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 647756 - [bin] Posting EOS message although not reached PLAYING yet
[bin] Posting EOS message although not reached PLAYING yet
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal critical
: 0.10.36
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 585268 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2011-04-14 10:18 UTC by Sebastian Dröge (slomo)
Modified: 2012-08-14 08:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
bin: Only post EOS messages after reaching the PLAYING state (5.23 KB, patch)
2011-04-18 12:27 UTC, Sebastian Dröge (slomo)
none Details | Review

Description Sebastian Dröge (slomo) 2011-04-14 10:18:46 UTC
With the encodebin unit test in -base an interesting race condition is exposed.

The unit test sets the pipeline to PLAYING, waits for the EOS message, then releases the request pads and then sets the pipeline to NULL. The EOS message arrives before the complete pipeline has finished the state change to PLAYING (the sinks are PLAYING already) and then the pad releasing will set internal elements of encodebin to NULL and remove them from encodebin. Because an async state change is still happening these elements are set to PLAYING again afterwards (although they're not a child element of encodebin anymore) and during disposal GstElement complains that the state is not NULL.

The assertion can be fixed by releasing the encodebin pads after setting the pipeline to NULL. It can also be fixed by checking in GstBin if an element is still the child of the bin before changing the state and by setting the state of the elements to NULL *after* removing them from the bin. I'll file a separate bug for the GstBin child element problem.
Comment 1 Sebastian Dröge (slomo) 2011-04-14 10:49:15 UTC
Bug #647760 and bug #647757 are related to this one and are the cause why the unit test actually results in the assertions. Nonetheless it's probably wrong to get EOS before the pipeline is in PLAYING.
Comment 2 Sebastian Dröge (slomo) 2011-04-14 10:55:39 UTC
Reproducing this requires reverting of the following commit and running test_encodebin_render_audio_dynamic with .forever for example


commit 10e0b85a5695eda8e7d358d49e00966a09df0569
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Thu Apr 14 12:55:00 2011 +0200

    encodebin: Release pads after setting the state to NULL in the unit test
    
    See bug #647756.
Comment 3 Sebastian Dröge (slomo) 2011-04-14 21:12:19 UTC
Just a random thought but maybe this is caused by making GstBus lockfree when posting messages. Maybe previously the async-done messages would be handled synchronously and the state change will be finished and only afterwards the EOS message can be handled. Now there's no locking anywhere and messages can be handled at the same time.
Comment 4 Sebastian Dröge (slomo) 2011-04-14 21:13:38 UTC
I'll check this tomorrow
Comment 5 Tim-Philipp Müller 2011-04-14 21:53:16 UTC
As I said on IRC, I think this is counter-intuitive and think that the pipeline should delay posting the EOS message until it's reached PLAYING state.
Comment 6 Sebastian Dröge (slomo) 2011-04-15 06:33:30 UTC
Sure, that's one way to fix it but if my thought from comment #3 is correct this is not only affecting the EOS messages but all messages. Previously messages would be posted by the top-level bin in the order they were posted by the childs, now later messages can be posted first if threads change while the messages are forwarded to the higher-level bins (and especially if more than just forwarding is done, like with the async-done messages).
Comment 7 Sebastian Dröge (slomo) 2011-04-15 08:13:07 UTC
Nope, the mutex was only taken when adding messages to the queue so that can't be the reason why this happens.

And it also happens with GstBus from last release. Can be easily shown by adding a g_assert (GST_STATE (pipeline) == GST_STATE_PLAYING) after the EOS message is received in the test.
Comment 8 Sebastian Dröge (slomo) 2011-04-17 17:23:13 UTC
Not marking as blocker, it's bad but definitely no regression :)
Comment 9 Sebastian Dröge (slomo) 2011-04-18 12:27:47 UTC
Created attachment 186196 [details] [review]
bin: Only post EOS messages after reaching the PLAYING state

Fixes bug #647756.
Comment 10 Sebastian Dröge (slomo) 2011-05-14 09:58:12 UTC
commit 7316a88387e3e5a36d792f4a6e95cbb9f05e9513
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Thu May 12 16:48:41 2011 +0200

    bin: Don't interprete pipelines without sink elements as always being in EOS
    
    Some tests (e.g. elements/capsfilter) have pipelines with dangling
    sinkpads and without a sink element. These pipelines can never post
    an EOS message (because this is only valid by a sink) and as such
    should never get an EOS message posted by the bin.


commit 4a836cae9fa9240d7ed3c6e5a9d7ea9afa484c0a
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Mon Apr 18 14:26:33 2011 +0200

    bin: Only post EOS messages after reaching the PLAYING state
    
    Fixes bug #647756.
Comment 11 Sebastian Dröge (slomo) 2012-08-14 08:04:46 UTC
*** Bug 585268 has been marked as a duplicate of this bug. ***