Opened 11 years ago

Closed 3 years ago

#2716 closed defect (fixed)

Memory leak at avformat_new_stream

Reported by: Anshul Owned by: Anshul
Priority: minor Component: avformat
Version: git-master Keywords: leak
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

Summary of the bug:
There is memory leak when we do avformat_new_stream,
when we use codec h264, we dont free the codec->privatedata
in avformat_free_context
How to reproduce:

to reproduce the bug we cant use ffmpeg directly, take the file
attached to ticket, and compile it using ffmpeg library

i am using latest version from git.

i dont know that if, i fall in developer category, but i can reproduce and have analyzed it too.

Attachments (1)

muxing.c (6.7 KB ) - added by Anshul 10 years ago.
this file produce same result and have no dependency other then ffmpeg

Download all attachments as: .zip

Change History (20)

comment:1 by Carl Eugen Hoyos, 11 years ago

Do copyrights not apply where you live?

in reply to:  1 comment:2 by Anshul, 11 years ago

Replying to cehoyos:

Do copyrights not apply where you live?

I they apply, can you explain how am i breaking laws.

comment:3 by Carl Eugen Hoyos, 11 years ago

I compared doc/examples/muxing.c to the file you attached here and had the feeling you removed something important on top.

in reply to:  3 ; comment:4 by Anshul, 11 years ago

Replying to cehoyos:

I compared doc/examples/muxing.c to the file you attached here and had the feeling you removed something important on top.

i wrote that attached file myself and took some reference from that file,
i didn’t copy, modify, merge, publish, distribute, sublicense, and/or sell copies of that file.
as on above copy right. If taking reference also need to put that license, i don’t have any problem
pasting that again.
Does that file really seems to be same as example/muxing.c

comment:5 by gjdfgh, 11 years ago

It seems a bit silly to insist on licenses for EXAMPLES.

Maybe effort should be made to relicense the affected files to public domain?

in reply to:  4 ; comment:6 by Carl Eugen Hoyos, 11 years ago

Replying to er.anshul.maheshwari@…:

i wrote that attached file myself

My comment:1 was meant mostly as a joke, I was just surprised that you posted a file named exactly like one from the examples directory and when I opened it in the browser the only obvious difference was the missing license header (which basically only contains the request to not remove it), sorry if you felt offended.

I consider your comment extremely offending though.

in reply to:  6 comment:7 by Anshul, 11 years ago

Replying to cehoyos:

Replying to er.anshul.maheshwari@…:

i wrote that attached file myself

My comment:1 was meant mostly as a joke, I was just surprised that you posted a file named exactly like one from the examples directory and when I opened it in the browser the only obvious difference was the missing license header (which basically only contains the request to not remove it), sorry if you felt offended.

I consider your comment extremely offending though.

Actually i am newbie, don't know lot of laws "Extremly sorry if mine comments were offending"

comment:8 by Anshul, 11 years ago

I checked the memory footprint of example, and found that codec->private_data is freed
by avcodec_close.

I think that if avformat is allocating some Context using libavcodec,
then libavformat should deallocate, but it is expecting user to call
avcodec_close.

If guys you feel that avcodec_close should be called by user, at least we should
mention in comment above avformat_new_stream.

Still 7 bytes are leak

