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 668762 - regression: playbin2: dvd menu doesn't come up with flags |= deinterlace
regression: playbin2: dvd menu doesn't come up with flags |= deinterlace
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal blocker
: 0.10.24
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-01-26 18:06 UTC by Tim-Philipp Müller
Modified: 2012-03-12 16:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
resindvd: fix wedge in preroll when playbin2 deinterlacing is enabled (1.71 KB, patch)
2012-01-30 18:49 UTC, Vincent Penquerc'h
committed Details | Review

Description Tim-Philipp Müller 2012-01-26 18:06:52 UTC
dvd menus don't seem to come up for me if the deinterlace flag is set on playbin2, which is what totem sets. Doing a seek in totem via the left arrow key makes them come up though.
Comment 1 Sebastian Dröge (slomo) 2012-01-27 15:37:22 UTC
Is this a still-frame menu?
Comment 2 Tim-Philipp Müller 2012-01-27 16:53:57 UTC
> Is this a still-frame menu?

Yes, it is.
Comment 3 Vincent Penquerc'h 2012-01-27 17:37:53 UTC
Rather unexpectedly traced to:

commit 76b29367e7b13cc1751724cb2678ebd9039678e1
Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk>
Date:   Thu Oct 6 11:53:26 2011 +0100

    playbin2: fix mismatch between video/ and video/x-dvd-subpicture
    
    The new code was checking for a prefix, and would find video/
    first. Check in two passes, first checking for a perfect match,
    and falling back to a prefix check if nothing was found.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=657261


The hang is in a subtitleoverlay pad block. Somehow.
Comment 4 Vincent Penquerc'h 2012-01-30 11:51:58 UTC
Removing that commit above makes it work, but it would re-break https://bugzilla.gnome.org/show_bug.cgi?id=657261.
Including that commit breaks deinterlaced playback till at least 2010, so there is almost certainly another bug that got exposed by this change, though I haven't had luck finding it so far.
Comment 5 Vincent Penquerc'h 2012-01-30 13:19:53 UTC
Traced to the delaying of events in basetransform.
Replacing the colorspace by a queue in the deinterlacing chain fixes preroll, as does removing the delaying code.
We end up with two threads blocked with an event in a pad blocked.
Not sure how to fix it yet though.
Comment 6 Vincent Penquerc'h 2012-01-30 14:36:09 UTC
Stuck trying to work out how to fix it, leaving it for now.
Comment 7 Vincent Penquerc'h 2012-01-30 17:19:48 UTC
Back hacking on it, I have a bodge that "fixes" it, but doesn't appear to fix the root cause. Somehow, two threads are stuck on a blocked pad, one with a sink message event, and one with a custom event from the DVD demuxer. and with in gst_subtitle_overlay_subtitle_sink_event. That AFAIK should not happen, and should be fixed.


diff --git a/gst/playback/gstsubtitleoverlay.c b/gst/playback/gstsubtitleoverlay.c
index 126ffd2..0f3657f 100644
--- a/gst/playback/gstsubtitleoverlay.c
+++ b/gst/playback/gstsubtitleoverlay.c
@@ -2082,7 +2082,11 @@ gst_subtitle_overlay_subtitle_sink_event (GstPad * pad, GstEvent * event)
       break;
   }
 
-  ret = gst_proxy_pad_event_default (pad, gst_event_ref (event));
+  if (GST_EVENT_TYPE (event) != GST_EVENT_CUSTOM_DOWNSTREAM_OOB) {
+    ret = gst_proxy_pad_event_default (pad, gst_event_ref (event));
+  } else {
+    ret = TRUE;
+  }
 
   if (GST_EVENT_TYPE (event) == GST_EVENT_NEWSEGMENT) {
     gboolean update;
Comment 8 Vincent Penquerc'h 2012-01-30 17:25:26 UTC
I'm seeing the issue with an animated menu DVD, so updating bug title accordingly.
Comment 9 Vincent Penquerc'h 2012-01-30 18:49:31 UTC
Created attachment 206463 [details] [review]
resindvd: fix wedge in preroll when playbin2 deinterlacing is enabled

When deinterlacing is enabled, an extra colorspace element is added.
Colorspace is a basetransform, and is then the only basetransform
element on the video path. A while ago, basetransform started delaying
events till caps were set on its source pad. These things conspired
to end up sending the DVD highlight events onto a blocked pad on
subtitleoverlay.

Ensuring these highlight events are only sent once we're in playing
mode fixes the issue.
Comment 10 Vincent Penquerc'h 2012-01-30 18:51:19 UTC
I'd like to get an ack from someone who's more comfortable with playbin2 and events/preroll behavior, as I'm not so certain this is a proper fix.
Comment 11 Sebastian Dröge (slomo) 2012-02-01 09:18:14 UTC
Should be fine
Comment 12 Vincent Penquerc'h 2012-02-01 12:07:47 UTC
Thanks, pushed, then:

commit 7a9fff74c6b6a3bc9559d624af3e8d71f8771db8
Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk>
Date:   Mon Jan 30 18:46:07 2012 +0000

    resindvd: fix wedge in preroll when playbin2 deinterlacing is enabled
    
    When deinterlacing is enabled, an extra colorspace element is added.
    Colorspace is a basetransform, and is then the only basetransform
    element on the video path. A while ago, basetransform started delaying
    events till caps were set on its source pad. These things conspired
    to end up sending the DVD highlight events onto a blocked pad on
    subtitleoverlay.
    
    Ensuring these highlight events are only sent once we're in playing
    mode fixes the issue.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=668762
Comment 13 Julian Andres Klode 2012-03-12 15:05:01 UTC
I still experience such a bug (http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=663582) with 0.10.23 which seems to include the bug fix according to the git repository. Did someone overlook someone, or is this another bug?
Comment 14 Tim-Philipp Müller 2012-03-12 16:47:30 UTC
Hard to say without more information.

Probably best to just file a new bug about it (and maybe mention this one for background).