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 759125 - GstBin: async-handling latency handling is decoupled from parent pipeline
GstBin: async-handling latency handling is decoupled from parent pipeline
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Mac OS
: Normal normal
: 1.7.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-12-07 15:23 UTC by Heinrich Fink
Modified: 2015-12-09 09:15 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
bin: Don't do latency handling if async-handling=true (890 bytes, patch)
2015-12-07 15:26 UTC, Sebastian Dröge (slomo)
none Details | Review
bin: Don't do latency handling if async-handling=true (1.25 KB, patch)
2015-12-07 15:37 UTC, Sebastian Dröge (slomo)
rejected Details | Review
bin: Post a LATENCY message with async-handling=TRUE if the PLAYING state is reached (1.44 KB, patch)
2015-12-08 16:32 UTC, Sebastian Dröge (slomo)
none Details | Review
bin: Post a LATENCY message with async-handling=TRUE if the PLAYING state is reached (1.39 KB, patch)
2015-12-09 09:14 UTC, Sebastian Dröge (slomo)
committed Details | Review

Description Heinrich Fink 2015-12-07 15:23:44 UTC
Since async-handling attempts to emulate a top-level pipeline object, latencies are configured within a GstBin with async-handling=TRUE independently of (a) fixed latencies set by gst_pipeline_set_latency and (b) other negotiated latencies in the pipeline. 

(a) will make it impossible to use bins with async-handling in a live pipeline where the overall latency has to be controlled by a fixed value (since the bin will ignore it and configure whatever it sees fit)

(b) if a bin with async-handling=TRUE has one sink, and the hosting pipeline of such a bin has other sinks (or other bins with sinks), then those sinks won't be in-sync with the one of the bin with async-handling, because potentially each bin might configure a different latency.
Comment 1 Sebastian Dröge (slomo) 2015-12-07 15:26:32 UTC
Created attachment 316882 [details] [review]
bin: Don't do latency handling if async-handling=true

Otherwise each bin might have a different latency in the end, causing
synchronization problems.
Comment 2 Sebastian Dröge (slomo) 2015-12-07 15:27:49 UTC
This has the problem that latency configuration would *never* happen unless explicitly triggered for such bins. Maybe for async-handling=true we should post a LATENCY message instead in this place?
Comment 3 Sebastian Dröge (slomo) 2015-12-07 15:37:14 UTC
Created attachment 316884 [details] [review]
bin: Don't do latency handling if async-handling=true

Otherwise each bin might have a different latency in the end, causing
synchronization problems.
Comment 4 Olivier Crête 2015-12-07 17:40:07 UTC
Changing the behaviour to not do the latency calculation here is an API break. I think the best we can do is to post a message if it's not truly the top level bin. Doing the latency queries in the bin and then re-doing later from the main loop should be fine. Ie, s/else if/if/ and leave the toplevel check as-is.
Comment 5 Sebastian Dröge (slomo) 2015-12-08 16:32:28 UTC
Created attachment 316955 [details] [review]
bin: Post a LATENCY message with async-handling=TRUE if the PLAYING state is reached

Otherwise each bin might have a different latency in the end, causing
synchronization problems.

The bin will still first handle latency internally as before, but gives the
overall pipeline the opportunity to update the latency of the whole pipeline
afterwards.
Comment 6 Sebastian Dröge (slomo) 2015-12-08 16:33:46 UTC
Good point, what about this one?

It's not great but otherwise applications that e.g. don't handle LATENCY messages at all would just stop working properly now.
Comment 7 Olivier Crête 2015-12-08 17:29:14 UTC
Review of attachment 316955 [details] [review]:

::: gst/gstbin.c
@@ +2673,2 @@
       GST_OBJECT_LOCK (bin);
+      toplevel = GST_OBJECT_PARENT (bin) == NULL;

Why change the definition of toplevel?
Comment 8 Sebastian Dröge (slomo) 2015-12-08 17:42:03 UTC
Unintentional. Will fix that in a bit :)
Comment 9 Sebastian Dröge (slomo) 2015-12-09 09:14:36 UTC
Created attachment 317009 [details] [review]
bin: Post a LATENCY message with async-handling=TRUE if the PLAYING state is reached

Otherwise each bin might have a different latency in the end, causing
synchronization problems.

The bin will still first handle latency internally as before, but gives the
overall pipeline the opportunity to update the latency of the whole pipeline
afterwards.
Comment 10 Sebastian Dröge (slomo) 2015-12-09 09:14:52 UTC
Attachment 317009 [details] pushed as 2d427c7 - bin: Post a LATENCY message with async-handling=TRUE if the PLAYING state is reached