GNOME Bugzilla – Bug 662330
[decodebin2] Should link and add elements to the bin before checking if they can reach READY state
Last modified: 2011-10-22 06:30:03 UTC
Created attachment 199584 [details] [review] playbin2: link elements before testing ready state One of the basic rule in GStreamer is to allocate the resources (e.g. open the X11 display) in the transition from NULL to READY. The auto-plugger take advantage of this by doing the state transition before adding and linking the element. The problem with that approach arrive when an element must share a context (see GstVideoContext proposal, bug #662321). As the element is not linked, it's not possible to query neighbour elements or application for an already allocated video context. This imply implementing hacks in the element to only allocate the display upon linking (e.g during a call to get_caps() if in ready state). As this is not straightforward and contributes to make things unclear, I propose to filter errors on elements and link the element before testing the ready state. Note that more patches are required in GstPlaySink, audovideosrc and other auto-plugger to fully move to this approach.
Review of attachment 199584 [details] [review]: Good idea and looks good in general, just two minor comments ::: gst/playback/gstdecodebin2.c @@ +178,3 @@ + + GMutex *filtered_lock; + GList *filtered; /* element for which error message are filterd */ You should be able to use the object lock here instead of introducing a new mutex. @@ +1090,3 @@ + g_mutex_free (decode_bin->filtered_lock); + decode_bin->filtered_lock = NULL; + } Just to be on the safe side, please also free the list of filtered elements here
Created attachment 199684 [details] [review] Link before testing, now using the decodebin lock
commit cf9da5c280603edde373f71ff1319aea4222130a Author: Nicolas Dufresne <nicolas.dufresne@collabora.com> Date: Thu Oct 13 11:34:49 2011 -0400 decodebin2: Link elements before testing if they can reach the READY state This is made possible by filtering errors. This is required to let harware accelerated element query the video context. The video context is used to determine if the HW is capable, and thus if the element is supported or not. Fixes bug #662330.