=14859== total heap usage: 10,873 allocs, 10,851 frees, 169,396,375 bytes allocated
==14859==
==14859== 7 bytes in 1 blocks are definitely lost in loss record 5 of 19
==14859== at 0x40294EB: memalign (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==14859== by 0x4A7A829: av_strdup (mem.c:241)
==14859== by 0x4A7BA2B: set_string (opt.c:172)
==14859== by 0x4A7C1BA: av_opt_set (opt.c:267)
==14859== by 0x4A7EB6A: av_opt_set_defaults2 (opt.c:1001)
==14859== by 0x4A7E9CF: av_opt_set_defaults (opt.c:961)
==14859== by 0x46315FF: avcodec_get_context_defaults3 (options.c:136)
==14859== by 0x46316D2: avcodec_alloc_context3 (options.c:157)
==14859== by 0x49755C8: avformat_new_stream (utils.c:3258)
==14859== by 0x4AA4FBD: add_stream (muxing.c:32)
==14859== by 0x4AA5269: init (muxing.c:121)
==14859== by 0x8048904: main (test_app.c:52)
==14859==
==14859== LEAK SUMMARY:
==14859== definitely lost: 7 bytes in 1 blocks
==14859== indirectly lost: 0 bytes in 0 blocks
==14859== possibly lost: 0 bytes in 0 blocks
==14859== still reachable: 2,074,154 bytes in 21 blocks
==14859== suppressed: 0 bytes in 0 blocks
==14859== Reachable blocks (those to which a pointer was found) are not shown.
==14859== To see them, rerun with: --leak-check=full --show-reachable=yes

I don't know how to remove that attached file, otherwise i have attached another file
with license on top. if any one know please help.

comment:9 by Anshul, 11 years ago

I got exactly where is the leak of 7 byte

In my case program counter never reach at the following position

#0  av_freep (arg=0x90c11d0) at libavutil/mem.c:219
#1  0x08878aac in av_opt_free (obj=0x90c0c20) at libavutil/opt.c:1216
#2  0x08595830 in avcodec_close (avctx=0x90c0880) at libavcodec/utils.c:2382
#3  0x0804c330 in close_video (oc=0x90c0020, st=0x90c0620) at muxing.c:375
#4  0x0804c864 in main (argc=2, argv=0xbfffed94) at muxing.c:496

since it never full fill the below condition in file libavcodec/utils.c

if (avctx->priv_data && avctx->codec && avctx->codec->priv_class)

av_opt_free(avctx->priv_data);

in my case avctx->codec is NULL

Last edited 10 years ago by Anshul (previous) (diff)

comment:10 by Anshul, 11 years ago

Owner: set to Anshul
Status: newopen

comment:11 by Anshul, 11 years ago

I do have two solution.

1.put some comment at avformat_new_stream, that what things might be required

2.just free things at avformat_free_context

please guide me this is the first time, i have assigned bug to me.

comment:12 by Anshul, 11 years ago

Analyzed by developer: set
Reproduced by developer: set

comment:13 by Carl Eugen Hoyos, 11 years ago

Analyzed by developer: unset
Reproduced by developer: unset

in reply to:  13 comment:14 by Anshul, 11 years ago

Replying to cehoyos:

hi cehoyos

i did send a patch, after analyzing and reproducing. is it something like any one as has to it.
then it will count.

thanks
anshul

"please pardon me if above statement is arrogant, this is because i don't know how to sugar them"

comment:15 by Carl Eugen Hoyos, 11 years ago

Keywords: memory removed
Resolution: fixed
Status: openclosed

Your patch was applied as 0f229f9b

comment:16 by Michael Niedermayer, 11 years ago

Priority: normalminor
Resolution: fixed
Status: closedreopened

Patch has been reverted as it causes races and crashes with VLC.
One could blame VLC for insufficient locking and mutexes but that wont make all applications do the extra needed locking. So its probably safer to keep the previously existing need for the user app to clean up the codec side. A patch in that direction is welcome!

by Anshul, 10 years ago

Attachment: muxing.c added

this file produce same result and have no dependency other then ffmpeg

in reply to:  4 comment:17 by Carl Eugen Hoyos, 10 years ago

Replying to er.anshul.maheshwari@…:

Replying to cehoyos:

I compared doc/examples/muxing.c to the file you attached here and had the feeling you removed something important on top.

i wrote that attached file myself and took some reference from that file,
i didn’t copy, modify, merge, publish, distribute, sublicense, and/or sell copies of that file.

This is still very hard to believe (actually impossible).

comment:18 by Anshul, 10 years ago

To play file with vlc to reproduce crashes following information is helpful

command:./vlc -I "dummy" --demux avformat,none --codec avcodec,none <filenae>

using none so that demuxer and codec should not be used other then ffmpeg's

influential environment var to be taken care while compiling vlc
AVCODEC_LIBS,AVCODEC_CFLAGS, AVFORMAT_CFLAGS, AVFORMAT_LIBS.

comment:19 by mkver, 3 years ago

Resolution: fixed
Status: reopenedclosed

The main leak has been fixed in b9d2005ea5d6837917a69bc2b8e98f5695f54e39 (which does an even more thorough cleanup than avcodec_close() does). So it seems that downstream users fixed their problems (probably by not using AVStream.codec any more).
The leak mentioned in comment 9 probably refers to the leak fixed in 7ddbb4e23a4d591ab35594418fa250cc54c8df58. Therefore I am closing the whole issue as fixed.

Note: See TracTickets for help on using tickets.