GNOME Bugzilla – Bug 316856
GstBin does state changes in wrong order sometimes
Last modified: 2005-10-03 18:10:35 UTC
Take a pipeline like: filesrc ! flacdec ! alsasink This will fail with a pad activation assertion issue, but the underlying problem is that GstBin propagates the state changes in the wrong order. Currently, GstBin does: sink null => ready filesrc null => ready flacdec null => ready filesrc ready => ready pipeline null => ready whereas it should do: sink null => ready flacdec null => ready filesrc null => ready pipeline null => ready The attached patch fixes the problem for the above case. Would be nice if someone more familiar with state changes and the different scenarios involved could take a look at it before I commit it. Maybe there is more that needs to be done for more complex scenarios. Cheers -Tim
Created attachment 52469 [details] [review] proposed fix
Created attachment 52470 [details] very short log extract with queue dumps to demonstrate problem
Created attachment 52471 [details] very short log extract with queue dumps with patch applied
The patch looks good, but I'd really like a test case. fakesrc ! identity ! fakesink should exhibit the same behavior -- I'd be surprised if it didn't. Can you make one like that, checking that you get the messages in the right order? It should be easy with gst_bus_poll(gst.MESSAGE_STATE_CHANGE, -1).
2005-09-23 Tim-Philipp Muller <tim at centricular dot net> * check/gst/gstbin.c: (test_children_state_change_order_flagged_sink), (test_children_state_change_order_semi_sink), (gst_bin_suite): Added test to check state change order in bins (can still be made to fail here under heavy disk load; bails out with 'Push on pad fakesink:sink0, but it was not activated in push mode'). * gst/gstbin.c: (gst_bin_class_init), (gst_bin_change_state): Fix state change order when there is only a semi sink (#316856) * gst/gstbus.c: (gst_bus_class_init): Use _class_peek_parent(), not _class_ref(); fix docs to say 'default main context' instead of 'mainloop' where that is what's meant. * gst/gstelement.c: (gst_element_commit_state), (gst_element_set_state): Fix typos in debug messages Committed patch and test case. Test case generally works fine but can be made to fail with an Push on pad fakesink:sink0, but it was not activated in push mode assertion/warning under heavy disk load when run in a loop. Dunno what that is or how to fix it. Cheers -Tim
The above test case failure occurs when the sink does a state change from PAUSED => READY state. Maybe some kind of race condition in gst_base_sink_preroll_queue_flush()? (According to the logs, the test succeeds when there's a buffer to be popped off the preroll queue and fails if there is no buffer on the preroll queue. That's the only difference, but it's probably just what causes the timing issue). Cheers -Tim
I should add that the test case that produces the above is test_children_state_change_order_flagged_sink() in check/gst/gstbin.c Cheers -Tim
I can't reproduce the above test case failure any longer. I presume it was due to the race condition in gst_pad_activate_push() that Andy fixed today. Cheers -Tim