Docs updates, clean up some headers.

Original commit message from CVS:
* docs/design/part-MT-refcounting.txt:
* docs/design/part-conventions.txt:
* docs/design/part-gstobject.txt:
* docs/design/part-relations.txt:
* docs/design/part-standards.txt:
* gst/gstbin.c: (gst_bin_add_func), (gst_bin_add),
(gst_bin_remove_func), (gst_bin_remove), (gst_bin_iterate_recurse),
(gst_bin_get_by_name), (gst_bin_get_by_interface),
(gst_bin_iterate_all_by_interface):
* gst/gstbuffer.h:
* gst/gstclock.h:
* gst/gstelement.c: (gst_element_class_init),
(gst_element_change_state), (gst_element_set_loop_function):
* gst/gstelement.h:
* gst/gstiterator.c:
* gst/gstobject.c: (gst_object_class_init), (gst_object_ref),
(gst_object_unref), (gst_object_sink), (gst_object_dispose),
(gst_object_dispatch_properties_changed), (gst_object_set_name),
(gst_object_set_parent), (gst_object_unparent),
(gst_object_check_uniqueness):
* gst/gstobject.h:
Docs updates, clean up some headers.
Free iterators in GstBin.
GstObject is now looking good.

ChangeLog

@@ -1,3 +1,28 @@
+2005-03-08  Wim Taymans  <wim@fluendo.com>
+
+	* docs/design/part-MT-refcounting.txt:
+	* docs/design/part-conventions.txt:
+	* docs/design/part-gstobject.txt:
+	* docs/design/part-relations.txt:
+	* docs/design/part-standards.txt:
+	* gst/gstbin.c: (gst_bin_add_func), (gst_bin_add),
+	(gst_bin_remove_func), (gst_bin_remove), (gst_bin_iterate_recurse),
+	(gst_bin_get_by_name), (gst_bin_get_by_interface),
+	(gst_bin_iterate_all_by_interface):
+	* gst/gstbuffer.h:
+	* gst/gstclock.h:
+	* gst/gstelement.c: (gst_element_class_init),
+	(gst_element_change_state), (gst_element_set_loop_function):
+	* gst/gstelement.h:
+	* gst/gstiterator.c:
+	* gst/gstobject.c: (gst_object_class_init), (gst_object_ref),
+	(gst_object_unref), (gst_object_sink), (gst_object_dispose),
+	(gst_object_dispatch_properties_changed), (gst_object_set_name),
+	(gst_object_set_parent), (gst_object_unparent),
+	(gst_object_check_uniqueness):
+	* gst/gstobject.h:
+	Docs updates, clean up some headers.
+
 2005-03-07  Wim Taymans  <wim@fluendo.com>
 
 	* check/.cvsignore:

docs/design/part-MT-refcounting.txt

@@ -260,6 +260,16 @@ Objects
     anymore of the pad. If you need to be sure it is, you need to extend the
     critical section to include the operations on the peer.
 
+    The following code is equivalent to the above but with using the functions
+    to access object properties.
+
+      peer = gst_pad_get_parent (pad);
+      if (peer) {
+        ... use peer ...
+
+        gst_object_unref (GST_OBJECT (peer));
+      }
+
   Example:
 
     Accessing the name of an object makes a copy of the name. The caller of the
@@ -272,6 +282,14 @@ Objects
 
       g_free (name);
 
+    or:
+
+      name = gst_object_get_name (object);
+
+      ... use name ...
+
+      g_free (name);
+    
 
 * Accessor methods
 

docs/design/part-conventions.txt

@@ -30,8 +30,6 @@ ASYNC.  Where there is a prefix, as in the element flags, this is usually droppe
 element flags should be cross-checked with the header, as there are currently two conventions in use: with and without 
 _FLAGS_ in the middle.
 
-FIXME: check flags for consistency.
-
 Drawing conventions
 ===================
 

docs/design/part-gstobject.txt

@@ -18,13 +18,15 @@ allows for new additions later.
   GstElement (inside a bin)
   GstPad (inside an element)
 
+
 Refcounting
 -----------
