[gst-devel] What can be done from within a state change?

[Wim Taymans]

On Mon, 2005-09-05 at 18:16 +0200, Andy Wingo wrote:
> This is a question for wim really, but we probably ask too many
> questions internally, and this one is kindof interesting from a 0.9
> architecture perspective. Also wim is not here today.
> 
> I have a deadlock running this pipeline:
> 
> dv1394src ! dvdemux ! video/x-dv,systemstream=\(boolean\)false ! fakesink
> 
> It's because dvdemux is adding a pad in its state change handler, which
> fires new-pad, which gstparse hooks into to connect fakesink, but it
> connects with filtered caps, which wants to create a capsfilter.
> gst_element_link_pads_filtered now wants to do a get_state on the bin
> before adding the capsfilter element, so it can sync its state with the
> rest of the pipeline, but that will deadlock when we get to doing
> get_state on dvdemux:
> 
> #6  gst_element_get_state (element=0x5f3920, state=0x0, pending=0x0, timeout=0x7fffff846460) at gstelement.c:1632
> #7  gst_bin_get_state (element=0x602bc0, state=0x7fffff8465a4, pending=0x7fffff8465a0, timeout=0x7fffff846590) at gstbin.c:1003
> #8  gst_element_get_state (element=0x602bc0, state=0x7fffff8465a4, pending=0x7fffff8465a0, timeout=0x7fffff846590) at gstelement.c:1632
> #9  gst_element_link_pads_filtered (src=0x5f3920, srcpadname=0x0, dest=0x6011b0, destpadname=0x0, filter=0x5dbf90) at gstutils.c:1294
> #10 gst_parse_found_pad (src=0x5f3920, pad=0x5296c0, data=0x604060) at grammar.y:382
> #11 g_closure_invoke () from /usr/lib/libgobject-2.0.so.0
> #12 g_signal_stop_emission () from /usr/lib/libgobject-2.0.so.0
> #13 g_signal_emit_valist () from /usr/lib/libgobject-2.0.so.0
> #14 g_signal_emit () from /usr/lib/libgobject-2.0.so.0
> #15 gst_element_add_pad (element=0x5f3920, pad=0x5296c0) at gstelement.c:542
> #16 gst_dvdemux_add_pads (dvdemux=0x5f3920) at gstdvdemux.c:214
> #17 gst_dvdemux_change_state (element=0x5f3920, transition=GST_STATE_CHANGE_NULL_TO_READY) at gstdvdemux.c:969
> 
> Now, where is the bug?
> 
> 1) dvdemux, for adding a pad in the state change, which would cause
> new-pad to be fired, from which the user can do anything?

There are cases where this should be allowed, not a bug IMO.

> 
> 2) gst_element_link_pads_filtered, for doing a get_state where it
> shouldn't?
Since we need to add an element to the pipeline to implement the
capsfilter, we
need to bring it to the same state as the rest of the pipeline, we can
only do this after getting the pipeline state...

> 
> 3) GstElement, for having a non-recursive state lock?

It should be possible to do a get_state even when the state change is
running in the same thread. There is only a couple of places where
control could potentially go back to the application (actually only when
calling the overridden change_state method). The lock should be
recursive, but with a flag that no two state changes can happen at the
same time (just g_warning if that happens, as opposed to a deadlock
now).

> 
> 4) dvdemux, for not dropping the state lock when firing the new-pad
> signal? (yuk)
No, this is not allowed.
> 
> Basically, what can you do in a state change handler and what can't you
> do?

Almost everything (should) be allowed, even unreffing elements if you
hold a valid ref to it. The only problem currently is that doing stuff
that takes the STATE_LOCK is deadlocking. A get_state should not
deadlock and a set_state should error. Something for the TODO list.

Wim
> 
> Regards,
-- 
Wim Taymans <wim at fluendo.com>