[vlc-devel] [PATCH 2/3] decoder: don't control the vout when not started
Thomas Guillem
thomas at gllm.fr
Thu Dec 17 09:50:48 UTC 2020
On Thu, Dec 17, 2020, at 10:06, Thomas Guillem wrote:
>
>
> On Thu, Dec 17, 2020, at 07:57, Steve Lhomme wrote:
> > On 2020-12-16 18:34, Thomas Guillem wrote:
> > > Calling vout functions that need to Hold the vout_control will deadlock
> > > if the vout is not yet started.
> > > ---
> > > src/input/decoder.c | 26 ++++++++++++++++++++------
> > > 1 file changed, 20 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/src/input/decoder.c b/src/input/decoder.c
> > > index b9cdc7d4a81..488998fc35e 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,12 +477,20 @@ 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 );
> >
> > It seems that it should change with the has_started below. If it was
> > started there's no need to change it.
>
> Naup, if the vout is recycled (2 medias played one after the other,
> has_started will always be false with the second media, and yet the
> vout will be started. That's the difference between "has_started" and
> "started".
Furthermore, on_vout_stopped is called when the decoder is destroyed so there is not need to reset this variable in that case.
>
> >
> > Same thing when set to false. It should also match the calls to
> > on_vout_stopped.
> >
> > > +
> > > if (has_started)
> > > decoder_Notify(p_owner, on_vout_started, p_vout, p_owner->vout_order);
> > > return 0;
> > > }
> > >
> > > error:
> > > + vlc_mutex_lock( &p_owner->lock );
> > > + p_owner->vout_started = false;
> > > + vlc_mutex_unlock( &p_owner->lock );
> > > +
> > > if (p_owner->vctx != NULL)
> > > {
> > > vlc_video_context_Release(p_owner->vctx);
> > > @@ -1023,6 +1032,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 )
> > > {
> > > @@ -1474,7 +1485,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
> > > @@ -1504,7 +1515,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;
> > > @@ -1530,7 +1541,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:
> > > @@ -1562,7 +1573,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;
> > > @@ -1792,6 +1803,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;
> > > @@ -2151,7 +2163,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 );
> > > @@ -2324,7 +2337,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 );
> > > }
> > > --
> > > 2.29.2
> > >
> > > _______________________________________________
> > > vlc-devel mailing list
> > > To unsubscribe or modify your subscription options:
> > > https://mailman.videolan.org/listinfo/vlc-devel
> > >
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> 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