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

[Thomas Guillem]

On Thu, Dec 17, 2020, at 12:24, Rémi Denis-Courmont wrote:
> 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.

I reset the flag here in case a new format update fails (when a previous vout is valid).
I could also reset it in  CreateVoutIfNeeded() when a new vout is created.

> 
> >      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/
> 
> 
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel