[Ffmpeg-devel] [PATCH] faac free extradata in encode close

[Baptiste Coudurier]

Hi

Michael Niedermayer wrote:
> Hi
> 
> On Tue, Feb 27, 2007 at 12:36:34PM +0100, Baptiste Coudurier wrote:
>> Hi
>>
>> M?ns Rullg?rd wrote:
>>> Baptiste Coudurier said:
>>>> Baptiste Coudurier wrote:
>>>>> Hi
>>>>>
>>>>> M?ns Rullg?rd wrote:
>>>>>> Baptiste Coudurier <baptiste.coudurier at smartjog.com> writes:
>>>>>>
>>>>>>> Hi
>>>>>>>
>>>>>>> $subj.
>>>>>>>
>>>>>>> Assuming it should be freed in encode_close, since it has been allocated
>>>>>>> in encode init, another idea ?
>>>>>>>
>>>>>>> Index: libavcodec/faac.c
>>>>>>> ===================================================================
>>>>>>> --- libavcodec/faac.c	(revision 7983)
>>>>>>> +++ libavcodec/faac.c	(working copy)
>>>>>>> @@ -115,9 +115,8 @@
>>>>>>>      FaacAudioContext *s = avctx->priv_data;
>>>>>>>
>>>>>>>      av_freep(&avctx->coded_frame);
>>>>>>> +    av_freep(&avctx->extradata);
>>>>>>>
>>>>>>> -    //if (avctx->extradata_size) free(avctx->extradata);
>>>>>>> -
>>>>>>>      faacEncClose(s->faac_handle);
>>>>>>>      return 0;
>>>>>>>  }
>>>>>> This is wrong.  The buffer was returned from faacEncGetDecoderSpecificInfo(),
>>>>>> and we don't know how it was allocated.  Hopefully, faacEncClose()
>>>>>> does the required cleanup, but I wouldn't count on it.
>>>>>>
>>>>>> Have you observed a leak here?
>>>>>>
>>>>> valgrind is your friend.
>>>>>
>>>> ==23423== 2 bytes in 1 blocks are definitely lost in loss record 1 of 4
>>>> ==23423==    at 0x40244B0: malloc (vg_replace_malloc.c:149)
>>>> ==23423==    by 0x422E0B3: faacEncGetDecoderSpecificInfo (in
>>>> /usr/lib/libfaac.so.0.0.0)
>>>> ==23423==    by 0x8446959: Faac_encode_init (faac.c:82)
>>>>
>>>> So ?
>>> Well, if faac is that stupid we have no choice.  You have to use normal free(),
>>> not av_free() though, or it will break horribly if the memalign hack is used.
>>>
>> Right, here is a patch to copy extradata to fit extradata recommendation:
>> * the allocated memory should be FF_INPUT_BUFFER_PADDING_SIZE bytes larger
>> * then extradata_size to avoid prolems if its read with the bitstream reader
>> * the bytewise contents of extradata must not depend on the architecture
>> or cpu endianness
>>
>> adts muxer reads extrata using bitstream reader.
>>
>> Any hint to avoid def/undef free ugliness ?
> 
> no but patch looks ok
> 

Ok, applied.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
SMARTJOG S.A.                                    http://www.smartjog.com
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
Phone: +33 1 49966312