[FFmpeg-devel] [PATCH] Fixed memory leaks associated with AVStream objects if FF_API_LAVF_AVCTX is defined

[Aaron Levinson]

On 4/21/2017 2:03 PM, James Almer wrote:
> On 4/21/2017 12:09 PM, Michael Niedermayer wrote:
>> On Thu, Apr 20, 2017 at 11:30:13PM -0700, Aaron Levinson wrote:
>>>  From 4f27e910aca6dae6642b4d73cf07fa0d6c4b1618 Mon Sep 17 00:00:00 2001
>>> From: Aaron Levinson <alevinsn at aracnet.com>
>>> Date: Thu, 20 Apr 2017 23:20:20 -0700
>>> Subject: [PATCH] Fixed memory leaks associated with AVStream objects if
>>>   FF_API_LAVF_AVCTX is defined
>>>
>>> Purpose: Fixed memory leaks associated with AVStream objects if
>>> FF_API_LAVF_AVCTX is defined.  If FF_API_LAVF_AVCTX is defined,
>>> AVStream::codec is set to an AVCodecContext object, and this wasn't
>>> being deallocated properly when the AVStream object was freed.  While
>>> FF_API_LAVF_AVCTX has to do with deprecated functionality, in
>>> practice, it will always be defined for typical builds currently,
>>> since it is defined in libavformat\version.h if
>>> LIBAVFORMAT_VERSION_MAJOR is less than 58, and
>>> LIBAVFORMAT_VERSION_MAJOR is currently set to 57.
>>>
>>> Comments:
>>>
>>> -- libavformat/utils.c: Now using avcodec_free_context() to properly
>>>     deallocate st->codec in free_stream() if FF_API_LAVF_AVCTX is
>>>     defined.
>>> ---
>>>   libavformat/utils.c | 4 +---
>>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> please use "make fate" to test your changes
>>
>> This one causes many all? tests to segfault
>
> The issue is coded_side_data in AVStream->codec.
>
> ffmpeg.c calls avcodec_copy_context() to copy the encoder AVCodecContext
> to the output's AVStream->codec because the latter is still needed by
> some internal code in lavf/utils.c and such.
> avcodec_copy_context() copies the coded_side_data pointer as part of its
> memcpy call but not the buffers, and by the time avcodec_free_context()
> is called for AVStream->codec the buffers said pointer points to was
> already freed by the avcodec_free_context() call for the encoder
> AVCodecContext.
>
> The proper solution: Remove the avcodec_copy_context() call in ffmpeg.c
> and make libavformat stop needing AVStream->codec internally. It should
> use AVStream->internal->avctx instead. Even if this is not done now, it
> will anyway when the deprecation period ends and it's time to remove
> AVStream->codec.
> The easy but ugly solution until the above is done: Copy the
> coded_side_data buffers in avcodec_copy_context().
>
> One could argue that by definition avcodec_copy_context() should copy
> said side data, but the function is clearly marked as "do not use, its
> behavior is ill defined" and the user is asked instead to copy using an
> intermediary AVCodecParameters context instead.

Now that James Almer's patches pertaining to avcodec_copy_context() have 
been applied, perhaps the patch I submitted a couple of weeks ago can be 
considered anew.  I've confirmed that make fate SAMPLES=fate-suite/ 
(64-bit, Linux) completes successfully with the patch that I submitted.

Thanks,
Aaron Levinson