ghostpad: Don't cache internal proxy pad target

The internal proxy pad target is simply a cache of the internal proxy pad
peer. This patch uses the well implement GstPad peer handling to obtain the
target. This fixes issues with target not being set in both direction when
two ghostpads are linked together (empty bin).

https://bugzilla.gnome.org/show_bug.cgi?id=658517
This commit is contained in:
Nicolas Dufresne 2011-10-25 17:26:50 -04:00 committed by Sebastian Dröge
parent cf69ce1df6
commit 391568efde
2 changed files with 95 additions and 175 deletions

View file

@ -54,29 +54,18 @@
#define GST_PROXY_PAD_CAST(obj) ((GstProxyPad *)obj)
#define GST_PROXY_PAD_PRIVATE(obj) (GST_PROXY_PAD_CAST (obj)->priv)
#define GST_PROXY_PAD_TARGET(pad) (GST_PROXY_PAD_PRIVATE (pad)->target)
#define GST_PROXY_PAD_TARGET(pad) (GST_PAD_PEER (GST_PROXY_PAD_INTERNAL (pad)))
#define GST_PROXY_PAD_INTERNAL(pad) (GST_PROXY_PAD_PRIVATE (pad)->internal)
#define GST_PROXY_PAD_RETARGET(pad) (GST_PROXY_PAD_PRIVATE (pad)->retarget)
#define GST_PROXY_GET_LOCK(pad) (GST_PROXY_PAD_PRIVATE (pad)->proxy_lock)
#define GST_PROXY_LOCK(pad) (g_mutex_lock (GST_PROXY_GET_LOCK (pad)))
#define GST_PROXY_UNLOCK(pad) (g_mutex_unlock (GST_PROXY_GET_LOCK (pad)))
struct _GstProxyPadPrivate
{
/* with PROXY_LOCK */
GMutex *proxy_lock;
GstPad *target;
GstPad *internal;
gboolean retarget;
};
G_DEFINE_TYPE (GstProxyPad, gst_proxy_pad, GST_TYPE_PAD);
static GstPad *gst_proxy_pad_get_target (GstPad * pad);
static void gst_proxy_pad_dispose (GObject * object);
static void gst_proxy_pad_finalize (GObject * object);
#if !defined(GST_DISABLE_LOADSAVE) && !defined(GST_REMOVE_DEPRECATED)
#ifdef GST_DISABLE_DEPRECATED
#include <libxml/parser.h>
@ -510,62 +499,16 @@ gst_proxy_pad_setcaps_default (GstPad * pad, GstCaps * caps)
return res;
}
static gboolean
gst_proxy_pad_set_target_unlocked (GstPad * pad, GstPad * target)
{
GstPad *oldtarget;
if (target) {
GST_LOG_OBJECT (pad, "setting target %s:%s", GST_DEBUG_PAD_NAME (target));
if (G_UNLIKELY (GST_PAD_DIRECTION (pad) != GST_PAD_DIRECTION (target)))
goto wrong_direction;
} else
GST_LOG_OBJECT (pad, "clearing target");
/* clear old target */
if ((oldtarget = GST_PROXY_PAD_TARGET (pad)))
gst_object_unref (oldtarget);
/* set and ref new target if any */
if (target)
GST_PROXY_PAD_TARGET (pad) = gst_object_ref (target);
else
GST_PROXY_PAD_TARGET (pad) = NULL;
return TRUE;
/* ERRORS */
wrong_direction:
{
GST_ERROR_OBJECT (pad,
"target pad doesn't have the same direction as ourself");
return FALSE;
}
}
static gboolean
gst_proxy_pad_set_target (GstPad * pad, GstPad * target)
{
gboolean result;
GST_PROXY_LOCK (pad);
result = gst_proxy_pad_set_target_unlocked (pad, target);
GST_PROXY_UNLOCK (pad);
return result;
}
static GstPad *
gst_proxy_pad_get_target (GstPad * pad)
{
GstPad *target;
GST_PROXY_LOCK (pad);
GST_OBJECT_LOCK (pad);
target = GST_PROXY_PAD_TARGET (pad);
if (target)
gst_object_ref (target);
GST_PROXY_UNLOCK (pad);
GST_OBJECT_UNLOCK (pad);
return target;
}
@ -591,11 +534,11 @@ gst_proxy_pad_get_internal (GstProxyPad * pad)
g_return_val_if_fail (GST_IS_PROXY_PAD (pad), NULL);
GST_PROXY_LOCK (pad);
GST_OBJECT_LOCK (pad);
internal = GST_PROXY_PAD_INTERNAL (pad);
if (internal)
gst_object_ref (internal);
GST_PROXY_UNLOCK (pad);
GST_OBJECT_UNLOCK (pad);
return GST_PROXY_PAD_CAST (internal);
}
@ -611,31 +554,15 @@ gst_proxy_pad_get_internal (GstProxyPad * pad)
void
gst_proxy_pad_unlink_default (GstPad * pad)
{
GstPad *internal;
/* don't do anything if this unlink resulted from retargeting the pad
* controlled by the ghostpad. We only want to invalidate the target pad when
* the element suddenly unlinked with our internal pad. */
if (GST_PROXY_PAD_RETARGET (pad))
return;
internal = GST_PROXY_PAD_INTERNAL (pad);
/* nothing to do anymore */
GST_DEBUG_OBJECT (pad, "pad is unlinked");
gst_proxy_pad_set_target (internal, NULL);
}
static void
gst_proxy_pad_class_init (GstProxyPadClass * klass)
{
GObjectClass *gobject_class = (GObjectClass *) klass;
g_type_class_add_private (klass, sizeof (GstProxyPadPrivate));
gobject_class->dispose = gst_proxy_pad_dispose;
gobject_class->finalize = gst_proxy_pad_finalize;
#if !defined(GST_DISABLE_LOADSAVE) && !defined(GST_REMOVE_DEPRECATED)
{
GstObjectClass *gstobject_class = (GstObjectClass *) klass;
@ -663,35 +590,6 @@ gst_proxy_pad_class_init (GstProxyPadClass * klass)
GST_DEBUG_REGISTER_FUNCPTR (gst_proxy_pad_checkgetrange_default);
}
static void
gst_proxy_pad_dispose (GObject * object)
{
GstPad *pad = GST_PAD (object);
GstPad **target_p;
GST_PROXY_LOCK (pad);
/* remove and unref the target */
target_p = &GST_PROXY_PAD_TARGET (pad);
gst_object_replace ((GstObject **) target_p, NULL);
/* The internal is only cleared by GstGhostPad::dispose, since it is the
* parent of non-ghost GstProxyPad and owns the refcount on the internal.
*/
GST_PROXY_UNLOCK (pad);
G_OBJECT_CLASS (gst_proxy_pad_parent_class)->dispose (object);
}
static void
gst_proxy_pad_finalize (GObject * object)
{
GstProxyPad *pad = GST_PROXY_PAD (object);
g_mutex_free (GST_PROXY_GET_LOCK (pad));
GST_PROXY_GET_LOCK (pad) = NULL;
G_OBJECT_CLASS (gst_proxy_pad_parent_class)->finalize (object);
}
static void
gst_proxy_pad_init (GstProxyPad * ppad)
{
@ -699,7 +597,6 @@ gst_proxy_pad_init (GstProxyPad * ppad)
GST_PROXY_PAD_PRIVATE (ppad) = G_TYPE_INSTANCE_GET_PRIVATE (ppad,
GST_TYPE_PROXY_PAD, GstProxyPadPrivate);
GST_PROXY_GET_LOCK (pad) = g_mutex_new ();
gst_pad_set_query_type_function (pad, gst_proxy_pad_query_type_default);
gst_pad_set_event_function (pad, gst_proxy_pad_event_default);
@ -954,17 +851,12 @@ GstPadLinkReturn
gst_ghost_pad_link_default (GstPad * pad, GstPad * peer)
{
GstPadLinkReturn ret;
GstPad *internal;
g_return_val_if_fail (GST_IS_GHOST_PAD (pad), GST_PAD_LINK_REFUSED);
g_return_val_if_fail (GST_IS_PAD (peer), GST_PAD_LINK_REFUSED);
GST_DEBUG_OBJECT (pad, "linking ghostpad");
internal = GST_PROXY_PAD_INTERNAL (pad);
if (!gst_proxy_pad_set_target (internal, peer))
goto target_failed;
ret = GST_PAD_LINK_OK;
/* if we are a source pad, we should call the peer link function
* if the peer has one, see design docs. */
@ -972,24 +864,10 @@ gst_ghost_pad_link_default (GstPad * pad, GstPad * peer)
if (GST_PAD_LINKFUNC (peer)) {
ret = GST_PAD_LINKFUNC (peer) (peer, pad);
if (ret != GST_PAD_LINK_OK)
goto link_failed;
GST_DEBUG_OBJECT (pad, "linking failed");
}
}
return ret;
/* ERRORS */
target_failed:
{
GST_DEBUG_OBJECT (pad, "setting target failed");
return GST_PAD_LINK_REFUSED;
}
link_failed:
{
GST_DEBUG_OBJECT (pad, "linking failed");
/* clear target again */
gst_proxy_pad_set_target (internal, NULL);
return ret;
}
}
/**
@ -1003,16 +881,9 @@ link_failed:
void
gst_ghost_pad_unlink_default (GstPad * pad)
{
GstPad *internal;
g_return_if_fail (GST_IS_GHOST_PAD (pad));
internal = GST_PROXY_PAD_INTERNAL (pad);
GST_DEBUG_OBJECT (pad, "unlinking ghostpad");
/* The target of the internal pad is no longer valid */
gst_proxy_pad_set_target (internal, NULL);
}
static void
@ -1046,18 +917,18 @@ on_src_target_notify (GstPad * target, GParamSpec * unused, gpointer user_data)
}
proxypad = GST_PROXY_PAD (GST_PAD_PEER (target));
GST_PROXY_LOCK (proxypad);
GST_OBJECT_LOCK (proxypad);
/* Now check if the proxypad's internal pad is still there and
* a ghostpad */
if (!GST_PROXY_PAD_INTERNAL (proxypad) ||
!GST_IS_GHOST_PAD (GST_PROXY_PAD_INTERNAL (proxypad))) {
GST_OBJECT_UNLOCK (proxypad);
GST_OBJECT_UNLOCK (target);
GST_PROXY_UNLOCK (proxypad);
goto done;
}
gpad = GST_GHOST_PAD (GST_PROXY_PAD_INTERNAL (proxypad));
g_object_ref (gpad);
GST_PROXY_UNLOCK (proxypad);
GST_OBJECT_UNLOCK (proxypad);
GST_OBJECT_UNLOCK (target);
gst_pad_set_caps (GST_PAD_CAST (gpad), caps);
@ -1069,6 +940,13 @@ done:
gst_caps_unref (caps);
}
static void
on_src_target_unlinked (GstPad * pad, GstPad * peer, gpointer user_data)
{
g_signal_handlers_disconnect_by_func (pad,
(gpointer) on_src_target_notify, NULL);
}
/**
* gst_ghost_pad_setcaps_default:
* @pad: the #GstPad to link.
@ -1148,7 +1026,7 @@ gst_ghost_pad_dispose (GObject * object)
gst_object_unref (peer);
}
GST_PROXY_LOCK (pad);
GST_OBJECT_LOCK (pad);
internal = GST_PROXY_PAD_INTERNAL (pad);
gst_pad_set_activatepull_function (internal, NULL);
@ -1162,7 +1040,7 @@ gst_ghost_pad_dispose (GObject * object)
gst_object_unparent (GST_OBJECT_CAST (internal));
GST_PROXY_PAD_INTERNAL (pad) = NULL;
GST_PROXY_UNLOCK (pad);
GST_OBJECT_UNLOCK (pad);
G_OBJECT_CLASS (gst_ghost_pad_parent_class)->dispose (object);
}
@ -1242,7 +1120,7 @@ gst_ghost_pad_construct (GstGhostPad * gpad)
gst_proxy_pad_checkgetrange_default);
}
GST_PROXY_LOCK (pad);
GST_OBJECT_LOCK (pad);
/* now make the ghostpad a parent of the internal pad */
if (!gst_object_set_parent (GST_OBJECT_CAST (internal),
@ -1266,16 +1144,16 @@ gst_ghost_pad_construct (GstGhostPad * gpad)
g_signal_connect (internal, "notify::caps", G_CALLBACK (on_int_notify),
pad);
/* call function to init values of the pad caps */
on_int_notify (internal, NULL, GST_GHOST_PAD_CAST (pad));
/* special activation functions for the internal pad */
gst_pad_set_activatepull_function (internal,
gst_ghost_pad_internal_activate_pull_default);
gst_pad_set_activatepush_function (internal,
gst_ghost_pad_internal_activate_push_default);
GST_PROXY_UNLOCK (pad);
GST_OBJECT_UNLOCK (pad);
/* call function to init values of the pad caps */
on_int_notify (internal, NULL, GST_GHOST_PAD_CAST (pad));
GST_GHOST_PAD_PRIVATE (gpad)->constructed = TRUE;
return TRUE;
@ -1287,7 +1165,7 @@ parent_failed:
GST_DEBUG_PAD_NAME (internal));
g_critical ("Could not set internal pad %s:%s",
GST_DEBUG_PAD_NAME (internal));
GST_PROXY_UNLOCK (pad);
GST_OBJECT_UNLOCK (pad);
gst_object_unref (internal);
return FALSE;
}
@ -1500,7 +1378,6 @@ gst_ghost_pad_set_target (GstGhostPad * gpad, GstPad * newtarget)
{
GstPad *internal;
GstPad *oldtarget;
gboolean result;
GstPadLinkReturn lret;
g_return_val_if_fail (GST_IS_GHOST_PAD (gpad), FALSE);
@ -1517,31 +1394,25 @@ gst_ghost_pad_set_target (GstGhostPad * gpad, GstPad * newtarget)
GST_DEBUG_OBJECT (gpad, "clearing target");
/* clear old target */
GST_PROXY_LOCK (gpad);
GST_OBJECT_LOCK (gpad);
if ((oldtarget = GST_PROXY_PAD_TARGET (gpad))) {
if (GST_PAD_IS_SRC (oldtarget)) {
g_signal_handlers_disconnect_by_func (oldtarget,
(gpointer) on_src_target_notify, NULL);
}
GST_PROXY_PAD_RETARGET (internal) = TRUE;
GST_OBJECT_UNLOCK (gpad);
/* unlink internal pad */
if (GST_PAD_IS_SRC (internal))
gst_pad_unlink (internal, oldtarget);
else
gst_pad_unlink (oldtarget, internal);
GST_PROXY_PAD_RETARGET (internal) = FALSE;
} else {
GST_OBJECT_UNLOCK (gpad);
}
result = gst_proxy_pad_set_target_unlocked (GST_PAD_CAST (gpad), newtarget);
GST_PROXY_UNLOCK (gpad);
if (result && newtarget) {
if (newtarget) {
if (GST_PAD_IS_SRC (newtarget)) {
g_signal_connect (newtarget, "notify::caps",
G_CALLBACK (on_src_target_notify), NULL);
g_signal_connect (newtarget, "unlinked",
G_CALLBACK (on_src_target_unlinked), NULL);
}
/* and link to internal pad without any checks */
@ -1558,17 +1429,13 @@ gst_ghost_pad_set_target (GstGhostPad * gpad, GstPad * newtarget)
goto link_failed;
}
return result;
return TRUE;
/* ERRORS */
link_failed:
{
GST_WARNING_OBJECT (gpad, "could not link internal and target, reason:%d",
lret);
/* and unset target again */
GST_PROXY_LOCK (gpad);
gst_proxy_pad_set_target_unlocked (GST_PAD_CAST (gpad), NULL);
GST_PROXY_UNLOCK (gpad);
return FALSE;
}
}

View file

@ -106,9 +106,8 @@ GST_START_TEST (test_remove2)
ret = gst_pad_link (srcpad, sinkpad);
GST_DEBUG ("linked srcpad and sinkpad");
fail_unless (ret == GST_PAD_LINK_OK);
/* the linking causes a proxypad to be created for srcpad,
* to which sinkpad gets linked. This proxypad has a ref to srcpad */
ASSERT_OBJECT_REFCOUNT (srcpad, "srcpad", 3);
/* Refcount should be unchanged, targets are now decuced using peer pad */
ASSERT_OBJECT_REFCOUNT (srcpad, "srcpad", 2);
ASSERT_OBJECT_REFCOUNT (sinkpad, "sinkpad", 2);
gst_object_unref (srcpad);
gst_object_unref (sinkpad);
@ -120,14 +119,14 @@ GST_START_TEST (test_remove2)
/* pad is still linked to ghostpad */
fail_if (!gst_pad_is_linked (srcpad));
ASSERT_OBJECT_REFCOUNT (src, "src", 1);
ASSERT_OBJECT_REFCOUNT (srcpad, "srcpad", 3);
ASSERT_OBJECT_REFCOUNT (srcpad, "srcpad", 2);
gst_object_unref (srcpad);
ASSERT_OBJECT_REFCOUNT (sinkpad, "sinkpad", 1);
/* cleanup */
/* now unlink the pads */
gst_pad_unlink (srcpad, sinkpad);
ASSERT_OBJECT_REFCOUNT (srcpad, "srcpad", 1); /* proxy has dropped ref */
ASSERT_OBJECT_REFCOUNT (srcpad, "srcpad", 1); /* we dropped our ref */
ASSERT_OBJECT_REFCOUNT (sinkpad, "sinkpad", 1);
ASSERT_OBJECT_REFCOUNT (src, "src", 1);
@ -361,15 +360,15 @@ GST_START_TEST (test_ghost_pads)
/* all objects above have one refcount owned by us as well */
ASSERT_OBJECT_REFCOUNT (fsrc, "fsrc", 3); /* parent and gisrc */
ASSERT_OBJECT_REFCOUNT (fsrc, "fsrc", 2); /* parent */
ASSERT_OBJECT_REFCOUNT (gsink, "gsink", 2); /* parent */
ASSERT_OBJECT_REFCOUNT (gsrc, "gsrc", 2); /* parent */
ASSERT_OBJECT_REFCOUNT (fsink, "fsink", 3); /* parent and gisink */
ASSERT_OBJECT_REFCOUNT (fsink, "fsink", 2); /* parent */
ASSERT_OBJECT_REFCOUNT (gisrc, "gisrc", 2); /* parent */
ASSERT_OBJECT_REFCOUNT (isink, "isink", 3); /* parent and gsink */
ASSERT_OBJECT_REFCOUNT (isink, "isink", 2); /* parent */
ASSERT_OBJECT_REFCOUNT (gisink, "gisink", 2); /* parent */
ASSERT_OBJECT_REFCOUNT (isrc, "isrc", 3); /* parent and gsrc */
ASSERT_OBJECT_REFCOUNT (isrc, "isrc", 2); /* parent */
ret = gst_element_set_state (b1, GST_STATE_PLAYING);
ret = gst_element_get_state (b1, NULL, NULL, GST_CLOCK_TIME_NONE);
@ -1027,6 +1026,59 @@ GST_START_TEST (test_ghost_pads_change_when_linked)
GST_END_TEST;
/* test that setting a ghostpad proxy pad as ghostpad target automatically set
* both ghostpad targets.
*
* fakesrc ! ( ) ! fakesink
*/
GST_START_TEST (test_ghost_pads_internal_link)
{
GstElement *pipeline, *src, *bin, *sink;
GstPad *sinkpad, *srcpad, *target;
GstProxyPad *proxypad;
pipeline = gst_element_factory_make ("pipeline", NULL);
bin = gst_element_factory_make ("bin", NULL);
src = gst_element_factory_make ("fakesrc", NULL);
sink = gst_element_factory_make ("fakesink", NULL);
gst_bin_add (GST_BIN (pipeline), src);
gst_bin_add (GST_BIN (pipeline), bin);
gst_bin_add (GST_BIN (pipeline), sink);
/* create the sink ghostpad */
sinkpad = gst_ghost_pad_new_no_target ("sink", GST_PAD_SINK);
proxypad = gst_proxy_pad_get_internal (GST_PROXY_PAD (sinkpad));
gst_element_add_pad (bin, sinkpad);
/* create the src ghostpad and link it to sink proxypad */
srcpad = gst_ghost_pad_new ("src", GST_PAD (proxypad));
gst_object_unref (proxypad);
gst_element_add_pad (bin, srcpad);
fail_unless (gst_element_link_many (src, bin, sink, NULL));
/* Check that both targets are set, and point to each other */
target = gst_ghost_pad_get_target (GST_GHOST_PAD (sinkpad));
fail_if (target == NULL);
proxypad = gst_proxy_pad_get_internal (GST_PROXY_PAD (srcpad));
fail_unless (target == GST_PAD (proxypad));
gst_object_unref (target);
gst_object_unref (proxypad);
target = gst_ghost_pad_get_target (GST_GHOST_PAD (srcpad));
fail_if (target == NULL);
proxypad = gst_proxy_pad_get_internal (GST_PROXY_PAD (sinkpad));
fail_unless (target == GST_PAD (proxypad));
gst_object_unref (target);
gst_object_unref (proxypad);
/* clean up */
gst_object_unref (pipeline);
}
GST_END_TEST;
static Suite *
gst_ghost_pad_suite (void)
@ -1051,6 +1103,7 @@ gst_ghost_pad_suite (void)
tcase_add_test (tc_chain, test_ghost_pads_sink_link_unlink);
tcase_add_test (tc_chain, test_ghost_pads_src_link_unlink);
tcase_add_test (tc_chain, test_ghost_pads_change_when_linked);
tcase_add_test (tc_chain, test_ghost_pads_internal_link);
return s;
}