[FFmpeg-devel] [PATCH] Fix for memory leak in mov format
Art Clarke
aclarke
Wed Jun 4 00:13:06 CEST 2008
On Fri, May 16, 2008 at 6:47 AM, Art Clarke <aclarke at vlideshow.com> wrote:
>
>
> 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
>
It's been over two weeks and no one had comments on this patch (either
rejected or accepted). Can someone let me know if it's OK, and if so, how
to commit it (I'd rather not keep my source tree out of sync with the tip).
Issue: mov format leaks memory when opening and closing a file.
Patch: attached.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: patch.txt
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080603/f404ed8f/attachment.txt>
More information about the ffmpeg-devel
mailing list