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 613351 - gst_element_seek() should have async counterpart
gst_element_seek() should have async counterpart
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal enhancement
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 617905
 
 
Reported: 2010-03-19 18:38 UTC by Bastien Nocera
Modified: 2016-11-12 14:03 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Bastien Nocera 2010-03-19 18:38:31 UTC
As I understand it, gst_element_seek() is nearly all async, apart from sometimes computationally intensive operations (index searches).

My current worst case scenario, in Totem, when seeking in a 6BG full HD Matroska file, looks about like this:
(bacon_video_widget_seek_time_no_lock:3883) took 0.176199 seconds
(totem_action_seek:1116) took 0.176254 seconds

I regularly see calls taking 0.1 sec, but a lot of them take 0.01 sec.
Comment 1 Sebastian Dröge (slomo) 2010-03-20 07:07:22 UTC
Well, it's not really async. Some elements even do IO do search for the new positions and I think we should keep it like that, otherwise it will be more complex for elements to report if a seek was really possible and if it wasn't to simply continue from the last position.

A gst_element_seek_async(), which does all this from another thread and calls a callback once finished, is a good idea though.
Comment 2 Wim Taymans 2012-06-15 14:21:28 UTC
I think it should always be async, the seek event returning TRUE when the seek event was queued to be handled and progress messages to report on completion/success/cancelation/failure of the seek operation.
Comment 3 Tim-Philipp Müller 2012-07-09 16:27:46 UTC
Same here, I think elements should not be doing I/O from the seeking thread (i.e. the application thread).

The TRUE/FALSE notification from the seek handler is not really useful in practice anyway, and to check seekability in principle we now have the SEEKING query anyway.

If notification is a concern, we could use PROGRESS message to notify the application of start/success/error, though perhaps that's more interesting for when we're bisecting in push mode or so.
Comment 4 Sebastian Dröge (slomo) 2012-07-10 07:24:20 UTC
Agreed, but the PROGRESS messages are useful in general here to notify the application that seeking succeeded or failed in the end. Not only to notify about any intermediate progress. Just has to be implemented in all the elements.
Comment 5 Edward Hervey 2013-07-18 06:24:15 UTC
Do we still have elements that do the actual blocking seek in the seeking thread (instead of the streaming thread) ?

In order to validate a specific seek has completed, we should have a way to report (via a message and its seqnum) the seek completed by using the same seqnum as the seek event.

If a certain seek is going to take a long time, shouldn't buffering messages be posted ?
Comment 6 Sebastian Dröge (slomo) 2013-07-18 09:06:34 UTC
(In reply to comment #5)
> Do we still have elements that do the actual blocking seek in the seeking
> thread (instead of the streaming thread) ?

Yes, most demuxers and also baseparse.

> In order to validate a specific seek has completed, we should have a way to
> report (via a message and its seqnum) the seek completed by using the same
> seqnum as the seek event.

The segment event after a seek is supposed to have the same seqnum as the seek event.

> If a certain seek is going to take a long time, shouldn't buffering messages be
> posted ?

PROGRESS messages probably, there's no buffering happening.
Comment 7 Tim-Philipp Müller 2016-11-12 12:44:57 UTC
I'm closing this bug because I don't think it's useful in the current form.

I have cloned bug #774314 for baseparse and bug #774315 for matroskademux for starters. Other elements can follow once we're done with those.
Comment 8 Bastien Nocera 2016-11-12 13:51:26 UTC
(In reply to Tim-Philipp Müller from comment #7)
> I'm closing this bug because I don't think it's useful in the current form.
> 
> I have cloned bug #774314 for baseparse and bug #774315 for matroskademux
> for starters. Other elements can follow once we're done with those.

Once you're done with those, qtdemux for MPEG-4 support would be a requirement for me to consider this fixed, if it indeed affects MPEG-4.
Comment 9 Tim-Philipp Müller 2016-11-12 14:03:33 UTC
Not sure if qtdemux does any data access in the seek handler. It will usually have all the info from the index already. Maybe it needs to read/unpack some index bits if it doesn't keep it all in memory, but I didn't see it doing that from the seek handler.