buffer: unify buffer merge methods

Add gst_buffer_append() which appends the memory blocks from one buffer to
another. Remove the old inefficient _merge() and _join() methods which forced a
premature memcpy in most cases.
Remove the _is_span() and _span() methods they are not needed anymore now that
we can _append(). Merging and spanning will be delayed until mapping or maybe
not at all when the element can deal with the different memory blocks.

docs/gst/gstreamer-sections.txt

@@ -202,9 +202,6 @@ gst_buffer_remove_all_memory
 gst_buffer_replace_all_memory
 gst_buffer_replace_memory
 
-gst_buffer_join
-gst_buffer_merge
-
 gst_buffer_map
 gst_buffer_unmap
 
@@ -223,8 +220,7 @@ gst_buffer_is_writable
 gst_buffer_make_writable
 gst_buffer_replace
 
-gst_buffer_is_span_fast
+gst_buffer_append
-gst_buffer_span
 
 gst_buffer_get_meta
 gst_buffer_add_meta

docs/random/porting-to-0.11.txt

@@ -248,6 +248,20 @@ The 0.11 porting guide
     Likewise GST_BUFFER_TIMESTAMP_IS_VALID() was changed to
     GST_BUFFER_PTS_IS_VALID and GST_BUFFER_DTS_IS_VALID
 
+    gst_buffer_join() was renamed to gst_buffer_append() and the memory is not
+    directly merged but appended.
+
+    gst_buffer_merge() was removed, it is the same as gst_buffer_join() but
+    without taking ownership of the arguments. Caller code should ref themselves
+    when needed. Note that the extra refs might force slower paths in
+    gst_buffer_join().
+
+    gst_buffer_is_span() and gst_buffer_span() are removed, use
+    gst_buffer_merge() and gst_buffer_resize() for the same effect. Merging and
+    spanning is delayed until the buffer is mapped and in some cases no merging
+    of memory is needed at all when the element can deal with individual memory
+    chunks.
+
 * GstBufferList
     The GstBufferList object is much simplified because most of the
     functionality in the groups is now part of the GstMemory in buffers.

gst/gstbuffer.c

@@ -88,8 +88,8 @@
  * GST_BUFFER_FLAG_IS_SET() to test if a certain #GstBufferFlag is set.
  *
  * Buffers can be efficiently merged into a larger buffer with
