[FFmpeg-devel] [libav-devel] [PATCH 1/4] avcodec/a64multienc: use av_frame_ref instead of copying the frame

[Andreas Cadhalpun]

On 23.02.2015 12:46, Luca Barbato wrote:
> On 23/02/15 12:23, Andreas Cadhalpun wrote:
>> On 23.02.2015 01:56, Michael Niedermayer wrote:
>>>> --- a/libavcodec/a64multienc.c
>>>> +++ b/libavcodec/a64multienc.c
>>>> @@ -317,7 +317,9 @@ static int a64multi_encode_frame(AVCodecContext
>>>> *avctx, AVPacket *pkt,
>>>>       } else {
>>>>           /* fill up mc_meta_charset with data until lifetime
>>>> exceeds */
>>>>           if (c->mc_frame_counter < c->mc_lifetime) {
>>>> -            *p = *pict;
>>>> +            ret = av_frame_ref(p, pict);
>>>> +            if (ret < 0)
>>>> +                return ret;
>>>
>>> I suspect this leaves a memleak, ill push it anyway as it allows
>>> regression testing the more complex subsequent fix
>>
>> Yes, this trades the crash for a memleak. :-/
>> Your simplification [1] fixes that.
>>
>
> coded_frame should be always allocated if I remember correctly and a
> cursory git grep tells me that.

Why should coded_frame be allocated? Is this documented somewhere?

It seems many audio encoders don't do that, as well as e.g. 
libopenh264enc and libwebpenc.

Anyway, the memleak can also be fixed by adding av_frame_unref(p) before 
the av_frame_ref.

Best regards,
Andreas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-avcodec-a64multienc-use-av_frame_ref-instead-of-copy.patch
Type: text/x-diff
Size: 1126 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150223/c1b5d235/attachment.bin>