From 29177515ffe035eda88c91ccdcf020a21e53524b Mon Sep 17 00:00:00 2001 From: Thomas Vander Stichele Date: Tue, 14 Dec 2004 11:47:44 +0000 Subject: [PATCH] add _get/_set_name_prefix() for debugging. Update docs. Original commit message from CVS: * check/gst/gstobject.c: * docs/design/part-gstelement.txt: * docs/design/part-gstobject.txt: * gst/gstobject.c: * gst/gstobject.h: add _get/_set_name_prefix() for debugging. Update docs. Finish unique name unit test --- ChangeLog | 10 ++++++ check/gst/gstobject.c | 47 +++++++++++++++++++++++++-- common | 2 +- docs/design/part-gstelement.txt | 42 ++++++++++++++++-------- docs/design/part-gstobject.txt | 53 +++++++++++++++++++++--------- gst/gstobject.c | 57 +++++++++++++++++++++++++++++++++ gst/gstobject.h | 21 +++++++----- tests/check/gst/gstobject.c | 47 +++++++++++++++++++++++++-- 8 files changed, 236 insertions(+), 43 deletions(-) diff --git a/ChangeLog b/ChangeLog index 7fa6ad74ce..09ca338e4b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2004-12-14 Thomas Vander Stichele + + * check/gst/gstobject.c: + * docs/design/part-gstelement.txt: + * docs/design/part-gstobject.txt: + * gst/gstobject.c: + * gst/gstobject.h: + add _get/_set_name_prefix() for debugging. Update docs. + Finish unique name unit test + 2004-12-13 Thomas Vander Stichele * docs/design/part-conventions.txt: diff --git a/check/gst/gstobject.c b/check/gst/gstobject.c index 3cac37e1f8..d0bfcaecdf 100644 --- a/check/gst/gstobject.c +++ b/check/gst/gstobject.c @@ -366,6 +366,8 @@ thread_name_object_default (int *i) for (j = *i; j < num_objects; j += num_threads) { GstObject *o = GST_OBJECT (g_list_nth_data (object_list, j)); + /* g_message ("THREAD %p: setting default name on object %d\n", + g_thread_self (), j); */ gst_object_set_name (o, NULL); /* a minimal sleep invokes a thread switch */ g_usleep (1); @@ -373,15 +375,42 @@ thread_name_object_default (int *i) /* thread is done, so let's return */ g_message ("THREAD %p: set name\n", g_thread_self ()); + g_free (i); return NULL; } +static gint +gst_object_name_compare (GstObject * o, GstObject * p) +{ + gint result; + + GST_LOCK (o); + GST_LOCK (p); + + if (o->name == NULL && p->name == NULL) { + result = 0; + } else if (o->name == NULL) { + result = -1; + } else if (p->name == NULL) { + result = 1; + } else { + result = strcmp (o->name, p->name); + } + + GST_UNLOCK (p); + GST_UNLOCK (o); + + return result; +} START_TEST (test_fake_object_name_threaded_unique) { GstObject *object; gint i; + gint *ip; + gchar *name1, *name2; + GList *l; g_message ("\nTEST: uniqueness of default names\n"); @@ -394,7 +423,9 @@ START_TEST (test_fake_object_name_threaded_unique) mark_point (); for (i = 0; i < num_threads; ++i) { - MAIN_START_THREAD_FUNCTION (i, thread_name_object_default, &i); + ip = g_new (gint, 1); + *ip = i; + MAIN_START_THREAD_FUNCTION (i, thread_name_object_default, ip); } mark_point (); @@ -404,6 +435,16 @@ START_TEST (test_fake_object_name_threaded_unique) /* sort GList based on object name */ /* FIXME: sort and test */ + g_list_sort (object_list, (GCompareFunc) gst_object_name_compare); + + name1 = gst_object_get_name (GST_OBJECT (object_list->data)); + for (l = object_list->next; l->next; l = l->next) { + g_message ("object with name %s\n", name1); + name2 = gst_object_get_name (GST_OBJECT (l->data)); + fail_if (strcmp (name1, name2) == 0, "Two objects with name %s", name2); + g_free (name1); + name1 = name2; + } /* free stuff */ g_list_foreach (object_list, (GFunc) g_object_unref, NULL); @@ -417,13 +458,15 @@ END_TEST TCase *tc_chain = tcase_create ("general"); suite_add_tcase (s, tc_chain); - tcase_add_test_raise_signal (tc_chain, test_fail_abstract_new, SIGSEGV); tcase_add_test (tc_chain, test_fake_object_new); tcase_add_test (tc_chain, test_fake_object_name); tcase_add_test (tc_chain, test_fake_object_name_threaded_wrong); tcase_add_test (tc_chain, test_fake_object_name_threaded_right); tcase_add_test (tc_chain, test_fake_object_name_threaded_unique); //tcase_add_checked_fixture (tc_chain, setup, teardown); + + /* SEGV tests go last so we can debug the others */ + tcase_add_test_raise_signal (tc_chain, test_fail_abstract_new, SIGSEGV); return s; } diff --git a/common b/common index 8404d8841f..b2638c1007 160000 --- a/common +++ b/common @@ -1 +1 @@ -Subproject commit 8404d8841f5fd58fe31de09090867115e97c5261 +Subproject commit b2638c100721f67b280c3b43b21f1ce1c9b5e316 diff --git a/docs/design/part-gstelement.txt b/docs/design/part-gstelement.txt index 9cb57eceb7..17585744ed 100644 --- a/docs/design/part-gstelement.txt +++ b/docs/design/part-gstelement.txt @@ -10,25 +10,39 @@ GstElement ========== -The Element is the most important object in the entire GStreamer system, as it defines the structure of the pipeline. -Elements include sources, filters, sinks, and containers (Bins). They may be an intrinsic part of the core GStreamer -library, or may be loaded from a plugin. In some cases they're even fabricated from completely different systems (see -the LADSPA plugin). They are generally created from a GstElementFactory, which will be covered in another chapter, but -for the intrinsic types they can be created with specific functions. +The Element is the most important object in the entire GStreamer system, as it +defines the structure of the pipeline. Elements include sources, filters, +sinks, and containers (Bins). They may be an intrinsic part of the core +GStreamer library, or may be loaded from a plugin. In some cases they're even +fabricated from completely different systems (see the LADSPA plugin). They +are generally created from a GstElementFactory, which will be covered in +another chapter, but for the intrinsic types they can be created with specific +functions. -Elements contains GstPads (also covered in another chapter), which are subsequently used to connect the Elements -together to form a pipeline capable of passing and processing data. They have a parent, which must be another Element. -This allows deeply nested pipelines, and the possibility of "black-box" meta-elements. +Elements contains GstPads (also covered in another chapter), which are +subsequently used to connect the Elements together to form a pipeline capable +of passing and processing data. They have a parent, which must be another +Element. This allows deeply nested pipelines, and the possibility of +"black-box" meta-elements. Name ---- -All elements are named, and while they should ideally be unique in any given pipeline, the do not have to be. The only -guaranteed unique name for an element is its complete path in the object hierarchy. Functions are provided to set and -get the name of the element. _set_name() creates a local copy of the string passed. _get_name() returns the actual -element's pointer. Therefore, the argument to _set_name() is the responsibility of the caller to free if necessary, -but the return from _get_name() is definitely not to be messed with by the caller. Accordingly, _get_name() returns an -const gchar *. +All elements are named, and while they should ideally be unique in any given +pipeline, they do not have to be. The only guaranteed unique name for an +element is its complete path in the object hierarchy. In other words, an +element's name is unique inside its parent. (This follows from GstObject's +name explanation) + +This uniqueness is guaranteed through all functions where either parentage +or name of an element is changed. + +Functions are provided +to set and get the name of the element. _set_name() creates a local copy of +the string passed. _get_name() returns the actual element's pointer. +Therefore, the argument to _set_name() is the responsibility of the caller to +free if necessary, but the return from _get_name() is definitely not to be +messed with by the caller. Accordingly, _get_name() returns an const gchar *. Providing a new name to an element causes it to free its internal copy of the name and make a copy of the new name. This means that you must consider the pointer returned by _get_name() to be short-lived. If you must make use of the diff --git a/docs/design/part-gstobject.txt b/docs/design/part-gstobject.txt index f6d6e5d778..a85480e21c 100644 --- a/docs/design/part-gstobject.txt +++ b/docs/design/part-gstobject.txt @@ -6,12 +6,13 @@ The base class for the entire GStreamer hierarchy is the GstObject. Parentage --------- -A pointer is available to store the current parent of the object. -This one of the two fundamental requires for a -hierarchical system such as GStreamer (for the other, read up on GstBin). Three functions are provided: _set_parent(), -_get_parent(), and _unparent(). The third is required because there is an explicit check in _set_parent(): an object -must not already have a parent if you wish to set one. You must unparent the object first. This allows for new -additions later. +A pointer is available to store the current parent of the object. This is one +of the two fundamental requires for a hierarchical system such as GStreamer +(for the other, read up on GstBin). Three functions are provided: +_set_parent(), _get_parent(), and _unparent(). The third is required because +there is an explicit check in _set_parent(): an object must not already have a +parent if you wish to set one. You must unparent the object first. This +allows for new additions later. - GstObject's that can be parented: GstElement (inside a bin) @@ -30,22 +31,42 @@ Naming - names of objects should be unique across parent - set_name() can fail because of this - as can gst_element_add_pad()/gst_bin_add_element() +- gst_object_set_name() only changes the object's name + +- objects also have a name_prefix that is used to prefix the object name + during debugging and identification +- there are object-specific set_name's() which also set the name_prefix + on the object. This is useful for debugging purposes to give the object + a more identifiable name. Typically a parent will call _set_name_prefix + on children, taking a lock on them to do so. Locking ------- -The GstObject contains the necessary primitives to lock the object in a thread-safe manner. This will be used to -provide general thread-safety as needed. However, this lock is generic, i.e. it covers the whole object. Note that -this does *not* mean that no other thread can modify the object at the same time that the lock is held. It only means -that any two sections of code that obey the lock are guaranteed to not be running simultaneously. +The GstObject contains the necessary primitives to lock the object in a +thread-safe manner. This will be used to provide general thread-safety as +needed. However, this lock is generic, i.e. it covers the whole object. -This lock will ideally be used for parentage and refcounting, which is reasonable, since they are the only possible -things to protect in the GstObject. +All members of the GstObject structure marked as +/*< public >*/ /* with LOCK */ +are protected by this lock. These members can only be accessed for reading +or writing while the lock is held. + +Note that this does *not* mean that no other thread can modify the object at +the same time that the lock is held. It only means that any two sections of +code that obey the lock are guaranteed to not be running simultaneously. "The +lock is voluntary and cooperative". + +This lock will ideally be used for parentage and refcounting, which is +reasonable, since they are the only possible things to protect in the +GstObject. Path Generation --------------- +FIXME: rethink this ? -Due to the base nature of the GstObject, it becomes the only reasonable place to put this particular function -(_get_path_string). It will generate a string describing the parent hierarchy of a given GstObject. Currently it is -forced to use several child-class-specific functions, because we do not properly use the base capabilities (parentage, -etc.) of GstObject properly. +Due to the base nature of the GstObject, it becomes the only reasonable place +to put this particular function (_get_path_string). It will generate a string +describing the parent hierarchy of a given GstObject. Currently it is forced +to use several child-class-specific functions, because we do not properly use +the base capabilities (parentage, etc.) of GstObject properly. diff --git a/gst/gstobject.c b/gst/gstobject.c index f2bfda9885..9d05c82475 100644 --- a/gst/gstobject.c +++ b/gst/gstobject.c @@ -667,6 +667,63 @@ gst_object_get_name (GstObject * object) return result; } +/** + * gst_object_set_name_prefix: + * @object: a #GstObject to set the name prefix of + * @name_prefix: new name prefix of object + * + * Sets the name prefix of the object. + * This function makes a copy of the provided name prefix, so the caller + * retains ownership of the name prefix it sent. + * + * + * MT safe. This function grabs and releases the object's LOCK. + */ +void +gst_object_set_name_prefix (GstObject * object, const gchar * name_prefix) +{ + g_return_if_fail (GST_IS_OBJECT (object)); + + GST_LOCK (object); + + if (object->name_prefix != NULL) + g_free (object->name_prefix); + + if (name_prefix == NULL) + object->name_prefix = NULL; + else + object->name_prefix = g_strdup (name_prefix); + + GST_UNLOCK (object); +} + +/** + * gst_object_get_name_prefix: + * @object: a #GstObject to get the name prefix of + * + * Returns a copy of the name prefix of the object. + * Caller should g_free() the return value after usage. + * For a prefixless object, this returns NULL, which you can safely g_free() + * as well. + * + * Returns: the name prefix of the object. g_free() after usage. + * + * MT safe. + */ +gchar * +gst_object_get_name_prefix (GstObject * object) +{ + gchar *result = NULL; + + g_return_val_if_fail (GST_IS_OBJECT (object), NULL); + + GST_LOCK (object); + result = g_strdup (object->name_prefix); + GST_UNLOCK (object); + + return result; +} + /** * gst_object_set_parent: * @object: GstObject to set parent of diff --git a/gst/gstobject.h b/gst/gstobject.h index 15eee3afcf..3d0f47636a 100644 --- a/gst/gstobject.h +++ b/gst/gstobject.h @@ -83,19 +83,22 @@ typedef enum #define GST_OBJECT_IS_FLOATING(obj) (GST_FLAG_IS_SET (obj, GST_OBJECT_FLOATING)) struct _GstObject { - GObject object; + GObject object; /*< public >*/ GstAtomicInt refcount; /*< public >*/ /* with LOCK */ - GMutex *lock; /* locking for all sorts of things */ - gchar *name; /* name */ - GstObject *parent; /* this object's parent, no refcount is held for the parent. */ - guint32 flags; + GMutex *lock; /* object LOCK */ + gchar *name; /* object name */ + GstObject *parent; /* this object's parent, weak ref */ + guint32 flags; + + /* FIXME: in padding, move on up */ + gchar *name_prefix; /* used for debugging */ /*< private >*/ - gpointer _gst_reserved[GST_PADDING]; + gpointer _gst_reserved[GST_PADDING - 1]; }; @@ -133,8 +136,10 @@ struct _GstObjectClass { GType gst_object_get_type (void); /* name routines */ -gboolean gst_object_set_name (GstObject *object, const gchar *name); -gchar* gst_object_get_name (GstObject *object); +gboolean gst_object_set_name (GstObject *object, const gchar *name_prefix); +gchar* gst_object_get_name (GstObject *object); +void gst_object_set_name_prefix (GstObject *object, const gchar *name_prefix); +gchar* gst_object_get_name_prefix (GstObject *object); /* parentage routines */ gboolean gst_object_set_parent (GstObject *object, GstObject *parent); diff --git a/tests/check/gst/gstobject.c b/tests/check/gst/gstobject.c index 3cac37e1f8..d0bfcaecdf 100644 --- a/tests/check/gst/gstobject.c +++ b/tests/check/gst/gstobject.c @@ -366,6 +366,8 @@ thread_name_object_default (int *i) for (j = *i; j < num_objects; j += num_threads) { GstObject *o = GST_OBJECT (g_list_nth_data (object_list, j)); + /* g_message ("THREAD %p: setting default name on object %d\n", + g_thread_self (), j); */ gst_object_set_name (o, NULL); /* a minimal sleep invokes a thread switch */ g_usleep (1); @@ -373,15 +375,42 @@ thread_name_object_default (int *i) /* thread is done, so let's return */ g_message ("THREAD %p: set name\n", g_thread_self ()); + g_free (i); return NULL; } +static gint +gst_object_name_compare (GstObject * o, GstObject * p) +{ + gint result; + + GST_LOCK (o); + GST_LOCK (p); + + if (o->name == NULL && p->name == NULL) { + result = 0; + } else if (o->name == NULL) { + result = -1; + } else if (p->name == NULL) { + result = 1; + } else { + result = strcmp (o->name, p->name); + } + + GST_UNLOCK (p); + GST_UNLOCK (o); + + return result; +} START_TEST (test_fake_object_name_threaded_unique) { GstObject *object; gint i; + gint *ip; + gchar *name1, *name2; + GList *l; g_message ("\nTEST: uniqueness of default names\n"); @@ -394,7 +423,9 @@ START_TEST (test_fake_object_name_threaded_unique) mark_point (); for (i = 0; i < num_threads; ++i) { - MAIN_START_THREAD_FUNCTION (i, thread_name_object_default, &i); + ip = g_new (gint, 1); + *ip = i; + MAIN_START_THREAD_FUNCTION (i, thread_name_object_default, ip); } mark_point (); @@ -404,6 +435,16 @@ START_TEST (test_fake_object_name_threaded_unique) /* sort GList based on object name */ /* FIXME: sort and test */ + g_list_sort (object_list, (GCompareFunc) gst_object_name_compare); + + name1 = gst_object_get_name (GST_OBJECT (object_list->data)); + for (l = object_list->next; l->next; l = l->next) { + g_message ("object with name %s\n", name1); + name2 = gst_object_get_name (GST_OBJECT (l->data)); + fail_if (strcmp (name1, name2) == 0, "Two objects with name %s", name2); + g_free (name1); + name1 = name2; + } /* free stuff */ g_list_foreach (object_list, (GFunc) g_object_unref, NULL); @@ -417,13 +458,15 @@ END_TEST TCase *tc_chain = tcase_create ("general"); suite_add_tcase (s, tc_chain); - tcase_add_test_raise_signal (tc_chain, test_fail_abstract_new, SIGSEGV); tcase_add_test (tc_chain, test_fake_object_new); tcase_add_test (tc_chain, test_fake_object_name); tcase_add_test (tc_chain, test_fake_object_name_threaded_wrong); tcase_add_test (tc_chain, test_fake_object_name_threaded_right); tcase_add_test (tc_chain, test_fake_object_name_threaded_unique); //tcase_add_checked_fixture (tc_chain, setup, teardown); + + /* SEGV tests go last so we can debug the others */ + tcase_add_test_raise_signal (tc_chain, test_fail_abstract_new, SIGSEGV); return s; }