[PATCH] 16 bytes leaked per frame

[Bradley Baetz <bbaetz@…>]

The fix for #11029 [92016b94983a4b8a6cb75dbc48893f5c05ce4620] applied a mythtv-specific patch to libavformat, chaning a call from av_init_packet(pkt) to av_new_packet(pkt, 0), in order to properly initialise some fields.

However, av_new_packet ends up mallocing (size+ FF_INPUT_BUFFER_PADDING_SIZE) (FF_INPUT_BUFFER_PADDING_SIZE is #defined as 16). This data is never freed, at least according to valgrind.

After watching a 2 hour show (recorded as an MPEG2 TS stream from an hdhomerun) 4MB of data was leaked:

==5837== 4,279,136 bytes in 267,446 blocks are definitely lost in loss record 2,969 of 2,971
==5837==    at 0x4C29BE8: memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5837==    by 0x4C29C97: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5837==    by 0xAE9DAC1: av_malloc (mem.c:95)
==5837==    by 0xA0A15C7: av_new_packet (avpacket.c:71)
==5837==    by 0x9D88A8A: ff_read_packet (utils.c:712)
==5837==    by 0x9D89C46: read_frame_internal (utils.c:1310)
==5837==    by 0x9D8A6F2: av_read_frame (utils.c:1413)
==5837==    by 0x5301CF5: AvFormatDecoder::GetFrame(DecodeTypes) (avformatdecoder.cpp:4509)
==5837==    by 0x52930E3: MythPlayer::DecoderGetFrame(DecodeTypes, bool) (mythplayer.cpp:3221)
==5837==    by 0x5293381: MythPlayer::DecoderLoop(bool) (mythplayer.cpp:3145)
==5837==    by 0x5284EE5: DecoderThread::run() (mythplayer.cpp:108)
==5837==    by 0x8938FCA: ??? (in /usr/lib/x86_64-linux-gnu/libQtCore.so.4.8.1)
==5837==    by 0x720BE99: start_thread (pthread_create.c:308)
==5837==    by 0x9398CBC: clone (clone.S:112)

Upstream appears to have fixed the same bug with ​http://git.videolan.org/?p=ffmpeg.git;a=commitdiff;h=1cc569dddadfedabe970ce7408dba7ddf381e98b which just initialises those fields. I suspect that that's the better fix (I could only find the git commit and no mailing list conversation; not sure if someone else reported the same issue or if that fix was prompted from a report by one of the mythtv developers)

Reverting the mythtv patch and applying the upstream one fixes the leak for me, and from code inspection this does the same as av_new_packet (without the malloc). (I can't easily check for the bug that that ticket has; it was apparently hard to reproduce, plus mythtv crashes very often for me under valgrind anyway for some reason)

Note that this data should be freed by av_free_packet, which implies that its not being called in this case. Not sure why not, but it may be internal to libavformat and related to the mythtv-local patch (the library isn't expecting allocated data at that point?) Or it may be another bug - AvFormatDecoder::GetFrame's use of av_dup_packet looks a bit odd (dupes the data and then just drops the original? Not sure though - documentation for av_dup_packet is a bit missing)

Full valgrind log and patch attached. (This was noticed while I'm trying to reproduce a much larger frontend leak)

[Bradley Baetz <bbaetz@…>]

Full valgrind log

[Bradley Baetz <bbaetz@…>]

patch

[Bradley Baetz <bbaetz@…>]

In 430900f7bce45d031ec948d867f0903e20544750/mythtv:

Error: Processor CommitTicketReference failed

GIT backend not available