- * gst_buffer_span(), which avoids memory copies when the gst_buffer_is_span_fast()
+ * gst_buffer_append(). Copying of memory will only be done when absolutely
- * function returns TRUE.
+ * needed.
  *
  * An element should either unref the buffer or push it out on a src pad
  * using gst_pad_push() (see #GstPad).
@@ -1311,7 +1311,7 @@ _gst_buffer_arr_is_span_fast (GstMemory ** mem[], gsize len[], guint n,
       mcur = cmem[i];
 
       if (mprv && mcur) {
-        /* check is memory is contiguous */
+        /* check if memory is contiguous */
         if (!gst_memory_is_span (mprv, mcur, &offs))
           return FALSE;
 
@@ -1385,114 +1385,50 @@ _gst_buffer_arr_span (GstMemory ** mem[], gsize len[], guint n, gsize offset,
 }
 
 /**
- * gst_buffer_is_span_fast:
+ * gst_buffer_append:
- * @buf1: the first #GstBuffer.
+ * @buf1: (transfer full): the first source #GstBuffer to append.
- * @buf2: the second #GstBuffer.
+ * @buf2: (transfer full): the second source #GstBuffer to append.
  *
- * Determines whether a gst_buffer_span() can be done without copying
+ * Append all the memory from @buf2 to @buf1. The result buffer will contain a
- * the contents, that is, whether the data areas are contiguous sub-buffers of
+ * concatenation of the memory of @buf1 and @buf2.
- * the same buffer.
  *
- * MT safe.
+ * Returns: (transfer full): the new #GstBuffer that contains the memory
- * Returns: TRUE if the buffers are contiguous,
+ *     of the two source buffers.
- * FALSE if a copy would be required.
- */
-gboolean
-gst_buffer_is_span_fast (GstBuffer * buf1, GstBuffer * buf2)
-{
-  GstMemory **mem[2];
-  gsize len[2];
-
-  g_return_val_if_fail (GST_IS_BUFFER (buf1), FALSE);
-  g_return_val_if_fail (GST_IS_BUFFER (buf2), FALSE);
-  g_return_val_if_fail (buf1->mini_object.refcount > 0, FALSE);
-  g_return_val_if_fail (buf2->mini_object.refcount > 0, FALSE);
-
-  mem[0] = GST_BUFFER_MEM_ARRAY (buf1);
-  len[0] = GST_BUFFER_MEM_LEN (buf1);
-  mem[1] = GST_BUFFER_MEM_ARRAY (buf2);
-  len[1] = GST_BUFFER_MEM_LEN (buf2);
-
-  return _gst_buffer_arr_is_span_fast (mem, len, 2, NULL, NULL);
-}
-
-/**
- * gst_buffer_span:
- * @buf1: the first source #GstBuffer to merge.
- * @offset: the offset in the first buffer from where the new
- * buffer should start.
- * @buf2: the second source #GstBuffer to merge.
- * @size: the total size of the new buffer.
- *
- * Creates a new buffer that consists of part of buf1 and buf2.
- * Logically, buf1 and buf2 are concatenated into a single larger
- * buffer, and a new buffer is created at the given offset inside
- * this space, with a given length.
- *
- * If the two source buffers are children of the same larger buffer,
- * and are contiguous, the new buffer will be a child of the shared
- * parent, and thus no copying is necessary. you can use
- * gst_buffer_is_span_fast() to determine if a memcpy will be needed.
- *
- * MT safe.
- *
- * Returns: (transfer full): the new #GstBuffer that spans the two source
- *     buffers, or NULL if the arguments are invalid.
  */
 GstBuffer *
-gst_buffer_span (GstBuffer * buf1, gsize offset, GstBuffer * buf2, gsize size)
+gst_buffer_append (GstBuffer * buf1, GstBuffer * buf2)
 {
-  GstBuffer *newbuf;
+  gsize i, len;
-  GstMemory *span;
-  GstMemory **mem[2];
-  gsize len[2], size1, size2;
 
   g_return_val_if_fail (GST_IS_BUFFER (buf1), NULL);
   g_return_val_if_fail (GST_IS_BUFFER (buf2), NULL);
-  g_return_val_if_fail (buf1->mini_object.refcount > 0, NULL);
-  g_return_val_if_fail (buf2->mini_object.refcount > 0, NULL);
-  size1 = gst_buffer_get_size (buf1);
-  size2 = gst_buffer_get_size (buf2);
-  g_return_val_if_fail (size1 + size2 > offset, NULL);
-  if (size == -1)
-    size = size1 + size2 - offset;
-  else
-    g_return_val_if_fail (size <= size1 + size2 - offset, NULL);
 
-  mem[0] = GST_BUFFER_MEM_ARRAY (buf1);
+  buf1 = gst_buffer_make_writable (buf1);
-  len[0] = GST_BUFFER_MEM_LEN (buf1);
+  buf2 = gst_buffer_make_writable (buf2);
-  mem[1] = GST_BUFFER_MEM_ARRAY (buf2);
-  len[1] = GST_BUFFER_MEM_LEN (buf2);
 
-  span = _gst_buffer_arr_span (mem, len, 2, offset, size, FALSE);
+  len = GST_BUFFER_MEM_LEN (buf2);
+  for (i = 0; i < len; i++) {
+    GstMemory *mem;
 
-  newbuf = gst_buffer_new ();
+    mem = GST_BUFFER_MEM_PTR (buf2, i);
-  _memory_add (newbuf, -1, span);
+    GST_BUFFER_MEM_PTR (buf2, i) = NULL;
-
+    _memory_add (buf1, -1, mem);
-  /* if the offset is 0, the new buffer has the same timestamp as buf1 */
-  if (offset == 0) {
-    GST_BUFFER_OFFSET (newbuf) = GST_BUFFER_OFFSET (buf1);
-    GST_BUFFER_PTS (newbuf) = GST_BUFFER_PTS (buf1);
-    GST_BUFFER_DTS (newbuf) = GST_BUFFER_DTS (buf1);
-
-    /* if we completely merged the two buffers (appended), we can
-     * calculate the duration too. Also make sure we's not messing with
-     * invalid DURATIONS */
-    if (size1 + size2 == size) {
-      if (GST_BUFFER_DURATION_IS_VALID (buf1) &&
-          GST_BUFFER_DURATION_IS_VALID (buf2)) {
-        /* add duration */
-        GST_BUFFER_DURATION (newbuf) = GST_BUFFER_DURATION (buf1) +
-            GST_BUFFER_DURATION (buf2);
-      }
-      if (GST_BUFFER_OFFSET_END_IS_VALID (buf2)) {
-        /* add offset_end */
-        GST_BUFFER_OFFSET_END (newbuf) = GST_BUFFER_OFFSET_END (buf2);
-      }
-    }
   }
+  GST_BUFFER_MEM_LEN (buf2) = 0;
+  gst_buffer_unref (buf2);
 
-  return newbuf;
+  /* we can calculate the duration too. Also make sure we's not messing
+   * with invalid DURATIONS */
+  if (GST_BUFFER_DURATION_IS_VALID (buf1) &&
+      GST_BUFFER_DURATION_IS_VALID (buf2)) {
+    /* add duration */
+    GST_BUFFER_DURATION (buf1) += GST_BUFFER_DURATION (buf2);
+  }
+  if (GST_BUFFER_OFFSET_END_IS_VALID (buf2)) {
+    /* set offset_end */
+    GST_BUFFER_OFFSET_END (buf1) = GST_BUFFER_OFFSET_END (buf2);
+  }
+  return buf1;
 }
 
 /**

gst/gstbuffer.h

@@ -481,9 +481,8 @@ gst_buffer_replace (GstBuffer **obuf, GstBuffer *nbuf)
 GstBuffer*      gst_buffer_copy_region          (GstBuffer *parent, GstBufferCopyFlags flags,
                                                  gsize offset, gsize size);
 
-/* span, two buffers, intelligently */
+/* append two buffers */
-gboolean        gst_buffer_is_span_fast         (GstBuffer *buf1, GstBuffer *buf2);
+GstBuffer*      gst_buffer_append               (GstBuffer *buf1, GstBuffer *buf2);
-GstBuffer*      gst_buffer_span                 (GstBuffer *buf1, gsize offset, GstBuffer *buf2, gsize size) G_GNUC_MALLOC;
 
 /* metadata */
 #include <gst/gstmeta.h>

gst/gstutils.c

@@ -2506,74 +2506,6 @@ gst_bin_remove_many (GstBin * bin, GstElement * element_1, ...)
   va_end (args);
 }
 
-/**
- * gst_buffer_merge:
- * @buf1: (transfer none): the first source #GstBuffer to merge.
- * @buf2: (transfer none): the second source #GstBuffer to merge.
- *
- * Create a new buffer that is the concatenation of the two source
- * buffers.  The original source buffers will not be modified or
- * unref'd.  Make sure you unref the source buffers if they are not used
- * anymore afterwards.
- *
- * If the buffers point to contiguous areas of memory, the buffer
- * is created without copying the data.
- *
- * Free-function: gst_buffer_unref
- *
- * Returns: (transfer full): the new #GstBuffer which is the concatenation
- *     of the source buffers.
- */
-GstBuffer *
-gst_buffer_merge (GstBuffer * buf1, GstBuffer * buf2)
-{
-  GstBuffer *result;
-  gsize size1, size2;
-
-  size1 = gst_buffer_get_size (buf1);
-  size2 = gst_buffer_get_size (buf2);
-
-  /* we're just a specific case of the more general gst_buffer_span() */
-  result = gst_buffer_span (buf1, 0, buf2, size1 + size2);
-
-  return result;
-}
-
-/**
- * gst_buffer_join:
- * @buf1: (transfer full): the first source #GstBuffer.
- * @buf2: (transfer full): the second source #GstBuffer.
- *
- * Create a new buffer that is the concatenation of the two source
- * buffers, and unrefs the original source buffers.
- *
- * If the buffers point to contiguous areas of memory, the buffer
- * is created without copying the data.
- *
- * This is a convenience function for C programmers. See also
- * gst_buffer_merge(), which does the same thing without
- * unreffing the input parameters. Language bindings without
- * explicit reference counting should not wrap this function.
- *
- * Returns: (transfer full): the new #GstBuffer which is the concatenation of
- * the source buffers.
- */
-GstBuffer *
-gst_buffer_join (GstBuffer * buf1, GstBuffer * buf2)
-{
-  GstBuffer *result;
-  gsize size1, size2;
-
-  size1 = gst_buffer_get_size (buf1);
-  size2 = gst_buffer_get_size (buf2);
-
-  result = gst_buffer_span (buf1, 0, buf2, size1 + size2);
-  gst_buffer_unref (buf1);
-  gst_buffer_unref (buf2);
-
-  return result;
-}
-
 typedef struct
 {
   GstQuery *query;

gst/gstutils.h

@@ -913,10 +913,6 @@ void                    gst_bin_add_many                (GstBin *bin, GstElement
 void                    gst_bin_remove_many             (GstBin *bin, GstElement *element_1, ...) G_GNUC_NULL_TERMINATED;
 GstPad *                gst_bin_find_unlinked_pad       (GstBin *bin, GstPadDirection direction);
 
-/* buffer functions */
-GstBuffer *             gst_buffer_merge                (GstBuffer * buf1, GstBuffer * buf2);
-GstBuffer *             gst_buffer_join                 (GstBuffer * buf1, GstBuffer * buf2);
-
 /* parse utility functions */
 GstElement *            gst_parse_bin_from_description      (const gchar     * bin_description,
                                                              gboolean          ghost_unlinked_pads,

libs/gst/base/gstadapter.c

@@ -342,8 +342,7 @@ gst_adapter_push (GstAdapter * adapter, GstBuffer * buf)
 }
 
 /* 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
+ * to form a single larger buffer of size 'size'.
- * where 'gst_buffer_is_span_fast' returns TRUE.
  *
  * Returns TRUE if it managed to merge anything.
  */
@@ -369,15 +368,12 @@ gst_adapter_try_to_merge_up (GstAdapter * adapter, gsize size)
 
   while (g != NULL && hsize < size) {
     cur = g->data;
-    if (!gst_buffer_is_span_fast (head, cur))
-      return ret;
-
     /* Merge the head buffer and the next in line */
     GST_LOG_OBJECT (adapter, "Merging buffers of size %" G_GSIZE_FORMAT " & %"
         G_GSIZE_FORMAT " in search of target %" G_GSIZE_FORMAT,
         hsize, gst_buffer_get_size (cur), size);
 
-    head = gst_buffer_join (head, cur);
+    head = gst_buffer_append (head, cur);
     hsize = gst_buffer_get_size (head);
     ret = TRUE;
 

tests/check/gst/gstbuffer.c

@@ -115,35 +115,6 @@ GST_START_TEST (test_subbuffer)
 
 GST_END_TEST;
 
-GST_START_TEST (test_is_span_fast)
-{
-  GstBuffer *buffer, *sub1, *sub2;
-
-  buffer = gst_buffer_new_and_alloc (4);
-
-  sub1 = gst_buffer_copy_region (buffer, GST_BUFFER_COPY_ALL, 0, 2);
-  fail_if (sub1 == NULL, "copy_region of buffer returned NULL");
-
-  sub2 = gst_buffer_copy_region (buffer, GST_BUFFER_COPY_ALL, 2, 2);
-  fail_if (sub2 == NULL, "copy_region of buffer returned NULL");
-
-  fail_if (gst_buffer_is_span_fast (buffer, sub2) == TRUE,
-      "a parent buffer can't be span_fasted");
-
-  fail_if (gst_buffer_is_span_fast (sub1, buffer) == TRUE,
-      "a parent buffer can't be span_fasted");
-
-  fail_if (gst_buffer_is_span_fast (sub1, sub2) == FALSE,
-      "two subbuffers next to each other should be span_fast");
-
-  /* clean up */
-  gst_buffer_unref (sub1);
-  gst_buffer_unref (sub2);
-  gst_buffer_unref (buffer);
-}
-
-GST_END_TEST;
-
 GST_START_TEST (test_span)
 {
   GstBuffer *buffer, *sub1, *sub2, *span;
@@ -155,10 +126,9 @@ GST_START_TEST (test_span)
   memcpy (info.data, "data", 4);
   gst_buffer_unmap (buffer, &info);
 
-  ASSERT_CRITICAL (gst_buffer_span (NULL, 1, NULL, 2));
+  ASSERT_CRITICAL (gst_buffer_append (NULL, NULL));
-  ASSERT_CRITICAL (gst_buffer_span (buffer, 1, NULL, 2));
+  ASSERT_CRITICAL (gst_buffer_append (buffer, NULL));
-  ASSERT_CRITICAL (gst_buffer_span (NULL, 1, buffer, 2));
+  ASSERT_CRITICAL (gst_buffer_append (NULL, buffer));
-  ASSERT_CRITICAL (gst_buffer_span (buffer, 0, buffer, 10));
 
   sub1 = gst_buffer_copy_region (buffer, GST_BUFFER_COPY_ALL, 0, 2);
   fail_if (sub1 == NULL, "copy_region of buffer returned NULL");
@@ -171,7 +141,9 @@ GST_START_TEST (test_span)
   ASSERT_BUFFER_REFCOUNT (sub2, "sub2", 1);
 
   /* span will create a new subbuffer from the parent */
-  span = gst_buffer_span (sub1, 0, sub2, 4);
+  gst_buffer_ref (sub1);
+  gst_buffer_ref (sub2);
+  span = gst_buffer_append (sub1, sub2);
   fail_unless (gst_buffer_map (span, &info, GST_MAP_READ));
   fail_unless (info.size == 4, "spanned buffer is wrong size");
   ASSERT_BUFFER_REFCOUNT (buffer, "parent", 1);
@@ -185,7 +157,9 @@ GST_START_TEST (test_span)
   ASSERT_BUFFER_REFCOUNT (buffer, "parent", 1);
 
   /* span from non-contiguous buffers will create new buffers */
-  span = gst_buffer_span (sub2, 0, sub1, 4);
+  gst_buffer_ref (sub1);
+  gst_buffer_ref (sub2);
+  span = gst_buffer_append (sub2, sub1);
   fail_unless (gst_buffer_map (span, &info, GST_MAP_READ));
   fail_unless (info.size == 4, "spanned buffer is wrong size");
   ASSERT_BUFFER_REFCOUNT (buffer, "parent", 1);
@@ -198,33 +172,6 @@ GST_START_TEST (test_span)
   gst_buffer_unref (span);
   ASSERT_BUFFER_REFCOUNT (buffer, "parent", 1);
 
-  /* span with different sizes */
-  span = gst_buffer_span (sub1, 1, sub2, 3);
-  fail_unless (gst_buffer_map (span, &info, GST_MAP_READ));
-  fail_unless (info.size == 3, "spanned buffer is wrong size");
-  ASSERT_BUFFER_REFCOUNT (buffer, "parent", 1);
-  ASSERT_BUFFER_REFCOUNT (sub1, "sub1", 1);
-  ASSERT_BUFFER_REFCOUNT (sub2, "sub2", 1);
-  ASSERT_BUFFER_REFCOUNT (span, "span", 1);
-  fail_unless (memcmp (info.data, "ata", 3) == 0,
-      "spanned buffer contains the wrong data");
-  gst_buffer_unmap (span, &info);
-  gst_buffer_unref (span);
-  ASSERT_BUFFER_REFCOUNT (buffer, "parent", 1);
-
-  span = gst_buffer_span (sub2, 0, sub1, 3);
-  fail_unless (gst_buffer_map (span, &info, GST_MAP_READ));
-  fail_unless (info.size == 3, "spanned buffer is wrong size");
-  ASSERT_BUFFER_REFCOUNT (buffer, "parent", 1);
-  ASSERT_BUFFER_REFCOUNT (sub1, "sub1", 1);
-  ASSERT_BUFFER_REFCOUNT (sub2, "sub2", 1);
-  ASSERT_BUFFER_REFCOUNT (span, "span", 1);
-  fail_unless (memcmp (info.data, "tad", 3) == 0,
-      "spanned buffer contains the wrong data");
-  gst_buffer_unmap (span, &info);
-  gst_buffer_unref (span);
-  ASSERT_BUFFER_REFCOUNT (buffer, "parent", 1);
-
   /* clean up */
   gst_buffer_unref (sub1);
   gst_buffer_unref (sub2);
@@ -667,7 +614,6 @@ gst_buffer_suite (void)
   tcase_add_test (tc_chain, test_subbuffer);
   tcase_add_test (tc_chain, test_subbuffer_make_writable);
   tcase_add_test (tc_chain, test_make_writable);
-  tcase_add_test (tc_chain, test_is_span_fast);
   tcase_add_test (tc_chain, test_span);
   tcase_add_test (tc_chain, test_metadata_writable);
   tcase_add_test (tc_chain, test_copy);

win32/common/libgstreamer.def

@@ -90,6 +90,7 @@ EXPORTS
 	gst_bin_remove_many
 	gst_bitmask_get_type
 	gst_buffer_add_meta
+	gst_buffer_append
 	gst_buffer_copy_flags_get_type
 	gst_buffer_copy_into
 	gst_buffer_copy_region
@@ -101,9 +102,7 @@ EXPORTS
 	gst_buffer_get_meta
 	gst_buffer_get_sizes
 	gst_buffer_get_type
-	gst_buffer_is_span_fast
 	gst_buffer_iterate_meta
-	gst_buffer_join
 	gst_buffer_list_foreach
 	gst_buffer_list_get
 	gst_buffer_list_get_type
@@ -115,7 +114,6 @@ EXPORTS
 	gst_buffer_map
 	gst_buffer_memcmp
 	gst_buffer_memset
-	gst_buffer_merge
 	gst_buffer_n_memory
 	gst_buffer_new
 	gst_buffer_new_allocate
@@ -144,7 +142,6 @@ EXPORTS
 	gst_buffer_remove_meta
 	gst_buffer_replace_memory
 	gst_buffer_resize
-	gst_buffer_span
 	gst_buffer_take_memory
 	gst_buffer_unmap
 	gst_buffering_mode_get_type