GNOME Bugzilla – Bug 637586
playbin2 fails to recognize subtitle caps from katedec
Last modified: 2010-12-21 16:44:15 UTC
As discussed with slomo, playbin2 tries to get caps from katedec through inputselector, but fails, as inputselector has no active pad yet. The sequence of events is: - katedec gets the first buffer (header), and sets src caps from its contents - katedec forwards any delayed events (including newsegment events) - subtitleoverlay tries to get caps from katedec, but fails
Reason for this is the code in gstplaybin2.c around line 2733, which sends a sink-message event downstream *after* the subtitle inputselector and this event is causing the subtitleoverlay to block. Unfortunately the subtitle input selector did not get a newsegment event yet and as such did not select a active sinkpad yet, causing the get_caps/get_negotiated_caps calls in subtitleoverlay to return ANY. One solution might be to delay the sink-message events to the selector_blocked callback in playbin2, which is called when the selector gets unblocked (i.e. got an event) (and this will also cause the events to be sent from the corresponding streaming threads). OTOH the sink-message events should probably be sent immediately after the newsegment event to allow correct synchronization in the sink.
Created attachment 176815 [details] [review] playbin2: delay stream-changed messages
Review of attachment 176815 [details] [review]: Looks good in general, just some cosmetic/minor things, after that I'll push this change :) ::: gst/playback/gstplaybin2.c @@ +2323,3 @@ +static gboolean +event_data_probe (GstPad * pad, GstMiniObject * object, gpointer data) Maybe call it stream_changed_data_probe. It's not really an event data probe @@ +2344,3 @@ + /* push the event first, then send the delayed one */ + gst_event_ref (GST_EVENT (object)); + gst_pad_send_event (pad, GST_EVENT (object)); Please cast with GST_EVENT_CAST() here. You already know that it is an event :) @@ +2797,3 @@ + + /* remove any data probe we might have, and replace */ + select->sinkpad_delayed_event = event; You'll be leaking any previously set event here. To be on the safe side unref any events here first Maybe also add a comment here why we're not sending the event directly but defer it to the data probe (1: send it from the correct streaming thread, 2: pushing the event could block, leading to a deadlock, 3: things might not be linked correctly yet, after the data probe is called it will)
Created attachment 176843 [details] [review] playbin2: delay stream-changed messages With requested changes.
Great, thanks :) commit f221466099b9193fb737cafc6bde1195aaeec06a Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk> Date: Tue Dec 21 10:26:40 2010 +0000 playbin2: delay stream-changed messages https://bugzilla.gnome.org/show_bug.cgi?id=637586