From: <wt...@fr...> - 2005-07-28 11:24:47
|
CVS Root: /cvs/gstreamer Module: gstreamer Changes by: wtay Date: Thu Jul 28 2005 04:24:45 PDT Log message: * check/gst/gstghostpad.c: (GST_START_TEST), (gst_ghost_pad_suite): Added some more tests for wrong hierarchy * docs/design/part-overview.txt: Some updates. * gst/gstbin.c: (gst_bin_remove_func), (gst_bin_dispose): Cleanups. * gst/gstelement.c: (gst_element_remove_pad), (gst_element_seek), (gst_element_dispose): Some more cleanups. * gst/gstpad.c: (gst_pad_link_check_compatible_unlocked), (gst_pad_link_check_hierarchy), (gst_pad_link_prepare), (gst_pad_get_caps_unlocked), (gst_pad_accept_caps), (gst_pad_set_caps), (gst_pad_send_event): Check for correct hierarchy when linking pads. Moving to strict requirement for ghostpads when linking elements in different bins. * gst/gstpad.h: Clean ups. Added WRONG_HIERARCHY return value. Modified files: . : ChangeLog check/gst : gstghostpad.c docs/design : part-overview.txt gst : gstbin.c gstelement.c gstpad.c gstpad.h Links: http://freedesktop.org/cgi-bin/viewcvs.cgi/gstreamer/gstreamer/ChangeLog.diff?r1=1.1310&r2=1.1311 http://freedesktop.org/cgi-bin/viewcvs.cgi/gstreamer/gstreamer/check/gst/gstghostpad.c.diff?r1=1.5&r2=1.6 http://freedesktop.org/cgi-bin/viewcvs.cgi/gstreamer/gstreamer/docs/design/part-overview.txt.diff?r1=1.2&r2=1.3 http://freedesktop.org/cgi-bin/viewcvs.cgi/gstreamer/gstreamer/gst/gstbin.c.diff?r1=1.243&r2=1.244 http://freedesktop.org/cgi-bin/viewcvs.cgi/gstreamer/gstreamer/gst/gstelement.c.diff?r1=1.349&r2=1.350 http://freedesktop.org/cgi-bin/viewcvs.cgi/gstreamer/gstreamer/gst/gstpad.c.diff?r1=1.420&r2=1.421 http://freedesktop.org/cgi-bin/viewcvs.cgi/gstreamer/gstreamer/gst/gstpad.h.diff?r1=1.182&r2=1.183 ====Begin Diffs==== Index: ChangeLog =================================================================== RCS file: /cvs/gstreamer/gstreamer/ChangeLog,v retrieving revision 1.1310 retrieving revision 1.1311 diff -u -d -r1.1310 -r1.1311 --- ChangeLog 28 Jul 2005 10:38:02 -0000 1.1310 +++ ChangeLog 28 Jul 2005 11:24:32 -0000 1.1311 @@ -1,3 +1,29 @@ +2005-07-28 Wim Taymans <wi...@fl...> + + * check/gst/gstghostpad.c: (GST_START_TEST), (gst_ghost_pad_suite): + Added some more tests for wrong hierarchy + * docs/design/part-overview.txt: + Some updates. + * gst/gstbin.c: (gst_bin_remove_func), (gst_bin_dispose): + Cleanups. + * gst/gstelement.c: (gst_element_remove_pad), (gst_element_seek), + (gst_element_dispose): + Some more cleanups. + * gst/gstpad.c: (gst_pad_link_check_compatible_unlocked), + (gst_pad_link_check_hierarchy), (gst_pad_link_prepare), + (gst_pad_get_caps_unlocked), (gst_pad_accept_caps), + (gst_pad_set_caps), (gst_pad_send_event): + Check for correct hierarchy when linking pads. Moving to + strict requirement for ghostpads when linking elements in + different bins. + * gst/gstpad.h: + Clean ups. Added WRONG_HIERARCHY return value. 2005-07-28 Ronald S. Bultje <rb...@ro...> * gst/base/gstbasetransform.c: (gst_base_transform_setcaps): Index: gstghostpad.c RCS file: /cvs/gstreamer/gstreamer/check/gst/gstghostpad.c,v retrieving revision 1.5 retrieving revision 1.6 diff -u -d -r1.5 -r1.6 --- gstghostpad.c 11 Jul 2005 15:10:39 -0000 1.5 +++ gstghostpad.c 28 Jul 2005 11:24:33 -0000 1.6 @@ -29,6 +29,99 @@ GST_OBJECT_REFCOUNT_VALUE (p)); } +/* test if removing a bin also cleans up the ghostpads + */ +GST_START_TEST (test_remove1) +{ + 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); + /* now remove the bin with the ghostpad, b2 is disposed + * now. */ + gst_bin_remove (GST_BIN (b1), b2); + /* pad cannot be linked now */ + fail_if (gst_pad_is_linked (srcpad)); +} +GST_END_TEST; +/* test if linking fails over different bins using a pipeline + * like this: + * + * fakesrc num_buffers=10 ! ( fakesink ) +GST_START_TEST (test_link) + GstPad *srcpad, *sinkpad, *gpad; + fail_unless (srcpad != NULL); + fail_unless (sinkpad != NULL); + /* linking in different hierarchies should fail */ + fail_unless (ret == GST_PAD_LINK_WRONG_HIERARCHY); + /* now setup a ghostpad */ + gpad = gst_ghost_pad_new ("sink", sinkpad); + /* need to ref as _add_pad takes ownership */ + gst_object_ref (gpad); + gst_element_add_pad (b2, gpad); + /* our new sinkpad */ + sinkpad = gpad; + /* and linking should work now */ +/* test if ghostpads are created automagically when using + * gst_element_link_pads. + * fakesrc num_buffers=10 ! ( identity ) ! fakesink GST_START_TEST (test_ghost_pads) { GstElement *b1, *b2, *src, *i1, *sink; @@ -134,6 +227,8 @@ TCase *tc_chain = tcase_create ("ghost pad tests"); suite_add_tcase (s, tc_chain); + tcase_add_test (tc_chain, test_remove1); + tcase_add_test (tc_chain, test_link); tcase_add_test (tc_chain, test_ghost_pads); return s; Index: part-overview.txt RCS file: /cvs/gstreamer/gstreamer/docs/design/part-overview.txt,v retrieving revision 1.2 retrieving revision 1.3 diff -u -d -r1.2 -r1.3 --- part-overview.txt 30 May 2005 15:48:45 -0000 1.2 +++ part-overview.txt 28 Jul 2005 11:24:33 -0000 1.3 @@ -41,6 +41,10 @@ vorbisdec element decodes the compressed data and sends it to the alsasink element. The alsasink element sends the samples to the audio card for playback. + Downstream and upstream are the term used to describe the direction in the + Pipeline. From source to sink is called "downstream" and "upstream" is + from sink to source. The task of the application is to construct a pipeline as above using existing elements. This is further explained in the pipeline building topic. @@ -198,7 +202,9 @@ out the buffer. When an element pad receives a buffer, if has to check if it understands the media - type of the buffer before starting processing it. + type of the buffer before starting processing it. The GStreamer core does this + automatically and will call the gst_pad_set_caps() function of the element before + sending the buffer to the element. Both gst_pad_push() and gst_pad_pull_range() have a return value indicating wether the operation succeeded. An error code means that no more data should be send @@ -273,10 +279,14 @@ One of the important functions of the pipeline is to select a global clock for all the elements in the pipeline. + The purpose of the clock is to provide a stricly increasing value at the rate + of one GST_SECOND per second. Clock values are expressed in nanoseconds. + Elements use the clock time to synchronized the playback of data. Before the pipeline is set to PAUSED, the pipeline asks each element if they can provide a clock. The clock is selected in the following order: - - If the application selected a clock, us that one. + - If the application selected a clock, use that one. - If a source element provides a clock, use that clock. - Select a clock from any other element that provides a clock, start with the sinks. @@ -284,7 +294,10 @@ In a typical playback pipeline this will select the clock provided by a sink element such as an audio sink. - + In capture pipelines, this will typically select the clock of the data producer, which + can in most cases not control the rate at which it delivers data. Pipeline states --------------- @@ -447,8 +460,9 @@ When the seek event reaches an element that will perform the seek operation, that element performs the following steps. - 1) send a FLUSH event to all downstream and upstream peer elements. - 2) make sure the streaming thread is not running. + 1) send a FLUSH_START event to all downstream and upstream peer elements. + 2) make sure the streaming thread is not running. The streaming thread will + always stop because of step 1). 3) perform the seek operation 4) send a FLUSH done event to all downstream and upstream peer elements. 5) send DISCONT event to inform all elements of the new position and to complete Index: gstbin.c RCS file: /cvs/gstreamer/gstreamer/gst/gstbin.c,v retrieving revision 1.243 retrieving revision 1.244 diff -u -d -r1.243 -r1.244 --- gstbin.c 18 Jul 2005 17:12:36 -0000 1.243 +++ gstbin.c 28 Jul 2005 11:24:33 -0000 1.244 @@ -483,7 +483,7 @@ bin->numchildren--; bin->children_cookie++; - /* check if we a sink */ + /* check if we removed a sink */ if (GST_FLAG_IS_SET (element, GST_ELEMENT_IS_SINK)) { GList *other_sink; @@ -691,6 +691,9 @@ return is_sink ? 0 : 1; +/* check if object has the given ancestor somewhere up in + * the hierarchy static gboolean has_ancestor (GstObject * object, GstObject * ancestor) @@ -1008,6 +1011,8 @@ * Each element will have its refcount increased, so unref * after use. * + * Not implemented yet. * MT safe. * Returns: a #GstIterator of #GstElements. gst_iterator_free after use. @@ -1060,7 +1065,8 @@ */ -/* FIXME, make me more elegant */ +/* FIXME, make me more elegant, want to use a topological sort algorithm + * based on indegrees (or outdegrees in our case) */ static GstElementStateReturn gst_bin_change_state (GstElement * element) @@ -1335,7 +1341,9 @@ gst_object_ref (object); g_list_free (bin->eosed); + bin->eosed = NULL; gst_object_unref (bin->child_bus); + bin->child_bus = NULL; gst_element_set_bus (GST_ELEMENT (bin), NULL); while (bin->children) { Index: gstelement.c RCS file: /cvs/gstreamer/gstreamer/gst/gstelement.c,v retrieving revision 1.349 retrieving revision 1.350 diff -u -d -r1.349 -r1.350 --- gstelement.c 27 Jul 2005 18:33:02 -0000 1.349 +++ gstelement.c 28 Jul 2005 11:24:33 -0000 1.350 @@ -590,35 +590,28 @@ gst_element_remove_pad (GstElement * element, GstPad * pad) GstPad *peer; - gchar *pad_name; g_return_val_if_fail (GST_IS_ELEMENT (element), FALSE); g_return_val_if_fail (GST_IS_PAD (pad), FALSE); /* locking pad to look at the name and parent */ GST_LOCK (pad); - pad_name = g_strdup (GST_PAD_NAME (pad)); - GST_CAT_INFO_OBJECT (GST_CAT_ELEMENT_PADS, element, "removing pad '%s'", - GST_STR_NULL (pad_name)); + GST_STR_NULL (GST_PAD_NAME (pad))); if (G_UNLIKELY (GST_PAD_PARENT (pad) != element)) goto not_our_pad; GST_UNLOCK (pad); - g_free (pad_name); - peer = gst_pad_get_peer (pad); /* unlink */ - if (peer != NULL) { + if ((peer = gst_pad_get_peer (pad))) { /* window for MT unsafeness, someone else could unlink here * and then we call unlink with wrong pads. The unlink * function would catch this and safely return failed. */ if (GST_PAD_IS_SRC (pad)) - gst_pad_unlink (pad, GST_PAD_CAST (peer)); + gst_pad_unlink (pad, peer); else - gst_pad_unlink (GST_PAD_CAST (peer), pad); + gst_pad_unlink (peer, pad); gst_object_unref (peer); } @@ -658,7 +651,6 @@ GST_ELEMENT_NAME (element)); GST_UNLOCK (element); GST_UNLOCK (pad); - g_free (pad_name); return FALSE; @@ -2056,7 +2048,7 @@ /* first we break all our links with the outside */ while (element->pads) { - gst_element_remove_pad (element, GST_PAD (element->pads->data)); + gst_element_remove_pad (element, GST_PAD_CAST (element->pads->data)); if (G_UNLIKELY (element->pads != 0)) { g_critical ("could not remove pads from element %s", Index: gstpad.c RCS file: /cvs/gstreamer/gstreamer/gst/gstpad.c,v retrieving revision 1.420 retrieving revision 1.421 diff -u -d -r1.420 -r1.421 --- gstpad.c 27 Jul 2005 18:33:02 -0000 1.420 +++ gstpad.c 28 Jul 2005 11:24:33 -0000 1.421 @@ -1327,6 +1327,12 @@ return result; +/* get the caps from both pads and see if the intersection + * is not empty. + * This function should be called with the pad LOCK on both + * pads gst_pad_link_check_compatible_unlocked (GstPad * src, GstPad * sink) @@ -1338,6 +1344,7 @@ GST_CAT_DEBUG (GST_CAT_CAPS, " src caps %" GST_PTR_FORMAT, srccaps); GST_CAT_DEBUG (GST_CAT_CAPS, "sink caps %" GST_PTR_FORMAT, sinkcaps); + /* if we have caps on both pads we can check the intersection */ if (srccaps && sinkcaps) { GstCaps *icaps; @@ -1349,6 +1356,7 @@ "intersection caps %p %" GST_PTR_FORMAT, icaps, icaps); if (!icaps || gst_caps_is_empty (icaps)) { + GST_CAT_DEBUG (GST_CAT_CAPS, "intersection is empty"); gst_caps_unref (icaps); return FALSE; } @@ -1358,6 +1366,49 @@ return TRUE; +/* check if the grandparents of both pads are the same. + * This check is required so that we don't try to link + * pads from elements in different bins without ghostpads. + * The LOCK should be helt on both pads +static gboolean +gst_pad_link_check_hierarchy (GstPad * src, GstPad * sink) + GstObject *psrc, *psink; + gboolean res = TRUE; + psrc = GST_OBJECT_PARENT (src); + psink = GST_OBJECT_PARENT (sink); + /* if one of the pads has no parent, we allow the link */ + if (psrc && psink) { + /* if the parents are the same, we have a loop */ + if (psrc == psink) { + GST_CAT_DEBUG (GST_CAT_CAPS, "pads have same parent %" GST_PTR_FORMAT, + psrc); + res = FALSE; + goto done; + } + /* if they both have a parent, we check the grandparents */ + psrc = gst_object_get_parent (psrc); + psink = gst_object_get_parent (psink); + if (psrc != psink) { + /* if they have grandparents but they are not the same */ + GST_CAT_DEBUG (GST_CAT_CAPS, + "pads have different grandparents %" GST_PTR_FORMAT " and %" + GST_PTR_FORMAT, psrc, psink); + if (psrc) + gst_object_unref (psrc); + if (psink) + gst_object_unref (psink); + } +done: + return res; /* FIXME leftover from an attempt at refactoring... */ /* call with the two pads unlocked */ @@ -1387,10 +1438,14 @@ if (G_UNLIKELY (GST_PAD_PEER (sinkpad) != NULL)) goto sink_was_linked; + /* check hierarchy, pads can only be linked if the grandparents + * are the same. */ + if (!gst_pad_link_check_hierarchy (srcpad, sinkpad)) + goto wrong_hierarchy; /* check pad caps for non-empty intersection */ - if (!gst_pad_link_check_compatible_unlocked (srcpad, sinkpad)) { + if (!gst_pad_link_check_compatible_unlocked (srcpad, sinkpad)) goto no_format; - } /* FIXME check pad scheduling for non-empty intersection */ @@ -1428,6 +1483,13 @@ GST_UNLOCK (srcpad); return GST_PAD_LINK_WAS_LINKED; +wrong_hierarchy: + { + GST_CAT_INFO (GST_CAT_PADS, "pads have wrong hierarchy"); + GST_UNLOCK (sinkpad); + GST_UNLOCK (srcpad); + return GST_PAD_LINK_WRONG_HIERARCHY; no_format: { GST_CAT_INFO (GST_CAT_PADS, "caps are incompatible"); Index: gstpad.h RCS file: /cvs/gstreamer/gstreamer/gst/gstpad.h,v retrieving revision 1.182 retrieving revision 1.183 diff -u -d -r1.182 -r1.183 --- gstpad.h 18 Jul 2005 12:49:52 -0000 1.182 +++ gstpad.h 28 Jul 2005 11:24:33 -0000 1.183 @@ -56,13 +56,24 @@ typedef struct _GstPadTemplateClass GstPadTemplateClass; typedef struct _GstStaticPadTemplate GstStaticPadTemplate; +/** + * GstPadLinkReturn: + * #GST_PAD_LINK_OK : link ok + * #GST_PAD_LINK_WRONG_HIERARCHY: pads have no common grandparent + * #GST_PAD_LINK_WAS_LINKED : pad was already linked + * #GST_PAD_LINK_WRONG_DIRECTION: pads have wrong direction + * #GST_PAD_LINK_NOFORMAT : pads do not have common format + * #GST_PAD_LINK_NOSCHED : pads cannot cooperate in scheduling + * #GST_PAD_LINK_REFUSED : refused for some reason typedef enum { - GST_PAD_LINK_NOSCHED = -5, /* pads cannot cooperate in scheduling */ - GST_PAD_LINK_NOFORMAT = -4, /* pads do not have common format */ - GST_PAD_LINK_REFUSED = -3, /* refused for some reason */ - GST_PAD_LINK_WRONG_DIRECTION = -2, /* pads have wrong direction */ - GST_PAD_LINK_WAS_LINKED = -1, /* pad was already linked */ - GST_PAD_LINK_OK = 0, /* link ok */ + GST_PAD_LINK_OK = 0, + GST_PAD_LINK_WRONG_HIERARCHY = -1, + GST_PAD_LINK_WAS_LINKED = -2, + GST_PAD_LINK_WRONG_DIRECTION = -3, + GST_PAD_LINK_NOFORMAT = -4, + GST_PAD_LINK_NOSCHED = -5, + GST_PAD_LINK_REFUSED = -6, } GstPadLinkReturn; #define GST_PAD_LINK_FAILED(ret) ((ret) < GST_PAD_LINK_OK) |