[vlc-devel] [PATCH 2/3] decoder: don't control the vout when not started

[Rémi Denis-Courmont]

Le torstaina 17. joulukuuta 2020, 11.52.04 EET Thomas Guillem a écrit :
> Calling vout functions that need to Hold the vout_control will deadlock
> if the vout is not yet started.
> ---
>  src/input/decoder.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/src/input/decoder.c b/src/input/decoder.c
> index 46bb17a3840..0b90d3e5ae6 100644
> --- a/src/input/decoder.c
> +++ b/src/input/decoder.c
> @@ -128,6 +128,7 @@ struct vlc_input_decoder_t
>      audio_output_t *p_aout;
> 
>      vout_thread_t   *p_vout;
> +    bool             vout_started;
>      enum vlc_vout_order vout_order;
> 
>      /* -- Theses variables need locking on read *and* write -- */
> @@ -476,6 +477,10 @@ static int ModuleThread_UpdateVideoFormat( decoder_t
> *p_dec, vlc_video_context * &has_started);
>      if (p_vout != NULL)
>      {
> +        vlc_mutex_lock( &p_owner->lock );
> +        p_owner->vout_started = true;
> +        vlc_mutex_unlock( &p_owner->lock );
> +
>          if (has_started)
>              decoder_Notify(p_owner, on_vout_started, p_vout,
> p_owner->vout_order); return 0;
> @@ -486,6 +491,7 @@ error:
>       * call */
>      vlc_mutex_lock( &p_owner->lock );
>      es_format_Clean( &p_owner->fmt );
> +    p_owner->vout_started = false;
>      vlc_mutex_unlock( &p_owner->lock );
> 

Not sure what that particular code path is for, really, but it clearing the 
flag does not seem correct to match with the description. I would expect that 
the flag is already false at this point.

>      if (p_owner->vctx != NULL)
> @@ -1029,6 +1035,8 @@ static int ModuleThread_PlayVideo( vlc_input_decoder_t
> *p_owner, picture_t *p_pi }
> 
>      vlc_mutex_lock( &p_owner->lock );
> +    assert( p_owner->vout_started );
> +
>      bool prerolled = p_owner->i_preroll_end != PREROLL_NONE;
>      if( prerolled && p_owner->i_preroll_end > p_picture->date )
>      {
> @@ -1480,7 +1488,7 @@ 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 )
> +        if( p_owner->p_vout && p_owner->vout_started )
>              vout_FlushAll( p_owner->p_vout );
> 
>          /* Reset the pool cancel state, previously set by
> @@ -1510,7 +1518,7 @@ static void DecoderThread_ChangePause(
> vlc_input_decoder_t *p_owner, bool paused {
>          case VIDEO_ES:
>              vlc_mutex_lock( &p_owner->lock );
> -            if( p_owner->p_vout != NULL )
> +            if( p_owner->p_vout != NULL && p_owner->vout_started )
>                  vout_ChangePause( p_owner->p_vout, paused, date );
>              vlc_mutex_unlock( &p_owner->lock );
>              break;
> @@ -1536,7 +1544,7 @@ static void DecoderThread_ChangeRate(
> vlc_input_decoder_t *p_owner, float rate ) switch( p_dec->fmt_out.i_cat )
>      {
>          case VIDEO_ES:
> -            if( p_owner->p_vout != NULL )
> +            if( p_owner->p_vout != NULL && p_owner->vout_started )
>                  vout_ChangeRate( p_owner->p_vout, rate );
>              break;
>          case AUDIO_ES:
> @@ -1568,7 +1576,7 @@ static void DecoderThread_ChangeDelay(
> vlc_input_decoder_t *p_owner, vlc_tick_t {
>          case VIDEO_ES:
>              vlc_mutex_lock( &p_owner->lock );
> -            if( p_owner->p_vout != NULL )
> +            if( p_owner->p_vout != NULL && p_owner->vout_started )
>                  vout_ChangeDelay( p_owner->p_vout, delay );
>              vlc_mutex_unlock( &p_owner->lock );
>              break;
> @@ -1798,6 +1806,7 @@ 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_started = false;
>      p_owner->i_spu_channel = VOUT_SPU_CHANNEL_INVALID;
>      p_owner->i_spu_order = 0;
>      p_owner->p_sout = p_sout;
> @@ -2157,7 +2166,8 @@ 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 ) +    if(
> p_dec->fmt_in.i_cat == VIDEO_ES && p_owner->p_vout != NULL +     &&
> p_owner->vout_started )
>      {
>          if (p_owner->out_pool)
>              picture_pool_Cancel( p_owner->out_pool, true );
> @@ -2330,7 +2340,8 @@ 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 )
> +        if( p_owner->dec.fmt_out.i_cat == VIDEO_ES && p_owner->p_vout
> +         && p_owner->vout_started )
>              vout_FlushAll( p_owner->p_vout );
>          vlc_mutex_unlock( &p_owner->lock );
>      }


-- 
レミ・デニ-クールモン
http://www.remlab.net/