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

@@ -1,3 +1,13 @@
+2004-12-14  Thomas Vander Stichele  <thomas at apestaart dot org>
+
+	* 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  <thomas at apestaart dot org>
 
 	* docs/design/part-conventions.txt:

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;
 }
 

common

@@ -1 +1 @@
-Subproject commit 8404d8841f5fd58fe31de09090867115e97c5261
+Subproject commit b2638c100721f67b280c3b43b21f1ce1c9b5e316

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.  
+The Element is the most important object in the entire GStreamer system, as it
-Elements include sources, filters, sinks, and containers (Bins).  They may be an intrinsic part of the core GStreamer
+defines the structure of the pipeline.  Elements include sources, filters,
-library, or may be loaded from a plugin.  In some cases they're even fabricated from completely different systems (see
+sinks, and containers (Bins).  They may be an intrinsic part of the core
-the LADSPA plugin).  They are generally created from a GstElementFactory, which will be covered in another chapter, but
+GStreamer library, or may be loaded from a plugin.  In some cases they're even
-for the intrinsic types they can be created with specific functions.
+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
+Elements contains GstPads (also covered in another chapter), which are
-together to form a pipeline capable of passing and processing data.  They have a parent, which must be another Element.  
+subsequently used to connect the Elements together to form a pipeline capable
-This allows deeply nested pipelines, and the possibility of "black-box" meta-elements.
+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 
+All elements are named, and while they should ideally be unique in any given
-guaranteed unique name for an element is its complete path in the object hierarchy.  Functions are provided to set and 
+pipeline, they do not have to be.  The only guaranteed unique name for an
-get the name of the element.  _set_name() creates a local copy of the string passed.  _get_name() returns the actual 
+element is its complete path in the object hierarchy.  In other words, an
-element's pointer.  Therefore, the argument to _set_name() is the responsibility of the caller to free if necessary, 
+element's name is unique inside its parent.  (This follows from GstObject's
-but the return from _get_name() is definitely not to be messed with by the caller.  Accordingly, _get_name() returns an 
+name explanation)
-const gchar *.
+
+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 

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.
+A pointer is available to store the current parent of the object.  This is one
-This one of the two fundamental requires for a 
+of the two fundamental requires for a hierarchical system such as GStreamer
-hierarchical system such as GStreamer (for the other, read up on GstBin).  Three functions are provided: _set_parent(), 
+(for the other, read up on GstBin).  Three functions are provided:
-_get_parent(), and _unparent().  The third is required because there is an explicit check in _set_parent(): an object 
+_set_parent(), _get_parent(), and _unparent().  The third is required because
-must not already have a parent if you wish to set one.  You must unparent the object first.  This allows for new 
+there is an explicit check in _set_parent(): an object must not already have a
-additions later.
+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 
+The GstObject contains the necessary primitives to lock the object in a
-provide general thread-safety as needed.  However, this lock is generic, i.e. it covers the whole object.  Note that 
+thread-safe manner.  This will be used to provide general thread-safety as
-this does *not* mean that no other thread can modify the object at the same time that the lock is held.  It only means 
+needed.  However, this lock is generic, i.e. it covers the whole object.
-that any two sections of code that obey the lock are guaranteed to not be running simultaneously.
 
-This lock will ideally be used for parentage and refcounting, which is reasonable, since they are the only possible 
+All members of the GstObject structure marked as
-things to protect in the GstObject.
+/*< 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
+Due to the base nature of the GstObject, it becomes the only reasonable place
-(_get_path_string).  It will generate a string describing the parent hierarchy of a given GstObject.  Currently it is
+to put this particular function (_get_path_string).  It will generate a string
-forced to use several child-class-specific functions, because we do not properly use the base capabilities (parentage,
+describing the parent hierarchy of a given GstObject.  Currently it is forced
-etc.) of GstObject properly.
+to use several child-class-specific functions, because we do not properly use
+the base capabilities (parentage, etc.) of GstObject properly.

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

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 */
+  GMutex    *lock;        /* object LOCK */
-  gchar 	*name; 		/* name */
+  gchar	    *name;        /* object name */
-  GstObject 	*parent; 	/* this object's parent, no refcount is held for the parent. */
+  GstObject *parent;      /* this object's parent, weak ref */
-  guint32 	 flags;
+  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);
+gboolean	gst_object_set_name		(GstObject *object, const gchar *name_prefix);
-gchar* 		gst_object_get_name		(GstObject *object);
+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);

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;
 }