-- GObject refcount is not threadsafe.
+- GObject refcount is not threadsafe. This will be changed in the future.
-  GStreamer sets it to a constant value on each _ref/_unref()
+  GStreamer for now sets it to a constant value on each _ref/_unref()
   and uses an atomic int "refcount" instead for threadsafe refcounting
   This implies you should always use gst_object_ref() and gst_object_unref() !
 
+
 Naming
 ------
 - names of objects cannot be changed when they are parented 
@@ -40,6 +42,7 @@ Naming
   a more identifiable name.  Typically a parent will call _set_name_prefix
   on children, taking a lock on them to do so.
 
+
 Locking
 -------
 
@@ -50,23 +53,38 @@ needed.  However, this lock is generic, i.e. it covers the whole object.
 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.
+or writing while the lock is held. All members should be copied or reffed
+if they are used after releasing the LOCK.
 
 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
+This lock will ideally be used for parentage, flags and naming, 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
+describing the parent hierarchy of a given GstObject.  
-to use several child-class-specific functions, because we do not properly use
+
-the base capabilities (parentage, etc.) of GstObject properly.
+
+Flags
+-----
+
+Each object in the GStreamer object hierarchy can have flags associated with it,
+which are used to describe a state or a feature of the object.
+GstObject has flags to mark its lifecycle: FLOATING, DISPOSING and DESTROYED.
+
+
+Class signals
+-------------
+
+It is possible to know when a new object is loaded by connecting to the
+GstObjectClass signal. This feature is not very much used and might be removed
+at some point.

docs/design/part-relations.txt

@@ -1,6 +1,10 @@
 Object relation types
 ---------------------
 
+This document describes the relations between objects that exist in GStreamer.
+It will also describe the way of handling the relation wrt locking and 
+refcounting.
+
 1) parent-child relation
 
         +---------+    +-------+

docs/design/part-standards.txt

@@ -6,11 +6,7 @@ far as the pointers go.  Therefore, some standards must be adhered to as far as
 
 Strings:
 Arguments passed into a function are owned by the caller, and the function will make a copy of the string for its own 
-internal use.  The string should be const gchar *.  Strings returned from a function remain the property of the 
+internal use.  The string should be const gchar *.  Strings returned from a function are always a copy of the
-function called, and the caller must make a copy if it is to use the string for an extended duration.
+original and should be freed after usage by the caller.
 
 Objects:
-The ownership of an object during a function call depends on the type of function.  If the function is simply returning 
-something from the object, such as _get_name(), the caller retains ownership.  If the object passed is to be 
-manipulated in some way, it is generally the case that the function will take over the ownership.  This should be 
-expressed as a reference increment on that object, but isn't in the general case (yet).

gst/gstbin.c

@@ -484,23 +484,27 @@ gst_bin_add_func (GstBin * bin, GstElement * element)
 
   /* ERROR handling here */
 adding_itself:
+  {
     GST_LOCK (bin);
     g_warning ("Cannot add bin %s to itself", GST_ELEMENT_NAME (bin));
     GST_UNLOCK (bin);
     return FALSE;
-
+  }
 duplicate_name:
+  {
     g_warning ("Name %s is not unique in bin %s, not adding",
         elem_name, GST_ELEMENT_NAME (bin));
     GST_UNLOCK (bin);
     g_free (elem_name);
     return FALSE;
-
+  }
 had_parent:
+  {
     g_warning ("Element %s already has parent", elem_name);
     GST_UNLOCK (bin);
     g_free (elem_name);
     return FALSE;
+  }
 }
 
 /**
@@ -537,10 +541,13 @@ gst_bin_add (GstBin * bin, GstElement * element)
 
   return result;
 
+  /* ERROR handling */
 no_function:
+  {
     g_warning ("adding elements to bin %s is not supported",
         GST_ELEMENT_NAME (bin));
     return FALSE;
+  }
 }
 
 /* remove an element from the bin
@@ -585,11 +592,15 @@ gst_bin_remove_func (GstBin * bin, GstElement * element)
 
   return TRUE;
 
+  /* ERROR handling */
 not_in_bin:
