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 753887 - queue2: "Process SEEKING query" ok, but should handle SEEK events too (regression with non-seekable sources)
queue2: "Process SEEKING query" ok, but should handle SEEK events too (regres...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal blocker
: 1.6.0
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-08-20 16:58 UTC by Alexandre Jousset
Modified: 2015-08-31 11:27 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Python2.7 minimal test file server (2.34 KB, text/x-python)
2015-08-28 13:32 UTC, Alexandre Jousset
  Details
queue2: fix seeking query when operating as queue (1.37 KB, patch)
2015-08-29 09:14 UTC, Tim-Philipp Müller
none Details | Review

Description Alexandre Jousset 2015-08-20 16:58:15 UTC
Commit 7b0b93d (discussed here: #733351) introduced a regression: queue2 always returns TRUE to GST_QUERY_SEEKING if format == GST_FORMAT_BYTES, but does not handle SEEK events itself.

That means that this:

gst-launch playbin uri=http://192.168.0.1/Song_1.ogg

...fails if the HTTP server on 192.168.0.1 does not (wish to) support ranges (and announces it with the header "Accept-Ranges: none" because souphttpsrc, when receiving a is_seekable() query returns correctly FALSE, but unfortunately, queue2 wrongly told oggdemux that the stream was seekable, and it is not. When oggdemux sends SEEK events upstream, that fails.

I think this is more or less related to #705071 but the WIP then included is not very up to date.
Comment 1 Tim-Philipp Müller 2015-08-20 21:06:44 UTC
Note that returning TRUE to QUERY_SEEKING doesn't imply that it's seekable, only if TRUE is returned AND parsing out the values returns seekable=TRUE as well means it's seekable.
Comment 2 Sebastian Dröge (slomo) 2015-08-24 14:42:03 UTC
Should we just revert the other patch for now?
Comment 3 Jan Alexander Steffens (heftig) 2015-08-24 15:11:34 UTC
I don't remember the details all that well right now. I suppose it's not queue2 that provides the seeking, but the pull-activated element behind it. queue2 needs to report which range it has available while ring-buffering a live stream.
Comment 4 Alexandre Jousset 2015-08-28 12:35:01 UTC
@Jan: Please correct me if I don't understand correctly, but why doesn't queue2 provide the seeking if it returned the range itself and is the one that holds data? The "live stream" is not seekable (at least in my case) and if queue2 wants to "emulate" this it should manage it. Thus the subject of this bug report.

@Tim-Philipp: I debug-printed the returned value and the parsed value too. they are the same in that case: TRUE.
Comment 5 Tim-Philipp Müller 2015-08-28 12:49:56 UTC
Do you have a public server to reproduce this with, or a quick/easy way to set up something locally that will add "Accept-Ranges: none" ?
Comment 6 Alexandre Jousset 2015-08-28 13:32:45 UTC
Created attachment 310183 [details]
Python2.7 minimal test file server

Here is a simplified (maybe not enough, I'm not a Python dev) version of a test server we use.

- Binds to 0.0.0.0:8080
- Uses current directory as document root
- Sends (among others) the « Accept-Ranges: none » header.
Comment 7 Alexandre Jousset 2015-08-28 13:33:53 UTC
I added as attachment a small Python 2.7 script to setup a minimal server.

If you don't wish to execute it I can run it on a public server and give you the URL to fetch.
Comment 8 Tim-Philipp Müller 2015-08-28 13:54:27 UTC
Thanks, that's helpful. I'll try it in a bit.
Comment 9 Tim-Philipp Müller 2015-08-29 09:14:55 UTC
Created attachment 310238 [details] [review]
queue2: fix seeking query when operating as queue

Minimal patch.

Still, I am tempted to say we should just revert the patch

 commit 7b0b93dafe4ac547552cdb66ade5d8aa0405e7b4
 Author: Jan Alexander Steffens (heftig) <jan.steffens@gmail.com>
 Date:   Thu Jul 17 16:33:29 2014 +0200

    queue2: Process SEEKING query
    
    Add QUERY_SEEKING handling to queue2, so RTMP live streams become
    seekable when a queue2 in download or ringbuffer mode is inserted:
    
    rtmpsrc ! queue2 ! flvdemux
    
    https://bugzilla.gnome.org/show_bug.cgi?id=733351


in its entirety for now. I suspect that almost all demuxers/parsers will not be able to handle any non-0 based byte range for seeking and assume that the resource is seekable in its entirety. So this patch enables a very specialised use case whilst potentially causing problems for a lot of other use cases. Or perhaps it should be opt-in then.
Comment 10 Sebastian Dröge (slomo) 2015-08-31 08:07:38 UTC
I agree that we should revert the patch for now and then carefully reconsider it after 1.6 together with fixing all the relevant demuxers, parsers and other elements.
Comment 11 Tim-Philipp Müller 2015-08-31 11:27:52 UTC
commit c55bfecc55a98c820fe3b27b5228da8ceb384851
Author: Tim-Philipp Müller <tim@centricular.com>
Date:   Mon Aug 31 12:07:10 2015 +0100

    Revert "queue2: Process SEEKING query"
    
    This caused problems with oggdemux when queue2 was
    operating in queue mode and the souphttpsrc upstream
    is not seekable because the server doesn't support
    range requests. It would then still claim seekability
    and then things go wrong from there.
    
    This reverts commit 7b0b93dafe4ac547552cdb66ade5d8aa0405e7b4.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=753887