Some more documentation.

Original commit message from CVS:
* docs/design/part-element-sink.txt:
* docs/design/part-element-source.txt:
* gst/base/gstbasesink.c: (gst_basesink_class_init),
(gst_basesink_event), (gst_basesink_activate):
* gst/base/gstbasesink.h:
* gst/base/gstbasesrc.c: (gst_basesrc_init), (gst_basesrc_unlock),
(gst_basesrc_activate):
* gst/base/gstbasesrc.h:
* gst/gstelement.c: (gst_element_pads_activate):
Some more documentation.
Fixed scheduling decision in _pads_activate().

ChangeLog

@@ -1,3 +1,17 @@
+2005-05-06  Wim Taymans  <wim@fluendo.com>
+
+	* docs/design/part-element-sink.txt:
+	* docs/design/part-element-source.txt:
+	* gst/base/gstbasesink.c: (gst_basesink_class_init),
+	(gst_basesink_event), (gst_basesink_activate):
+	* gst/base/gstbasesink.h:
+	* gst/base/gstbasesrc.c: (gst_basesrc_init), (gst_basesrc_unlock),
+	(gst_basesrc_activate):
+	* gst/base/gstbasesrc.h:
+	* gst/gstelement.c: (gst_element_pads_activate):
+	Some more documentation.
+	Fixed scheduling decision in _pads_activate().
+
 2005-05-05  Andy Wingo  <wingo@pobox.com>
 
 	* check/pipelines/simple_launch_lines.c (test_2_elements): "Fix"

docs/design/part-element-sink.txt

@@ -0,0 +1,15 @@
+Sink elements
+-------------
+
+Sink elements consume data. They normally have no source pads.
+
+typical sink elements include:
+
+ - audio/video renderers
+ - network sinks
+ - filesinks
+
+Sinks are harder to construct than other element types as they are 
+treated specially by the GStreamer core.
+
+

docs/design/part-element-source.txt

@@ -0,0 +1,67 @@
+Source elements
+---------------
+
+A source element is an element that provides data to the pipeline. It
+does typically not have any sink (input) pads.
+
+Typical source elements include:
+
+ - file readers
+ - network elements
+ - capture elements (video/audio/...)
+ - generators (signals/video/audio/...)
+
+A source element can operate in three ways:
+
+  - it is fully seekable, this means that random access can be performed
+    on it in an efficient way. (a file reader,...). This also typically
+    means that the source is not live.
+
+  - data can be obtained from it with a variable size. This means that
+    the source can give N bytes of data. An example is an audio source.
+    A video source always provides the same amount of data (one video 
+    frame). Note that this is not a fully seekable source.
+
+  - it is a live source, this means that data arrives when it is ready.
+    An example of this is a video or network source.
+
+When writing a source, one has to look at how the source can operate to
+decide on the scheduling methods to implement on the source.
+
+  - fully seekable sources implement a getrange function on the source pad.
+
+  - sources that can give N bytes but cannot do seeking also  implement a
+    getrange function but state that they cannot do random access.
+
+  - sources that are purely live sources implement a task to push out
+    data.
+
+Any source that has a getrange function must also implement a push based
+scheduling mode. In this mode the source starts a task that gets N bytes
+and pushes them out. Whenever possible, the peer element will select the
+getrange based scheduling method of the source, though.
+
+A source with a getrange function must activate itself in the pad activate
+function. This is needed because the downstream peer element will decide
+and activate the source element in its state change function before the
+source's state change function is called.
+
+
+Source base classes
+-------------------
+
+GstBaseSource:
+
+  This base class provides an implementation of a random access source and
+  is very well suited for file reader like sources.
+
+
+
+
+
+
+
+
+
+
+

gst/base/gstbasesink.c

@@ -44,6 +44,7 @@ enum
   LAST_SIGNAL
 };
 
+/* FIXME, need to figure out a better way to handle the pull mode */
 #define DEFAULT_SIZE 1024
 #define DEFAULT_HAS_LOOP FALSE
 #define DEFAULT_HAS_CHAIN TRUE
@@ -140,6 +141,8 @@ gst_basesink_class_init (GstBaseSinkClass * klass)
       g_param_spec_boolean ("has-chain", "has-chain",
           "Enable chain-based operation", DEFAULT_HAS_CHAIN,
           G_PARAM_READWRITE | G_PARAM_CONSTRUCT));
