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 683504 - playsink: deadlock when disabling subtitles and suboptimal disabling of subtitles
playsink: deadlock when disabling subtitles and suboptimal disabling of subti...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal normal
: 1.2.4
Assigned To: GStreamer Maintainers
GStreamer Maintainers
playback
: 668459 700635 708782 727339 (view as bug list)
Depends on:
Blocks: 700635
 
 
Reported: 2012-09-06 14:53 UTC by Arnaud Vrac
Modified: 2014-06-10 12:11 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
playsink: fix text sink getter when default subtitleoverlay is used (1017 bytes, patch)
2012-09-06 14:53 UTC, Arnaud Vrac
rejected Details | Review

Description Arnaud Vrac 2012-09-06 14:53:39 UTC
Created attachment 223660 [details] [review]
playsink: fix text sink getter when default subtitleoverlay is used

When the default subtitleoverlay element is used in playsink, the gst_play_sink_get_sink returns NULL when called with GST_PLAY_SINK_TYPE_TEXT. The attached patch returns the subtitleoverlay element instead.
Comment 1 Tim-Philipp Müller 2013-05-15 09:48:48 UTC
<__tim> I don't know, I don't understand the rationale for that - why would we return an overlay element in get_sink ? you just say what you're doing there, but not why :)
<rawoul> this allows changing the overlay properties
<rawoul> like 'silent'
<rawoul> to toggle subtitles
<__tim> you should be able to do that by setting current-text to -1
<rawoul> I just tested and it doesn't seem to work
<rawoul> I see  [17.165] playbin: <playbin0> Changing current text stream 0 -> -1
<rawoul> but the subtitles are still displayed and 'current-text' is still 0
<__tim> well, that needs fixing then :)
Comment 2 Arnaud Vrac 2013-05-15 10:31:45 UTC
As discussed, the proper way to do toggle subtitles is to add/remove the TEXT flag from playbin/playsink.

However removing the flag causes a deadlock in playsink (in 1.0.7 at least):

set subtitle track -1
playsink: <playsink> Triggering reconfiguration
playsink: <playsink> locking from thread 0x80f48e0
playsink: <playsink> locked from thread 0x80f48e0
playsink: <playsink> unlocking from thread 0x80f48e0
playsink: <playsink> locking from thread 0x7559d5b0
playsink: <playsink> locked from thread 0x7559d5b0
playsink: <playsink:video_sink> Video pad blocked
playsink: <playsink> All pads blocked -- reconfiguring
playsink: <playsink> Video pad is raw: 1
playsink: <playsink> Audio pad is raw: 0
playsink: <playsink> reconfiguring
playsink: <playsink> locking from thread 0x7559d5b0
playsink: <playsink> locked from thread 0x7559d5b0
playsink: <playsink> audio:1, video:1, vis:0, text:0
playsink: <playsink> adding video, raw 1
playsink: <playsink> adding video chain
playsink: <playsink> adding audio
playsink: <playsink> adding audio chain
playsink: <playsink> no vis needed
playsink: <playsink> no text needed
streamsynchronizer: <streamsynchronizer0> Releasing stream 2
playsink: <playsink> locking from thread 0x74a07690

gst_pad_set_active(pad, false) in streamsynchronizer locks the streaming thread, and playsink is also locked while reconfiguring.
Comment 3 Sebastian Dröge (slomo) 2013-05-15 10:33:48 UTC
Can you get a backtrace of that deadlock?

There are two problems here:
a) playsink should just set the silent property on subtitleoverlay if there's already one
b) streamsynchronizer should not deadlock when adding/removing subtitleoverlay (it's not entirely clear to me why it deadlocks though)
Comment 4 Arnaud Vrac 2013-05-15 10:48:37 UTC
Here is the backtrace of the locked threads:


Comment 5 Sebastian Dröge (slomo) 2013-05-27 11:18:25 UTC
Problem in the backtrace is that data flow is happening, causing a pad block while playsink is reconfiguring as result of a pad block, thus we block on the stream lock in two threads. The reconfiguration shouldn't have happened at all before all pads were blocked.
Comment 6 Tim-Philipp Müller 2013-09-25 21:37:03 UTC
*** Bug 708782 has been marked as a duplicate of this bug. ***
Comment 7 Nicolas Dufresne (ndufresne) 2013-10-01 22:52:52 UTC
*** Bug 668459 has been marked as a duplicate of this bug. ***
Comment 8 Nicolas Dufresne (ndufresne) 2013-10-01 23:14:28 UTC
For those looking for a file to reproduce:

http://people.collabora.com/~nicolas/Subtitles/Test_AdvancedSubsV2_StandardDef.mkv
Comment 9 Sebastian Dröge (slomo) 2014-02-16 11:44:42 UTC
(In reply to comment #5)
> Problem in the backtrace is that data flow is happening, causing a pad block
> while playsink is reconfiguring as result of a pad block, thus we block on the
> stream lock in two threads. The reconfiguration shouldn't have happened at all
> before all pads were blocked.

... and this might happen because pad probes might be called multiple times. A blocking probe will be called once for every thread that causes a block to happen.
Comment 10 Sebastian Dröge (slomo) 2014-02-16 14:28:38 UTC
Looking into that
Comment 11 Sebastian Dröge (slomo) 2014-02-16 14:36:20 UTC
Please test if this fixes all your problems.

commit 4dd30bbd166db003f9963f41918b056df338dcf0
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Sun Feb 16 15:32:47 2014 +0100

    playsink: Only remove the complete text chain if the text pad goes away
    
    If the text pads does not go away we just set the overlay to silent, which
    allows us to immediately re-enable subs later again. However before this
    change we also released the streamsynchronizer text pads, which deadlocked
    because there was still dataflow going on. Just do this only if we remove
    the complete chain.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=683504
Comment 12 Nicolas Dufresne (ndufresne) 2014-02-17 14:43:40 UTC
(y) This has been around for a long time.
Comment 13 Sebastian Dröge (slomo) 2014-05-01 11:25:33 UTC
*** Bug 727339 has been marked as a duplicate of this bug. ***
Comment 14 Bastien Nocera 2014-06-10 12:11:48 UTC
*** Bug 700635 has been marked as a duplicate of this bug. ***