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 727949 - bin: Make sure to post EOS message always after reaching the PLAYING state
bin: Make sure to post EOS message always after reaching the PLAYING state
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal normal
: 1.3.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-04-10 09:49 UTC by Sebastian Dröge (slomo)
Modified: 2014-05-01 18:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
0001-basesink-If-we-re-pre-rolling-because-of-EOS-make-su.patch (1.64 KB, patch)
2014-04-10 09:49 UTC, Sebastian Dröge (slomo)
rejected Details | Review
basesink: Add test for checking that EOS always comes after the state change to PLAYING (2.91 KB, patch)
2014-04-17 19:12 UTC, Sebastian Dröge (slomo)
committed Details | Review

Description Sebastian Dröge (slomo) 2014-04-10 09:49:02 UTC
See attached patch.

Without this it can happen that we pre-roll into PAUSED state, the application sets the pipeline to PLAYING based on the state-changed message, then we immediately go to PLAYING. And only then the code in the EOS event handler causes the EOS message to be posted.

After this change we won't get into PLAYING and will instead post the EOS message from basesink's change_state function.
Comment 1 Sebastian Dröge (slomo) 2014-04-10 09:49:44 UTC
Created attachment 273966 [details] [review]
0001-basesink-If-we-re-pre-rolling-because-of-EOS-make-su.patch
Comment 2 Wim Taymans 2014-04-10 10:39:24 UTC
As discussed on IRC:

 - basesink should block the streaming thread after posting the EOS
 - basesink should wake up the streaming thread and make it post a new EOS when
   the state changes from PAUSED->PLAYING
 - basesink should unblock the streaming thread when going to READY or being
   flushed.

The idea is to always have the events in the following order: async-done, state-change, EOS. 

Because we block the streaming thread now in EOS, it is now completely impossible to push something after EOS (this is a good thing).
Comment 3 Sebastian Dröge (slomo) 2014-04-17 19:12:07 UTC
Created attachment 274628 [details] [review]
basesink: Add test for checking that EOS always comes after the state change to PLAYING
Comment 4 Sebastian Dröge (slomo) 2014-04-17 19:12:41 UTC
This test here fails almost every time because EOS comes before the state change to PLAYING.
Comment 5 Sebastian Dröge (slomo) 2014-05-01 18:10:39 UTC
Actually the problem was in GstBin, basesink did it all correctly although the code seems a bit weird and bitrotten in a few places.

commit 129e4940c0e2e63715a310b525e1ccdafaf13c10
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Thu May 1 18:42:47 2014 +0200

    bin: Always first post the state-changed message for PAUSED->READY before posting any pending EOS message
    
    https://bugzilla.gnome.org/show_bug.cgi?id=727949

commit 644159bb9b9d352816ef591e4066017926c1bb55
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Thu Apr 17 21:10:55 2014 +0200

    basesink: Add test for checking that EOS always comes after the state change to PLAYING
    
    https://bugzilla.gnome.org/show_bug.cgi?id=727949