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 588944 - mpegpsdemux doesn't support seeking with GST_FORMAT_TIME anymore
mpegpsdemux doesn't support seeking with GST_FORMAT_TIME anymore
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal major
: 0.10.14
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2009-07-18 11:21 UTC by Robin Stocker
Modified: 2009-08-01 11:25 UTC
See Also:
GNOME target: ---
GNOME version: 2.25/2.26



Description Robin Stocker 2009-07-18 11:21:14 UTC
It seems there has been a regression in mpegpsdemux in that it doesn't support seeking with GST_FORMAT_TIME anymore. A query "gst_query_new_seeking (GST_FORMAT_TIME)" returns 0 for "seekable". With GST_FORMAT_BYTES it returns 1 for the same file.

The same also occurs with matroskademux, so the cause might be in code that both use. It doesn't occur with OGV and AVI files. I'm using the newest gstreamer and gst-plugins-* from git.

Severity is set to "major" as the end result is that e.g. in Totem, all VOB, MPEG and MKV files are no longer seekable.
Comment 1 Sebastian Dröge (slomo) 2009-07-19 10:01:51 UTC
Does this happen when the demuxer is in PAUSED or PLAYING state or only if it's in READY/NULL? Which was the last version where it worked for you?
Comment 2 Robin Stocker 2009-07-19 15:37:16 UTC
I only tested it in PAUSED and PLAYING state. It probably also happens in READY and NULL then.

As for when this regression happened, I don't know yet. I'd say it started about 1 month ago. I'll try to bisect gst-plugins-bad.
Comment 3 Tim-Philipp Müller 2009-07-19 15:48:45 UTC
FWIW, I think something changed in the totem backend. It seems to rely on the seeking query being implemented now to determine seekability. IIRC flv and matroska files are also affected.
Comment 4 Sebastian Dröge (slomo) 2009-07-19 18:39:38 UTC
(In reply to comment #3)
> FWIW, I think something changed in the totem backend. It seems to rely on the
> seeking query being implemented now to determine seekability. IIRC flv and
> matroska files are also affected.

Uh, that would be bad and should be reverted IMHO until everything actually supports the seeking query. A short look at the commit history doesn't show a commit like this though, maybe I missed it ;)

Comment 5 Sebastian Dröge (slomo) 2009-07-19 18:46:38 UTC
A short look at latest version from GIT shows, that totem assumes seekability if the seeking query succeeds and returns TRUE or if the seeking query failed (i.e. isn't implemented) but a duration is known.
Comment 6 Tim-Philipp Müller 2009-07-19 18:54:40 UTC
I know, I looked as well, but didn't see anything obvious. Maybe the point at which the checks are done changed? Just seems odd that multiple formats would 'break' in this respect at the same time, unless it was something core-related, but then you'd think it would affect all formats. In any case, we should fix the demuxers in question to support the SEEKING query, as you said.
Comment 7 Robin Stocker 2009-07-19 19:35:40 UTC
(In reply to comment #3)
> FWIW, I think something changed in the totem backend. It seems to rely on the
> seeking query being implemented now to determine seekability.

But totem has done that for more than a year, and it was possible to seek in these files.

I installed an old gst-plugins-bad (commit 1ef77b20f7879100adfe484651a6500014fcf11a, which is from 2008-11-27) and seeking doesn't work with it either.

So it has to be something in the core which changed. I'll bisect gstreamer now.
Comment 8 Robin Stocker 2009-07-19 20:46:03 UTC
Ok, I bisected gstreamer and the bad commit is:

commit 4bb3702886610bf258ce953cc52282ba5dab5c9d
Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk>
Date:   Fri Jun 5 11:37:24 2009 +0200

    basesrc: reply to QUERY_SEEKING with original format.  Fixes #584838.
Comment 9 Robin Stocker 2009-07-19 21:01:33 UTC
Added a comment to bug #584838.
Comment 10 Tim-Philipp Müller 2009-07-19 21:12:15 UTC
Oh, right, that makes perfect sense. Thanks for tracking this down. So before that commit, demuxers that didn't handle the SEEKING query would just pass the query upstream, and basesrc would then completely ignore the fact it was asked for seekability in TIME format and just reply with 'yes, look I can seek in BYTE format', and the demuxer would never know. And the query originator (totem) didn't double check the format after getting the query result, since it (rightfully) didn't expect it to change.

So, to sum up:

 - totem somehow doesn't do the 'assume seekability if
   we have a duration' fallback any more properly, which
   hasn't been noticed before because of that bug in basesrc

 - we should make all these demuxers implement SEEKING
   queries
Comment 11 Robin Stocker 2009-07-19 23:29:05 UTC
Yes. To clarify: totem only falls back on the duration when the query failed, not when it was answered with FALSE.
Comment 12 Robin Stocker 2009-07-20 22:53:29 UTC
Sebastian Dröge: I just saw your commit bb03d8ff18779f9cb9ebcb598373c101aaa239f7 on gst-plugins-good (http://cgit.freedesktop.org/gstreamer/gst-plugins-good/commit/?id=bb03d8ff18779f9cb9ebcb598373c101aaa239f7). Just to get some clarification on how demuxers should answer the seeking query; shouldn't the following line in your patch

  gst_query_set_seeking (query, GST_FORMAT_TIME, FALSE, -1, -1);

read like this?

  gst_query_set_seeking (query, fmt, FALSE, -1, -1);
Comment 13 Sebastian Dröge (slomo) 2009-07-21 06:00:22 UTC
Yes, you're right Robin. I've fixed that now, thanks :)
Comment 14 Sebastian Dröge (slomo) 2009-07-21 11:41:39 UTC
I hope this is correct, please try :)

commit 95e50d3598397a1248e756231a69114229a032e6
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Tue Jul 21 13:39:21 2009 +0200

    mpegpsdemux: Implement SEEKING query
    
    Fixes bug #588944.

commit 1f88ceeba8c908360d508cab5ee439eaff89309f
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Tue Jul 21 13:33:58 2009 +0200

    mpegtsdemux: Implement SEEKING query
    
    Partially fixes bug #588944.
Comment 15 Robin Stocker 2009-07-21 17:32:00 UTC
Ok, from cursory testing with the newest plugins from git:

Seeking now works for MKV, OGV, MPEG PS, AVI, MP3, OGG, WAV.

It doesn't work for FLAC, FLV, MPEG TS, AC3.
Comment 16 Tim-Philipp Müller 2009-07-21 17:52:01 UTC
> It doesn't work for FLAC, FLV, MPEG TS, AC3.

Don't think raw AC3 ever worked, or did it? (See bug #383478). 

Comment 17 Sebastian Dröge (slomo) 2009-07-21 18:04:42 UTC
MPEG TS should work (and does for me). Could you file new bugs for the others?
Comment 18 Robin Stocker 2009-07-22 19:54:36 UTC
The MPEG TS I tried was with AC3 in it, so that's probably why it didn't work here.

I filed bug #589423 for flacparse and #589424 for flvdemux.
Comment 19 Sebastian Dröge (slomo) 2009-07-23 05:59:53 UTC
MPEG TS with AC3 should still be seekable with commit 1f88ceeba8c908360d508cab5ee439eaff89309f. Can you try a different file maybe and if it doesn't work file a bug? :)
Comment 20 Robin Stocker 2009-08-01 11:25:39 UTC
Ok, filed bug #590446 for MPEG TS.