On Fri, 13 May 2016 10:58:02 +0100 Mark Thompson <s...@jkqxz.net> wrote:
> On 13/05/16 10:42, wm4 wrote: > > On Fri, 13 May 2016 10:54:17 +0300 > > Andrey Turkin <andrey.tur...@gmail.com> wrote: > > > >> 2016-05-13 10:35 GMT+03:00 wm4 <nfx...@googlemail.com>: > >> > >>> On Thu, 12 May 2016 22:35:48 +0300 > >>> Andrey Turkin <andrey.tur...@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. > > No: this part of the patch does nothing because it's already unreffed in the > avcodec_close() called immediately above. Maybe the unref should actually be > in > avcodec_free_context() only (if treating it like rc_override and those > fields), > but it shouldn't be in both. Oh I see. You could argue that avodec_open() is the thing which takes over ownership. But the real sketchy thing is that avcodec_close() unrefs it even if the context was not opened. This is all such an inconsistent mess. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel