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 438610 - [dvddemux] fails after flushing seek
[dvddemux] fails after flushing seek
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-ugly
0.10.5
Other Linux
: Normal normal
: 0.10.6
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2007-05-15 14:23 UTC by Mark Nauwelaerts
Modified: 2007-05-16 12:52 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to clear stored flow return (3.72 KB, patch)
2007-05-15 14:26 UTC, Mark Nauwelaerts
none Details | Review
slightly cleaner version (4.27 KB, patch)
2007-05-16 12:52 UTC, Tim-Philipp Müller
committed Details | Review

Description Mark Nauwelaerts 2007-05-15 14:23:14 UTC
Basically, dvddemux stores downstream flow returns so it can aggregate these.
In case of a flushing seek, FLUSH_START comes through and can make (some) downstream subsequently return WRONG_STATE.  If the seek (and FLUSH_STOP) have finished and streaming resumes, the stored WRONG_STATE has not been cleared, making dvddemux (or mpegdemux) wrongly conclude/aggregate there is a real problem downstream.  This typically then leads to the pipeline pausing/terminating.

[well, yes, there is flupsdemux as well, but it does not (yet) seem to have a taste for subtitles, and that makes working/testing on bug #415754 a bit difficult ;-)]
Comment 1 Mark Nauwelaerts 2007-05-15 14:26:31 UTC
Created attachment 88208 [details] [review]
Patch to clear stored flow return

* When receiving FLUSH_STOP; clear internally stored downstream flow returns

Note this patches across dvddemux and mpegdemux (due to inheritance etc involved).
Comment 2 Tim-Philipp Müller 2007-05-15 16:36:40 UTC
Argh, what a dumb mistake. I bet this also solves the problems people have with Thoggen's cropping dialog.

Is there a reason for the extensive

  if (flow != GST_FLOW_NOT_LINKED) stream->last_flow = GST_FLOW_OK

logic that I'm missing (ie. opposed to just resetting the last_flow), or is this just a copy'n'paste artefact?

Comment 3 Mark Nauwelaerts 2007-05-15 19:15:46 UTC
Use of the flow variable may indeed be an artefact.
As for the if-logic, I thought it was not necessary (or prudent) to reset a state indicating the pad is not linked.  Then again, it may also not hurt doing the reset anyway, and letting it be re-discovered that some are not linked.
Comment 4 Tim-Philipp Müller 2007-05-16 12:52:21 UTC
Created attachment 88287 [details] [review]
slightly cleaner version
Comment 5 Tim-Philipp Müller 2007-05-16 12:52:58 UTC
Thanks a lot, committed:

 2007-05-16  Tim-Philipp Müller  <tim at centricular dot net>

        Based on patch by: Mark Nauwelaerts  <manauw skynet be>

        * gst/mpegstream/gstdvddemux.c: (gst_dvd_demux_process_event):
        * gst/mpegstream/gstmpegdemux.c: (gst_mpeg_demux_class_init),
        (gst_mpeg_demux_process_event), (gst_mpeg_streams_reset_last_flow):
        * gst/mpegstream/gstmpegdemux.h:
          Reset last_flow values for the various streams after a flushing
          seek, otherwise we might aggregate wrong flow returns afterwards
          that will make upstream pause silently. This should fix seeking
          in DVDs and also fix the Thoggen cropping dialog (#438610).

Can't believe this wasn't noticed before the 0.10.5 release :-/