[FFmpeg-devel] [PATCH] libavcodec/options.c: handle hw_frames_ctx where necessary

[wm4]

On Fri, 13 May 2016 10:54:17 +0300
Andrey Turkin <andrey.turkin at gmail.com> wrote:

> 2016-05-13 10:35 GMT+03:00 wm4 <nfxjfg at googlemail.com>:
> 
> > On Thu, 12 May 2016 22:35:48 +0300
> > Andrey Turkin <andrey.turkin at gmail.com> wrote:
> >  
> > > Few functions didn't handle hw_frames_ctx references causing resources  
> > leaks and even crashes.  
> > > ---
> > >  libavcodec/options.c | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > >
> > > diff --git a/libavcodec/options.c b/libavcodec/options.c
> > > index ea2563b..8682262 100644
> > > --- a/libavcodec/options.c
> > > +++ b/libavcodec/options.c
> > > @@ -175,6 +175,7 @@ void avcodec_free_context(AVCodecContext **pavctx)
> > >      av_freep(&avctx->intra_matrix);
> > >      av_freep(&avctx->inter_matrix);
> > >      av_freep(&avctx->rc_override);
> > > +    av_buffer_unref(&avctx->hw_frames_ctx);
> > >
> > >      av_freep(pavctx);
> > >  }  
> >
> > I would have thought this is the responsibility of the API user?
> >
> >  
> AVCodecContext documentation says it is set by a user but then managed and
> owned by libavcodec (which is a logical thing to do for any shared
> reference).

Even so it's a breaking API change and should be treated as such. An
API user could for example have a separate variable with the same
buffer ref somewhere, which would lead to a double-free. Even more so
because an API user might have noticed the leak, and concluded the ref
must be released manually.

Since this looks like an unintentional bug and there's no release with
it included yet, we can probably skip major jumps. But it should still
be mentioned in APIchanges, and be sent to Libav too.