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

[Michael Niedermayer]

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

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070228/dd11f21c/attachment.pgp>