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 566936 - [ghostpads] unlink function wrongly called on target
[ghostpads] unlink function wrongly called on target
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal normal
: 0.10.23
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2009-01-07 19:15 UTC by Guillaume Emont (guijemont)
Modified: 2009-01-23 10:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch for the problem (8.11 KB, patch)
2009-01-13 16:27 UTC, Wim Taymans
committed Details | Review

Description Guillaume Emont (guijemont) 2009-01-07 19:15:46 UTC
Using a playbin2 with uri and suburi set, going from STATE_PLAYING to STATE_NULL and then back to STATE_PLAYING breaks the display of subtitles.

Example python code that triggers the bug:
------8<----------
#video and subs contain respectively a video uri and a subtitle file uri
pipeline = gst.element_factory_make('playbin2')
pipeline.set_property('uri', video)
pipeline.set_property('suburi', subs)
pipeline.set_state(gst.STATE_PLAYING)
time.sleep(5)
pipeline.set_state(gst.STATE_NULL)
pipeline.get_state(0)
pipeline.set_state(gst.STATE_PLAYING)
time.sleep(5)
pipeline.set_state(gst.STATE_NULL)
------8<----------
Expected result:
 The subtitles are shown in both iterations
Currentt result:
 The subtitles are not shown the second time the video is played

Setting again the suburi does not change anything.
The subtitles are displayed if playbin is used instead of playbin2.
Comment 1 Wim Taymans 2009-01-13 11:11:45 UTC
It's another bug in ghostpads.

We have these pads:

 srcpad -> ghostpad1 -> ghostpad2 -> text_sink

The sinkpad of the text_overlay is ghosted, then the ghosted pad is ghosted again. When srcpad is unlinked from ghostpad1, the unlink function of the target of ghostpad2 (text_sink) is called. Now textoverlay thinks it's unlinked and thus doesn't do any overlays.

Comment 2 Wim Taymans 2009-01-13 16:27:58 UTC
Created attachment 126357 [details] [review]
Patch for the problem

This patch removes the code to signal the unlink function on the target pad. Calling this function is the wrong thing to do because the target is still linked to the internal proxy pads.

This patch also makes playbin2 subtitles reusable.
Comment 3 Jan Schmidt 2009-01-15 23:20:18 UTC
I'm suspicious of this patch, because it changes the do_unlink function without changing the do_link in anyway, even though do_link also calls the peer's link function in some circumstances. If that's correct, it seems logical that the unlink function should be called in some circumstances also.
Comment 4 Wim Taymans 2009-01-16 11:36:15 UTC
to Comment #3: It's not supposed to be symmetrical like that.

A srcpad is required to call the link function on the _peer_ pad passed in the do_link function (core requirement to avoid some potential races we have not had to deal with yet). 

In the unlink function, the pad is not required to call any unlink function on the peer pad, because core does that and the peer pad is not passed to the unlink function for that reason.

In any case, the unlink/link function should never be called on the _target_ pad because we don't unlink/link the internal pad from/to the target when we simply unlink/link the ghostpad.
Comment 5 Jan Schmidt 2009-01-16 11:43:03 UTC
Hmm, ok. I don't think this should go in this release though. I'd prefer to have more time to bed down any change to ghostpad behaviour.
Comment 6 Wim Taymans 2009-01-23 10:47:30 UTC
commit ae76b3d60626e495a3227df613803a4c6b6b4101
Author: Wim Taymans <wim.taymans@collabora.co.uk>
Date:   Wed Jan 21 12:21:49 2009 +0100

    do not call the unlink function on the target pad when the ghostpad
    is unlinked.
    Add some unit tests for this behaviour.
    Fixes #566936.