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 785957 - adaptivedemux: leaks pad probes, causing gradual increase in CPU
adaptivedemux: leaks pad probes, causing gradual increase in CPU
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal major
: 1.12.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-08-07 17:48 UTC by Tom Bailey
Modified: 2017-08-08 17:15 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch for IDLE probe leak (1.94 KB, patch)
2017-08-07 17:48 UTC, Tom Bailey
none Details | Review
Patch for leaking pad probes (2nd attempt) (1.12 KB, patch)
2017-08-08 13:03 UTC, Tom Bailey
committed Details | Review

Description Tom Bailey 2017-08-07 17:48:19 UTC
Created attachment 357134 [details] [review]
Patch for IDLE probe leak

While testing our application it was observed that CPU utilization slowly increased over time, which on low spec (i.e. embedded) hardware caused playback to halt after several hours. This was tracked down to code in adaptivedemux which added an IDLE probe to the elements source pad every time it downloaded a fragment. These probes were never removed meaning an ever-increasing number of probe callbacks had to be dispatched.

The attached patch fixes this resource leak by removing the IDLE probe when it is no longer needed. It would probably be more optimal to use the same probe throughout the lifetime of the element but the current logic relies on the fact that the pad calls the probe callback when it is added if the pad is currently idle, and it is not possible to determine this from outside the pad
Comment 1 Tim-Philipp Müller 2017-08-07 18:32:26 UTC
Comment on attachment 357134 [details] [review]
Patch for IDLE probe leak

I wonder if just returning GST_PAD_PROBE_REMOVE from the probe would do the trick as well here? As far as I can tell we always wait for the probe to be called.
Comment 2 Tom Bailey 2017-08-08 13:02:57 UTC
Hi Tim-Philipp,

That's a great suggestion, I didn't think of that. I've tried it and it also fixes the problem of leaking pad probes. I've updated the patch.

Tom
Comment 3 Tom Bailey 2017-08-08 13:03:46 UTC
Created attachment 357190 [details] [review]
Patch for leaking pad probes (2nd attempt)
Comment 4 Tim-Philipp Müller 2017-08-08 17:15:18 UTC
Many thanks for the patch, pushed to master and 1.12 branch:

commit 4064683d36ba8eb8732b904be0bdaedc38b013fc
Author: Tom Bailey <tom.bailey@youview.com>
Date:   Wed Aug 2 18:17:08 2017 +0100

    adaptivedemux: Fix leak of pad probes in GstAdaptiveDemuxStream
    
    This commit ensures that the idle probe which GstAdaptiveDemuxStream
    adds to the upstream source pad is removed after use. Previously a new
    probe was added to the pad whenever a fragment was downloaded, meaning
    the number of pad probe callbacks being executed increased continually.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=785957