+  /* FIXME, this next value should be configured using an event from the
+   * upstream element */
   g_object_class_install_property (G_OBJECT_CLASS (klass),
       PROP_PREROLL_QUEUE_LEN,
       g_param_spec_uint ("preroll-queue-len", "preroll-queue-len",
@@ -502,6 +505,7 @@ gst_basesink_event (GstPad * pad, GstEvent * event)
 
       GST_STREAM_LOCK (pad);
 
+      /* EOS also finishes the preroll */
       gst_basesink_finish_preroll (basesink, pad, NULL);
 
       GST_LOCK (basesink);
@@ -743,8 +747,10 @@ gst_basesink_activate (GstPad * pad, GstActivateMode mode)
 {
   gboolean result = FALSE;
   GstBaseSink *basesink;
+  GstBaseSinkClass *bclass;
 
   basesink = GST_BASESINK (GST_OBJECT_PARENT (pad));
+  bclass = GST_BASESINK_GET_CLASS (basesink);
 
   switch (mode) {
     case GST_ACTIVATE_PUSH:
@@ -774,6 +780,10 @@ gst_basesink_activate (GstPad * pad, GstActivateMode mode)
       }
       GST_UNLOCK (basesink);
 
+      /* unlock any subclasses */
+      if (bclass->unlock)
+        bclass->unlock (basesink);
+
       /* unlock preroll */
       GST_PREROLL_LOCK (pad);
       GST_PREROLL_SIGNAL (pad);

gst/base/gstbasesink.h

@@ -41,14 +41,19 @@ G_BEGIN_DECLS
 typedef struct _GstBaseSink GstBaseSink;
 typedef struct _GstBaseSinkClass GstBaseSinkClass;
 
+/* a base class for implementing chain based sinks
+ *
+ * Preroll, EOS, state changes are all handled. 
+ */
 struct _GstBaseSink {
   GstElement 	 element;
 
   GstPad 	*sinkpad;
   GstActivateMode	pad_mode;
 
-  GQueue	*preroll_queue; /* with PREROLL_LOCK */
+  /*< protected >*/ /* with PREROLL_LOCK */
-  gint		 preroll_queue_max_len; /* with PREROLL_LOCK */
+  GQueue	*preroll_queue;
+  gint		 preroll_queue_max_len;
 
   guint64	 offset;
   gboolean 	 has_loop;
@@ -66,15 +71,24 @@ struct _GstBaseSink {
 struct _GstBaseSinkClass {
   GstElementClass parent_class;
 
+  /* get caps from subclass */
   GstCaps*      (*get_caps)     (GstBaseSink *sink);
+  /* notify subclass of new caps */
   gboolean      (*set_caps)     (GstBaseSink *sink, GstCaps *caps);
 
+  /* allocate a new buffer with given caps */
   GstBuffer*    (*buffer_alloc) (GstBaseSink *sink, guint64 offset, guint size,
 		                 GstCaps *caps);
 
+  /* get the start and end times for syncing on this buffer */
   void		(*get_times)    (GstBaseSink *sink, GstBuffer *buffer, 
 		                 GstClockTime *start, GstClockTime *end);
 
+  /* unlock any pending access to the resource. subclasses should unlock
+   * any function ASAP. */
+  gboolean      (*unlock)       (GstBaseSink *sink);
+
+  /* notify subclass of event, preroll buffer or real buffer */
   gboolean      (*event)        (GstBaseSink *sink, GstEvent *event);
   GstFlowReturn (*preroll)      (GstBaseSink *sink, GstBuffer *buffer);
   GstFlowReturn (*render)       (GstBaseSink *sink, GstBuffer *buffer);

gst/base/gstbasesrc.c

@@ -171,6 +171,7 @@ gst_basesrc_init (GstBaseSrc * basesrc, gpointer g_class)
   basesrc->segment_start = -1;
   basesrc->segment_end = -1;
   basesrc->blocksize = DEFAULT_BLOCKSIZE;
+  basesrc->clock_id = NULL;
 
   GST_FLAG_UNSET (basesrc, GST_BASESRC_STARTED);
 }
@@ -598,10 +599,18 @@ gst_basesrc_unlock (GstBaseSrc * basesrc)
   GstBaseSrcClass *bclass;
   gboolean result = FALSE;
 
+  /* unblock whatever the subclass is doing */
   bclass = GST_BASESRC_GET_CLASS (basesrc);
   if (bclass->unlock)
     result = bclass->unlock (basesrc);
 
+  /* and unblock the clock as well, if any */
+  GST_LOCK (basesrc);
+  if (basesrc->clock_id) {
+    gst_clock_id_unschedule (basesrc->clock_id);
+  }
+  GST_UNLOCK (basesrc);
+
   return result;
 }
 
@@ -747,6 +756,7 @@ gst_basesrc_activate (GstPad * pad, GstActivateMode mode)
       break;
     case GST_ACTIVATE_NONE:
       /* step 1, unblock clock sync (if any) */
+      gst_basesrc_unlock (basesrc);
 
       /* step 2, make sure streaming finishes */
       GST_STREAM_LOCK (pad);

gst/base/gstbasesrc.h

@@ -41,7 +41,14 @@ typedef enum {
   GST_BASESRC_FLAG_LAST         = GST_ELEMENT_FLAG_LAST + 2
 } GstFileSrcFlags;
 
-
+/* base class for random access sources
+ *
+ * This class is mostly usefull for elements that do byte based
+ * access to a random access resource, like files.
+ *
+ * Seeking, flushing, scheduling and sync is all handled by this
+ * base class.
+ */
 typedef struct _GstBaseSrc GstBaseSrc;
 typedef struct _GstBaseSrcClass GstBaseSrcClass;
 
@@ -50,38 +57,54 @@ struct _GstBaseSrc {
 
   GstPad 	*srcpad;
 
-  gint 		 blocksize;
+  /*< protected >*/ /* with LOCK */
+  gint 		 blocksize;		/* size of buffers when operating push based */
+  gboolean	 has_loop;		/* some scheduling properties */
+  gboolean	 has_getrange;
+  gboolean       seekable;
+  gboolean       random_access;
 
-  gint64	 segment_start;
+  GstClockID     clock_id;		/* for syncing */
+  GstClockTime   end_time;
+
+  /* with STREAM_LOCK */
+  gint64	 segment_start;		/* start and end positions for seeking */
   gint64	 segment_end;
   gboolean	 segment_loop;
 
-  gboolean	 has_loop;
+  guint64 	 offset;		/* current offset in the resource */
-  gboolean	 has_getrange;
+  guint64        size;			/* total size of the resource */
-
-  gboolean       seekable;
-  guint64 	 offset;
-  guint64        size;
 };
 
 struct _GstBaseSrcClass {
   GstElementClass parent_class;
 
+  /* get caps from subclass */
   GstCaps*      (*get_caps)     (GstBaseSrc *src);
+  /* notify the subclass of new caps */
   gboolean      (*set_caps)     (GstBaseSrc *src, GstCaps *caps);
 
+  /* start and stop processing, ideal for opening/closing the resource */
   gboolean      (*start)        (GstBaseSrc *src);
   gboolean      (*stop)         (GstBaseSrc *src);
 
+  /* given a buffer, return start and stop time when it should be pushed
+   * out. The base class will sync on the clock using these times. */
   void          (*get_times)    (GstBaseSrc *src, GstBuffer *buffer,
                                  GstClockTime *start, GstClockTime *end);
 
+  /* get the total size of the resource in bytes */
   gboolean      (*get_size)     (GstBaseSrc *src, guint64 *size);
 
+  /* check if the resource is seekable */
   gboolean      (*is_seekable)  (GstBaseSrc *src);
+  /* unlock any pending access to the resource. subclasses should unlock
+   * any function ASAP. */
   gboolean      (*unlock)       (GstBaseSrc *src);
 
+  /* notify subclasses of an event */
   gboolean      (*event)        (GstBaseSrc *src, GstEvent *event);
+  /* ask the subclass to create a buffer */
   GstFlowReturn (*create)       (GstBaseSrc *src, guint64 offset, guint size, 
 		                 GstBuffer **buf);
 };

gst/gstelement.c

@@ -2018,8 +2018,8 @@ restart:
         /* If the pad is a sink with loop and the peer has a get function,
          * we can activate the sinkpad,  FIXME, logic is reversed as
          * check_pull_range() checks the peer of the given pad. */
-        if ((GST_PAD_IS_SINK (pad) && pad_get && peer_loop) ||
+        if ((GST_PAD_IS_SINK (pad) && pad_get && pad_loop) ||
-            (GST_PAD_IS_SRC (pad) && peer_get && pad_loop)) {
+            (GST_PAD_IS_SRC (pad) && peer_get && peer_loop)) {
           GST_CAT_DEBUG_OBJECT (GST_CAT_STATES, element,
               "%sactivating pad %s pull mode", (active ? "" : "(de)"),
               GST_OBJECT_NAME (pad));

libs/gst/base/gstbasesink.c

@@ -44,6 +44,7 @@ enum
   LAST_SIGNAL
 };
 
+/* FIXME, need to figure out a better way to handle the pull mode */
 #define DEFAULT_SIZE 1024
 #define DEFAULT_HAS_LOOP FALSE
 #define DEFAULT_HAS_CHAIN TRUE
@@ -140,6 +141,8 @@ gst_basesink_class_init (GstBaseSinkClass * klass)
       g_param_spec_boolean ("has-chain", "has-chain",
           "Enable chain-based operation", DEFAULT_HAS_CHAIN,
           G_PARAM_READWRITE | G_PARAM_CONSTRUCT));
+  /* FIXME, this next value should be configured using an event from the
+   * upstream element */
   g_object_class_install_property (G_OBJECT_CLASS (klass),
       PROP_PREROLL_QUEUE_LEN,
       g_param_spec_uint ("preroll-queue-len", "preroll-queue-len",
@@ -502,6 +505,7 @@ gst_basesink_event (GstPad * pad, GstEvent * event)
 
       GST_STREAM_LOCK (pad);
 
+      /* EOS also finishes the preroll */
       gst_basesink_finish_preroll (basesink, pad, NULL);
 
       GST_LOCK (basesink);
@@ -743,8 +747,10 @@ gst_basesink_activate (GstPad * pad, GstActivateMode mode)
 {
   gboolean result = FALSE;
   GstBaseSink *basesink;
+  GstBaseSinkClass *bclass;
 
   basesink = GST_BASESINK (GST_OBJECT_PARENT (pad));
+  bclass = GST_BASESINK_GET_CLASS (basesink);
 
   switch (mode) {
     case GST_ACTIVATE_PUSH:
@@ -774,6 +780,10 @@ gst_basesink_activate (GstPad * pad, GstActivateMode mode)
       }
       GST_UNLOCK (basesink);
 
+      /* unlock any subclasses */
+      if (bclass->unlock)
+        bclass->unlock (basesink);
+
       /* unlock preroll */
       GST_PREROLL_LOCK (pad);
       GST_PREROLL_SIGNAL (pad);

libs/gst/base/gstbasesink.h

@@ -41,14 +41,19 @@ G_BEGIN_DECLS
 typedef struct _GstBaseSink GstBaseSink;
 typedef struct _GstBaseSinkClass GstBaseSinkClass;
 
+/* a base class for implementing chain based sinks
+ *
+ * Preroll, EOS, state changes are all handled. 
+ */
 struct _GstBaseSink {
   GstElement 	 element;
 
   GstPad 	*sinkpad;
   GstActivateMode	pad_mode;
 
-  GQueue	*preroll_queue; /* with PREROLL_LOCK */
+  /*< protected >*/ /* with PREROLL_LOCK */
-  gint		 preroll_queue_max_len; /* with PREROLL_LOCK */
+  GQueue	*preroll_queue;
+  gint		 preroll_queue_max_len;
 
   guint64	 offset;
   gboolean 	 has_loop;
@@ -66,15 +71,24 @@ struct _GstBaseSink {
 struct _GstBaseSinkClass {
   GstElementClass parent_class;
 
+  /* get caps from subclass */
   GstCaps*      (*get_caps)     (GstBaseSink *sink);
+  /* notify subclass of new caps */
   gboolean      (*set_caps)     (GstBaseSink *sink, GstCaps *caps);
 
+  /* allocate a new buffer with given caps */
   GstBuffer*    (*buffer_alloc) (GstBaseSink *sink, guint64 offset, guint size,
 		                 GstCaps *caps);
 
+  /* get the start and end times for syncing on this buffer */
   void		(*get_times)    (GstBaseSink *sink, GstBuffer *buffer, 
 		                 GstClockTime *start, GstClockTime *end);
 
+  /* unlock any pending access to the resource. subclasses should unlock
+   * any function ASAP. */
+  gboolean      (*unlock)       (GstBaseSink *sink);
+
+  /* notify subclass of event, preroll buffer or real buffer */
   gboolean      (*event)        (GstBaseSink *sink, GstEvent *event);
   GstFlowReturn (*preroll)      (GstBaseSink *sink, GstBuffer *buffer);
   GstFlowReturn (*render)       (GstBaseSink *sink, GstBuffer *buffer);

libs/gst/base/gstbasesrc.c

@@ -171,6 +171,7 @@ gst_basesrc_init (GstBaseSrc * basesrc, gpointer g_class)
   basesrc->segment_start = -1;
   basesrc->segment_end = -1;
   basesrc->blocksize = DEFAULT_BLOCKSIZE;
+  basesrc->clock_id = NULL;
 
   GST_FLAG_UNSET (basesrc, GST_BASESRC_STARTED);
 }
@@ -598,10 +599,18 @@ gst_basesrc_unlock (GstBaseSrc * basesrc)
   GstBaseSrcClass *bclass;
   gboolean result = FALSE;
 
+  /* unblock whatever the subclass is doing */
   bclass = GST_BASESRC_GET_CLASS (basesrc);
   if (bclass->unlock)
     result = bclass->unlock (basesrc);
 
+  /* and unblock the clock as well, if any */
+  GST_LOCK (basesrc);
+  if (basesrc->clock_id) {
+    gst_clock_id_unschedule (basesrc->clock_id);
+  }
+  GST_UNLOCK (basesrc);
+
   return result;
 }
 
@@ -747,6 +756,7 @@ gst_basesrc_activate (GstPad * pad, GstActivateMode mode)
       break;
     case GST_ACTIVATE_NONE:
       /* step 1, unblock clock sync (if any) */
+      gst_basesrc_unlock (basesrc);
 
       /* step 2, make sure streaming finishes */
       GST_STREAM_LOCK (pad);

libs/gst/base/gstbasesrc.h

@@ -41,7 +41,14 @@ typedef enum {
   GST_BASESRC_FLAG_LAST         = GST_ELEMENT_FLAG_LAST + 2
 } GstFileSrcFlags;
 
-
+/* base class for random access sources
+ *
+ * This class is mostly usefull for elements that do byte based
+ * access to a random access resource, like files.
+ *
+ * Seeking, flushing, scheduling and sync is all handled by this
+ * base class.
+ */
 typedef struct _GstBaseSrc GstBaseSrc;
 typedef struct _GstBaseSrcClass GstBaseSrcClass;
 
@@ -50,38 +57,54 @@ struct _GstBaseSrc {
 
   GstPad 	*srcpad;
 
-  gint 		 blocksize;
+  /*< protected >*/ /* with LOCK */
+  gint 		 blocksize;		/* size of buffers when operating push based */
+  gboolean	 has_loop;		/* some scheduling properties */
+  gboolean	 has_getrange;
+  gboolean       seekable;
+  gboolean       random_access;
 
-  gint64	 segment_start;
+  GstClockID     clock_id;		/* for syncing */
+  GstClockTime   end_time;
+
+  /* with STREAM_LOCK */
+  gint64	 segment_start;		/* start and end positions for seeking */
   gint64	 segment_end;
   gboolean	 segment_loop;
 
-  gboolean	 has_loop;
+  guint64 	 offset;		/* current offset in the resource */
-  gboolean	 has_getrange;
+  guint64        size;			/* total size of the resource */
-
-  gboolean       seekable;
-  guint64 	 offset;
-  guint64        size;
 };
 
 struct _GstBaseSrcClass {
   GstElementClass parent_class;
 
+  /* get caps from subclass */
   GstCaps*      (*get_caps)     (GstBaseSrc *src);
+  /* notify the subclass of new caps */
   gboolean      (*set_caps)     (GstBaseSrc *src, GstCaps *caps);
 
+  /* start and stop processing, ideal for opening/closing the resource */
   gboolean      (*start)        (GstBaseSrc *src);
   gboolean      (*stop)         (GstBaseSrc *src);
 
+  /* given a buffer, return start and stop time when it should be pushed
+   * out. The base class will sync on the clock using these times. */
   void          (*get_times)    (GstBaseSrc *src, GstBuffer *buffer,
                                  GstClockTime *start, GstClockTime *end);
 
+  /* get the total size of the resource in bytes */
   gboolean      (*get_size)     (GstBaseSrc *src, guint64 *size);
 
+  /* check if the resource is seekable */
   gboolean      (*is_seekable)  (GstBaseSrc *src);
+  /* unlock any pending access to the resource. subclasses should unlock
+   * any function ASAP. */
   gboolean      (*unlock)       (GstBaseSrc *src);
 
+  /* notify subclasses of an event */
   gboolean      (*event)        (GstBaseSrc *src, GstEvent *event);
+  /* ask the subclass to create a buffer */
   GstFlowReturn (*create)       (GstBaseSrc *src, guint64 offset, guint size, 
 		                 GstBuffer **buf);
 };