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 570910 - Removing the target of a ghostpad does not re-target the ghostpad to NULL
Removing the target of a ghostpad does not re-target the ghostpad to NULL
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-02-07 23:52 UTC by Michael Smith
Modified: 2009-02-16 11:59 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Unit test for removing the target of a ghostpad (1.95 KB, patch)
2009-02-07 23:53 UTC, Michael Smith
committed Details | Review
patch (1.55 KB, patch)
2009-02-07 23:59 UTC, David Schleef
none Details | Review
Fixed version of david's patch (3.83 KB, patch)
2009-02-09 19:31 UTC, Michael Smith
none Details | Review

Description Michael Smith 2009-02-07 23:52:56 UTC
Unit test attached, it:
 - creates a bin with a sink inside it.
 - creates a ghostpad on that bin targetted at the sink inside it.
 - removes the sink from the bin
 - checks that the ghostpad no longer points at the sink's sinkpad.

It currently fails.

In my larger application, I do roughly the same, then (having removed my element from the pipeline (p1)) I reuse that element in another pipeline (p2).

After that, I unref (and so it gets disposed) p1. Unfortunately, since there's something in p1 that still points at my element, this incorrectly affects p2.

(I will go into detail later; have to head out now).
Comment 1 Michael Smith 2009-02-07 23:53:37 UTC
Created attachment 128198 [details] [review]
Unit test for removing the target of a ghostpad
Comment 2 David Schleef 2009-02-07 23:59:06 UTC
Created attachment 128199 [details] [review]
patch

This is one way of fixing the problem, but I don't trust it until someone who understands ghost pads has looked at it.

From 3a468692887fd9c5f341d3f186de31acd003ebb2 Mon Sep 17 00:00:00 2001
From: David Schleef <ds@schleef.org>
Date: Sat, 7 Feb 2009 15:23:36 -0800
Subject: [PATCH] bin: untarget ghost pads of element being removed

When removing an element from a bin, iterate over ghostpads
on the bin and retarget those that point to pads of the
element being removed.
---
 gst/gstbin.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)
Comment 3 Michael Smith 2009-02-09 19:31:06 UTC
Created attachment 128319 [details] [review]
Fixed version of david's patch

This fixes up the refcounting in David's patch.

It then changes one of the exiting ghostpad tests - which was explicitly checking that a ref was retained in this case. I think the test is wrong.

This seems inelegant - my thought it that we should be able to fix it in gstghostpad.c, rather than changing how gstbin removes elements. However, my attempt at doing so ended up in deadlocks. Advice from someone who understands the current ghostpad implementation properly would be very welcome.
Comment 4 Wim Taymans 2009-02-16 10:24:15 UTC
It seems the proxypad simply does not have an unlink function. If we add an unlink function for the proxypad we can set the target to NULL in it.
Comment 5 Wim Taymans 2009-02-16 11:59:13 UTC
commit 26f368f7e751661d53895f8c00b07bb13486bffa
Author: Wim Taymans <wim.taymans@collabora.co.uk>
Date:   Mon Feb 16 12:58:34 2009 +0100

    Clear target when the target pad disappears
    
    When the target pad disappears (because it was explicitly unlinked or the
    element was removed/unreffed) make sure we receive a notify with the unlink
    function on the proxy pad and clear the target. We use a simple flag to not do
    this and cause deadlocks when the target was changed explicitly using the
    ghostpad functions.
    
    Update the unit test because we now unref the target sooner (and correctly).