[vlc-devel] [PATCH 11/11] input: move vout start/stop handling in resource
Steve Lhomme
robux4 at ycbcr.xyz
Wed Jul 1 11:00:44 CEST 2020
Comments below.
On 2020-06-30 17:27, Thomas Guillem wrote:
> The start state is now know by the input resource and protected with its lock.
> Move vout_StopDisplay() and vout_ChangeSource() to input/resource.c since the
> execution of these functions depend on the start state.
> ---
> src/input/decoder.c | 51 +++++++++++++++++---------------------------
> src/input/resource.c | 41 ++++++++++++++++++++++++++++++-----
> src/input/resource.h | 5 +++--
> 3 files changed, 59 insertions(+), 38 deletions(-)
>
> diff --git a/src/input/decoder.c b/src/input/decoder.c
> index ed7cb6803b8..ec3711cc685 100644
> --- a/src/input/decoder.c
> +++ b/src/input/decoder.c
> @@ -128,7 +128,6 @@ struct vlc_input_decoder_t
>
> vout_thread_t *p_vout;
> enum vlc_vout_order vout_order;
> - bool vout_thread_started;
>
> /* -- Theses variables need locking on read *and* write -- */
> /* Preroll */
> @@ -463,26 +462,21 @@ static int ModuleThread_UpdateVideoFormat( decoder_t *p_dec, vlc_video_context *
> return -1;
> }
> }
> - if (p_owner->vout_thread_started)
> - {
> - int res = vout_ChangeSource(p_owner->p_vout, &p_dec->fmt_out.video);
> - if (res == 0)
> - // the display/thread is started and can handle the new source format
> - return 0;
> - }
>
> vout_configuration_t cfg = {
> .vout = p_owner->p_vout, .clock = p_owner->p_clock, .fmt = &p_dec->fmt_out.video,
> .mouse_event = MouseEvent, .mouse_opaque = p_dec,
> };
> + bool started;
> vout_thread_t *p_vout =
> - input_resource_RequestVout(p_owner->p_resource, vctx, &cfg, NULL);
> + input_resource_RequestVout(p_owner->p_resource, vctx, &cfg, NULL, &started);
> if (p_vout != NULL)
> {
> - p_owner->vout_thread_started = true;
> - decoder_Notify(p_owner, on_vout_started, p_vout, p_owner->vout_order);
> + if (started)
> + decoder_Notify(p_owner, on_vout_started, p_vout, p_owner->vout_order);
> return 0;
> }
> +
> return -1;
> }
>
> @@ -543,7 +537,7 @@ static int CreateVoutIfNeeded(vlc_input_decoder_t *p_owner)
>
> enum vlc_vout_order order;
> const vout_configuration_t cfg = { .vout = p_vout, .fmt = NULL };
> - p_vout = input_resource_RequestVout( p_owner->p_resource, NULL, &cfg, &order );
> + p_vout = input_resource_RequestVout( p_owner->p_resource, NULL, &cfg, &order, NULL );
>
> vlc_mutex_lock( &p_owner->lock );
> p_owner->p_vout = p_vout;
> @@ -663,7 +657,6 @@ static subpicture_t *ModuleThread_NewSpuBuffer( decoder_t *p_dec,
>
> vout_Release(p_owner->p_vout);
> p_owner->p_vout = NULL; // the DecoderThread should not use the old vout anymore
> - p_owner->vout_thread_started = false;
> vlc_mutex_unlock( &p_owner->lock );
> }
> return NULL;
> @@ -684,7 +677,6 @@ static subpicture_t *ModuleThread_NewSpuBuffer( decoder_t *p_dec,
> p_owner->i_spu_channel);
> vout_Release(p_owner->p_vout);
> p_owner->p_vout = NULL; // the DecoderThread should not use the old vout anymore
> - p_owner->vout_thread_started = false;
> }
>
> enum vlc_vout_order channel_order;
> @@ -702,7 +694,6 @@ static subpicture_t *ModuleThread_NewSpuBuffer( decoder_t *p_dec,
> }
>
> p_owner->p_vout = p_vout;
> - p_owner->vout_thread_started = true;
> p_owner->vout_order = channel_order;
> vlc_mutex_unlock(&p_owner->lock);
>
> @@ -1021,7 +1012,7 @@ static void ModuleThread_QueueCc( decoder_t *p_videodec, block_t *p_cc,
> static int ModuleThread_PlayVideo( vlc_input_decoder_t *p_owner, picture_t *p_picture )
> {
> decoder_t *p_dec = &p_owner->dec;
> - vout_thread_t *p_vout = p_owner->vout_thread_started ? p_owner->p_vout : NULL;
> + vout_thread_t *p_vout = p_owner->p_vout;
>
> if( p_picture->date == VLC_TICK_INVALID )
> /* FIXME: VLC_TICK_INVALID -- verify video_output */
> @@ -1482,12 +1473,12 @@ static void DecoderThread_Flush( vlc_input_decoder_t *p_owner )
> }
> else if( p_dec->fmt_out.i_cat == VIDEO_ES )
> {
> - if( p_owner->p_vout && p_owner->vout_thread_started )
> + if( p_owner->p_vout )
> vout_FlushAll( p_owner->p_vout );
> }
> else if( p_dec->fmt_out.i_cat == SPU_ES )
> {
> - if( p_owner->p_vout && p_owner->vout_thread_started )
> + if( p_owner->p_vout )
> {
> assert( p_owner->i_spu_channel != VOUT_SPU_CHANNEL_INVALID );
> vout_FlushSubpictureChannel( p_owner->p_vout, p_owner->i_spu_channel );
> @@ -1796,7 +1787,6 @@ CreateDecoder( vlc_object_t *p_parent,
> p_owner->cbs_userdata = cbs_userdata;
> p_owner->p_aout = NULL;
> p_owner->p_vout = NULL;
> - p_owner->vout_thread_started = false;
> p_owner->i_spu_channel = VOUT_SPU_CHANNEL_INVALID;
> p_owner->i_spu_order = 0;
> p_owner->p_sout = p_sout;
> @@ -1973,15 +1963,16 @@ static void DeleteDecoder( vlc_input_decoder_t *p_owner )
>
> if (vout != NULL)
> {
> - if( p_owner->vout_thread_started)
> - {
> - /* Reset the cancel state that was set before joining the
> - * decoder thread */
> - vout_StopDisplay(vout);
> - p_owner->vout_thread_started = false;
> + /* Hold the vout since PutVout will likely release it and a
> + * last reference is needed for notify callbacks */
> + vout_Hold(vout);
> +
> + bool stopped;
> + input_resource_PutVout(p_owner->p_resource, vout, &stopped);
> + if (stopped)
> decoder_Notify(p_owner, on_vout_stopped, vout);
> - }
> - input_resource_PutVout(p_owner->p_resource, vout);
> +
> + vout_Release(vout);
> }
> break;
> }
> @@ -1995,7 +1986,6 @@ static void DeleteDecoder( vlc_input_decoder_t *p_owner )
> vout_UnregisterSubpictureChannel( p_owner->p_vout,
> p_owner->i_spu_channel );
> vout_Release(p_owner->p_vout);
> - p_owner->vout_thread_started = false;
> }
> break;
> }
> @@ -2157,7 +2147,7 @@ void vlc_input_decoder_Delete( vlc_input_decoder_t *p_owner )
> *
> * This unblocks the thread, allowing the decoder module to join all its
> * worker threads (if any) and the decoder thread to terminate. */
> - if( p_dec->fmt_in.i_cat == VIDEO_ES && p_owner->p_vout != NULL && p_owner->vout_thread_started )
> + if( p_dec->fmt_in.i_cat == VIDEO_ES && p_owner->p_vout != NULL )
> {
> if (p_owner->out_pool)
> picture_pool_Cancel( p_owner->out_pool, true );
> @@ -2303,8 +2293,7 @@ void vlc_input_decoder_Flush( vlc_input_decoder_t *p_owner )
> * after being unstuck. */
>
> vlc_mutex_lock( &p_owner->lock );
> - if( p_owner->dec.fmt_out.i_cat == VIDEO_ES
> - && p_owner->p_vout && p_owner->vout_thread_started )
> + if( p_owner->dec.fmt_out.i_cat == VIDEO_ES && p_owner->p_vout )
> vout_FlushAll( p_owner->p_vout );
> vlc_mutex_unlock( &p_owner->lock );
> }
> diff --git a/src/input/resource.c b/src/input/resource.c
> index b82d04a2897..04d554c1d48 100644
> --- a/src/input/resource.c
> +++ b/src/input/resource.c
> @@ -48,6 +48,7 @@ struct vout_resource
> {
> vout_thread_t *vout;
> enum vlc_vout_order order;
> + bool started;
>
> struct vlc_list node;
> };
> @@ -105,6 +106,7 @@ vout_resource_Create(vout_thread_t *vout)
> if (unlikely(vout_rsc == NULL))
> return NULL;
>
> + vout_rsc->started = false;
> vout_rsc->vout = vout;
> return vout_rsc;
> }
> @@ -376,12 +378,21 @@ void input_resource_SetInput( input_resource_t *p_resource, input_thread_t *p_in
> }
>
> static void input_resource_PutVoutLocked(input_resource_t *p_resource,
> - vout_thread_t *vout)
> + vout_thread_t *vout, bool *stopped)
> {
> assert(vout != NULL);
> struct vout_resource *vout_rsc = resource_GetVoutRsc(p_resource, vout);
> assert(vout_rsc != NULL);
>
> + if (stopped != NULL)
> + *stopped = vout_rsc->started;
I think the variable should be "was_stopped" or "has_stopped" because it
doesn't report the state but whether the call stopped it or not.
Otherwise *stopped should be !vout_rsc->started.
> +
> + if (vout_rsc->started)
> + {
> + vout_StopDisplay(vout_rsc->vout);
> + vout_rsc->started = false;
> + }
> +
> if (vout_rsc == resource_GetFirstVoutRsc(p_resource))
> {
> assert(p_resource->vout_rsc_free == NULL || p_resource->vout_rsc_free == vout_rsc);
> @@ -403,10 +414,10 @@ static void input_resource_PutVoutLocked(input_resource_t *p_resource,
> }
>
> void input_resource_PutVout(input_resource_t *p_resource,
> - vout_thread_t *vout)
> + vout_thread_t *vout, bool *stopped)
> {
> vlc_mutex_lock( &p_resource->lock );
> - input_resource_PutVoutLocked( p_resource, vout );
> + input_resource_PutVoutLocked( p_resource, vout, stopped );
> vlc_mutex_unlock( &p_resource->lock );
> }
>
> @@ -455,11 +466,15 @@ RequestVoutRsc(input_resource_t *p_resource)
> vout_thread_t *input_resource_RequestVout(input_resource_t *p_resource,
> vlc_video_context *vctx,
> const vout_configuration_t *cfg,
> - enum vlc_vout_order *order)
> + enum vlc_vout_order *order,
> + bool *started)
> {
> vlc_mutex_lock( &p_resource->lock );
> struct vout_resource *vout_rsc = NULL;
>
> + if (started != NULL)
> + *started = false;
> +
> vout_configuration_t dcfg = *cfg;
> if (dcfg.vout == NULL)
> {
> @@ -490,8 +505,19 @@ vout_thread_t *input_resource_RequestVout(input_resource_t *p_resource,
> return dcfg.vout;
> }
>
> + if (vout_rsc->started)
> + {
> + assert(cfg->vout != NULL);
> + int ret = vout_ChangeSource(dcfg.vout, dcfg.fmt);
> + if (ret == 0)
> + {
> + vlc_mutex_unlock(&p_resource->lock);
> + return dcfg.vout;
Here the call doesn't report the started state properly. It says it's
not started even though it may be started.
> + }
> + }
> +
> if (vout_Request(&dcfg, vctx, p_resource->p_input)) {
> - input_resource_PutVoutLocked(p_resource, dcfg.vout);
> + input_resource_PutVoutLocked(p_resource, dcfg.vout, NULL);
> vlc_mutex_unlock(&p_resource->lock);
> return NULL;
> }
> @@ -505,7 +531,12 @@ vout_thread_t *input_resource_RequestVout(input_resource_t *p_resource,
> input_ControlPush(p_resource->p_input, INPUT_CONTROL_SET_INITIAL_VIEWPOINT,
> ¶m);
> }
> + vout_rsc->started = true;
You could move this just after the successful vout_Request() call to
make it clearer why it's started at this point.
> vlc_mutex_unlock( &p_resource->lock );
> +
> + if (started != NULL)
> + *started = true;
> +
> return dcfg.vout;
> }
>
> diff --git a/src/input/resource.h b/src/input/resource.h
> index 7a725aabcf6..219049ee9b8 100644
> --- a/src/input/resource.h
> +++ b/src/input/resource.h
> @@ -39,8 +39,9 @@ sout_instance_t *input_resource_RequestSout( input_resource_t *, sout_instance_t
>
> vout_thread_t *input_resource_RequestVout(input_resource_t *, vlc_video_context *,
> const vout_configuration_t *,
> - enum vlc_vout_order *order);
> -void input_resource_PutVout(input_resource_t *, vout_thread_t *);
> + enum vlc_vout_order *order,
> + bool *started);
> +void input_resource_PutVout(input_resource_t *, vout_thread_t *, bool *stopped);
>
> /**
> * This function returns one of the current vout if any.
> --
> 2.20.1
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>
More information about the vlc-devel
mailing list