After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 674801 - tsdemux: more memory leaks
tsdemux: more memory leaks
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 0.10.24
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-04-25 15:08 UTC by Holger Kaelberer
Modified: 2012-10-06 11:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Don't leak buffers when hitting PES header parsing errors (707 bytes, patch)
2012-04-25 15:08 UTC, Holger Kaelberer
none Details | Review
Valgrind output (394.36 KB, text/plain)
2012-04-25 15:20 UTC, Holger Kaelberer
  Details

Description Holger Kaelberer 2012-04-25 15:08:46 UTC
Created attachment 212789 [details] [review]
Don't leak buffers when hitting PES header parsing errors

tsdemux (0.10) leaks memory in some cases (with fixes from end of march/begginning of april included!). On live mpegts streams (DVB-S) we have seen about 15mb/min for some streams (Arte with 4 mpeg2 streams). It seems to happen only in streams with multiple audio-streams wheres single-audio streams don't seem to leak. When compared to pipelines with mpegtsdemux instead of tsdemux the same streams (and pipelines) don't show any leaks. Can be verified with a simple gst-launch playbin2 ... pipe.

Does not seem to be related to UNLINKED pads (tested with pads connected to input-selector with always-ok=TRUE/FALSE).

There are at least 2 sources of the leaks, with one identfied and attached patch:

1. Leaked buffers on PES header parsing erros as reported by valgrind's memcheck:

==25074== 398,728 (336,904 direct, 61,824 indirect) bytes in 3,662 blocks are definitely lost in loss record 5,247 of 5,248
==25074==    at 0x4026864: malloc (vg_replace_malloc.c:236)
==25074==    by 0x41C4B03: g_malloc (gmem.c:164)
==25074==    by 0x41DC8B0: g_slice_alloc (gslice.c:842)
==25074==    by 0x41DCB74: g_slice_alloc0 (gslice.c:854)
==25074==    by 0x415E8C6: g_type_create_instance (gtype.c:1869)
==25074==    by 0x40AC1E4: gst_mini_object_new (gstminiobject.c:217)
==25074==    by 0x40847CF: gst_buffer_new (gstbuffer.c:503)
==25074==    by 0x4085215: gst_buffer_create_sub (gstbuffer.c:761)
==25074==    by 0x50A50A1: gst_adapter_take_buffer (gstadapter.c:700)
==25074==    by 0x60ACFFF: mpegts_packetizer_next_packet (mpegtspacketizer.c:2310)
==25074==    by 0x60A5FDF: mpegts_base_chain (mpegtsbase.c:1382)
==25074==    by 0x60A657A: mpegts_base_loop (mpegtsbase.c:1558)

Tracing down buffer pointers revealed leaked buffers on "Error parsing PES header ...". The attached patch fixes this leak for us. As the tsdemux codebae is new to me, you should verify that this has no side effects ;-) Thanks!

This problem can be reproduced with a simple pipeline like

gst-launch filesrc location=/mnt/ltmp/vid/arte.ts ! tsdemux name=demux ! queue  ! mad ! autoaudiosink demux. ! queue ! mpeg2dec ! xvimagesink demux.

on the following captured stream:
http://www.math.uni-bielefeld.de/~hkaelber/vid/leak.ts

2. With this patch there are still smaller leaks (6MB/min) on some live streams with >1 audio stream (Arte, Pro7). These leaks seem to only show up on live streams. Could not yet reproduce with captured streams played back via filesrc. Still searching ... 

Thanks for any help. Please let me know if I can provide any further input to narrow down the root of the problem.

Thanks,
  Holger
Comment 1 Holger Kaelberer 2012-04-25 15:20:08 UTC
Created attachment 212794 [details]
Valgrind output

complete output of 

G_SLICE=always-malloc valgrind --trace-children=yes --tool=memcheck --leak-check=full gst-launch playbin2 uri=file:///mnt/ltmp/vid/leak.ts video-sink=xvimagesink &>valgrind-output.txt
Comment 2 Wim Taymans 2012-05-01 07:03:46 UTC
some more leaks where fixed:

commit 15293543e93ad0af80aee36d6408466716a02e43
Author: Wim Taymans <wim.taymans@collabora.co.uk>
Date:   Mon Apr 30 12:36:46 2012 +0200

    tsbase: unref bad packets too

commit 8019ab8f3a4665432d82564bfeb96e702084b01f
Author: Wim Taymans <wim.taymans@collabora.co.uk>
Date:   Mon Apr 30 12:28:42 2012 +0200

    tsdemux: reset the state of the stream when flushing
    
    We need to reset the stream on a flush or else old packets could be added to the
    list and leak.

commit b2716793c6f24b87a37b4858348f67652343cf3d
Author: Wim Taymans <wim.taymans@collabora.co.uk>
Date:   Mon Apr 30 12:28:31 2012 +0200

    tsdemux: add more debug

commit 1d0657ca233b35d6e04ccf9f9131a68e8d1b4b3c
Author: Wim Taymans <wim.taymans@collabora.co.uk>
Date:   Mon Apr 30 12:26:45 2012 +0200

    mpegts: check flowreturn before parsing next
    
    First check the flow return code before attempting to parse the next packet. If
    we do it the other way, we leak the parsed packet.
Comment 3 Holger Kaelberer 2012-05-04 07:59:12 UTC
Thanks for the commits. 
This looks good now for multiple audio-streams I set always-ok=TRUE in the input-selector.

BUT: with always-ok=FALSE I see memory usage increasing rapidly and in the beginning constantly, after a while this seems to jump about several MB per update step in top, but in the end growing ... Looks like leaking. Also with mpegtsdemux! Therefore, if leak, probably not an tsdemux specific issue. Can't imagine that I am the first to run into this. Do I miss something?
Comment 4 Holger Kaelberer 2012-05-04 16:57:16 UTC
I compared two clean stacks of gst plugins + libs (git 0.10 vs. RELEASE 0.10.36 stack) running the following pipe:

gst-launch playbin2 uri=file:///file/with/2/audio/streams video-sink=xvimagesink

This showed the following:

- 0.10.36 leaks vs. git 0.10 does not
- no tsdemux issue (tested also with flutsdemux and mpegtsdemux)
- 0.10.36 leaks only with always-ok=FALSE on input-selector pads
- 0.10.36 leaks only on input container with > 1 audio stream
- pipelines where identical
- tried to port obvious candidates from git to 0.10.36 (coreelements, tsdemux) which did not make a difference

So, this is neither a tsdemux issue nor a git 0.10 issue, the bug can be closed. Nonetheless we need to identify the lib/plugin responsible as we run 0.10.36 + backports from git as needed in production. Any hints from you git masters, which changes since 0.10.36 could be responsible for the seen differences?
Comment 5 Holger Kaelberer 2012-05-05 09:46:56 UTC
Found it, missed this one:

commit df6d0b06969f9fe2cc8a6238bca7a4a57c17e621
Author: Edward Hervey <edward.hervey@collabora.co.uk>
Date:   Mon Apr 2 15:13:24 2012 +0200

    baseparse: always attempt to push if not-linked
    
    This avoids ending up with plenty of pending data (since we'll only
     try to parse/push one frame from the incoming buffer).
    
    Fixes increasing memory consumption when parsers aren't linked

Puuhh ...