From: Wim Taymans Date: Thu, 31 Aug 2006 15:19:44 +0000 (+0000) Subject: docs/design/part-gstghostpad.txt: Update ascii art in documentation. X-Git-Tag: RELEASE-0_10_10~16 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=47e5ba2f1522e597b348dee1e8989a9d18f0838c;p=platform%2Fupstream%2Fgstreamer.git docs/design/part-gstghostpad.txt: Update ascii art in documentation. Original commit message from CVS: * docs/design/part-gstghostpad.txt: Update ascii art in documentation. * gst/gstghostpad.c: (gst_proxy_pad_do_internal_link), (gst_proxy_pad_set_target_unlocked), (gst_proxy_pad_init), (gst_ghost_pad_parent_set), (gst_ghost_pad_parent_unset), (gst_ghost_pad_internal_do_activate_push), (gst_ghost_pad_internal_do_activate_pull), (gst_ghost_pad_do_activate_push), (gst_ghost_pad_do_activate_pull), (gst_ghost_pad_do_link), (gst_ghost_pad_do_unlink), (gst_ghost_pad_dispose), (gst_ghost_pad_new_full), (gst_ghost_pad_set_target): Small cleanups and leak fixes. Remove some checks now that the internal pad is never NULL. Fix the case where linking pads without a target would create nasty criticals. Fixes #341029. Don't assign a GstPadLinkReturn to a gboolean and mess up the return value of _set_target(). * tests/check/gst/gstghostpad.c: (GST_START_TEST), (gst_ghost_pad_suite): Some more tests for creating and linking untargeted ghostpads. --- diff --git a/ChangeLog b/ChangeLog index ab90f2a..088c5eb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,28 @@ +2006-08-31 Wim Taymans + + * docs/design/part-gstghostpad.txt: + Update ascii art in documentation. + + * gst/gstghostpad.c: (gst_proxy_pad_do_internal_link), + (gst_proxy_pad_set_target_unlocked), (gst_proxy_pad_init), + (gst_ghost_pad_parent_set), (gst_ghost_pad_parent_unset), + (gst_ghost_pad_internal_do_activate_push), + (gst_ghost_pad_internal_do_activate_pull), + (gst_ghost_pad_do_activate_push), (gst_ghost_pad_do_activate_pull), + (gst_ghost_pad_do_link), (gst_ghost_pad_do_unlink), + (gst_ghost_pad_dispose), (gst_ghost_pad_new_full), + (gst_ghost_pad_set_target): + Small cleanups and leak fixes. + Remove some checks now that the internal pad is never NULL. + Fix the case where linking pads without a target would create nasty + criticals. Fixes #341029. + Don't assign a GstPadLinkReturn to a gboolean and mess up the return + value of _set_target(). + + * tests/check/gst/gstghostpad.c: (GST_START_TEST), + (gst_ghost_pad_suite): + Some more tests for creating and linking untargeted ghostpads. + 2006-08-31 Edward Hervey * docs/gst/gstreamer-sections.txt: diff --git a/docs/design/part-gstghostpad.txt b/docs/design/part-gstghostpad.txt index 242f897..d5c987c 100644 --- a/docs/design/part-gstghostpad.txt +++ b/docs/design/part-gstghostpad.txt @@ -9,9 +9,9 @@ Ghostpads - Must look like a real GstPad on both sides. - target of Ghostpad must be changeable + - target can be initially NULL - -* a GhostPad is implemented using a smaller GstProxyPad class: +* a GhostPad is implemented using a private GstProxyPad class: GstProxyPad @@ -20,18 +20,58 @@ Ghostpads |------------------| | GstPad *target | (------------------) + | GstPad *internal | + (------------------) GstGhostPad (------------------) -\ | GstPad | | - |------------------| > GstProxyPad - | GstPad *target | | + |------------------| | + | GstPad *target | > GstProxyPad + |------------------| | + | GstPad *internal | | |------------------| -/ - | GstPad *internal | - (------------------) + | | + (------------------) + + A GstGhostPad (X) is _always_ created together with a GstProxyPad (Y). + The internal pad pointers are set to point to the eachother. The + GstProxyPad pairs have opposite directions, the GstGhostPad has the same + direction as the (future) ghosted pad (target). + (- X --------) + | | + | target * | + |------------| + | internal *----+ + (------------) | + ^ V + | (- Y --------) + | | | + | | target * | + | |------------| + +----* internal | + (------------) + + Which we will abreviate to: + + (- X --------) + | | + | target *--------->// + (------------) + | + (- Y --------) + | target *----->// + (------------) + + The GstGhostPad (X) is also set as the parent of the GstProxyPad (Y). + + The target is a pointer to the internal pads peer. It is an optimisation to + quickly get to the peer of a ghostpad without having to dereference the + internal->peer. + Some use case follow with a description of how the datastructure is modified. @@ -40,11 +80,11 @@ Ghostpads gst_ghost_pad_new (char *name, GstPad *target) - 1) create new GstGhostPad X + 1) create new GstGhostPad X + GstProxyPad Y 2) X name set to @name - 3) X direction is the same as the target - 4) the target is set to @target - 5) internal is NULL + 3) X direction is the same as the target, Y is opposite. + 4) the target of X is set to @target + 5) Y is linked to @target 6) link/unlink and activate functions are set up on GstGhostPad. @@ -53,10 +93,12 @@ Ghostpads (- X --------) | | | |------) | target *------------------> | sink | - |------------| |------) - | internal *---->// (-------------- - (------------) - + (------------) -------> |------) + | / (-------------- + (- Y --------) / (pad link) + //<-----* target |/ + (------------) + - Automatically takes same direction as target. - target is filled in automatically. @@ -65,20 +107,20 @@ Ghostpads gst_ghost_pad_new_no_target (char *name, GstPadDirection dir) - 1) create new GstGhostPad X + 1) create new GstGhostPad X + GstProxyPad Y 2) X name set to @name 3) X direction is @dir - 4) internal is NULL 5) link/unlink and activate functions are set up on GstGhostPad. - - (- X --------) - | | - | target *------>// - |------------| - | internal *---->// - (------------) + (- X --------) + | | + | target *--------->// + (------------) + | + (- Y --------) + | target *----->// + (------------) - allows for setting the target later @@ -87,59 +129,69 @@ Ghostpads gst_ghost_pad_set_target (char *name, GstPad *newtarget) - (- X --------) - | | - | target *------>// - |------------| - | internal *---->// - (------------) + (- X --------) + | | + | target *--------->// + (------------) + | + (- Y --------) + | target *----->// + (------------) 1) assert direction of newtarget == X direction 2) target is set to newtarget + 3) internal pad Y is linked to newtarget - (-------- - (- X --------) | - | | |------) - | target *------------->| sink | - |------------| |------) - | internal *--->// (-------- - (------------) - + (-------------- + (- X --------) | + | | |------) + | target *------------------> | sink | + (------------) -------> |------) + | / (-------------- + (- Y --------) / (pad link) + //<-----* target |/ + (------------) -* Setting target on an targetted unlinked ghostpad +* Setting target on a targetted unlinked ghostpad gst_ghost_pad_set_target (char *name, GstPad *newtarget) - (-------- - (- X --------) | - | | |------) - | target *------------->| sink | - |------------| |------) - | internal *--->// (-------- - (------------) - - 1) assert direction of newtarget == X direction - 2) target is set to newtarget - - (-------- - (- X --------) | - | | |------) - | target *------------->| sink | - |------------| |------) - | internal *--->// (-------- - (------------) + (-------------- + (- X --------) | + | | |-------) + | target *------------------> | sink1 | + (------------) -------> |-------) + | / (-------------- + (- Y --------) / (pad link) + //<-----* target |/ + (------------) + + 1) assert direction of newtarget (sink2) == X direction + 2) unlink internal pad Y and oldtarget + 3) target is set to newtarget (sink2) + 4) internal pad Y is linked to newtarget + (-------------- + (- X --------) | + | | |-------) + | target *------------------> | sink2 | + (------------) -------> |-------) + | / (-------------- + (- Y --------) / (pad link) + //<-----* target |/ + (------------) * Linking a pad to an untargetted ghostpad: gst_pad_link (src, X) - - (- X --------) - | | - | target *------>// - |------------| - | internal *---->// + (- X --------) + | | + | target *--------->// + (------------) + | + (- Y --------) + | target *----->// (------------) -------) | @@ -148,28 +200,28 @@ Ghostpads (-----| -------) + X is a sink GstGhostPad without a target. The internal GstProxyPad Y has + the same direction as the src pad (peer). 1) link function is called - a) new GstProxyPad Y is created - b) Y direction is same as peer - c) Y target is set to peer - d) X internal pad is set to Y (X is parent of Y) - e) Y is activated in the same mode as X - f) core makes link from src to X + a) Y direction is same as @src + b) Y target is set to @src + c) Y is activated in the same mode as X + d) core makes link from @src to X (- X --------) | | | target *----->// - >|------------| - (real pad link) / | internal * | - / (----------|-) - / | - -------) / V - | / (- Y ------) - (-----|/ | | - | src |<-------------* target | - (-----| (----------) + >(------------) + (real pad link) / | + / (- Y ------) + / -----* target | + -------) / / (----------) + | / / + (-----|/ / + | src |<---- + (-----| -------) @@ -177,39 +229,37 @@ Ghostpads gst_pad_link (src, X) - (-------- - (- X --------) | - | | |------) - | target *------------->| sink | - |------------| |------) - | internal *--->// (-------- - (------------) - -------) - | - (-----| - | src | - (-----| - -------) + (-------- + (- X --------) | + | | |------) + | target *------------->| sink | + (------------) >|------) + | / (-------- + | / + | / + -------) | / (real pad link) + | (- Y ------) / + (-----| | |/ + | src | //<----* target | + (-----| (----------) + -------) 1) link function is called - a) new GstProxyPad Y is created - b) Y direction is same as peer - c) Y target is set to peer - d) X internal pad is set to Y (X is parent of Y) - e) link is made from Y to X target (sink) - f) Y is activated in the same mode as X - g) core makes link from src to X + a) Y direction is same as @src + b) Y target is set to @src + c) Y is activated in the same mode as X + d) core makes link from @src to X (-------- (- X --------) | | | |------) | target *------------->| sink | - >|------------| >|------) - (real pad link) / | internal * | / (-------- - / (----------|-) / + >(------------) >|------) + (real pad link) / | / (-------- + / | / / | / - -------) / V / (real pad link) + -------) / | / (real pad link) | / (- Y ------) / (-----|/ | |/ | src |<-------------* target | @@ -221,77 +271,73 @@ Ghostpads gst_ghost_pad_set_target (char *name, GstPad *newtarget) - + (- X --------) - | | - | target *----->// - >|------------| - (real pad link) / | internal * | - / (----------|-) + | | + | target *------>// + >(------------) + (real pad link) / | + / | / | - -------) / V + -------) / | | / (- Y ------) (-----|/ | | | src |<-------------* target | (-----| (----------) -------) - 1) assert direction of newtarget == X direction - 2) X target is set to newtarget - 3) Y (X internal) is linked to newtarget - + 1) assert direction of @newtarget == X direction + 2) X target is set to @newtarget + 3) Y is linked to @newtarget (-------- (- X --------) | | | |------) | target *------------->| sink | - >|------------| >|------) - (real pad link) / | internal * | / (-------- - / (----------|-) / + >(------------) >|------) + (real pad link) / | / (-------- + / | / / | / - -------) / V / (real pad link) + -------) / | / (real pad link) | / (- Y ------) / (-----|/ | |/ | src |<-------------* target | (-----| (----------) -------) - * Setting target on targetted linked ghostpad: gst_ghost_pad_set_target (char *name, GstPad *newtarget) - (-------- (- X --------) | - | | |------) - | target *------------->| sink | - >|------------| >|------) - (real pad link) / | internal * | / (-------- - / (----------|-) / + | | |-------) + | target *------------->| sink1 | + >(------------) >|-------) + (real pad link) / | / (-------- + / | / / | / - -------) / V / (real pad link) + -------) / | / (real pad link) | / (- Y ------) / (-----|/ | |/ | src |<-------------* target | (-----| (----------) -------) - 1) assert direction of newtarget == X direction + 1) assert direction of @newtarget == X direction 2) Y and X target are unlinked - 2) X target is set to newtarget - 3) Y (X internal) is linked to newtarget - + 2) X target is set to @newtarget + 3) Y is linked to @newtarget (-------- (- X --------) | - | | |------) - | target *------------->| sink | - >|------------| >|------) - (real pad link) / | internal * | / (-------- - / (----------|-) / + | | |-------) + | target *------------->| sink2 | + >(------------) >|-------) + (real pad link) / | / (-------- + / | / / | / - -------) / V / (real pad link) + -------) / | / (real pad link) | / (- Y ------) / (-----|/ | |/ | src |<-------------* target | diff --git a/gst/gstghostpad.c b/gst/gstghostpad.c index f418128..1dcd42e 100644 --- a/gst/gstghostpad.c +++ b/gst/gstghostpad.c @@ -58,7 +58,6 @@ #define GST_PROXY_PAD_TARGET(pad) (GST_PROXY_PAD (pad)->target) #define GST_PROXY_PAD_INTERNAL(pad) (GST_PROXY_PAD (pad)->internal) - typedef struct _GstProxyPad GstProxyPad; typedef struct _GstProxyPadClass GstProxyPadClass; @@ -136,8 +135,6 @@ gst_proxy_pad_do_event (GstPad * pad, GstEvent * event) gboolean res = FALSE; GstPad *internal = gst_proxy_pad_get_internal (pad); - g_return_val_if_fail (internal != NULL, FALSE); - res = gst_pad_push_event (internal, event); gst_object_unref (internal); @@ -161,8 +158,8 @@ gst_proxy_pad_do_query (GstPad * pad, GstQuery * query) static GList * gst_proxy_pad_do_internal_link (GstPad * pad) { - GstPad *target = gst_proxy_pad_get_target (pad); GList *res = NULL; + GstPad *target = gst_proxy_pad_get_target (pad); if (target) { res = gst_pad_get_internal_links (target); @@ -179,8 +176,6 @@ gst_proxy_pad_do_bufferalloc (GstPad * pad, guint64 offset, guint size, GstFlowReturn result; GstPad *internal = gst_proxy_pad_get_internal (pad); - g_return_val_if_fail (internal != NULL, GST_FLOW_NOT_LINKED); - result = gst_pad_alloc_buffer (internal, offset, size, caps, buf); gst_object_unref (internal); @@ -193,8 +188,6 @@ gst_proxy_pad_do_chain (GstPad * pad, GstBuffer * buffer) GstPad *internal = gst_proxy_pad_get_internal (pad); GstFlowReturn res; - g_return_val_if_fail (internal != NULL, GST_FLOW_NOT_LINKED); - res = gst_pad_push (internal, buffer); gst_object_unref (internal); @@ -208,8 +201,6 @@ gst_proxy_pad_do_getrange (GstPad * pad, guint64 offset, guint size, GstPad *internal = gst_proxy_pad_get_internal (pad); GstFlowReturn res; - g_return_val_if_fail (internal != NULL, GST_FLOW_NOT_LINKED); - res = gst_pad_pull_range (internal, offset, size, buffer); gst_object_unref (internal); @@ -222,8 +213,6 @@ gst_proxy_pad_do_checkgetrange (GstPad * pad) gboolean result; GstPad *internal = gst_proxy_pad_get_internal (pad); - g_return_val_if_fail (internal != NULL, FALSE); - result = gst_pad_check_pull_range (internal); gst_object_unref (internal); @@ -332,6 +321,7 @@ gst_proxy_pad_set_target_unlocked (GstPad * pad, GstPad * target) return TRUE; + /* ERRORS */ wrong_direction: { GST_ERROR_OBJECT (pad, @@ -422,6 +412,7 @@ gst_proxy_pad_init (GstProxyPad * ppad) gst_pad_set_query_function (pad, GST_DEBUG_FUNCPTR (gst_proxy_pad_do_query)); gst_pad_set_internal_link_function (pad, GST_DEBUG_FUNCPTR (gst_proxy_pad_do_internal_link)); + gst_pad_set_getcaps_function (pad, GST_DEBUG_FUNCPTR (gst_proxy_pad_do_getcaps)); gst_pad_set_acceptcaps_function (pad, @@ -430,7 +421,6 @@ gst_proxy_pad_init (GstProxyPad * ppad) GST_DEBUG_FUNCPTR (gst_proxy_pad_do_fixatecaps)); gst_pad_set_setcaps_function (pad, GST_DEBUG_FUNCPTR (gst_proxy_pad_do_setcaps)); - } #ifndef GST_DISABLE_LOADSAVE @@ -510,33 +500,57 @@ gst_critical (const gchar * format, ...) * _ the internal and target are unlinked as soon as the GhostPad is removed * from it's parent. */ - static void gst_ghost_pad_parent_set (GstObject * object, GstObject * parent) { - GstPad *gpad = GST_PAD (object); - GstPad *target = gst_proxy_pad_get_target (gpad); - GstPad *internal = gst_proxy_pad_get_internal (gpad); + GstPad *gpad; + GstPad *target; + GstPad *internal; + GstPadLinkReturn ret; + + gpad = GST_PAD (object); + + /* internal is never NULL */ + internal = gst_proxy_pad_get_internal (gpad); + target = gst_proxy_pad_get_target (gpad); if (target) { if (GST_PAD_IS_SRC (internal)) - gst_pad_link (internal, target); + ret = gst_pad_link (internal, target); else - gst_pad_link (target, internal); + ret = gst_pad_link (target, internal); gst_object_unref (target); + + if (ret != GST_PAD_LINK_OK) + goto link_failed; } gst_object_unref (internal); if (GST_OBJECT_CLASS (gst_ghost_pad_parent_class)->parent_set) GST_OBJECT_CLASS (gst_ghost_pad_parent_class)->parent_set (object, parent); + + return; + + /* ERRORS */ +link_failed: + { + /* a warning is all we can do */ + gst_object_unref (internal); + g_warning ("could not link internal ghostpad"); + return; + } } static void gst_ghost_pad_parent_unset (GstObject * object, GstObject * parent) { - GstPad *gpad = GST_PAD (object); - GstPad *target = gst_proxy_pad_get_target (gpad); - GstPad *internal = gst_proxy_pad_get_internal (gpad); + GstPad *gpad; + GstPad *target; + GstPad *internal; + + gpad = GST_PAD (object); + internal = gst_proxy_pad_get_internal (gpad); + target = gst_proxy_pad_get_target (gpad); if (target) { if (GST_PAD_IS_SRC (internal)) @@ -570,29 +584,18 @@ static gboolean gst_ghost_pad_internal_do_activate_push (GstPad * pad, gboolean active) { gboolean ret; + GstPad *other; - if (GST_PAD_DIRECTION (pad) == GST_PAD_SINK) { - GstPad *parent = gst_proxy_pad_get_internal (pad); - - if (parent) { - g_return_val_if_fail (GST_IS_GHOST_PAD (parent), FALSE); - - ret = gst_pad_activate_push (parent, active); - - gst_object_unref (parent); - } else { - ret = FALSE; - } - } else { - GstPad *peer = gst_pad_get_peer (pad); + if (GST_PAD_DIRECTION (pad) == GST_PAD_SINK) + other = gst_proxy_pad_get_internal (pad); + else + other = gst_pad_get_peer (pad); - if (peer) { - ret = gst_pad_activate_push (peer, active); - gst_object_unref (peer); - } else { - ret = FALSE; - } - } + if (G_LIKELY (other)) { + ret = gst_pad_activate_push (other, active); + gst_object_unref (other); + } else + ret = FALSE; return ret; } @@ -601,29 +604,18 @@ static gboolean gst_ghost_pad_internal_do_activate_pull (GstPad * pad, gboolean active) { gboolean ret; + GstPad *other; - if (GST_PAD_DIRECTION (pad) == GST_PAD_SINK) { - GstPad *peer = gst_pad_get_peer (pad); - - if (peer) { - ret = gst_pad_activate_pull (peer, active); - gst_object_unref (peer); - } else { - ret = FALSE; - } - } else { - GstPad *parent = gst_proxy_pad_get_internal (pad); - - if (parent) { - g_return_val_if_fail (GST_IS_GHOST_PAD (parent), FALSE); + if (GST_PAD_DIRECTION (pad) == GST_PAD_SINK) + other = gst_pad_get_peer (pad); + else + other = gst_proxy_pad_get_internal (pad); - ret = gst_pad_activate_pull (parent, active); - - gst_object_unref (parent); - } else { - ret = FALSE; - } - } + if (G_LIKELY (other)) { + ret = gst_pad_activate_pull (other, active); + gst_object_unref (other); + } else + ret = FALSE; return ret; } @@ -633,19 +625,16 @@ gst_ghost_pad_do_activate_push (GstPad * pad, gboolean active) { gboolean ret; + ret = TRUE; + if (GST_PAD_DIRECTION (pad) == GST_PAD_SINK) { GstPad *internal = gst_proxy_pad_get_internal (pad); if (internal) { ret = gst_pad_activate_push (internal, active); gst_object_unref (internal); - } else { - ret = TRUE; } - } else { - ret = TRUE; } - return ret; } @@ -653,26 +642,18 @@ static gboolean gst_ghost_pad_do_activate_pull (GstPad * pad, gboolean active) { gboolean ret; + GstPad *other; - if (GST_PAD_DIRECTION (pad) == GST_PAD_SINK) { - GstPad *peer = gst_pad_get_peer (pad); + if (GST_PAD_DIRECTION (pad) == GST_PAD_SINK) + other = gst_pad_get_peer (pad); + else + other = gst_proxy_pad_get_internal (pad); - if (peer) { - ret = gst_pad_activate_pull (peer, active); - gst_object_unref (peer); - } else { - ret = FALSE; - } - } else { - GstPad *internal = gst_proxy_pad_get_internal (pad); - - if (internal) { - ret = gst_pad_activate_pull (internal, active); - gst_object_unref (internal); - } else { - ret = FALSE; - } - } + if (G_LIKELY (other)) { + ret = gst_pad_activate_pull (other, active); + gst_object_unref (other); + } else + ret = FALSE; return ret; } @@ -680,44 +661,55 @@ gst_ghost_pad_do_activate_pull (GstPad * pad, gboolean active) static GstPadLinkReturn gst_ghost_pad_do_link (GstPad * pad, GstPad * peer) { - GstPad *internal, *target; - GstPadLinkReturn ret = GST_PAD_LINK_OK; + GstPadLinkReturn ret; + GstPad *internal; - target = gst_proxy_pad_get_target (pad); - g_return_val_if_fail (target != NULL, GST_PAD_LINK_NOSCHED); + ret = GST_PAD_LINK_OK; internal = gst_proxy_pad_get_internal (pad); - gst_proxy_pad_set_target (internal, peer); + if (!gst_proxy_pad_set_target (internal, peer)) + goto failed; /* if we are a source pad, we should call the peer link function * if the peer has one */ if (GST_PAD_IS_SRC (pad)) { - if (GST_PAD_LINKFUNC (peer) && ret == GST_PAD_LINK_OK) + if (GST_PAD_LINKFUNC (peer)) ret = GST_PAD_LINKFUNC (peer) (peer, pad); } - - gst_object_unref (target); +done: gst_object_unref (internal); return ret; + + /* ERRORS */ +failed: + { + ret = GST_PAD_LINK_REFUSED; + goto done; + } } static void gst_ghost_pad_do_unlink (GstPad * pad) { - GstPad *target = gst_proxy_pad_get_target (pad); - GstPad *internal = gst_proxy_pad_get_internal (pad); + GstPad *target; + GstPad *internal; + + target = gst_proxy_pad_get_target (pad); + internal = gst_proxy_pad_get_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); - if (target && target->unlinkfunc) - target->unlinkfunc (target); + if (target) { + if (GST_PAD_UNLINKFUNC (target)) + GST_PAD_UNLINKFUNC (target) (target); - if (target) gst_object_unref (target); + } + gst_object_unref (internal); } @@ -771,9 +763,9 @@ gst_ghost_pad_dispose (GObject * object) } GST_PROXY_PAD_INTERNAL (internal) = NULL; - /* - disposes of the internal pad, since the ghostpad is the only possible object - that has a refcount on the internal pad. + + /* disposes of the internal pad, since the ghostpad is the only possible object + * that has a refcount on the internal pad. */ gst_object_unparent (GST_OBJECT_CAST (internal)); @@ -819,7 +811,6 @@ gst_ghost_pad_new_full (const gchar * name, GstPadDirection dir, /* INTERNAL PAD */ - internal = g_object_new (GST_TYPE_PROXY_PAD, "name", NULL, "direction", (dir == GST_PAD_SRC) ? GST_PAD_SINK : GST_PAD_SRC, NULL); @@ -840,37 +831,45 @@ gst_ghost_pad_new_full (const gchar * name, GstPadDirection dir, GST_PROXY_LOCK (ret); if (!gst_object_set_parent (GST_OBJECT_CAST (internal), - GST_OBJECT_CAST (ret))) { - gst_critical ("Could not set internal pad%" GST_PTR_FORMAT, internal); - goto beach; - } - - /* - The ghostpad is the parent of the internal pad and is the only object that - can have a refcount on the internal pad. - At this point, the GstGhostPad has a refcount of 1, and the internal pad has - a refcount of 1. - When the refcount of the GstGhostPad drops to 0, the ghostpad will dispose - it's refcount on the internal pad in the dispose method by un-parenting it. - This is why we don't take extra refcounts in the assignments below + GST_OBJECT_CAST (ret))) + goto parent_failed; + + /* The ghostpad is the parent of the internal pad and is the only object that + * can have a refcount on the internal pad. + * At this point, the GstGhostPad has a refcount of 1, and the internal pad has + * a refcount of 1. + * When the refcount of the GstGhostPad drops to 0, the ghostpad will dispose + * it's refcount on the internal pad in the dispose method by un-parenting it. + * This is why we don't take extra refcounts in the assignments below */ GST_PROXY_PAD_INTERNAL (ret) = internal; - GST_PROXY_PAD_INTERNAL (internal) = GST_PAD (ret); + GST_PROXY_PAD_INTERNAL (internal) = ret; /* could be more general here, iterating over all writable properties... * taking the short road for now tho */ - GST_GHOST_PAD (ret)->notify_id = g_signal_connect (internal, "notify::caps", - G_CALLBACK (on_int_notify), ret); - on_int_notify (internal, NULL, GST_GHOST_PAD (ret)); + GST_GHOST_PAD_CAST (ret)->notify_id = + g_signal_connect (internal, "notify::caps", G_CALLBACK (on_int_notify), + ret); + /* call function to init values */ + on_int_notify (internal, NULL, GST_GHOST_PAD_CAST (ret)); + gst_pad_set_activatepull_function (GST_PAD (internal), GST_DEBUG_FUNCPTR (gst_ghost_pad_internal_do_activate_pull)); gst_pad_set_activatepush_function (GST_PAD (internal), GST_DEBUG_FUNCPTR (gst_ghost_pad_internal_do_activate_push)); -beach: GST_PROXY_UNLOCK (ret); return ret; + + /* ERRORS */ +parent_failed: + { + gst_critical ("Could not set internal pad %" GST_PTR_FORMAT, internal); + GST_PROXY_UNLOCK (ret); + gst_object_unref (ret); + return NULL; + } } /** @@ -979,13 +978,12 @@ set_target_failed: * @templ: the #GstPadTemplate to create the ghostpad from. * * Create a new ghostpad based on @templ, without setting a target. The - * direction will be taken from @templ. + * direction will be taken from the @templ. * * Returns: a new #GstPad, or NULL in case of an error. * * Since: 0.10.10 */ - GstPad * gst_ghost_pad_new_no_target_from_template (const gchar * name, GstPadTemplate * templ) @@ -1034,11 +1032,12 @@ gst_ghost_pad_set_target (GstGhostPad * gpad, GstPad * newtarget) GstPad *oldtarget; GstObject *parent; gboolean result; + GstPadLinkReturn lret; g_return_val_if_fail (GST_IS_GHOST_PAD (gpad), FALSE); GST_PROXY_LOCK (gpad); - internal = GST_PROXY_PAD_INTERNAL (GST_PAD (gpad)); + internal = GST_PROXY_PAD_INTERNAL (GST_PAD_CAST (gpad)); GST_DEBUG_OBJECT (gpad, "set target %s:%s", GST_DEBUG_PAD_NAME (newtarget)); @@ -1059,9 +1058,13 @@ gst_ghost_pad_set_target (GstGhostPad * gpad, GstPad * newtarget) /* and link to internal pad if we are not unparent-ed */ if ((parent = gst_object_get_parent (GST_OBJECT (gpad)))) { if (GST_PAD_IS_SRC (internal)) - result = gst_pad_link (internal, newtarget); + lret = gst_pad_link (internal, newtarget); else - result = gst_pad_link (newtarget, internal); + lret = gst_pad_link (newtarget, internal); + + /* FIXME, do something with lret, possibly checking the return value and + * undoing the set_target operation if it failed */ + gst_object_unref (parent); } else { /* we need to connect the internal pad once we have a parent */ diff --git a/tests/check/gst/gstghostpad.c b/tests/check/gst/gstghostpad.c index f9cc49f..7e7d0ae 100644 --- a/tests/check/gst/gstghostpad.c +++ b/tests/check/gst/gstghostpad.c @@ -148,17 +148,26 @@ GST_START_TEST (test_remove2) GST_END_TEST; -#if 0 -/* test if a ghost pad without a target can be linked - * It can't because it has incompatible caps... +/* test if a ghost pad without a target can be linked and + * unlinked. An untargeted ghostpad has a default ANY caps unless there + * is a padtemplate that says something else. */ -GST_START_TEST (test_ghost_pad_notarget) +GST_START_TEST (test_ghost_pads_notarget) { GstElement *b1, *b2, *sink; - GstPad *srcpad, *sinkpad; + GstPad *srcpad, *sinkpad, *peer; GstPadLinkReturn ret; + gboolean bret; + GstBus *bus; + GstCaps *caps; b1 = gst_element_factory_make ("pipeline", NULL); + + /* make sure all messages are discarded */ + bus = gst_pipeline_get_bus (GST_PIPELINE (b1)); + gst_bus_set_flushing (bus, TRUE); + gst_object_unref (bus); + b2 = gst_element_factory_make ("bin", NULL); sink = gst_element_factory_make ("fakesink", NULL); @@ -172,10 +181,33 @@ GST_START_TEST (test_ghost_pad_notarget) ret = gst_pad_link (srcpad, sinkpad); fail_unless (ret == GST_PAD_LINK_OK); + + /* check if the peers are ok */ + peer = gst_pad_get_peer (srcpad); + fail_unless (peer == sinkpad); + gst_object_unref (peer); + + peer = gst_pad_get_peer (sinkpad); + fail_unless (peer == srcpad); + gst_object_unref (peer); + + /* check caps, untargetted pad should return ANY or the padtemplate caps + * when it was created from a template */ + caps = gst_pad_get_caps (srcpad); + fail_unless (gst_caps_is_any (caps)); + gst_caps_unref (caps); + + /* unlink */ + bret = gst_pad_unlink (srcpad, sinkpad); + fail_unless (bret == TRUE); + + /* cleanup */ + gst_object_unref (srcpad); + gst_object_unref (sinkpad); + gst_object_unref (b1); } GST_END_TEST; -#endif /* test if linking fails over different bins using a pipeline * like this: @@ -225,6 +257,10 @@ GST_START_TEST (test_link) gst_element_set_state (b1, GST_STATE_READY); gst_element_set_state (b1, GST_STATE_NULL); ASSERT_OBJECT_REFCOUNT (b1, "pipeline", 1); + + gst_object_unref (srcpad); + gst_object_unref (sinkpad); + gst_object_unref (b1); } GST_END_TEST; @@ -345,8 +381,8 @@ GST_START_TEST (test_ghost_pads_bin) GstBin *sinkbin; GstElement *src; GstElement *sink; - GstPad *srcghost; - GstPad *sinkghost; + GstPad *srcpad, *srcghost, *target; + GstPad *sinkpad, *sinkghost; pipeline = GST_BIN (gst_pipeline_new ("pipe")); ASSERT_OBJECT_REFCOUNT (pipeline, "pipeline", 1); @@ -361,27 +397,35 @@ GST_START_TEST (test_ghost_pads_bin) src = gst_element_factory_make ("fakesrc", "src"); gst_bin_add (srcbin, src); - srcghost = gst_ghost_pad_new ("src", gst_element_get_pad (src, "src")); + srcpad = gst_element_get_pad (src, "src"); + srcghost = gst_ghost_pad_new ("src", srcpad); + gst_object_unref (srcpad); gst_element_add_pad (GST_ELEMENT (srcbin), srcghost); sink = gst_element_factory_make ("fakesink", "sink"); gst_bin_add (sinkbin, sink); - sinkghost = gst_ghost_pad_new ("sink", gst_element_get_pad (sink, "sink")); + sinkpad = gst_element_get_pad (sink, "sink"); + sinkghost = gst_ghost_pad_new ("sink", sinkpad); + gst_object_unref (sinkpad); gst_element_add_pad (GST_ELEMENT (sinkbin), sinkghost); gst_element_link (GST_ELEMENT (srcbin), GST_ELEMENT (sinkbin)); fail_unless (GST_PAD_PEER (srcghost) != NULL); fail_unless (GST_PAD_PEER (sinkghost) != NULL); - fail_unless (GST_PAD_PEER (gst_ghost_pad_get_target (GST_GHOST_PAD - (srcghost))) != NULL); - fail_unless (GST_PAD_PEER (gst_ghost_pad_get_target (GST_GHOST_PAD - (sinkghost))) != NULL); + target = gst_ghost_pad_get_target (GST_GHOST_PAD (srcghost)); + fail_unless (GST_PAD_PEER (target) != NULL); + gst_object_unref (target); + target = gst_ghost_pad_get_target (GST_GHOST_PAD (sinkghost)); + fail_unless (GST_PAD_PEER (target) != NULL); + gst_object_unref (target); /* flush the message, dropping the b1 refcount to 1 */ gst_element_set_state (GST_ELEMENT (pipeline), GST_STATE_READY); gst_element_set_state (GST_ELEMENT (pipeline), GST_STATE_NULL); ASSERT_OBJECT_REFCOUNT (pipeline, "pipeline", 1); + + gst_object_unref (pipeline); } GST_END_TEST; @@ -437,6 +481,9 @@ GST_START_TEST (test_ghost_pads_block) g_mutex_free (block_data.mutex); g_cond_free (block_data.cond); + + ASSERT_OBJECT_REFCOUNT (pipeline, "pipeline", 1); + gst_object_unref (pipeline); } GST_END_TEST; @@ -475,6 +522,9 @@ GST_START_TEST (test_ghost_pads_probes) g_mutex_free (block_data.mutex); g_cond_free (block_data.cond); + + ASSERT_OBJECT_REFCOUNT (pipeline, "pipeline", 1); + gst_object_unref (pipeline); } GST_END_TEST; @@ -569,7 +619,7 @@ gst_ghost_pad_suite (void) tcase_add_test (tc_chain, test_link); tcase_add_test (tc_chain, test_ghost_pads); tcase_add_test (tc_chain, test_ghost_pads_bin); -/* tcase_add_test (tc_chain, test_ghost_pad_notarget); */ + tcase_add_test (tc_chain, test_ghost_pads_notarget); tcase_add_test (tc_chain, test_ghost_pads_block); tcase_add_test (tc_chain, test_ghost_pads_probes); tcase_add_test (tc_chain, test_ghost_pads_new_from_template);