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 316856 - GstBin does state changes in wrong order sometimes
GstBin does state changes in wrong order sometimes
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: High major
: 0.9.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2005-09-21 16:22 UTC by Tim-Philipp Müller
Modified: 2005-10-03 18:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed fix (673 bytes, patch)
2005-09-21 16:26 UTC, Tim-Philipp Müller
committed Details | Review
very short log extract with queue dumps to demonstrate problem (17.27 KB, text/plain)
2005-09-21 16:28 UTC, Tim-Philipp Müller
  Details
very short log extract with queue dumps with patch applied (15.03 KB, text/plain)
2005-09-21 16:28 UTC, Tim-Philipp Müller
  Details

Description Tim-Philipp Müller 2005-09-21 16:22:54 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
Comment 1 Tim-Philipp Müller 2005-09-21 16:26:45 UTC
Created attachment 52469 [details] [review]
proposed fix
Comment 2 Tim-Philipp Müller 2005-09-21 16:28:10 UTC
Created attachment 52470 [details]
very short log extract with queue dumps to demonstrate problem
Comment 3 Tim-Philipp Müller 2005-09-21 16:28:45 UTC
Created attachment 52471 [details]
very short log extract with queue dumps with patch applied
Comment 4 Andy Wingo 2005-09-22 10:33:24 UTC
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).
Comment 5 Tim-Philipp Müller 2005-09-23 17:38:06 UTC
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
 
Comment 6 Tim-Philipp Müller 2005-09-24 13:59:33 UTC
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
Comment 7 Tim-Philipp Müller 2005-09-24 14:16:34 UTC
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
Comment 8 Tim-Philipp Müller 2005-10-03 18:10:35 UTC
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