From 65006292ce2c4d97043941403999def2c77cc9ac Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Sat, 8 Oct 2005 09:24:25 +0000 Subject: [PATCH] gst/: Make ChildProxy threadsafe and fix mem leaks. Original commit message from CVS: * gst/gstbin.c: (gst_bin_child_proxy_get_child_by_index), (gst_bin_child_proxy_get_children_count): * gst/gstchildproxy.c: (gst_child_proxy_get_child_by_name), (gst_child_proxy_lookup), (gst_child_proxy_get_property), (gst_child_proxy_get_valist), (gst_child_proxy_set_property), (gst_child_proxy_set_valist): * gst/parse/grammar.y: Make ChildProxy threadsafe and fix mem leaks. --- ChangeLog | 11 ++++++ gst/gstbin.c | 27 +++++++++++-- gst/gstchildproxy.c | 96 ++++++++++++++++++++++++++++++++------------- gst/parse/grammar.y | 2 + 4 files changed, 104 insertions(+), 32 deletions(-) diff --git a/ChangeLog b/ChangeLog index 1298575c31..afc953da89 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +2005-10-08 Wim Taymans + + * gst/gstbin.c: (gst_bin_child_proxy_get_child_by_index), + (gst_bin_child_proxy_get_children_count): + * gst/gstchildproxy.c: (gst_child_proxy_get_child_by_name), + (gst_child_proxy_lookup), (gst_child_proxy_get_property), + (gst_child_proxy_get_valist), (gst_child_proxy_set_property), + (gst_child_proxy_set_valist): + * gst/parse/grammar.y: + Make ChildProxy threadsafe and fix mem leaks. + 2005-10-08 Thomas Vander Stichele * gst/gst.c: (init_post): diff --git a/gst/gstbin.c b/gst/gstbin.c index 7cdc38067b..db4650ade7 100644 --- a/gst/gstbin.c +++ b/gst/gstbin.c @@ -112,7 +112,7 @@ static void gst_bin_restore_thyself (GstObject * object, xmlNodePtr self); static gint bin_element_is_sink (GstElement * child, GstBin * bin); -/* Bin signals and args */ +/* Bin signals and properties */ enum { ELEMENT_ADDED, @@ -122,7 +122,7 @@ enum enum { - ARG_0 + PROP_0 /* FILL ME */ }; @@ -185,13 +185,32 @@ static GstObject * gst_bin_child_proxy_get_child_by_index (GstChildProxy * child_proxy, guint index) { - return g_list_nth_data (GST_BIN (child_proxy)->children, index); + GstObject *res; + GstBin *bin; + + bin = GST_BIN_CAST (child_proxy); + + GST_LOCK (bin); + if ((res = g_list_nth_data (bin->children, index))) + gst_object_ref (res); + GST_UNLOCK (bin); + + return res; } guint gst_bin_child_proxy_get_children_count (GstChildProxy * child_proxy) { - return GST_BIN (child_proxy)->numchildren; + guint num; + GstBin *bin; + + bin = GST_BIN_CAST (child_proxy); + + GST_LOCK (bin); + num = bin->numchildren; + GST_UNLOCK (bin); + + return num; } static void diff --git a/gst/gstchildproxy.c b/gst/gstchildproxy.c index de2c119b19..be550339b2 100644 --- a/gst/gstchildproxy.c +++ b/gst/gstchildproxy.c @@ -61,31 +61,46 @@ static guint signals[LAST_SIGNAL] = { 0 }; * * Implementors can use #GstObject together with gst_object_get_name() * - * Returns: the child object or %NULL if not found + * Returns: the child object or %NULL if not found. Unref after usage. + * + * MT safe. */ GstObject * gst_child_proxy_get_child_by_name (GstChildProxy * parent, const gchar * name) { guint count, i; - GstObject *object; - const gchar *object_name; + GstObject *object, *result; + gchar *object_name; g_return_val_if_fail (GST_IS_CHILD_PROXY (parent), NULL); g_return_val_if_fail (name != NULL, NULL); + result = NULL; + count = gst_child_proxy_get_children_count (parent); for (i = 0; i < count; i++) { - object = gst_child_proxy_get_child_by_index (parent, i); + gboolean eq; + + if (!(object = gst_child_proxy_get_child_by_index (parent, i))) + continue; + object_name = gst_object_get_name (object); if (object_name == NULL) { g_warning ("child %u of parent %s has no name", i, GST_OBJECT_NAME (parent)); - continue; + goto next; } - if (g_str_equal (object_name, name)) - return object; + eq = g_str_equal (object_name, name); + g_free (object_name); + + if (eq) { + result = object; + break; + } + next: + gst_object_unref (object); } - return NULL; + return result; } /** @@ -95,7 +110,10 @@ gst_child_proxy_get_child_by_name (GstChildProxy * parent, const gchar * name) * * Fetches a child by its number. * - * Returns: the child object or %NULL if not found (index too high) + * Returns: the child object or %NULL if not found (index too high). Unref + * after usage. + * + * MT safe. */ GstObject * gst_child_proxy_get_child_by_index (GstChildProxy * parent, guint index) @@ -113,6 +131,8 @@ gst_child_proxy_get_child_by_index (GstChildProxy * parent, guint index) * Gets the number of child objects this parent contains. * * Returns: the number of child objects + * + * MT safe. */ guint gst_child_proxy_get_children_count (GstChildProxy * parent) @@ -132,7 +152,10 @@ gst_child_proxy_get_children_count (GstChildProxy * parent) * Looks up which object and #GParamSpec would be effected by the given @name. * * Returns: TRUE if @target and @pspec could be found. FALSE otherwise. In that - * case the values for @pspec and @target are not modified + * case the values for @pspec and @target are not modified. Unref @target after + * usage. + * + * MT safe. */ gboolean gst_child_proxy_lookup (GstObject * object, const gchar * name, @@ -144,21 +167,26 @@ gst_child_proxy_lookup (GstObject * object, const gchar * name, g_return_val_if_fail (GST_IS_OBJECT (object), FALSE); g_return_val_if_fail (name != NULL, FALSE); + gst_object_ref (object); + current = names = g_strsplit (name, "::", -1); while (current[1]) { + GstObject *next; + if (!GST_IS_CHILD_PROXY (object)) { GST_INFO ("object %s is not a parent, so you cannot request a child by name %s", GST_OBJECT_NAME (object), current[0]); break; } - object = - gst_child_proxy_get_child_by_name (GST_CHILD_PROXY (object), + next = gst_child_proxy_get_child_by_name (GST_CHILD_PROXY (object), current[0]); - if (!object) { + if (!next) { GST_INFO ("no such object %s", current[0]); break; } + gst_object_unref (object); + object = next; current++; }; if (current[1] == NULL) { @@ -169,11 +197,14 @@ gst_child_proxy_lookup (GstObject * object, const gchar * name, } else { if (pspec) *pspec = spec; - if (target) + if (target) { + gst_object_ref (object); *target = object; + } res = TRUE; } } + gst_object_unref (object); g_strfreev (names); return res; } @@ -198,12 +229,20 @@ gst_child_proxy_get_property (GstObject * object, const gchar * name, g_return_if_fail (name != NULL); g_return_if_fail (!G_IS_VALUE (value)); - if (!gst_child_proxy_lookup (object, name, &target, &pspec)) { + if (!gst_child_proxy_lookup (object, name, &target, &pspec)) + goto not_found; + + g_object_get_property (G_OBJECT (target), pspec->name, value); + gst_object_unref (target); + + return; + +not_found: + { g_warning ("cannot get property %s from object %s", name, GST_OBJECT_NAME (object)); return; } - g_object_get_property (G_OBJECT (target), pspec->name, value); } /** @@ -224,8 +263,6 @@ gst_child_proxy_get_valist (GstObject * object, g_return_if_fail (G_IS_OBJECT (object)); - g_object_ref (object); - name = first_property_name; /* iterate over pairs */ @@ -239,8 +276,6 @@ gst_child_proxy_get_valist (GstObject * object, g_value_unset (&value); name = va_arg (var_args, gchar *); } - - g_object_unref (object); } /** @@ -282,12 +317,19 @@ gst_child_proxy_set_property (GstObject * object, const gchar * name, g_return_if_fail (name != NULL); g_return_if_fail (!G_IS_VALUE (value)); - if (!gst_child_proxy_lookup (object, name, &target, &pspec)) { + if (!gst_child_proxy_lookup (object, name, &target, &pspec)) + goto not_found; + + g_object_set_property (G_OBJECT (target), pspec->name, value); + gst_object_unref (target); + return; + +not_found: + { g_warning ("cannot set property %s on object %s", name, GST_OBJECT_NAME (object)); return; } - g_object_set_property (G_OBJECT (target), pspec->name, value); } /** @@ -308,8 +350,6 @@ gst_child_proxy_set_valist (GstObject * object, g_return_if_fail (G_IS_OBJECT (object)); - g_object_ref (object); - name = first_property_name; /* iterate over pairs */ @@ -320,21 +360,21 @@ gst_child_proxy_set_valist (GstObject * object, if (!gst_child_proxy_lookup (object, name, &target, &pspec)) { g_warning ("no such property %s in object %s", name, GST_OBJECT_NAME (object)); - g_object_unref (object); + continue; } g_value_init (&value, G_PARAM_SPEC_VALUE_TYPE (pspec)); G_VALUE_COLLECT (&value, var_args, G_VALUE_NOCOPY_CONTENTS, &error); if (error) { g_warning ("error copying value: %s", error); - g_object_unref (object); + gst_object_unref (target); return; } g_object_set_property (G_OBJECT (target), pspec->name, &value); + gst_object_unref (target); + g_value_unset (&value); name = va_arg (var_args, gchar *); } - - g_object_unref (object); } /** diff --git a/gst/parse/grammar.y b/gst/parse/grammar.y index dc552c767b..980999cb30 100644 --- a/gst/parse/grammar.y +++ b/gst/parse/grammar.y @@ -267,6 +267,7 @@ gst_parse_element_set (gchar *value, GstElement *element, graph_t *graph) if (!gst_value_deserialize (&v, pos)) goto error; g_object_set_property (G_OBJECT (target), pspec->name, &v); + gst_object_unref (target); } else { SET_ERROR (((graph_t *) graph)->error, GST_PARSE_ERROR_NO_SUCH_PROPERTY, _("no property \"%s\" in element \"%s\""), value, GST_ELEMENT_NAME (element)); } @@ -280,6 +281,7 @@ out: return; error: + gst_object_unref (target); SET_ERROR (((graph_t *) graph)->error, GST_PARSE_ERROR_COULD_NOT_SET_PROPERTY, _("could not set property \"%s\" in element \"%s\" to \"%s\""), value, GST_ELEMENT_NAME (element), pos);