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 <alevi...@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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to