-  g_warning ("Element %s is not in bin %s", elem_name, GST_ELEMENT_NAME (bin));
+  {
+    g_warning ("Element %s is not in bin %s", elem_name,
+        GST_ELEMENT_NAME (bin));
     GST_UNLOCK (bin);
     g_free (elem_name);
     return FALSE;
+  }
 }
 
 /**
@@ -630,10 +641,13 @@ gst_bin_remove (GstBin * bin, GstElement * element)
 
   return result;
 
+  /* ERROR handling */
 no_function:
+  {
     g_warning ("removing elements from bin %s is not supported",
         GST_ELEMENT_NAME (bin));
     return FALSE;
+  }
 }
 
 static GstIteratorItem
@@ -725,10 +739,8 @@ gst_bin_iterate_recurse (GstBin * bin)
   GST_UNLOCK (bin);
 
   return result;
-  return NULL;
 }
 
-
 /**
  * gst_bin_child_state_change:
  * @bin: #GstBin with the child
@@ -1070,6 +1082,7 @@ gst_bin_get_by_name (GstBin * bin, const gchar * name)
   children = gst_bin_iterate_recurse (bin);
   result = gst_iterator_find_custom (children,
       (GCompareFunc) compare_name, (gpointer) name);
+  gst_iterator_free (children);
 
   return GST_ELEMENT_CAST (result);
 }
@@ -1154,6 +1167,7 @@ gst_bin_get_by_interface (GstBin * bin, GType interface)
   children = gst_bin_iterate_recurse (bin);
   result = gst_iterator_find_custom (children, (GCompareFunc) compare_interface,
       GINT_TO_POINTER (interface));
+  gst_iterator_free (children);
 
   return GST_ELEMENT_CAST (result);
 }
@@ -1182,6 +1196,7 @@ gst_bin_iterate_all_by_interface (GstBin * bin, GType interface)
   children = gst_bin_iterate_recurse (bin);
   result = gst_iterator_filter (children, (GCompareFunc) compare_interface,
       GINT_TO_POINTER (interface));
+  gst_iterator_free (children);
 
   return result;
 }

gst/gstbuffer.h

@@ -81,11 +81,9 @@ extern GType _gst_buffer_type;
  * @GST_BUFFER_ORIGINAL: buffer is not a copy of another buffer.
  * @GST_BUFFER_DONTFREE: do not try to free the data when this buffer is
  * unreferenced.
- * @GST_BUFFER_KEY_UNIT: the buffer holds a key unit, a unit that can be
+ * @GST_BUFFER_PREROLL: the buffer is part of a preroll and should not be 
- * decoded independently of other buffers.
+ * displayed.
- * This flag has been deprecated, see #GST_BUFFER_DELTA_UNIT.
+ * @GST_BUFFER_DISCONT: the buffer marks a discontinuity in the stream.
- * @GST_BUFFER_DONTKEEP: the buffer should not be ref()ed, but copied instead
- * before doing anything with it (for specially allocated hw buffers and such)
  * @GST_BUFFER_IN_CAPS: the buffer has been added as a field in a #GstCaps.
  * @GST_BUFFER_GAP: the buffer has been created to fill a gap in the stream.
  * @GST_BUFFER_DELTA_UNIT: this unit cannot be decoded independently.

gst/gstclock.h

@@ -60,7 +60,7 @@ G_STMT_START { 						\
 #define GST_TIME_TO_TIMESPEC(t,ts)			\
 G_STMT_START { 						\
   (ts).tv_sec  =  (t) / GST_SECOND;			\
-  (ts).tv_usec = ((t) - (ts).tv_sec * GST_SECOND) / GST_NSECOND;	\
+  (ts).tv_nsec = ((t) - (ts).tv_sec * GST_SECOND) / GST_NSECOND;	\
 } G_STMT_END
 
 /* timestamp debugging macros */

gst/gstelement.c

@@ -1716,48 +1716,6 @@ gst_element_class_get_pad_template (GstElementClass * element_class,
   return NULL;
 }
 
