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 733351 - queue2: Process SEEKING query
queue2: Process SEEKING query
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other All
: Normal enhancement
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-07-18 11:49 UTC by Jan Alexander Steffens (heftig)
Modified: 2018-11-03 12:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (2.97 KB, patch)
2014-07-18 11:49 UTC, Jan Alexander Steffens (heftig)
needs-work Details | Review
patch (3.15 KB, patch)
2014-11-12 13:39 UTC, Jan Alexander Steffens (heftig)
none Details | Review
patch (2.58 KB, patch)
2015-03-16 10:06 UTC, Jan Alexander Steffens (heftig)
needs-work Details | Review

Description Jan Alexander Steffens (heftig) 2014-07-18 11:49:54 UTC
Created attachment 281085 [details] [review]
patch

In order to seek FLV streams, flvdemux creates a seek index;
however, this index is not created if upstream is not seekable.
gst_flv_demux_check_seekability was copied nearly verbatim from
GstBaseParse.

This commit adds QUERY_SEEKING handling to queue2, so RTMP live
streams become seekable when a queue2 in download or ringbuffer
mode is inserted:

rtmpsrc ! queue2 ! flvdemux

Alternatively, flvdemux could be altered to not require
seekability. I am unsure what is the best approach.
Comment 1 Jan Alexander Steffens (heftig) 2014-07-21 11:33:37 UTC
A patch for the mentioned alternative is at #733494.
Comment 2 Jan Alexander Steffens (heftig) 2014-08-07 13:28:53 UTC
Any comment?
Comment 3 Sebastian Dröge (slomo) 2014-08-11 08:49:01 UTC
Review of attachment 281085 [details] [review]:

::: plugins/elements/gstqueue2.c
@@ +3043,3 @@
+        if (segment_end < 0) {
+          GST_DEBUG_OBJECT (queue, "fixing seekable segment to maximum");
+          segment_end = G_MAXINT64;

IIRC -1 for the seeking query also has the meaning that the complete range can be seeked.


However I'm not sure it's valid to assume that upstream is seekable if the seeking query failed upstream. The queue will be seekable in the already downloaded area, but otherwise no seeking is possible.
Comment 4 Jan Alexander Steffens (heftig) 2014-08-11 13:11:32 UTC
The code in baseparse (and flvdemux) that handles checking seekability is:

  gst_query_parse_seeking (query, NULL, &seekable, &start, &stop);

  /* try harder to query upstream size if we didn't get it the first time */
  if (seekable && stop == -1) {
    GST_DEBUG_OBJECT (parse, "doing duration query to fix up unset stop");
    gst_pad_peer_query_duration (parse->sinkpad, GST_FORMAT_BYTES, &stop);
  }

  /* if upstream doesn't know the size, it's likely that it's not seekable in
   * practice even if it technically may be seekable */
  if (seekable && (start != 0 || stop <= start)) {
    GST_DEBUG_OBJECT (parse, "seekable but unknown start/stop -> disable");
    seekable = FALSE;
  }

I want to seek in a livestream that does not have a known duration, so the duration query also returns -1 and this code claims unseekable.
Comment 5 Jan Alexander Steffens (heftig) 2014-11-12 13:39:56 UTC
Created attachment 290514 [details] [review]
patch

Here's a slightly different version that uses the current range to fill in the seekable segment if upstream is not seekable.
Comment 6 Jan Alexander Steffens (heftig) 2015-03-10 10:42:15 UTC
Please review the patch.
Comment 7 Sebastian Dröge (slomo) 2015-03-11 16:03:50 UTC
Review of attachment 290514 [details] [review]:

Mostly looks good, I would call that function something like handle_seeking_query() or similar though.

::: plugins/elements/gstqueue2.c
@@ +2923,3 @@
+  if (segment_end <= segment_start) {
+    GST_DEBUG_OBJECT (queue, "no range to seek");
+    return FALSE;

I think this should not fail the query but just set it to unseekable.
Comment 8 Jan Alexander Steffens (heftig) 2015-03-16 10:06:41 UTC
Created attachment 299493 [details] [review]
patch

Refactored the code again. If upstream is not seekable, the query now fails only when format != BYTES.
Comment 9 Sebastian Dröge (slomo) 2015-03-23 09:48:12 UTC
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
Comment 10 Tim-Philipp Müller 2015-08-31 11:29:59 UTC
Comment on attachment 299493 [details] [review]
patch

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
Comment 11 GStreamer system administrator 2018-11-03 12:21:35 UTC
-- 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/66.