diff --git a/ChangeLog b/ChangeLog index be7771e635..88caec318d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,31 @@ +2005-07-29 Wim Taymans + + * check/gst/gstghostpad.c: (GST_START_TEST), (gst_ghost_pad_suite): + Added test for removing an element with ghostpad from a bin. + Fixed test as current implementation does the right thing. + + * gst/gstghostpad.c: (gst_proxy_pad_class_init), + (gst_proxy_pad_do_query_type), (gst_proxy_pad_do_event), + (gst_proxy_pad_do_query), (gst_proxy_pad_do_internal_link), + (gst_proxy_pad_do_bufferalloc), (gst_proxy_pad_do_activate), + (gst_proxy_pad_do_activatepull), (gst_proxy_pad_do_activatepush), + (gst_proxy_pad_do_chain), (gst_proxy_pad_do_getrange), + (gst_proxy_pad_do_checkgetrange), (gst_proxy_pad_do_getcaps), + (gst_proxy_pad_do_acceptcaps), (gst_proxy_pad_do_fixatecaps), + (gst_proxy_pad_do_setcaps), (gst_proxy_pad_set_target), + (gst_proxy_pad_get_target), (gst_proxy_pad_init), + (gst_proxy_pad_dispose), (gst_proxy_pad_finalize), + (gst_ghost_pad_class_init), (gst_ghost_pad_do_activate_push), + (gst_ghost_pad_do_link), (gst_ghost_pad_do_unlink), + (gst_ghost_pad_set_internal), (gst_ghost_pad_dispose), + (gst_ghost_pad_new_notarget), (gst_ghost_pad_new), + (gst_ghost_pad_get_target), (gst_ghost_pad_set_target): + * gst/gstghostpad.h: + Clean up ghostpads, remove properties for internal stuff. + Make threadsafe. + Fix refcounting. + Prepare for switching targets, not all use cases work yet. + 2005-07-29 Wim Taymans * docs/design/part-gstghostpad.txt: diff --git a/check/gst/gstghostpad.c b/check/gst/gstghostpad.c index 580c6301e8..274148337f 100644 --- a/check/gst/gstghostpad.c +++ b/check/gst/gstghostpad.c @@ -70,6 +70,46 @@ GST_START_TEST (test_remove1) GST_END_TEST; +/* test if removing a bin also cleans up the ghostpads + */ +GST_START_TEST (test_remove2) +{ + GstElement *b1, *b2, *src, *sink; + GstPad *srcpad, *sinkpad; + GstPadLinkReturn ret; + + b1 = gst_element_factory_make ("pipeline", NULL); + b2 = gst_element_factory_make ("bin", NULL); + src = gst_element_factory_make ("fakesrc", NULL); + sink = gst_element_factory_make ("fakesink", NULL); + + fail_unless (gst_bin_add (GST_BIN (b2), sink)); + fail_unless (gst_bin_add (GST_BIN (b1), src)); + fail_unless (gst_bin_add (GST_BIN (b1), b2)); + + sinkpad = gst_element_get_pad (sink, "sink"); + gst_element_add_pad (b2, gst_ghost_pad_new ("sink", sinkpad)); + gst_object_unref (sinkpad); + + srcpad = gst_element_get_pad (src, "src"); + /* get the ghostpad */ + sinkpad = gst_element_get_pad (b2, "sink"); + + ret = gst_pad_link (srcpad, sinkpad); + fail_unless (ret == GST_PAD_LINK_OK); + gst_object_unref (srcpad); + gst_object_unref (sinkpad); + + /* now remove the sink from the bin */ + gst_bin_remove (GST_BIN (b2), sink); + + srcpad = gst_element_get_pad (src, "src"); + /* pad is still linked to ghostpad */ + fail_if (!gst_pad_is_linked (srcpad)); +} + +GST_END_TEST; + /* test if linking fails over different bins using a pipeline * like this: * @@ -186,23 +226,16 @@ GST_START_TEST (test_ghost_pads) /* unreffing the bin will unref all elements, which will unlink and unparent * all pads */ - /* FIXME: ghost pads need to drop their internal pad in the unlink function, - * but can't right now. So internal pads have a ref from their parent, and the - * internal pads' targets have refs from the internals. When we do the last - * unref on the ghost pads, these refs should go away. - */ - assert_gstrefcount (fsrc, 2); /* gisrc */ assert_gstrefcount (gsink, 1); assert_gstrefcount (gsrc, 1); assert_gstrefcount (fsink, 2); /* gisink */ - assert_gstrefcount (gisrc, 2); /* gsink -- fixme drop ref in unlink */ + assert_gstrefcount (gisrc, 1); /* gsink */ assert_gstrefcount (isink, 2); /* gsink */ - assert_gstrefcount (gisink, 2); /* gsrc -- fixme drop ref in unlink */ + assert_gstrefcount (gisink, 1); /* gsrc */ assert_gstrefcount (isrc, 2); /* gsrc */ - /* while the fixme isn't fixed, check cleanup */ gst_object_unref (gsink); assert_gstrefcount (isink, 1); assert_gstrefcount (gisrc, 1); @@ -216,6 +249,11 @@ GST_START_TEST (test_ghost_pads) assert_gstrefcount (fsink, 2); /* gisrc */ gst_object_unref (gisink); assert_gstrefcount (fsink, 1); + + gst_object_unref (fsrc); + gst_object_unref (isrc); + gst_object_unref (isink); + gst_object_unref (fsink); } GST_END_TEST; @@ -228,6 +266,7 @@ gst_ghost_pad_suite (void) suite_add_tcase (s, tc_chain); tcase_add_test (tc_chain, test_remove1); + tcase_add_test (tc_chain, test_remove2); tcase_add_test (tc_chain, test_link); tcase_add_test (tc_chain, test_ghost_pads); diff --git a/gst/gstghostpad.c b/gst/gstghostpad.c index 81628377bc..15cdba1d15 100644 --- a/gst/gstghostpad.c +++ b/gst/gstghostpad.c @@ -37,21 +37,18 @@ typedef struct _GstProxyPad GstProxyPad; typedef struct _GstProxyPadClass GstProxyPadClass; - -enum -{ - PROXY_PROP_0, - PROXY_PROP_TARGET -}; +#define GST_PROXY_GET_LOCK(pad) (GST_PROXY_PAD (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 _GstProxyPad { GstPad pad; + /* with PROXY_LOCK */ + GMutex *proxy_lock; GstPad *target; - GMutex *property_lock; - /*< private > */ gpointer _gst_reserved[1]; }; @@ -67,12 +64,9 @@ struct _GstProxyPadClass 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_set_property (GObject * object, guint prop_id, - const GValue * value, GParamSpec * pspec); -static void gst_proxy_pad_get_property (GObject * object, guint prop_id, - GValue * value, GParamSpec * pspec); static void gst_proxy_pad_finalize (GObject * object); #ifndef GST_DISABLE_LOADSAVE @@ -88,12 +82,6 @@ gst_proxy_pad_class_init (GstProxyPadClass * klass) gobject_class->dispose = GST_DEBUG_FUNCPTR (gst_proxy_pad_dispose); gobject_class->finalize = GST_DEBUG_FUNCPTR (gst_proxy_pad_finalize); - gobject_class->set_property = GST_DEBUG_FUNCPTR (gst_proxy_pad_set_property); - gobject_class->get_property = GST_DEBUG_FUNCPTR (gst_proxy_pad_get_property); - - g_object_class_install_property (G_OBJECT_CLASS (klass), PROXY_PROP_TARGET, - g_param_spec_object ("target", "Target", "The proxy pad's target", - GST_TYPE_PAD, G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY)); #ifndef GST_DISABLE_LOADSAVE { @@ -108,41 +96,57 @@ gst_proxy_pad_class_init (GstProxyPadClass * klass) const GstQueryType * gst_proxy_pad_do_query_type (GstPad * pad) { - GstPad *target = GST_PROXY_PAD_TARGET (pad); + GstPad *target = gst_proxy_pad_get_target (pad); + const GstQueryType *res; g_return_val_if_fail (target != NULL, NULL); - return gst_pad_get_query_types (target); + res = gst_pad_get_query_types (target); + gst_object_unref (target); + + return res; } static gboolean gst_proxy_pad_do_event (GstPad * pad, GstEvent * event) { - GstPad *target = GST_PROXY_PAD_TARGET (pad); + gboolean res; + GstPad *target = gst_proxy_pad_get_target (pad); g_return_val_if_fail (target != NULL, FALSE); - return gst_pad_send_event (target, event); + res = gst_pad_send_event (target, event); + gst_object_unref (target); + + return res; } static gboolean gst_proxy_pad_do_query (GstPad * pad, GstQuery * query) { - GstPad *target = GST_PROXY_PAD_TARGET (pad); + gboolean res; + GstPad *target = gst_proxy_pad_get_target (pad); g_return_val_if_fail (target != NULL, FALSE); - return gst_pad_query (target, query); + res = gst_pad_query (target, query); + gst_object_unref (target); + + return res; } static GList * gst_proxy_pad_do_internal_link (GstPad * pad) { - GstPad *target = GST_PROXY_PAD_TARGET (pad); + GstPad *target = gst_proxy_pad_get_target (pad); + GList *res; g_return_val_if_fail (target != NULL, NULL); - return gst_pad_get_internal_links (target); + res = gst_pad_get_internal_links (target); + gst_object_unref (target); + + return res; } static GstFlowReturn @@ -150,7 +154,7 @@ gst_proxy_pad_do_bufferalloc (GstPad * pad, guint64 offset, guint size, GstCaps * caps, GstBuffer ** buf) { GstFlowReturn result; - GstPad *target = GST_PROXY_PAD_TARGET (pad); + GstPad *target = gst_proxy_pad_get_target (pad); GstPad *peer; g_return_val_if_fail (target != NULL, GST_FLOW_NOT_LINKED); @@ -166,24 +170,27 @@ gst_proxy_pad_do_bufferalloc (GstPad * pad, guint64 offset, guint size, result = GST_FLOW_NOT_LINKED; } + gst_object_unref (target); + return result; } static gboolean gst_proxy_pad_do_activate (GstPad * pad) { - GstPad *target = GST_PROXY_PAD_TARGET (pad); + gboolean res; - g_return_val_if_fail (target != NULL, FALSE); + res = gst_pad_activate_push (pad, TRUE); - return gst_pad_activate_push (pad, TRUE); + return res; } static gboolean gst_proxy_pad_do_activatepull (GstPad * pad, gboolean active) { GstActivateMode old; - GstPad *target = GST_PROXY_PAD_TARGET (pad); + GstPad *target = gst_proxy_pad_get_target (pad); + gboolean res; g_return_val_if_fail (target != NULL, FALSE); @@ -193,16 +200,20 @@ gst_proxy_pad_do_activatepull (GstPad * pad, gboolean active) if ((active && old == GST_ACTIVATE_PULL) || (!active && old == GST_ACTIVATE_NONE)) - return TRUE; + res = TRUE; else - return gst_pad_activate_pull (target, active); + res = gst_pad_activate_pull (target, active); + gst_object_unref (target); + + return res; } static gboolean gst_proxy_pad_do_activatepush (GstPad * pad, gboolean active) { GstActivateMode old; - GstPad *target = GST_PROXY_PAD_TARGET (pad); + GstPad *target = gst_proxy_pad_get_target (pad); + gboolean res; g_return_val_if_fail (target != NULL, FALSE); @@ -212,40 +223,52 @@ gst_proxy_pad_do_activatepush (GstPad * pad, gboolean active) if ((active && old == GST_ACTIVATE_PUSH) || (!active && old == GST_ACTIVATE_NONE)) - return TRUE; + res = TRUE; else - return gst_pad_activate_push (target, active); + res = gst_pad_activate_push (target, active); + gst_object_unref (target); + + return res; } static GstFlowReturn gst_proxy_pad_do_chain (GstPad * pad, GstBuffer * buffer) { - GstPad *target = GST_PROXY_PAD_TARGET (pad); + GstPad *target = gst_proxy_pad_get_target (pad); + GstFlowReturn res; g_return_val_if_fail (target != NULL, GST_FLOW_NOT_LINKED); - return gst_pad_chain (target, buffer); + res = gst_pad_chain (target, buffer); + gst_object_unref (target); + + return res; } static GstFlowReturn gst_proxy_pad_do_getrange (GstPad * pad, guint64 offset, guint size, GstBuffer ** buffer) { - GstPad *target = GST_PROXY_PAD_TARGET (pad); + GstPad *target = gst_proxy_pad_get_target (pad); + GstFlowReturn res; g_return_val_if_fail (target != NULL, GST_FLOW_NOT_LINKED); - return gst_pad_get_range (target, offset, size, buffer); + res = gst_pad_get_range (target, offset, size, buffer); + gst_object_unref (target); + + return res; } static gboolean gst_proxy_pad_do_checkgetrange (GstPad * pad) { gboolean result; - GstPad *target = GST_PROXY_PAD_TARGET (pad); + GstPad *target = gst_proxy_pad_get_target (pad); GstPad *peer; g_return_val_if_fail (target != NULL, FALSE); + peer = gst_pad_get_peer (target); if (peer) { result = gst_pad_check_pull_range (peer); @@ -253,115 +276,126 @@ gst_proxy_pad_do_checkgetrange (GstPad * pad) } else { result = FALSE; } + gst_object_unref (target); + return result; } static GstCaps * gst_proxy_pad_do_getcaps (GstPad * pad) { - GstPad *target = GST_PROXY_PAD_TARGET (pad); + GstPad *target = gst_proxy_pad_get_target (pad); + GstCaps *res; g_return_val_if_fail (target != NULL, NULL); - return gst_pad_get_caps (target); + res = gst_pad_get_caps (target); + gst_object_unref (target); + + return res; } static gboolean gst_proxy_pad_do_acceptcaps (GstPad * pad, GstCaps * caps) { - GstPad *target = GST_PROXY_PAD_TARGET (pad); + GstPad *target = gst_proxy_pad_get_target (pad); + gboolean res; g_return_val_if_fail (target != NULL, FALSE); - return gst_pad_accept_caps (target, caps); + res = gst_pad_accept_caps (target, caps); + gst_object_unref (target); + + return res; } static void gst_proxy_pad_do_fixatecaps (GstPad * pad, GstCaps * caps) { - GstPad *target = GST_PROXY_PAD_TARGET (pad); + GstPad *target = gst_proxy_pad_get_target (pad); g_return_if_fail (target != NULL); gst_pad_fixate_caps (target, caps); + gst_object_unref (target); } static gboolean gst_proxy_pad_do_setcaps (GstPad * pad, GstCaps * caps) { - GstPad *target = GST_PROXY_PAD_TARGET (pad); + GstPad *target = gst_proxy_pad_get_target (pad); + gboolean res; g_return_val_if_fail (target != NULL, FALSE); - return gst_pad_set_caps (target, caps); + res = gst_pad_set_caps (target, caps); + gst_object_unref (target); + + return res; } #define SETFUNC(member, kind) \ if (target->member) \ gst_pad_set_##kind##_function (pad, gst_proxy_pad_do_##kind) -static void -gst_proxy_pad_set_property (GObject * object, guint prop_id, - const GValue * value, GParamSpec * pspec) +static gboolean +gst_proxy_pad_set_target (GstPad * pad, GstPad * target) { - GstPad *pad = GST_PAD (object); - - switch (prop_id) { - case PROXY_PROP_TARGET:{ - GstPad *target; - - target = GST_PAD_CAST (gst_object_ref - (GST_OBJECT_CAST (g_value_get_object (value)))); - GST_PROXY_PAD_TARGET (object) = target; - - /* really, all these should have default implementations so I can set them - * in the _init() instead of here */ - SETFUNC (querytypefunc, query_type); - SETFUNC (eventfunc, event); - SETFUNC (queryfunc, query); - SETFUNC (intlinkfunc, internal_link); - SETFUNC (activatefunc, activate); - SETFUNC (activatepullfunc, activatepull); - SETFUNC (activatepushfunc, activatepush); - SETFUNC (getcapsfunc, getcaps); - SETFUNC (acceptcapsfunc, acceptcaps); - SETFUNC (fixatecapsfunc, fixatecaps); - SETFUNC (setcapsfunc, setcaps); - - if (GST_PAD_DIRECTION (pad) == GST_PAD_SINK) { - SETFUNC (bufferallocfunc, bufferalloc); - SETFUNC (chainfunc, chain); - } else { - SETFUNC (getrangefunc, getrange); - SETFUNC (checkgetrangefunc, checkgetrange); - } - - break; - } - default: - G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); - break; + GST_PROXY_LOCK (pad); + /* clear old target */ + if (GST_PROXY_PAD_TARGET (pad)) { + gst_object_unref (GST_PROXY_PAD_TARGET (pad)); + GST_PROXY_PAD_TARGET (pad) = NULL; } + + if (target) { + /* set and ref new target if any */ + GST_PROXY_PAD_TARGET (pad) = gst_object_ref (target); + + /* really, all these should have default implementations so I can set them + * in the _init() instead of here */ + SETFUNC (querytypefunc, query_type); + SETFUNC (eventfunc, event); + SETFUNC (queryfunc, query); + SETFUNC (intlinkfunc, internal_link); + SETFUNC (activatefunc, activate); + SETFUNC (activatepullfunc, activatepull); + SETFUNC (activatepushfunc, activatepush); + SETFUNC (getcapsfunc, getcaps); + SETFUNC (acceptcapsfunc, acceptcaps); + SETFUNC (fixatecapsfunc, fixatecaps); + SETFUNC (setcapsfunc, setcaps); + + if (GST_PAD_DIRECTION (pad) == GST_PAD_SINK) { + SETFUNC (bufferallocfunc, bufferalloc); + SETFUNC (chainfunc, chain); + } else { + SETFUNC (getrangefunc, getrange); + SETFUNC (checkgetrangefunc, checkgetrange); + } + } + GST_PROXY_UNLOCK (pad); + + return TRUE; } -static void -gst_proxy_pad_get_property (GObject * object, guint prop_id, - GValue * value, GParamSpec * pspec) +static GstPad * +gst_proxy_pad_get_target (GstPad * pad) { - switch (prop_id) { - case PROXY_PROP_TARGET: - g_value_set_object (value, GST_PROXY_PAD_TARGET (object)); - break; - default: - G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); - break; - } + GstPad *target; + + GST_PROXY_LOCK (pad); + target = GST_PROXY_PAD_TARGET (pad); + gst_object_ref (target); + GST_PROXY_UNLOCK (pad); + + return target; } static void gst_proxy_pad_init (GstProxyPad * pad) { - pad->property_lock = g_mutex_new (); + pad->proxy_lock = g_mutex_new (); } static void @@ -369,9 +403,9 @@ gst_proxy_pad_dispose (GObject * object) { GstPad *pad = GST_PAD (object); - if (GST_PROXY_PAD_TARGET (pad)) { - gst_object_replace ((GstObject **) & GST_PROXY_PAD_TARGET (pad), NULL); - } + GST_PROXY_LOCK (pad); + gst_object_replace ((GstObject **) & GST_PROXY_PAD_TARGET (pad), NULL); + GST_PROXY_UNLOCK (pad); G_OBJECT_CLASS (gst_proxy_pad_parent_class)->dispose (object); } @@ -381,8 +415,8 @@ gst_proxy_pad_finalize (GObject * object) { GstProxyPad *pad = GST_PROXY_PAD (object); - g_mutex_free (pad->property_lock); - pad->property_lock = NULL; + g_mutex_free (pad->proxy_lock); + pad->proxy_lock = NULL; G_OBJECT_CLASS (gst_proxy_pad_parent_class)->finalize (object); } @@ -422,16 +456,11 @@ gst_proxy_pad_save_thyself (GstObject * object, xmlNodePtr parent) */ -enum -{ - GHOST_PROP_0, - GHOST_PROP_INTERNAL -}; - struct _GstGhostPad { GstProxyPad pad; + /* with PROXY_LOCK */ GstPad *internal; gulong notify_id; @@ -450,13 +479,10 @@ struct _GstGhostPadClass G_DEFINE_TYPE (GstGhostPad, gst_ghost_pad, GST_TYPE_PROXY_PAD); +static gboolean gst_ghost_pad_set_internal (GstGhostPad * pad, + GstPad * internal); static void gst_ghost_pad_dispose (GObject * object); -static void gst_ghost_pad_set_property (GObject * object, guint prop_id, - const GValue * value, GParamSpec * pspec); -static void gst_ghost_pad_get_property (GObject * object, guint prop_id, - GValue * value, GParamSpec * pspec); - /* Work around g_logv's use of G_GNUC_PRINTF because gcc chokes on %P, which we * use for GST_PTR_FORMAT. */ @@ -476,12 +502,6 @@ gst_ghost_pad_class_init (GstGhostPadClass * klass) GObjectClass *gobject_class = (GObjectClass *) klass; gobject_class->dispose = GST_DEBUG_FUNCPTR (gst_ghost_pad_dispose); - gobject_class->set_property = GST_DEBUG_FUNCPTR (gst_ghost_pad_set_property); - gobject_class->get_property = GST_DEBUG_FUNCPTR (gst_ghost_pad_get_property); - - g_object_class_install_property (G_OBJECT_CLASS (klass), GHOST_PROP_INTERNAL, - g_param_spec_object ("internal", "Internal", - "The ghost pad's internal pad", GST_TYPE_PAD, G_PARAM_READWRITE)); } /* will only be called for src pads (afaict) */ @@ -511,10 +531,10 @@ gst_ghost_pad_do_activate_push (GstPad * pad, gboolean active) ret = gst_proxy_pad_do_activatepush (pad, active); - GST_LOCK (pad); + GST_PROXY_LOCK (pad); if ((internal = GST_GHOST_PAD (pad)->internal)) gst_object_ref (internal); - GST_UNLOCK (pad); + GST_PROXY_UNLOCK (pad); if (internal) { ret &= gst_pad_activate_push (internal, active); @@ -528,41 +548,49 @@ static GstPadLinkReturn gst_ghost_pad_do_link (GstPad * pad, GstPad * peer) { GstPad *internal, *target; + GstPadLinkReturn ret; + + target = gst_proxy_pad_get_target (pad); - target = GST_PROXY_PAD_TARGET (pad); g_return_val_if_fail (target != NULL, GST_PAD_LINK_NOSCHED); /* proxy the peer into the bin */ internal = g_object_new (GST_TYPE_PROXY_PAD, "name", NULL, "direction", GST_PAD_DIRECTION (peer), - "template", GST_PAD_PAD_TEMPLATE (peer), "target", peer, NULL); - g_object_set (pad, "internal", internal, NULL); + "template", GST_PAD_PAD_TEMPLATE (peer), NULL); - if ((GST_PAD_IS_SRC (internal) && - gst_pad_link (internal, target) == GST_PAD_LINK_OK) || - (GST_PAD_IS_SINK (internal) && - (gst_pad_link (target, internal) == GST_PAD_LINK_OK))) { + gst_proxy_pad_set_target (internal, peer); + gst_ghost_pad_set_internal (GST_GHOST_PAD (pad), internal); + + if (GST_PAD_IS_SRC (internal)) + ret = gst_pad_link (internal, target); + else + ret = gst_pad_link (target, internal); + + gst_object_unref (target); + + if (ret == GST_PAD_LINK_OK) gst_pad_set_active (internal, GST_PAD_ACTIVATE_MODE (pad)); - return GST_PAD_LINK_OK; - } else { - g_object_set (pad, "internal", NULL, NULL); - return GST_PAD_LINK_REFUSED; - } + else + gst_ghost_pad_set_internal (GST_GHOST_PAD (pad), NULL); + + return ret; } static void gst_ghost_pad_do_unlink (GstPad * pad) { - GstPad *target = GST_PROXY_PAD_TARGET (pad); + GstPad *target = gst_proxy_pad_get_target (pad); g_return_if_fail (target != NULL); if (target->unlinkfunc) target->unlinkfunc (target); - /* FIXME: should do this here, but locks in deep_notify prevent it */ - /* g_object_set (pad, "internal", NULL, NULL); */ + gst_ghost_pad_set_internal (GST_GHOST_PAD (pad), NULL); + + gst_object_unref (target); } static void @@ -581,84 +609,57 @@ on_int_notify (GstPad * internal, GParamSpec * unused, GstGhostPad * pad) gst_caps_unref (caps); } -static void -gst_ghost_pad_set_property (GObject * object, guint prop_id, - const GValue * value, GParamSpec * pspec) +static gboolean +gst_ghost_pad_set_internal (GstGhostPad * pad, GstPad * internal) { - GstGhostPad *pad = GST_GHOST_PAD (object); + GST_PROXY_LOCK (pad); + /* first remove old internal pad */ + if (pad->internal) { + GstPad *intpeer; - switch (prop_id) { - case GHOST_PROP_INTERNAL:{ - GstPad *internal; + gst_pad_set_activatepull_function (pad->internal, NULL); - g_mutex_lock (GST_PROXY_PAD (pad)->property_lock); + g_signal_handler_disconnect (pad->internal, pad->notify_id); - if (pad->internal) { - GstPad *intpeer; - - gst_pad_set_activatepull_function (pad->internal, NULL); - - g_signal_handler_disconnect (pad->internal, pad->notify_id); - - intpeer = gst_pad_get_peer (pad->internal); - if (intpeer) { - if (GST_PAD_IS_SRC (pad->internal)) { - gst_pad_unlink (pad->internal, intpeer); - } else { - gst_pad_unlink (intpeer, pad->internal); - } - gst_object_unref (intpeer); - } - - /* should dispose it */ - gst_object_unparent (GST_OBJECT_CAST (pad->internal)); - } - - internal = g_value_get_object (value); /* no extra refcount... */ - - if (internal) { - if (!gst_object_set_parent (GST_OBJECT_CAST (internal), - GST_OBJECT_CAST (pad))) { - gst_critical ("Could not set internal pad %" GST_PTR_FORMAT, - internal); - g_mutex_unlock (GST_PROXY_PAD (pad)->property_lock); - return; - } - - /* could be more general here, iterating over all writable properties... - * taking the short road for now tho */ - pad->notify_id = g_signal_connect (internal, "notify::caps", - G_CALLBACK (on_int_notify), pad); - on_int_notify (internal, NULL, pad); - gst_pad_set_activatepull_function (internal, - gst_ghost_proxy_pad_do_activate_pull); - - /* a ref was taken by set_parent */ - } - - pad->internal = internal; - - g_mutex_unlock (GST_PROXY_PAD (pad)->property_lock); - - break; + intpeer = gst_pad_get_peer (pad->internal); + if (intpeer) { + if (GST_PAD_IS_SRC (pad->internal)) + gst_pad_unlink (pad->internal, intpeer); + else + gst_pad_unlink (intpeer, pad->internal); + gst_object_unref (intpeer); } - default: - G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); - break; + /* should dispose it */ + gst_object_unparent (GST_OBJECT_CAST (pad->internal)); } -} -static void -gst_ghost_pad_get_property (GObject * object, guint prop_id, - GValue * value, GParamSpec * pspec) -{ - switch (prop_id) { - case GHOST_PROP_INTERNAL: - g_value_set_object (value, GST_GHOST_PAD (object)->internal); - break; - default: - G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); - break; + /* then set new internal pad */ + if (internal) { + if (!gst_object_set_parent (GST_OBJECT_CAST (internal), + GST_OBJECT_CAST (pad))) + goto could_not_set; + + /* could be more general here, iterating over all writable properties... + * taking the short road for now tho */ + pad->notify_id = g_signal_connect (internal, "notify::caps", + G_CALLBACK (on_int_notify), pad); + on_int_notify (internal, NULL, pad); + gst_pad_set_activatepull_function (internal, + gst_ghost_proxy_pad_do_activate_pull); + /* a ref was taken by set_parent */ + } + pad->internal = internal; + + GST_PROXY_UNLOCK (pad); + + return TRUE; + + /* ERRORS */ +could_not_set: + { + gst_critical ("Could not set internal pad %" GST_PTR_FORMAT, internal); + GST_PROXY_UNLOCK (pad); + return FALSE; } } @@ -671,16 +672,46 @@ gst_ghost_pad_init (GstGhostPad * pad) static void gst_ghost_pad_dispose (GObject * object) { - g_object_set (object, "internal", NULL, NULL); + gst_ghost_pad_set_internal (GST_GHOST_PAD (object), NULL); G_OBJECT_CLASS (gst_ghost_pad_parent_class)->dispose (object); } +/** + * gst_ghost_pad_new_notarget: + * @name: the name of the new pad, or NULL to assign a default name. + * @dir: the direction of the ghostpad + * + * Create a new ghostpad without a target with the given direction. + * A target can be set on the ghostpad later with the + * #gst_ghost_pad_set_target() function. + * + * The created ghostpad will not have a padtemplate. + * + * Returns: a new #GstPad, or NULL in case of an error. + */ +GstPad * +gst_ghost_pad_new_notarget (const gchar * name, GstPadDirection dir) +{ + GstPad *ret; + + ret = g_object_new (GST_TYPE_GHOST_PAD, "name", name, "direction", dir, NULL); + + gst_pad_set_activatepush_function (ret, gst_ghost_pad_do_activate_push); + gst_pad_set_link_function (ret, gst_ghost_pad_do_link); + gst_pad_set_unlink_function (ret, gst_ghost_pad_do_unlink); + + return ret; +} + /** * gst_ghost_pad_new: * @name: the name of the new pad, or NULL to assign a default name. * @target: the pad to ghost. * + * Create a new ghostpad with @target as the target. The direction and + * padtemplate will be taken from the target pad. + * * Will ref the target. * * Returns: a new #GstPad, or NULL in case of an error. @@ -691,16 +722,47 @@ gst_ghost_pad_new (const gchar * name, GstPad * target) GstPad *ret; g_return_val_if_fail (GST_IS_PAD (target), NULL); - g_return_val_if_fail (!GST_PAD_IS_LINKED (target), NULL); - - ret = g_object_new (GST_TYPE_GHOST_PAD, - "name", name, - "direction", GST_PAD_DIRECTION (target), - "template", GST_PAD_PAD_TEMPLATE (target), "target", target, NULL); - - gst_pad_set_activatepush_function (ret, gst_ghost_pad_do_activate_push); - gst_pad_set_link_function (ret, gst_ghost_pad_do_link); - gst_pad_set_unlink_function (ret, gst_ghost_pad_do_unlink); + g_return_val_if_fail (!gst_pad_is_linked (target), NULL); + if ((ret = gst_ghost_pad_new_notarget (name, GST_PAD_DIRECTION (target)))) { + g_object_set (G_OBJECT (ret), + "template", GST_PAD_PAD_TEMPLATE (target), NULL); + gst_ghost_pad_set_target (GST_GHOST_PAD (ret), target); + } return ret; } + +/** + * gst_ghost_pad_get_target: + * @gpad: the #GstGhostpad + * + * Get the target pad of #gpad. Unref after usage. + * + * Returns: the target #GstPad, can be NULL if the ghostpad + * has no target set. Unref after usage. + */ +GstPad * +gst_ghost_pad_get_target (GstGhostPad * gpad) +{ + g_return_val_if_fail (GST_IS_GHOST_PAD (gpad), NULL); + + return gst_proxy_pad_get_target (GST_PAD_CAST (gpad)); +} + +/** + * gst_ghost_pad_set_target: + * @gpad: the #GstGhostpad + * @newtarget: the new pad target + * + * Set the new target of the ghostpad @gpad. Any existing target + * is unlinked. + * + * Returns: TRUE if the new target could be set, FALSE otherwise. + */ +gboolean +gst_ghost_pad_set_target (GstGhostPad * gpad, GstPad * newtarget) +{ + g_return_val_if_fail (GST_IS_GHOST_PAD (gpad), FALSE); + + return gst_proxy_pad_set_target (GST_PAD_CAST (gpad), newtarget); +} diff --git a/gst/gstghostpad.h b/gst/gstghostpad.h index 51f2f848c5..24a8ba4a90 100644 --- a/gst/gstghostpad.h +++ b/gst/gstghostpad.h @@ -31,23 +31,23 @@ G_BEGIN_DECLS - #define GST_TYPE_GHOST_PAD (gst_ghost_pad_get_type ()) #define GST_IS_GHOST_PAD(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), GST_TYPE_GHOST_PAD)) #define GST_IS_GHOST_PAD_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), GST_TYPE_GHOST_PAD)) #define GST_GHOST_PAD(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), GST_TYPE_GHOST_PAD, GstGhostPad)) #define GST_GHOST_PAD_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), GST_TYPE_GHOST_PAD, GstGhostPadClass)) - typedef struct _GstGhostPad GstGhostPad; typedef struct _GstGhostPadClass GstGhostPadClass; - GType gst_ghost_pad_get_type (void); -GstPad* gst_ghost_pad_new (const gchar *name, GstPad *target); +GstPad* gst_ghost_pad_new (const gchar *name, GstPad *target); +GstPad* gst_ghost_pad_new_notarget (const gchar *name, GstPadDirection dir); + +GstPad* gst_ghost_pad_get_target (GstGhostPad *gpad); +gboolean gst_ghost_pad_set_target (GstGhostPad *gpad, GstPad *newtarget); G_END_DECLS - #endif /* __GST_GHOST_PAD_H__ */ diff --git a/tests/check/gst/gstghostpad.c b/tests/check/gst/gstghostpad.c index 580c6301e8..274148337f 100644 --- a/tests/check/gst/gstghostpad.c +++ b/tests/check/gst/gstghostpad.c @@ -70,6 +70,46 @@ GST_START_TEST (test_remove1) GST_END_TEST; +/* test if removing a bin also cleans up the ghostpads + */ +GST_START_TEST (test_remove2) +{ + GstElement *b1, *b2, *src, *sink; + GstPad *srcpad, *sinkpad; + GstPadLinkReturn ret; + + b1 = gst_element_factory_make ("pipeline", NULL); + b2 = gst_element_factory_make ("bin", NULL); + src = gst_element_factory_make ("fakesrc", NULL); + sink = gst_element_factory_make ("fakesink", NULL); + + fail_unless (gst_bin_add (GST_BIN (b2), sink)); + fail_unless (gst_bin_add (GST_BIN (b1), src)); + fail_unless (gst_bin_add (GST_BIN (b1), b2)); + + sinkpad = gst_element_get_pad (sink, "sink"); + gst_element_add_pad (b2, gst_ghost_pad_new ("sink", sinkpad)); + gst_object_unref (sinkpad); + + srcpad = gst_element_get_pad (src, "src"); + /* get the ghostpad */ + sinkpad = gst_element_get_pad (b2, "sink"); + + ret = gst_pad_link (srcpad, sinkpad); + fail_unless (ret == GST_PAD_LINK_OK); + gst_object_unref (srcpad); + gst_object_unref (sinkpad); + + /* now remove the sink from the bin */ + gst_bin_remove (GST_BIN (b2), sink); + + srcpad = gst_element_get_pad (src, "src"); + /* pad is still linked to ghostpad */ + fail_if (!gst_pad_is_linked (srcpad)); +} + +GST_END_TEST; + /* test if linking fails over different bins using a pipeline * like this: * @@ -186,23 +226,16 @@ GST_START_TEST (test_ghost_pads) /* unreffing the bin will unref all elements, which will unlink and unparent * all pads */ - /* FIXME: ghost pads need to drop their internal pad in the unlink function, - * but can't right now. So internal pads have a ref from their parent, and the - * internal pads' targets have refs from the internals. When we do the last - * unref on the ghost pads, these refs should go away. - */ - assert_gstrefcount (fsrc, 2); /* gisrc */ assert_gstrefcount (gsink, 1); assert_gstrefcount (gsrc, 1); assert_gstrefcount (fsink, 2); /* gisink */ - assert_gstrefcount (gisrc, 2); /* gsink -- fixme drop ref in unlink */ + assert_gstrefcount (gisrc, 1); /* gsink */ assert_gstrefcount (isink, 2); /* gsink */ - assert_gstrefcount (gisink, 2); /* gsrc -- fixme drop ref in unlink */ + assert_gstrefcount (gisink, 1); /* gsrc */ assert_gstrefcount (isrc, 2); /* gsrc */ - /* while the fixme isn't fixed, check cleanup */ gst_object_unref (gsink); assert_gstrefcount (isink, 1); assert_gstrefcount (gisrc, 1); @@ -216,6 +249,11 @@ GST_START_TEST (test_ghost_pads) assert_gstrefcount (fsink, 2); /* gisrc */ gst_object_unref (gisink); assert_gstrefcount (fsink, 1); + + gst_object_unref (fsrc); + gst_object_unref (isrc); + gst_object_unref (isink); + gst_object_unref (fsink); } GST_END_TEST; @@ -228,6 +266,7 @@ gst_ghost_pad_suite (void) suite_add_tcase (s, tc_chain); tcase_add_test (tc_chain, test_remove1); + tcase_add_test (tc_chain, test_remove2); tcase_add_test (tc_chain, test_link); tcase_add_test (tc_chain, test_ghost_pads);