From bb876588ac998061cef3b3343a8c00f01d119063 Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Wed, 28 Mar 2012 12:44:44 +0200 Subject: [PATCH] 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 | 6 +- docs/random/porting-to-0.11.txt | 14 +++++ gst/gstbuffer.c | 134 +++++++++++----------------------------- gst/gstbuffer.h | 5 +- gst/gstutils.c | 68 -------------------- gst/gstutils.h | 4 -- libs/gst/base/gstadapter.c | 8 +-- tests/check/gst/gstbuffer.c | 72 +++------------------ win32/common/libgstreamer.def | 5 +- 9 files changed, 64 insertions(+), 252 deletions(-) diff --git a/docs/gst/gstreamer-sections.txt b/docs/gst/gstreamer-sections.txt index 2e43a41..bbdabe5 100644 --- a/docs/gst/gstreamer-sections.txt +++ b/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_span +gst_buffer_append gst_buffer_get_meta gst_buffer_add_meta diff --git a/docs/random/porting-to-0.11.txt b/docs/random/porting-to-0.11.txt index 0181bcd..9a1642b 100644 --- a/docs/random/porting-to-0.11.txt +++ b/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. diff --git a/gst/gstbuffer.c b/gst/gstbuffer.c index ccfee8c..664bdf7 100644 --- a/gst/gstbuffer.c +++ b/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() - * function returns TRUE. + * gst_buffer_append(). Copying of memory will only be done when absolutely + * 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: - * @buf1: the first #GstBuffer. - * @buf2: the second #GstBuffer. + * gst_buffer_append: + * @buf1: (transfer full): the first source #GstBuffer to append. + * @buf2: (transfer full): the second source #GstBuffer to append. * - * Determines whether a gst_buffer_span() can be done without copying - * the contents, that is, whether the data areas are contiguous sub-buffers of - * the same buffer. + * Append all the memory from @buf2 to @buf1. The result buffer will contain a + * concatenation of the memory of @buf1 and @buf2. * - * MT safe. - * Returns: TRUE if the buffers are contiguous, - * 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. + * Returns: (transfer full): the new #GstBuffer that contains the memory + * of the two source buffers. */ GstBuffer * -gst_buffer_span (GstBuffer * buf1, gsize offset, GstBuffer * buf2, gsize size) +gst_buffer_append (GstBuffer * buf1, GstBuffer * buf2) { - GstBuffer *newbuf; - GstMemory *span; - GstMemory **mem[2]; - gsize len[2], size1, size2; + gsize i, len; 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); - len[0] = GST_BUFFER_MEM_LEN (buf1); - mem[1] = GST_BUFFER_MEM_ARRAY (buf2); - len[1] = GST_BUFFER_MEM_LEN (buf2); + buf1 = gst_buffer_make_writable (buf1); + buf2 = gst_buffer_make_writable (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 (); - _memory_add (newbuf, -1, span); - - /* 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); - } - } + mem = GST_BUFFER_MEM_PTR (buf2, i); + GST_BUFFER_MEM_PTR (buf2, i) = NULL; + _memory_add (buf1, -1, mem); } - - return newbuf; + GST_BUFFER_MEM_LEN (buf2) = 0; + gst_buffer_unref (buf2); + + /* 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; } /** diff --git a/gst/gstbuffer.h b/gst/gstbuffer.h index f5e67fb..28c6284 100644 --- a/gst/gstbuffer.h +++ b/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 */ -gboolean gst_buffer_is_span_fast (GstBuffer *buf1, GstBuffer *buf2); -GstBuffer* gst_buffer_span (GstBuffer *buf1, gsize offset, GstBuffer *buf2, gsize size) G_GNUC_MALLOC; +/* append two buffers */ +GstBuffer* gst_buffer_append (GstBuffer *buf1, GstBuffer *buf2); /* metadata */ #include diff --git a/gst/gstutils.c b/gst/gstutils.c index 9e7f551..7771862 100644 --- a/gst/gstutils.c +++ b/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; diff --git a/gst/gstutils.h b/gst/gstutils.h index 18e4616..e562457 100644 --- a/gst/gstutils.h +++ b/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, diff --git a/libs/gst/base/gstadapter.c b/libs/gst/base/gstadapter.c index 4ac6725..99bb975 100644 --- a/libs/gst/base/gstadapter.c +++ b/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 - * where 'gst_buffer_is_span_fast' returns TRUE. + * to form a single larger buffer of size 'size'. * * 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; diff --git a/tests/check/gst/gstbuffer.c b/tests/check/gst/gstbuffer.c index d009e68..190825e 100644 --- a/tests/check/gst/gstbuffer.c +++ b/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_span (buffer, 1, NULL, 2)); - ASSERT_CRITICAL (gst_buffer_span (NULL, 1, buffer, 2)); - ASSERT_CRITICAL (gst_buffer_span (buffer, 0, buffer, 10)); + ASSERT_CRITICAL (gst_buffer_append (NULL, NULL)); + ASSERT_CRITICAL (gst_buffer_append (buffer, NULL)); + ASSERT_CRITICAL (gst_buffer_append (NULL, buffer)); 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); diff --git a/win32/common/libgstreamer.def b/win32/common/libgstreamer.def index ca592f7..9a97067 100644 --- a/win32/common/libgstreamer.def +++ b/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 -- 2.7.4