Opened 7 years ago

Closed 7 years ago

#11329 closed Patch - Bug Fix (fixed)

[PATCH] 16 bytes leaked per frame

Reported by: Bradley Baetz <bbaetz@…> Owned by: Jim Stichnoth
Priority: minor Milestone: 0.26.1
Component: MythTV - Video Decoding Version: 0.26-fixes
Severity: medium Keywords: patch
Cc: Ticket locked: no

Description

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)

Attachments (2)

valgrind.5837.gz (354.6 KB) - added by Bradley Baetz <bbaetz@…> 7 years ago.
Full valgrind log
mythtv-ffmpeg-leak.patch (565 bytes) - added by Bradley Baetz <bbaetz@…> 7 years ago.
patch

Download all attachments as: .zip

Change History (5)

Changed 7 years ago by Bradley Baetz <bbaetz@…>

Attachment: valgrind.5837.gz added

Full valgrind log

Changed 7 years ago by Bradley Baetz <bbaetz@…>

Attachment: mythtv-ffmpeg-leak.patch added

patch

comment:1 Changed 7 years ago by Jim Stichnoth

Milestone: unknown0.27
Owner: set to Jim Stichnoth
Status: newaccepted

comment:2 Changed 7 years ago by Jim Stichnoth

Milestone: 0.270.26.1

comment:3 Changed 7 years ago by Bradley Baetz <bbaetz@…>

Resolution: fixed
Status: acceptedclosed

In 430900f7bce45d031ec948d867f0903e20544750/mythtv:

Fix a memory leak introduced in 92016b949. Fixes #11329.

Signed-off-by: Jim Stichnoth <jstichnoth@…>

Note: See TracTickets for help on using tickets.