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 771479 - streamsynchronizer: Setting invalid segment.base if SEEK events are handled in parallel and some streams are finished flushing before others started
streamsynchronizer: Setting invalid segment.base if SEEK events are handled i...
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-09-15 09:13 UTC by Sebastian Dröge (slomo)
Modified: 2018-11-03 11:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
basesink: Use timestamp based running time instead of element start time (2.12 KB, patch)
2016-09-19 14:02 UTC, Sebastian Dröge (slomo)
rejected Details | Review

Description Sebastian Dröge (slomo) 2016-09-15 09:13:04 UTC
Problem here is that we send steps only to the video sink, which makes sense. BUFFERS format makes no sense for audio, but for video it is frames.

That however causes the audio sink to not move anywhere, which has to negative side effects:

1) in reverse mode, this means that the video position goes backwards with every step but audio stays the same. The position query returns the maximum of all values, so it always returns the (constant) audio position

2) in general, we move video but not audio. After a while queues run full and we lock up because the audio did not consume any data at the same time.


My idea for a solution here is to keep track of how much video we stepped (position queries and the step messages), and then do a TIME step on the audio sink in the same way to catch up. And only then forward the step-done message.
Comment 1 Sebastian Dröge (slomo) 2016-09-17 21:04:43 UTC
That it already does, but there are other problems. I have a fix but need to debug it a little bit more.
Comment 2 Sebastian Dröge (slomo) 2016-09-19 14:02:48 UTC
Created attachment 335858 [details] [review]
basesink: Use timestamp based running time instead of element start time

This is not correct, but it works around the problem of flush events
happening in the middle of a step event, which breaks running time
calculations and causes the step event to step for a huge amount usually.
Comment 3 Sebastian Dröge (slomo) 2016-09-20 15:32:08 UTC
So the problem here is basically https://bugzilla.gnome.org/show_bug.cgi?id=750013#c15 and https://bugzilla.gnome.org/show_bug.cgi?id=760408

When streamsynchronizer gets a flush-stop, it takes over the group start time from the furthest non-flushes stream. This generally works fine if SEEK events are handled by a single demuxer, and it flushes all streams one after another before it starts again.

However in GES (or when using multiple uridecodebins), this is not the case. We then end up with the group start time after a flushing seek to be either a) the old one, or b) some random value based on where we're at at this point.


The solution for this seems to be to ensure that a flushing seek in streamsynchronizer always blocks and serializes all the flush-stop events. This however can lead to deadlocks in the multiple uridecodebins case, or when only some but not all streams are seeked (as seeks are per stream, but we need to combine all here). It also doesn't handle properly if there is a new seek while the current one is still being handled (sidenote: aggregator seems to fail on that too).

We also can't just reset the group start time whenever a single flush-stop is received, as some streams might already/still be running while others are being flushed currently... and we might end up with wrong values then for the segment events.
Comment 4 Sebastian Dröge (slomo) 2016-09-20 19:14:18 UTC
commit 0c151f6bb27b3261a0a813720bbcb1fa9e9c7279
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Tue Sep 20 15:12:22 2016 -0400

    streamsynchronizer: Correctly calculate group start times in reverse playback mode
    
    We have to calculate from the segment.stop, not the segment.start, as
    playback goes from stop to start. This fix works around another race
    condition in streamsynchronizer in my testcase.
    
    See https://bugzilla.gnome.org/show_bug.cgi?id=771479


Still the original bug is valid.
Comment 5 GStreamer system administrator 2018-11-03 11:49:59 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/issues/293.