gst/gstelement.c: Don't output the same debug statement twice.
authorJan Schmidt <thaytan@mad.scientist.com>
Fri, 13 Apr 2007 11:53:00 +0000 (11:53 +0000)
committerJan Schmidt <thaytan@mad.scientist.com>
Fri, 13 Apr 2007 11:53:00 +0000 (11:53 +0000)
Original commit message from CVS:
* gst/gstelement.c: (gst_element_get_state_func):
Don't output the same debug statement twice.
* libs/gst/base/gstadapter.c: (gst_adapter_try_to_merge_up),
(gst_adapter_peek), (gst_adapter_take_buffer):
Optimise the case where we have buffers at the head of the queue that
can be joined quickly (because they're contiguous sub-buffers) by
merging them together rather than copying data out into new memory.
* gst/parse/grammar.y:
* tests/check/pipelines/parse-launch.c:
Fix a leak in an error path for parse_launch, and add a check
for it to the testsuite.

ChangeLog
gst/gstelement.c
gst/parse/grammar.y
libs/gst/base/gstadapter.c
tests/check/pipelines/parse-launch.c

index fe287fa..67ed0a8 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,21 @@
 2007-04-13  Jan Schmidt  <thaytan@mad.scientist.com>
 
+       * gst/gstelement.c: (gst_element_get_state_func):
+       Don't output the same debug statement twice.
+
+       * libs/gst/base/gstadapter.c: (gst_adapter_try_to_merge_up),
+       (gst_adapter_peek), (gst_adapter_take_buffer):
+       Optimise the case where we have buffers at the head of the queue that
+       can be joined quickly (because they're contiguous sub-buffers) by
+       merging them together rather than copying data out into new memory.
+
+       * gst/parse/grammar.y:
+       * tests/check/pipelines/parse-launch.c:
+       Fix a leak in an error path for parse_launch, and add a check 
+       for it to the testsuite.
+
+2007-04-13  Jan Schmidt  <thaytan@mad.scientist.com>
+
        * plugins/elements/gstmultiqueue.c: (gst_multi_queue_release_pad):
          Don't deadlock when releasing a pad - gst_pad_set_active may try
          and take the multiqueue lock too.
index 9392ff3..a4fca18 100644 (file)
@@ -1804,9 +1804,6 @@ gst_element_get_state_func (GstElement * element,
   GST_CAT_DEBUG_OBJECT (GST_CAT_STATES, element, "RETURN is %s",
       gst_element_state_change_return_get_name (ret));
 
-  GST_CAT_DEBUG_OBJECT (GST_CAT_STATES, element, "RETURN is %s",
-      gst_element_state_change_return_get_name (ret));
-
   /* we got an error, report immediatly */
   if (ret == GST_STATE_CHANGE_FAILURE)
     goto done;
index 0b16b81..d6d1a7c 100644 (file)
@@ -196,7 +196,7 @@ YYPRINTF(const char *format, ...)
 
 #endif /* GST_DISABLE_GST_DEBUG */
 
-#define GST_BIN_MAKE(res, type, chainval, assign) \
+#define GST_BIN_MAKE(res, type, chainval, assign, free_string) \
 G_STMT_START { \
   chain_t *chain = chainval; \
   GSList *walk; \
@@ -206,6 +206,9 @@ G_STMT_START { \
         _("specified empty bin \"%s\", not allowed"), type); \
     g_slist_foreach (assign, (GFunc) gst_parse_strfree, NULL); \
     g_slist_free (assign); \
+    gst_object_unref (bin); \
+    if (free_string) \
+      gst_parse_strfree (type); /* Need to clean up the string */ \
     YYERROR; \
   } else if (!bin) { \
     SET_ERROR (((graph_t *) graph)->error, GST_PARSE_ERROR_NO_SUCH_ELEMENT, \
@@ -504,14 +507,14 @@ element:  IDENTIFIER                    { $$ = gst_element_factory_make ($1, NULL);
 assignments:   /* NOP */                     { $$ = NULL; }
        |       assignments ASSIGNMENT        { $$ = g_slist_prepend ($1, $2); }
        ;               
-bin:           '(' assignments chain ')' { GST_BIN_MAKE ($$, "bin", $3, $2); }
-        |       BINREF assignments chain ')'  { GST_BIN_MAKE ($$, $1, $3, $2); 
+bin:           '(' assignments chain ')' { GST_BIN_MAKE ($$, "bin", $3, $2, FALSE); }
+        |       BINREF assignments chain ')'  { GST_BIN_MAKE ($$, $1, $3, $2, TRUE); 
                                                gst_parse_strfree ($1);
                                              }
-        |       BINREF assignments ')'       { GST_BIN_MAKE ($$, $1, NULL, $2); 
+        |       BINREF assignments ')'       { GST_BIN_MAKE ($$, $1, NULL, $2, TRUE); 
                                                gst_parse_strfree ($1);
                                              }
-        |       BINREF assignments error ')'  { GST_BIN_MAKE ($$, $1, NULL, $2); 
+        |       BINREF assignments error ')'  { GST_BIN_MAKE ($$, $1, NULL, $2, TRUE); 
                                                gst_parse_strfree ($1);
                                              }
        ;
index cf4597c..144db20 100644 (file)
@@ -240,6 +240,50 @@ gst_adapter_peek_into (GstAdapter * adapter, guint8 * data, guint size)
   }
 }
 
+/* Internal method only. Tries to merge buffers at the head of the queue
+ * to form a single larger buffer of size 'size'. Only merges buffers that
+ * where 'gst_buffer_is_span_fast' returns TRUE.
+ *
+ * Returns TRUE if it managed to merge anything.
+ */
+static gboolean
+gst_adapter_try_to_merge_up (GstAdapter * adapter, guint size)
+{
+  GstBuffer *cur, *head;
+  GSList *g;
+
+  g = adapter->buflist;
+  if (g == NULL)
+    return FALSE;
+
+  head = g->data;
+  g = g_slist_next (g);
+
+  /* How large do we want our head buffer? The requested size, plus whatever's
+   * been skipped already */
+  size += adapter->skip;
+
+  while (g != NULL && GST_BUFFER_SIZE (head) < size) {
+    cur = g->data;
+    if (!gst_buffer_is_span_fast (head, cur))
+      return TRUE;
+
+    /* Merge the head buffer and the next in line */
+    GST_LOG_OBJECT (adapter,
+        "Merging buffers of size %u & %u in search of target %u",
+        GST_BUFFER_SIZE (head), GST_BUFFER_SIZE (cur), size);
+
+    head = gst_buffer_join (head, cur);
+
+    /* Delete the front list item, and store our new buffer in the 2nd list 
+     * item */
+    adapter->buflist = g_slist_delete_link (adapter->buflist, adapter->buflist);
+    g->data = head;
+  }
+
+  return FALSE;
+}
+
 /**
  * gst_adapter_peek:
  * @adapter: a #GstAdapter
@@ -284,10 +328,16 @@ gst_adapter_peek (GstAdapter * adapter, guint size)
   if (GST_BUFFER_SIZE (cur) >= size + adapter->skip)
     return GST_BUFFER_DATA (cur) + adapter->skip;
 
-  /* FIXME, optimize further - we may be able to efficiently
-   * merge buffers in our pool to gather a big enough chunk to 
-   * return it from the head buffer directly */
+  /* We may be able to efficiently merge buffers in our pool to 
+   * gather a big enough chunk to return it from the head buffer directly */
+  if (gst_adapter_try_to_merge_up (adapter, size)) {
+    /* Merged something! Check if there's enough avail now */
+    cur = adapter->buflist->data;
+    if (GST_BUFFER_SIZE (cur) >= size + adapter->skip)
+      return GST_BUFFER_DATA (cur) + adapter->skip;
+  }
 
+  /* Gonna need to copy stuff out */
   if (adapter->assembled_size < size) {
     adapter->assembled_size = (size / DEFAULT_SIZE + 1) * DEFAULT_SIZE;
     GST_DEBUG_OBJECT (adapter, "setting size of internal buffer to %u",
@@ -465,7 +515,7 @@ gst_adapter_take_buffer (GstAdapter * adapter, guint nbytes)
   GST_LOG_OBJECT (adapter, "taking buffer of %u bytes", nbytes);
 
   /* we don't have enough data, return NULL. This is unlikely
-   * as one usually does an _available() first instead of peeking a
+   * as one usually does an _available() first instead of grabbing a
    * random size. */
   if (G_UNLIKELY (nbytes > adapter->size))
     return NULL;
@@ -482,6 +532,20 @@ gst_adapter_take_buffer (GstAdapter * adapter, guint nbytes)
     return buffer;
   }
 
+  if (gst_adapter_try_to_merge_up (adapter, nbytes)) {
+    /* Merged something, let's try again for sub-buffering */
+    cur = adapter->buflist->data;
+    if (GST_BUFFER_SIZE (cur) >= nbytes + adapter->skip) {
+      GST_LOG_OBJECT (adapter, "providing buffer of %d bytes via sub-buffer",
+          nbytes);
+      buffer = gst_buffer_create_sub (cur, adapter->skip, nbytes);
+
+      gst_adapter_flush (adapter, nbytes);
+
+      return buffer;
+    }
+  }
+
   buffer = gst_buffer_new_and_alloc (nbytes);
 
   /* we have enough assembled data, copy from there */
index 4f1a19e..c27ecf0 100644 (file)
@@ -295,6 +295,10 @@ static const gchar *expected_failures[] = {
   "fakesink silent=true ! fakesink silent=true",
   /* checks multi-chain link without src element fails. */
   "! identity silent=true ! identity silent=true ! fakesink silent=true",
+  /* Empty bin not allowed */
+  "bin.( )",
+  /* bin with non-existent element counts as empty, and not allowed */
+  "bin.( non_existent_element )",
   /* END: */
   NULL
 };