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

[James Almer]

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.