-/**
- * gst_element_get_pad_template_list:
- * @element: a #GstElement to get pad templates of.
- *
- * Retrieves a list of the pad templates associated with the element.
- * (FIXME: Should be deprecated in favor of
- *  gst_element_class_get_pad_template_list).
- *
- * Returns: the #GList of padtemplates.
- */
-GList *
-gst_element_get_pad_template_list (GstElement * element)
-{
-  g_return_val_if_fail (element != NULL, NULL);
-  g_return_val_if_fail (GST_IS_ELEMENT (element), NULL);
-
-  return GST_ELEMENT_GET_CLASS (element)->padtemplates;
-}
-
-/**
- * gst_element_get_pad_template:
- * @element: a #GstElement to get the pad template of.
- * @name: the name of the #GstPadTemplate to get.
- *
- * Retrieves a padtemplate from this element with the
- * given name.
- * (FIXME: Should be deprecated in favor of gst_element_class_get_pad_template).
- *
- * Returns: the #GstPadTemplate with the given name, or NULL if none was found.
- * No unreferencing is necessary.
- */
-GstPadTemplate *
-gst_element_get_pad_template (GstElement * element, const gchar * name)
-{
-  g_return_val_if_fail (element != NULL, NULL);
-  g_return_val_if_fail (GST_IS_ELEMENT (element), NULL);
-  g_return_val_if_fail (name != NULL, NULL);
-
-  return gst_element_class_get_pad_template (GST_ELEMENT_GET_CLASS (element),
-      name);
-}
-
 static void
 gst_element_error_func (GstElement * element, GstElement * source,
     GError * error, gchar * debug)

gst/gstelement.h

@@ -281,9 +281,14 @@ struct _GstElementClass
   gpointer _gst_reserved[GST_PADDING - 1];
 };
 
+GType			gst_element_get_type		(void);
+
+/* element class pad templates */
 void			gst_element_class_add_pad_template	(GstElementClass *klass, GstPadTemplate *templ);
-void                    gst_element_class_install_std_props	(GstElementClass *klass,
+GstPadTemplate*		gst_element_class_get_pad_template	(GstElementClass *element_class, const gchar *name);
-								 const gchar      *first_name, ...);
+GList*                  gst_element_class_get_pad_template_list (GstElementClass *element_class);
+
+/* element class details */
 void			gst_element_class_set_details		(GstElementClass *klass,
 								 const GstElementDetails *details);
 
