[FFmpeg-devel] [PATCH] Fix for memory leak in mov format
Art Clarke
aclarke
Fri May 16 15:37:06 CEST 2008
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: FfmpegLeak.c
Type: text/x-csrc
Size: 800 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080516/cacf1ed5/attachment.c>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Makefile
Type: application/octet-stream
Size: 178 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080516/cacf1ed5/attachment.obj>
More information about the ffmpeg-devel
mailing list