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 740694 - decodebin: Take STREAM_LOCK before sending sticky events.
decodebin: Take STREAM_LOCK before sending sticky events.
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
unspecified
Other All
: Normal normal
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 740547 740563
 
 
Reported: 2014-11-25 16:04 UTC by Mathieu Duponchelle
Modified: 2014-11-26 18:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
decodebin2: Take STREAM_LOCK before sending sticky events. (2.18 KB, patch)
2014-11-25 16:04 UTC, Mathieu Duponchelle
accepted-commit_now Details | Review
decodebin2: Take STREAM_LOCK before sending sticky events. (2.19 KB, patch)
2014-11-25 16:35 UTC, Mathieu Duponchelle
accepted-commit_now Details | Review
decodebin2: Take STREAM_LOCK before sending sticky events. (2.19 KB, patch)
2014-11-25 17:19 UTC, Thibault Saunier
committed Details | Review

Description Mathieu Duponchelle 2014-11-25 16:04:28 UTC
There was a race where:

1) we would put the element to PAUSED
2) It would get data sent to it from upstream
3) It would thus send caps
3) caps_notify_cb would continue autoplugging
4) caps would flow downstream, the last pad would get exposed
5) we were still not done sending the sticky events

Taking the stream lock on the new element's sinkpad and only
releasing it when sticky events have all been sent prevents
the caps from reaching the source pad of the element before
we're all set.
Comment 1 Mathieu Duponchelle 2014-11-25 16:04:30 UTC
Created attachment 291473 [details] [review]
decodebin2: Take STREAM_LOCK before sending sticky events.
Comment 2 Sebastian Dröge (slomo) 2014-11-25 16:24:46 UTC
Review of attachment 291473 [details] [review]:

Looks good except for a typo

::: gst/playback/gstdecodebin2.c
@@ +2371,3 @@
+    /* First lock element's sinkpad stream lock so no data reaches
+     * the possible new element added when caps are sent by element
+     * while we're still sending stickies */

sticky events you mean, not stickies ;)
Comment 3 Mathieu Duponchelle 2014-11-25 16:35:44 UTC
Created attachment 291474 [details] [review]
decodebin2: Take STREAM_LOCK before sending sticky events.

There was a race where:

1) we would put the element to PAUSED
2) It would get data sent to it from upstream
3) It would thus send caps
3) caps_notify_cb would continue autoplugging
4) caps would flow downstream, the last pad would get exposed
5) we were still not done sending the sticky events

Taking the stream lock on the new element's sinkpad and only
releasing it when sticky events have all been sent prevents
the caps from reaching the source pad of the element before
we're all set.
Comment 4 Thibault Saunier 2014-11-25 17:19:46 UTC
Created attachment 291480 [details] [review]
decodebin2: Take STREAM_LOCK before sending sticky events.

There was a race where:

1) we would put the element to PAUSED
2) It would get data sent to it from upstream
3) It would thus send caps
3) caps_notify_cb would continue autoplugging
4) caps would flow downstream, the last pad would get exposed
5) we were still not done sending the sticky events

Taking the stream lock on the new element's sinkpad and only
releasing it when sticky events have all been sent prevents
the caps from reaching the source pad of the element before
we're all set.
Comment 5 Mathieu Duponchelle 2014-11-26 18:41:39 UTC
Author: Mathieu Duponchelle <mathieu.duponchelle@opencreed.com>
Date:   Tue Nov 25 16:46:50 2014 +0100

    decodebin2: Take STREAM_LOCK before sending sticky events.
    
    There was a race where:
    
    1) we would put the element to PAUSED
    2) It would get data sent to it from upstream
    3) It would thus send caps
    3) caps_notify_cb would continue autoplugging
    4) caps would flow downstream, the last pad would get exposed
    5) we were still not done sending the sticky events
    
    Taking the stream lock on the new element's sinkpad and only
    releasing it when sticky events have all been sent prevents
    the caps from reaching the source pad of the element before
    we're all set.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=740694
Comment 6 Mathieu Duponchelle 2014-11-26 18:43:18 UTC
Review of attachment 291480 [details] [review]:

commited