@@ -291,7 +296,6 @@ void			gst_element_class_set_details		(GstElementClass *klass,
 
 void 			gst_element_default_error		(GObject *object, GstObject *orig, GError *error, gchar *debug);
 
-GType			gst_element_get_type		(void);
 void			gst_element_set_loop_function	(GstElement *element,
 							 GstElementLoopFunction loop);
 
@@ -353,34 +357,7 @@ GstPad*			gst_element_get_static_pad	(GstElement *element, const gchar *name);
 GstPad*			gst_element_get_request_pad	(GstElement *element, const gchar *name);
 void			gst_element_release_request_pad	(GstElement *element, GstPad *pad);
 
-GstIterator *gst_element_iterate_pads (GstElement * element);
+GstIterator *		gst_element_iterate_pads 	(GstElement * element);
-
-GstPad*			gst_element_get_compatible_pad	(GstElement *element, GstPad *pad);
-GstPad*			gst_element_get_compatible_pad_filtered (GstElement *element, GstPad *pad, 
-							 const GstCaps *filtercaps);
-
-GstPadTemplate*		gst_element_class_get_pad_template	(GstElementClass *element_class, const gchar *name);
-GList*                  gst_element_class_get_pad_template_list (GstElementClass *element_class);
-GstPadTemplate*		gst_element_get_pad_template		(GstElement *element, const gchar *name);
-GList*			gst_element_get_pad_template_list	(GstElement *element);
-GstPadTemplate*		gst_element_get_compatible_pad_template (GstElement *element, GstPadTemplate *compattempl);
-
-gboolean		gst_element_link		(GstElement *src, GstElement *dest);
-gboolean		gst_element_link_many 		(GstElement *element_1, 
-							 GstElement *element_2, ...);
-gboolean		gst_element_link_filtered 	(GstElement *src, GstElement *dest,
-							 const GstCaps *filtercaps);
-void			gst_element_unlink 		(GstElement *src, GstElement *dest);
-void			gst_element_unlink_many 	(GstElement *element_1, 
-							 GstElement *element_2, ...);
-
-gboolean		gst_element_link_pads		(GstElement *src, const gchar *srcpadname,
-							 GstElement *dest, const gchar *destpadname);
-gboolean		gst_element_link_pads_filtered 	(GstElement *src, const gchar *srcpadname,
-							 GstElement *dest, const gchar *destpadname,
-							 const GstCaps *filtercaps);
-void			gst_element_unlink_pads		(GstElement *src, const gchar *srcpadname,
-							 GstElement *dest, const gchar *destpadname);
 
 G_CONST_RETURN GstEventMask*
 			gst_element_get_event_masks	(GstElement *element);
@@ -417,8 +394,6 @@ GstElementStateReturn	gst_element_set_state		(GstElement *element, GstElementSta
 
 void 			gst_element_wait_state_change 	(GstElement *element);
 	
-G_CONST_RETURN gchar*	gst_element_state_get_name	(GstElementState state);
-
 GstElementFactory*	gst_element_get_factory		(GstElement *element);
 
 GstBin*			gst_element_get_managing_bin	(GstElement *element);

gst/gstiterator.c

@@ -53,6 +53,9 @@ gst_iterator_init (GstIterator * it,
  * Create a new iterator. This function is mainly used for objects
  * implementing the next/resync/free function to iterate a data structure.
  *
+ * For each item retrieved, the @item function is called with the lock
+ * held. The @free function is called when the iterator is freed.
+ * 
  * Returns: the new #GstIterator.
  *
  * MT safe.
@@ -124,9 +127,8 @@ gst_list_iterator_free (GstListIterator * it)
  * @master_cookie: pointer to a guint32 to protect the list.
  * @list: pointer to the list
  * @owner: object owning the list
- * @ref: function to ref each item
+ * @item: function to call for each item
- * @unref: function to unref each item
+ * @free: function to call when the iterator is freed
- * @free: function to free the owner of the list
  *
  * Create a new iterator designed for iterating @list. 
  * 

gst/gstobject.c

@@ -344,9 +344,7 @@ gst_object_unref (GstObject * object)
  * creating a new object to symbolically 'take ownership' of the object.
  * Use #gst_object_set_parent to have this done for you.
  *
- * This function takes the object lock.
+ * MT safe. This function grabs and releases the object lock.
- *
- * MT safe.
  */
 void
 gst_object_sink (GstObject * object)
@@ -375,6 +373,9 @@ gst_object_sink (GstObject * object)
  * function, it does not take any locks. You might want to lock
  * the object owning the oldobj pointer before calling this
  * function.
+ *
+ * Make sure not to LOCK the oldobj because it might be unreffed
+ * which could cause a deadlock when it is disposed.
  */
 void
 gst_object_replace (GstObject ** oldobj, GstObject * newobj)
@@ -530,13 +531,13 @@ gst_object_dispatch_properties_changed (GObject * object,
  * @excluded_props: a set of user-specified properties to exclude or
  *  NULL to show all changes.
  *
- * Adds a default deep_notify signal callback to an
+ * A default deep_notify signal callback for an element. The user data 
- * element. The user data should contain a pointer to an array of
+ * should contain a pointer to an array of strings that should be excluded 
- * strings that should be excluded from the notify.
+ * from the notify. The default handler will print the new value of the property 
- * The default handler will print the new value of the property 
  * using g_print.
  *
- * MT safe.
+ * MT safe. This function grabs and releases the object's LOCK or getting the
+ *          path string of the object.
  */
 void
 gst_object_default_deep_notify (GObject * object, GstObject * orig,
@@ -673,7 +674,7 @@ had_parent:
  *
  * Returns: the name of the object. g_free() after usage.
  *
- * MT safe.
+ * MT safe. This function grabs and releases the object's LOCK.
  */
 gchar *
 gst_object_get_name (GstObject * object)
@@ -722,7 +723,7 @@ gst_object_set_name_prefix (GstObject * object, const gchar * name_prefix)
  *
  * Returns: the name prefix of the object. g_free() after usage.
  *
- * MT safe.
+ * MT safe. This function grabs and releases the object's LOCK.
  */
 gchar *
 gst_object_get_name_prefix (GstObject * object)
@@ -746,15 +747,13 @@ gst_object_get_name_prefix (GstObject * object)
  * Sets the parent of @object. The object's reference count will be incremented,
  * and any floating reference will be removed (see gst_object_sink()).
  *
- * This function takes the object lock.
- *
  * Causes the parent-set signal to be emitted.
  *
  * Returns: TRUE if the parent could be set or FALSE when the object
  * already had a parent, the object and parent are the same or wrong
  * parameters were provided.
  *
- * MT safe.
+ * MT safe. Grabs and releases the object's LOCK.
  */
 gboolean
 gst_object_set_parent (GstObject * object, GstObject * parent)
@@ -782,9 +781,10 @@ gst_object_set_parent (GstObject * object, GstObject * parent)
   }
 
   g_signal_emit (G_OBJECT (object), gst_object_signals[PARENT_SET], 0, parent);
+
   return TRUE;
 
-  /* ERROR */
+  /* ERROR handling */
 had_parent:
   {
     GST_UNLOCK (object);
@@ -802,7 +802,7 @@ had_parent:
  * Returns: parent of the object, this can be NULL if the object has no
  *   parent. unref after usage.
  *
- * MT safe.
+ * MT safe. Grabs and releases the object's LOCK.
  */
 GstObject *
 gst_object_get_parent (GstObject * object)
@@ -828,7 +828,7 @@ gst_object_get_parent (GstObject * object)
  * This function decreases the refcount of the object so the object
  * might not point to valid memory anymore after calling this function.
  *
- * MT safe.
+ * MT safe. Grabs and releases the object's lock.
  */
 void
 gst_object_unparent (GstObject * object)
@@ -867,7 +867,7 @@ gst_object_unparent (GstObject * object)
  *
  * Returns: TRUE if the name does not appear in the list, FALSE if it does.
  *
- * MT safe.
+ * MT safe. Grabs and releases the LOCK of each object in the list.
  */
 gboolean
 gst_object_check_uniqueness (GList * list, const gchar * name)
@@ -1001,7 +1001,8 @@ gst_object_get_property (GObject * object, guint prop_id,
  * Returns: a string describing the path of the object. You must
  *          g_free() the string after usage. 
  *
- * MT safe.
+ * MT safe. Grabs and releases the object's LOCK for all objects
+ *          in the hierarchy.
  */
 gchar *
 gst_object_get_path_string (GstObject * object)

gst/gstobject.h

@@ -21,7 +21,6 @@
  * Boston, MA 02111-1307, USA.
  */
 
-
 #ifndef __GST_OBJECT_H__
 #define __GST_OBJECT_H__
 
@@ -79,6 +78,7 @@ typedef enum
 #define GST_FLAG_SET(obj,flag)          G_STMT_START{ (GST_FLAGS (obj) |= (1<<(flag))); }G_STMT_END
 #define GST_FLAG_UNSET(obj,flag)        G_STMT_START{ (GST_FLAGS (obj) &= ~(1<<(flag))); }G_STMT_END
 
+#define GST_OBJECT_IS_DISPOSING(obj)    (GST_FLAG_IS_SET (obj, GST_OBJECT_DISPOSING))
 #define GST_OBJECT_IS_DESTROYED(obj)    (GST_FLAG_IS_SET (obj, GST_OBJECT_DESTROYED))
 #define GST_OBJECT_IS_FLOATING(obj)     (GST_FLAG_IS_SET (obj, GST_OBJECT_FLOATING))
 
@@ -91,9 +91,9 @@ struct _GstObject {
   /*< public >*/ /* with LOCK */
   GMutex        *lock;        /* object LOCK */
   gchar         *name;        /* object name */
+  gchar         *name_prefix; /* used for debugging */
   GstObject     *parent;      /* this object's parent, weak ref */
   guint32        flags;
-  gchar         *name_prefix; /* used for debugging */
 
   /*< private >*/
   gpointer _gst_reserved[GST_PADDING];