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 797098 - downloadbuffer: does not recover from failed seek
downloadbuffer: does not recover from failed seek
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-09-07 20:14 UTC by Alicia Boya García
Modified: 2018-11-03 12:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
downloadbuffer: Handle failed seeks (1.76 KB, patch)
2018-09-07 23:57 UTC, Alicia Boya García
needs-work Details | Review

Description Alicia Boya García 2018-09-07 20:14:14 UTC
This is some code from perform_seek_to_offset() from gstdownloadbuffer.c

  /* until we receive the FLUSH_STOP from this seek, we skip data */
  dlbuf->seeking = TRUE;
  dlbuf->write_pos = offset;
  dlbuf->filling = FALSE;
  GST_DOWNLOAD_BUFFER_MUTEX_UNLOCK (dlbuf);

  GST_DEBUG_OBJECT (dlbuf, "Seeking to %" G_GUINT64_FORMAT, offset);

  event =
      gst_event_new_seek (1.0, GST_FORMAT_BYTES,
      GST_SEEK_FLAG_FLUSH | GST_SEEK_FLAG_ACCURATE, GST_SEEK_TYPE_SET, offset,
      GST_SEEK_TYPE_NONE, -1);

  res = gst_pad_push_event (dlbuf->sinkpad, event);
  GST_DOWNLOAD_BUFFER_MUTEX_LOCK (dlbuf);

  return res;

Problem is: what happens if the seek fails? `write_pos` has already been set to the attempted seek position.

What I've seen happening next is that then downloadbuffer waits endlessly for that data with the offset of the seek to arrive; which never happens because gst_download_buffer_chain() has this check:

  /* while we didn't receive the newsegment, we're seeking and we skip data */
  if (dlbuf->seeking)
    goto out_seeking;

(and downloadbuffer falsely believes the seek went through)
Comment 1 Alicia Boya García 2018-09-07 23:57:10 UTC
Created attachment 373561 [details] [review]
downloadbuffer: Handle failed seeks

downloadbuffer should be usable with non-seekable sources too, even if
that sometimes requires downloading the entire file to disk on one go.
Comment 2 Alicia Boya García 2018-09-08 00:17:44 UTC
Review of attachment 373561 [details] [review]:

::: plugins/elements/gstdownloadbuffer.c
@@ +626,3 @@
 
+  /* Note: We keep the lock while waiting for the event. Should the seek fail,
+   * we don't want the chain function to lose any data. */

This won't work when the seek succeeds, it will deadlock when the source synchronously sends an event (e.g. appsrc sends FLUSH_START).
Comment 3 GStreamer system administrator 2018-11-03 12:47:58 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/309.