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 772742 - mpegtsdemux: Implement efficient program updates
mpegtsdemux: Implement efficient program updates
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
unspecified
Other All
: Normal normal
: 1.10.0
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on: 772741
Blocks:
 
 
Reported: 2016-10-11 10:39 UTC by Edward Hervey
Modified: 2016-10-31 14:35 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
mpegtsdemux: Implement efficient program updates (11.10 KB, patch)
2016-10-11 10:39 UTC, Edward Hervey 		needs-work Details | Review

Description Edward Hervey 2016-10-11 10:39:18 UTC 
See commit message
Comment 1 Edward Hervey 2016-10-11 10:39:23 UTC 
Created attachment 337397 [details] [review]
mpegtsdemux: Implement efficient program updates

If the parent bin can handle it, only add/remove the new/gone stream
instead of re-adding/re-moving everything
Comment 2 Edward Hervey 2016-10-12 13:12:15 UTC 
commit 6622e2dacf3d61d880ac7bcccf5680f62ccac7a0
Author: Edward Hervey <edward@centricular.com>
Date:   Tue Oct 11 11:11:16 2016 +0200

    mpegtsdemux: Implement efficient program updates
    
    If the parent bin can handle it, only add/remove the new/gone stream
    instead of re-adding/re-moving everything
    
    https://bugzilla.gnome.org/show_bug.cgi?id=772742
Comment 3 Sebastian Dröge (slomo) 2016-10-12 13:42:39 UTC 
Review of attachment 337397 [details] [review]:

more review later

::: gst/mpegtsdemux/mpegtsbase.c
@@ +213,3 @@
+  base->streams_aware = GST_OBJECT_PARENT (base)
+      && GST_OBJECT_FLAG_IS_SET (GST_OBJECT_PARENT (base),
+      GST_BIN_FLAG_STREAMS_AWARE);

This is not threadsafe and will also be wrong if the element is added to the bin after this.
Comment 4 Edward Hervey 2016-10-13 07:08:34 UTC 
_reset() is called in 3 places:
* init (when created)
* READY=>PAUSED
* PAUSED=>READY

Since elements are added *before* being set to PAUSED, this is not an issue. Usage of that streams-aware value will also be in the streaming-thread, i.e. after _reset() is called.

Elements that are streams-aware will (or should) set this when initializing themselves (they either are or are not streams-aware). So not an issue there again.
Comment 5 Sebastian Dröge (slomo) 2016-10-13 09:15:54 UTC 
Ok then! :)