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 537033 - Downwards state changes can fail due to bug in core state change algorithm
Downwards state changes can fail due to bug in core state change algorithm
Status: RESOLVED DUPLICATE of bug 368536
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2008-06-06 21:02 UTC by Michael Smith
Modified: 2008-06-09 19:26 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Michael Smith 2008-06-06 21:02:36 UTC
I found this testing playbin2, using the test7 example program. To reproduce:
 - make it so that the audio sink will fail. I did this in two different ways
    - delete all the audio sinks that autoaudiosink can use
    - force playbin2 to use an alsasink, and make alsa fail by already having the device open.
 - run it with a file that exists.

See these messages:

(lt-test7:2221): GStreamer-CRITICAL **: 
Trying to dispose element inputselector1, but it is not in the NULL state.
You need to explicitly set elements to the NULL state before
dropping the final reference, to allow them to clean up.

(lt-test7:2221): GStreamer-CRITICAL **: 
Trying to dispose element player, but it is not in the NULL state.
You need to explicitly set elements to the NULL state before
dropping the final reference, to allow them to clean up.


Investigating deeper, it seems like what happens is this:
 - playsink has reached PAUSED
 - inside playsink, the audiobin (abin) has state NULL (it failed to go to READY because the audio sink wasn't working).
 - we then do a downwards state change to NULL for the whole pipeline.
 - we do PAUSED->READY on playsink
 - we iterate over playsink's children, setting them all to READY.
 - for abin, setting to READY is actually an _upwards_ state change.
 - that fails (again because we don't have a working sink).
 - Having failed the state change here, we abort the whole state change.
 - so we never set most of the elements to NULL.
 - then later, on dispose(), we print out the above errors.

So it looks like, in this case, a downwards state change is triggering _upwards_ (and thus possibly failing) state changes deeper in the bins. This seems to me that it must always be wrong.

As a hack, I tried this, which "fixes" the problem. This just forces bin state changes to never change children in the opposite direction. I doubt this is the correct fix, so I'm not bothering to attach it as a proper patch.

Thoughts and suggestions welcomed!

--- gstbin.c	25 Apr 2008 17:54:28 -0000	1.379
+++ gstbin.c	6 Jun 2008 21:01:55 -0000
@@ -1866,9 +1866,11 @@
   GstStateChangeReturn ret;
   gboolean locked;
   GList *found;
+  GstState element_state;
 
   /* peel off the locked flag */
   GST_OBJECT_LOCK (element);
+  element_state = GST_STATE (element);
   locked = GST_OBJECT_FLAG_IS_SET (element, GST_ELEMENT_LOCKED_STATE);
   /* get previous state return */
   ret = GST_STATE_RETURN (element);
@@ -1917,6 +1919,10 @@
   GST_OBJECT_UNLOCK (bin);
 
 no_preroll:
+  if (((next < current) && (next > element_state)) ||
+      ((next > current) && (next < element_state)))
+    return GST_STATE_CHANGE_SUCCESS;
+
   GST_DEBUG_OBJECT (bin,
       "setting element %s to %s, base_time %" GST_TIME_FORMAT,
       GST_ELEMENT_NAME (element), gst_element_state_get_name (next),
Comment 1 Michael Smith 2008-06-06 21:05:11 UTC
FWIW, this hack does pass the core testsuite.
Comment 2 Michael Smith 2008-06-09 19:26:15 UTC
Turns out to be a duplicate.

We should do something about this soon, though.


*** This bug has been marked as a duplicate of 368536 ***