[FFmpeg-devel] [PATCH] Fix for memory leak in mov format

Art Clarke aclarke
Fri May 16 15:47:16 CEST 2008


On Fri, May 16, 2008 at 9:41 AM, Art Clarke <aclarke at vlideshow.com> wrote:

> On Fri, May 16, 2008 at 9:37 AM, Art Clarke <aclarke at vlideshow.com> wrote:
>
>> On Thu, May 15, 2008 at 9:27 AM, Art Clarke <aclarke at vlideshow.com>
>> wrote:
>>
>>> Hi folks,
>>>
>>> This is my first patch submitted, so please let me know if you need
>>> additional information or if I screwed up the format in some way.
>>>
>>> Description of problem:
>>> The mov demuxer allocates some private data on each AVStream it adds, but
>>> never frees the data.
>>>
>>> How to reproduce:
>>> Use av_open_input_file() to open a .mov file, then call
>>> av_close_input_file().  Run program through a memory checking tool (e.g.
>>> valgrind).
>>>
>>> Description of fix:
>>> When closing a .mov format file, free the allocated MOVContext object and
>>> null the AVFormatContext->priv_data value.
>>>
>>> Pre-Patch Results (SVN r13158)
>>> "make test" tests pass (assuming "configure" has no options passed in).
>>>
>>> Valgrind results (test opens .mov file, decodes all audio and video,
>>> remixes audio in special ways, and then encodes a new FLV file):
>>> valgrind --leak-check=full ../aaffmpeg_test
>>> com::vlideshow::aaffmpeg::AudioSamplesMixerTest 8
>>> ==21759== Memcheck, a memory error detector.
>>> ==21759== Copyright (C) 2002-2007, and GNU GPL'd, by Julian Seward et al.
>>> ==21759== Using LibVEX rev 1804, a library for dynamic binary
>>> translation.
>>> ==21759== Copyright (C) 2004-2007, and GNU GPL'd, by OpenWorks LLP.
>>> ==21759== Using valgrind-3.3.0, a dynamic binary instrumentation
>>> framework.
>>> ==21759== Copyright (C) 2000-2007, and GNU GPL'd, by Julian Seward et al.
>>> ==21759== For more details, rerun with: -v
>>> ==21759==
>>> [mov,mp4,m4a,3gp,3g2,mj2 @ 0x411e540]negative ctts, ignoring
>>>
>>> com::vlideshow::aaffmpeg::AudioSamplesMixerTest: .
>>>
>>> tests summary: ok:1
>>> ==21759==
>>> ==21759== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 34 from 1)
>>> ==21759== malloc/free: in use at exit: 360 bytes in 3 blocks.
>>> ==21759== malloc/free: 29,788 allocs, 29,785 frees, 12,794,101 bytes
>>> allocated.
>>> ==21759== For counts of detected errors, rerun with: -v
>>> ==21759== searching for pointers to 3 not-freed blocks.
>>> ==21759== checked 1,441,900 bytes.
>>> ==21759==
>>> ==21759== 360 bytes in 3 blocks are definitely lost in loss record 1 of 1
>>> ==21759==    at 0x4004802: memalign (vg_replace_malloc.c:460)
>>> ==21759==    by 0x462FDCA: av_malloc (mem.c:61)
>>> ==21759==    by 0x462FE59: av_mallocz (mem.c:134)
>>> ==21759==    by 0x40E7E0F: mov_read_trak (mov.c:1253)
>>> ==21759==    by 0x40E4FC5: mov_read_default (mov.c:215)
>>> ==21759==    by 0x40E5A82: mov_read_moov (mov.c:455)
>>> ==21759==    by 0x40E4FC5: mov_read_default (mov.c:215)
>>> ==21759==    by 0x40E922C: mov_read_header (mov.c:1736)
>>> ==21759==    by 0x40A4F0B: av_open_input_stream (utils.c:395)
>>> ==21759==    by 0x40A51D7: av_open_input_file (utils.c:481)
>>> ==21759==    by 0x4059B7B:
>>> com::vlideshow::aaffmpeg::Container::openInputURL(char const*,
>>> com::vlideshow::aaffmpeg::IContainerFormat*) (Container.cpp:91)
>>> ==21759==    by 0x4059F4D: com::vlideshow::aaffmpeg::Container::open(char
>>> const*, com::vlideshow::aaffmpeg::IContainer::Type,
>>> com::vlideshow::aaffmpeg::IContainerFormat*) (Container.cpp:66)
>>> ==21759==
>>> ==21759== LEAK SUMMARY:
>>> ==21759==    definitely lost: 360 bytes in 3 blocks.
>>> ==21759==      possibly lost: 0 bytes in 0 blocks.
>>> ==21759==    still reachable: 0 bytes in 0 blocks.
>>> ==21759==         suppressed: 0 bytes in 0 blocks.
>>>
>>> Post-Patch results:
>>> "make test" tests pass (assuming "configure has no options passed in).
>>>
>>> Valgrind results (test opens .mov file, decodes all audio and video,
>>> remixes audio in special ways, and then encodes a new FLV file)
>>> valgrind --leak-check=full ../aaffmpeg_test
>>> com::vlideshow::aaffmpeg::AudioSamplesMixerTest 8
>>> ==21917== Memcheck, a memory error detector.
>>> ==21917== Copyright (C) 2002-2007, and GNU GPL'd, by Julian Seward et al.
>>> ==21917== Using LibVEX rev 1804, a library for dynamic binary
>>> translation.
>>> ==21917== Copyright (C) 2004-2007, and GNU GPL'd, by OpenWorks LLP.
>>> ==21917== Using valgrind-3.3.0, a dynamic binary instrumentation
>>> framework.
>>> ==21917== Copyright (C) 2000-2007, and GNU GPL'd, by Julian Seward et al.
>>> ==21917== For more details, rerun with: -v
>>> ==21917==
>>> [mov,mp4,m4a,3gp,3g2,mj2 @ 0x411e540]negative ctts, ignoring
>>>
>>> com::vlideshow::aaffmpeg::AudioSamplesMixerTest: .
>>>
>>> tests summary: ok:1
>>> ==21917==
>>> ==21917== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 34 from 1)
>>> ==21917== malloc/free: in use at exit: 0 bytes in 0 blocks.
>>> ==21917== malloc/free: 29,788 allocs, 29,788 frees, 12,794,101 bytes
>>> allocated.
>>> ==21917== For counts of detected errors, rerun with: -v
>>> ==21917== All heap blocks were freed -- no leaks are possible.
>>>
>>> Unified diff of patch:
>>> svn diff -x --unified
>>> Index: libavformat/mov.c
>>> ===================================================================
>>> --- libavformat/mov.c   (revision 13158)
>>> +++ libavformat/mov.c   (working copy)
>>> @@ -1894,6 +1894,7 @@
>>>          av_freep(&sc->drefs);
>>>          if (sc->pb && sc->pb != s->pb)
>>>              url_fclose(sc->pb);
>>> +        av_freep(&sc);
>>>      }
>>>      if(mov->dv_demux){
>>>          for(i=0; i<mov->dv_fctx->nb_streams; i++){
>>>
>>>
>>>
>>> (I know there's a two week gap between submitting a patch and ping back
>> about a response, so feel free to ignore this until then :) ).
>>
>> In the event it would be helpful, attached is some really simple sample
>> code (which I place in the public domain) that demonstrates the bug .  Just
>> pass in a .mov file as the input.  Passing in a .flv or a .mp3 (those were
>> the other formats I tested) don't leak (as expected).  Passing a .mov does
>> leak before the fix, and doesn't after the fix.
>>
>> - Art
>>
>>
> Lastly, I incorrect specified that to reproduce you just needed to open and
> close the file.  That is not correct.
>
> You need to also call av_find_stream_info() as well, so that libavformat
> creates the AVStream object.  The sample code does the correct reproduction
> of the problem.
>
> Thanks,
>
> - Art
>
OK, this time I mean lastly (sorry for the noisiness, but I figure correct
is better than not).

I made a last minute change in the sample code without testing it before I
posted it.  Shame on me.  I incorrectly assigned the filename before
checking if you passed it in on the command line.  The corrected sample code
is attached.  The patch remains correct.

- Art "take 50 lashes and always run one last test before submitting" Clarke
-------------- next part --------------
A non-text attachment was scrubbed...
Name: FfmpegLeak.c
Type: text/x-csrc
Size: 817 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080516/41b5ac0b/attachment.c>



More information about the ffmpeg-devel mailing list