[vlc-devel] [PATCH] decoder_helpers: fix clean order

[Thomas Guillem]

On Wed, Feb 20, 2019, at 10:09, Steve Lhomme wrote:
> On 19/02/2019 13:24, Thomas Guillem wrote:
> > The module need to be unloaded before the dec struct is cleaned.
> >
> > This fixes an undefined behavior with threaded decoders like avcodec when they
> > called decoder_UpdateVideoFormat()/dec_NewPicture() from an other thread just
> > before they were unloaded.
> 
> Are there cases where the decoder is still calling 
> decoder_UpdateVideoFormat() and decoder_NewPicture() while being about 
> to be unloaded ?

It is the case with avcodec with multi thread or every asynchronous decoders (MediaCodec/VideoToolBox).

> I don't see how this can work if one thread of the 
> decoder is calling decoder_UpdateVideoFormat() while another thread is 
> unloading the decoder DLLs. If that can happen I think the undefined 
> behavior is the least of our problem.

This won't happen since decoder implementations use mutex or semaphores to prevent that.

> 
> AFAIK lavc always call decoder_UpdateVideoFormat()/decoder_NewPicture() 
> from the same (decoder?) thread.
> 
> This is an important thing because the decoder pool will be released 
> there as well as the video context reference count.
> 
> > ---
> >   src/input/decoder_helpers.c | 11 ++++++-----
> >   1 file changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/input/decoder_helpers.c b/src/input/decoder_helpers.c
> > index 3cc02de8d6..b0a7c1890a 100644
> > --- a/src/input/decoder_helpers.c
> > +++ b/src/input/decoder_helpers.c
> > @@ -48,6 +48,12 @@ void decoder_Init( decoder_t *p_dec, const es_format_t *restrict p_fmt )
> >   
> >   void decoder_Clean( decoder_t *p_dec )
> >   {
> > +    if ( p_dec->p_module != NULL )
> > +    {
> > +        module_unneed(p_dec, p_dec->p_module);
> > +        p_dec->p_module = NULL;
> > +    }
> > +
> >       es_format_Clean( &p_dec->fmt_in );
> >       es_format_Clean( &p_dec->fmt_out );
> >   
> > @@ -56,11 +62,6 @@ void decoder_Clean( decoder_t *p_dec )
> >           vlc_meta_Delete(p_dec->p_description);
> >           p_dec->p_description = NULL;
> >       }
> > -    if ( p_dec->p_module != NULL )
> > -    {
> > -        module_unneed(p_dec, p_dec->p_module);
> > -        p_dec->p_module = NULL;
> > -    }
> >   }
> >   
> >   void decoder_Destroy( decoder_t *p_dec )
> > -- 
> > 2.20.1
> >
> > _______________________________________________
> > 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