GNOME Bugzilla – Bug 761439
basesink: Backward step operations fail and may lead to negative position value in Totem
Last modified: 2018-11-03 12:32:48 UTC
To reproduce: - start playing a video - (optional to make bug more obvious) seek in the middle - hold comma Notice that the state of the timeline becomes completely inconsistent. See attached screenshot of Big Buck Bunny, but this seems reproducible with any video.
Created attachment 320247 [details] screenshot
I'll assume this is easily reproduceable with the "seek" test program in GStreamer, and is possibly container specific.
We'll definitely need more research then that.
Cosimo, can you please try to grab the GStreamer debug log as per: https://wiki.gnome.org/Apps/Videos/BugReporting ?
This is the code that does the stepping. Nothing too complicated really: https://git.gnome.org/browse/totem/tree/src/backend/bacon-video-widget.c#n4195
Created attachment 320387 [details] log
Just for clarity, is that only with Theora/Ogg ?
Nicolas, no I can reproduce also with an MP4 container with H.264 video.
I confirm, looks bad. So backward stepping seems to break apart. I don't see anything wrong in totem, I do see bunch of assertions: (totem:22123): GStreamer-CRITICAL **: gst_segment_do_seek: assertion 'start <= stop' failed A small unrelated note though, if you step back (and it didn't fail like this ^, totem stays in backward playback mode, not sure this is the expected behaviour, but who knows, I didn't know totem was exposing this kind of stuff).
You can also use gst-plugins-base/tests/examples/playback for reproducing these kind of things btw, it might give more control over what exactly is happening than totem. For the backward playback, to do backwards stepping you need to first issue a negative-rate seek. Stepping can only happen in the current playback direction. Also it should be completely unrelated to container format and codecs. Stepping is handled completely in basesink (except for the seeks to change playback direction).
(In reply to Sebastian Dröge (slomo) from comment #10) > You can also use gst-plugins-base/tests/examples/playback for reproducing > these kind of things btw, it might give more control over what exactly is > happening than totem. > > For the backward playback, to do backwards stepping you need to first issue > a negative-rate seek. Stepping can only happen in the current playback > direction. Totem does that, I have reviewed. It does not wait for the preroll to complete, so might be a bit racy, but the behaviour indicate a bug in any case. I'm surprised though, as I thought one could flip the direction without having to seek to a start/stop position (passing NONE as positions, seek still need to be flushing I suppose).
Yes, seeking with start=stop=NONE should be possible (separate bugs otherwise). But you definitely need to seek. You don't necessarily need to FLUSH seek though, it just takes longer for the direction change to take place otherwise. So yes there's definitely a bug here, I just mean that the playback example could be useful for debugging here as it gives more control than totem.
This is still 100% reproducible on current Fedora 26 with GStreamer 1.11.2.
I think this looks wrong: 5890 GST_DEBUG ("Setting playback direction to %s at %"G_GINT64_FORMAT"", DIRECTION_STR, cur); 5891 event = gst_event_new_seek (target_rate, 5892 GST_FORMAT_TIME, GST_SEEK_FLAG_FLUSH | GST_SEEK_FLAG_ACCURATE, 5893 GST_SEEK_TYPE_SET, forward ? cur : G_GINT64_CONSTANT (0), 5894 GST_SEEK_TYPE_SET, forward ? G_GINT64_CONSTANT (0) : cur); 5895 if (gst_element_send_event (bvw->priv->play, event) == FALSE) { 5896 GST_WARNING ("Failed to set playback direction to %s", DIRECTION_STR); 5897 } else { 5898 gst_element_get_state (bvw->priv->play, NULL, NULL, GST_CLOCK_TIME_NONE); 5899 bvw->priv->rate = target_rate; In the seek event, if we're going forward, both start and stop are set, and the segment is set from cur to 0. Should it not be from cur to something unset ?
(In reply to Vincent Penquerc'h from comment #14) > I think this looks wrong: > > 5890 GST_DEBUG ("Setting playback direction to %s at > %"G_GINT64_FORMAT"", DIRECTION_STR, cur); > 5891 event = gst_event_new_seek (target_rate, > 5892 GST_FORMAT_TIME, GST_SEEK_FLAG_FLUSH | GST_SEEK_FLAG_ACCURATE, > 5893 GST_SEEK_TYPE_SET, forward ? cur : G_GINT64_CONSTANT (0), > 5894 GST_SEEK_TYPE_SET, forward ? G_GINT64_CONSTANT (0) : cur); > 5895 if (gst_element_send_event (bvw->priv->play, event) == FALSE) { > 5896 GST_WARNING ("Failed to set playback direction to %s", > DIRECTION_STR); > 5897 } else { > 5898 gst_element_get_state (bvw->priv->play, NULL, NULL, > GST_CLOCK_TIME_NONE); > 5899 bvw->priv->rate = target_rate; > > In the seek event, if we're going forward, both start and stop are set, and > the segment is set from cur to 0. Should it not be from cur to something > unset ? You mean passing -1 instead of 0 as the "stop" argument? That's what it throws: (totem:21224): GStreamer-CRITICAL **: gst_segment_position_from_running_time: assertion 'segment->format == format' failed The documentation isn't exactly great explaining what the stop value is, but it says: " For negative rates, playback will start from the newly configured stop position (if any). If the stop position is updated, it must be different from -1 (GST_CLOCK_TIME_NONE) for negative rates. " So 0 looks correct.
I mean passing GST_SEEK_TYPE_NONE instead of 0, in the forward case (so positive rate). AFAICT, the forward case in the code above creates a seek from current to 0. I guess I'm missing something here, I'm just not sure what.
Created attachment 351960 [details] [review] fix wrong time when stepping backwards Tracked down.
Comment on attachment 351960 [details] [review] fix wrong time when stepping backwards Can you please make a unit test for this? It seems to make sense but it might break again if you don't write a test :)
Created attachment 352509 [details] [review] with unit test
With patch and unit test.
-- 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/gstreamer/issues/154.