GNOME Bugzilla – Bug 584838
basesrc: confusing QUERY_SEEKING handling
Last modified: 2009-07-24 10:58:21 UTC
Different interpretations of (response to) GST_QUERY_SEEKING seem possible: 1) Reply with "preferred" format and seekability in that format (typically TRUE if the preferred on), thereby discarding original format in query. This is currently done by basesrc which responds with the src's format and seekability (leading to a TRUE seekability in BYTES for filesrc for a TIME seek query). 2) It has been suggested it might be more appropriate/intuitive to preserve the original queried format, and to respond with the ability to seek in that format. This is currently done by oggdemux (and presumably widely elsewhere). The point here is to consider/interpret 1) as a bug :)
Created attachment 135949 [details] [review] Modify QUERY_SEEKING response Fix basesrc to act according to 2) above.
Makes sense. In the reply for an unsupported format, I would set the stop to -1, as we don't know the stop position in this format. Alternatively one can try to convert the input format to the format of the segment along with the stop position if any.
Created attachment 136008 [details] [review] Modify QUERY_SEEKING response Modified patch as suggested (use -1 for stop). No (gst_pad_query_convert) conversions have been done, since: * if seeking == FALSE, the other info is typically discarded anyway * other responders (e.g. demuxer) bother even less in e.g. non-TIME case
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.
Totem currently determines if seeking is possible in a file by querying for TIME. Since this change, seeking is disabled for MPEG, MKV and probably others (see bug #588944). What do you suggest?
IMO this fix is correct and just exposed a bug in these plugins and/or totem; let's discuss this further in #588944.
Alternatively, if the SEEKING query does not refer to basesrc's native format, it might return FALSE to the query itself (i.e. not handle it at all, as in "do not know about this format, whether TRUE or FALSE). That might reflect better that there is a "gap" somewhere (demuxer not handling SEEKING query) and give plugins some more slack.
> Alternatively, if the SEEKING query does not refer to basesrc's native format, > it might return FALSE to the query itself (i.e. not handle it at all, as in "do > not know about this format, whether TRUE or FALSE). That might reflect better > that there is a "gap" somewhere (demuxer not handling SEEKING query) and give > plugins some more slack. Maybe that's what we should be doing for the upcoming release then, so we don't break totem until the next set of plugin releases containing the fixes for this gets released?
Let's re-open this (sorry Jan!)
Where's your proposed patch? I'm trying to make pre-release tarballs :)
Created attachment 138927 [details] [review] don't answer SEEKING query when format doesn't match How about this patch?
Created attachment 138928 [details] [review] Proposed patch As suggested, only handle the seeking query if format matches src's format.
Please also add a FIXME 0.11 there, in 0.11 the current behaviour should be used IMHO as with the patch it's impossible to distinguish if a source can't seek in this format or if it only doesn't handle the SEEKING query. It might also make sense to revert to the current behaviour for 0.10.25 after all plugins are fixed.
I like Robin's patch better because it has a comment ;)
In the meantime, the SEEKING query has been implemented in many demuxers by Sebastian where it was missing: MPEG PS, MPEG TS, FLAC, FLV, MXF and Musepack. I don't know if this patch is still necessary (if there are any demuxers left which don't yet implement it). (By the way, I don't have push rights.)
> In the meantime, the SEEKING query has been implemented in many demuxers by > Sebastian where it was missing: MPEG PS, MPEG TS, FLAC, FLV, MXF and Musepack. > I don't know if this patch is still necessary (if there are any demuxers left > which don't yet implement it). It's still needed for all those poor souls who don't run up-to-date git, but only releases. They will get a newer core dropped in with the last -good/-ugly/-bad releases where the SEEKING query implementation is still missing for those demuxers/decoders. > (By the way, I don't have push rights.) I'll push it.
commit 527da05476e6d72ba04d1f11fe55eb797d88dc0d Author: Robin Stocker <robin@nibor.org> Date: Fri Jul 24 09:50:19 2009 +0100 basesrc: don't handle SEEKING queries for formats that don't match the one the source operates in Return FALSE in basesrc's default query handler when we get a SEEKING query for a format that's not the one the source operates in. Previously (ie. before, in the git version) we would return TRUE in that case and seekable=FALSE, which is more correct, but causes backwards compatibility problems. (Before that we would change the format of the query when answering, which was completely broken since callers don't expect that or check for it). Since the SEEKING query is a fairly recent addition, not all demuxers, parsers and decoders implement it yet, in which case any SEEKING query by an application will just be passed upstream where it will then be handled by basesrc. Now, if e.g. totem does a SEEKING query for TIME format and we have a demuxer that doesn't implement the query, basesrc would answer it with seekable=FALSE in most cases, and totem can only take that as authoritative answer, not knowing that the demuxer doesn't implement the SEEKING query. To avoid this, we make basesrc return FALSE to SEEKING queries in unhandled formats. That way applications like totem can fall back on assuming seekability depending on whether a duration is available, or somesuch. Downstream elements doing such queries are likely to equate an unhandled query with a non-seekable response as well, so this should be an acceptable fix for the time being. See #584838, #588944, #589423 and #589424.