diff --git a/ChangeLog b/ChangeLog index e1825c1fb1..8380431cff 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,15 @@ +2006-01-17 Jan Schmidt + + * gst/gstbuffer.c: (gst_buffer_is_metadata_writable), + (gst_buffer_make_metadata_writable): + * gst/gstbuffer.h: + * libs/gst/base/gstbasetransform.c: + (gst_base_transform_prepare_output_buf): + * plugins/elements/gstcapsfilter.c: (gst_capsfilter_prepare_buf): + * tests/check/gst/gstbuffer.c: (GST_START_TEST), (gst_test_suite): + Replace gst_buffer_(make|is)_metadata_writable patch now + that the release is out. + 2006-01-17 Andy Wingo * gst/gstregistry.c: Reflow design comment. Update so as to speak diff --git a/gst/gstbuffer.c b/gst/gstbuffer.c index 684ef8ee8a..6248f1c6c8 100644 --- a/gst/gstbuffer.c +++ b/gst/gstbuffer.c @@ -83,10 +83,15 @@ * To efficiently create a smaller buffer out of an existing one, you can * use gst_buffer_create_sub(). * - * If the plug-in wants to modify the buffer in-place, it should first obtain + * If a plug-in wants to modify the buffer data in-place, it should first obtain * a buffer that is safe to modify by using gst_buffer_make_writable(). This * function is optimized so that a copy will only be made when it is necessary. * + * A plugin that only wishes to modify the metadata of a buffer, such as the offset, + * timestamp or caps, should use gst_buffer_make_metadata_writable(), which will + * create a subbuffer of the original buffer to ensure the caller has sole ownership, + * and not copy the buffer data. + * * Several flags of the buffer can be set and unset with the * GST_BUFFER_FLAG_SET() and GST_BUFFER_FLAG_UNSET() macros. Use * GST_BUFFER_FLAG_IS_SET() to test if a certain #GstBufferFlag is set. @@ -333,6 +338,44 @@ gst_buffer_set_caps (GstBuffer * buffer, GstCaps * caps) gst_caps_replace (&GST_BUFFER_CAPS (buffer), caps); } +/** + * gst_buffer_is_metadata_writable: + * @buf: a #GstBuffer + * + * Similar to gst_buffer_is_writable, but this only ensures that the + * refcount of the buffer is 1, indicating that the caller is the sole + * owner and can change the buffer metadata, such as caps and timestamps. + */ +gboolean +gst_buffer_is_metadata_writable (GstBuffer * buf) +{ + return (GST_MINI_OBJECT_REFCOUNT_VALUE (GST_MINI_OBJECT (buf)) == 1); +} + +/** + * gst_buffer_make_metadata_writable: + * @buf: a #GstBuffer + * + * Similar to gst_buffer_make_writable, but does not ensure that the buffer + * data array is writable. Instead, this just ensures that the returned buffer + * is solely owned by the caller, by creating a subbuffer of the original + * buffer if necessary. + */ +GstBuffer * +gst_buffer_make_metadata_writable (GstBuffer * buf) +{ + GstBuffer *ret; + + if (gst_buffer_is_metadata_writable (buf)) { + ret = buf; + } else { + ret = gst_buffer_create_sub (buf, 0, GST_BUFFER_SIZE (buf)); + gst_buffer_unref (buf); + } + + return ret; +} + typedef struct _GstSubBuffer GstSubBuffer; typedef struct _GstSubBufferClass GstSubBufferClass; diff --git a/gst/gstbuffer.h b/gst/gstbuffer.h index 23fa6a8c46..90ccb112f6 100644 --- a/gst/gstbuffer.h +++ b/gst/gstbuffer.h @@ -310,7 +310,10 @@ G_STMT_START { \ * gst_buffer_is_writable: * @buf: a #GstBuffer * - * Tests if you can safely write data into a buffer's data array. + * Tests if you can safely write data into a buffer's data array or validly + * modify the caps and timestamp metadata. Metadata in a GstBuffer is always + * writable, but it is only safe to change it when there is only one owner + * of the buffer - ie, the buffer is 1. */ #define gst_buffer_is_writable(buf) gst_mini_object_is_writable (GST_MINI_OBJECT (buf)) /** @@ -323,6 +326,11 @@ G_STMT_START { \ */ #define gst_buffer_make_writable(buf) GST_BUFFER_CAST (gst_mini_object_make_writable (GST_MINI_OBJECT (buf))) +/* Ensure that the metadata of the buffer is writable, even if the buffer data + * isn't */ +gboolean gst_buffer_is_metadata_writable (GstBuffer *buf); +GstBuffer* gst_buffer_make_metadata_writable (GstBuffer *buf); + /** * gst_buffer_replace: * @obuf: pointer to a pointer to a #GstBuffer to be replaced. diff --git a/libs/gst/base/gstbasetransform.c b/libs/gst/base/gstbasetransform.c index 9d8b9df032..1331c65ab8 100644 --- a/libs/gst/base/gstbasetransform.c +++ b/libs/gst/base/gstbasetransform.c @@ -866,7 +866,7 @@ gst_base_transform_prepare_output_buf (GstBaseTransform * trans, /* If the output buffer metadata is modifiable, copy timestamps and * buffer flags */ - if (*out_buf != in_buf && GST_MINI_OBJECT_REFCOUNT_VALUE (*out_buf) == 1) { + if (*out_buf != in_buf && gst_buffer_is_metadata_writable (*out_buf) == 1) { if (copy_inbuf && gst_buffer_is_writable (*out_buf)) memcpy (GST_BUFFER_DATA (*out_buf), GST_BUFFER_DATA (in_buf), out_size); diff --git a/plugins/elements/gstcapsfilter.c b/plugins/elements/gstcapsfilter.c index 2b20380b7b..07481570cd 100644 --- a/plugins/elements/gstcapsfilter.c +++ b/plugins/elements/gstcapsfilter.c @@ -260,7 +260,7 @@ gst_capsfilter_prepare_buf (GstBaseTransform * trans, GstBuffer * input, if (gst_caps_is_fixed (out_caps) && !gst_caps_is_empty (out_caps)) { GST_DEBUG_OBJECT (trans, "Have fixed output caps %" GST_PTR_FORMAT " to apply to buffer with no caps", out_caps); - if (gst_buffer_is_writable (input)) { + if (gst_buffer_is_metadata_writable (input)) { gst_buffer_ref (input); *buf = input; } else { diff --git a/tests/check/gst/gstbuffer.c b/tests/check/gst/gstbuffer.c index 24611b40f1..0335fb4215 100644 --- a/tests/check/gst/gstbuffer.c +++ b/tests/check/gst/gstbuffer.c @@ -268,6 +268,41 @@ GST_START_TEST (test_subbuffer_make_writable) GST_END_TEST; +GST_START_TEST (test_metadata_writable) +{ + GstBuffer *buffer, *sub1; + + buffer = gst_buffer_new_and_alloc (4); + /* Buffer with refcount 1 should have writable metadata */ + fail_unless (gst_buffer_is_metadata_writable (buffer) == TRUE); + + /* Check that a buffer with refcount 2 does not have writable metadata */ + gst_buffer_ref (buffer); + ASSERT_BUFFER_REFCOUNT (buffer, "buffer", 2); + fail_unless (gst_buffer_is_metadata_writable (buffer) == FALSE); + + /* Check that make_metadata_writable produces a new sub-buffer with + * writable metadata. */ + sub1 = gst_buffer_make_metadata_writable (buffer); + fail_if (sub1 == buffer); + fail_unless (gst_buffer_is_metadata_writable (sub1) == TRUE); + + /* Check that the original metadata is still not writable + * (subbuffer should be holding a reference, and so should we) */ + ASSERT_BUFFER_REFCOUNT (buffer, "buffer", 2); + fail_unless (gst_buffer_is_metadata_writable (buffer) == FALSE); + + /* Drop the subbuffer and check that the metadata is now writable again */ + ASSERT_BUFFER_REFCOUNT (sub1, "sub1", 1); + gst_buffer_unref (sub1); + fail_unless (gst_buffer_is_metadata_writable (buffer) == TRUE); + + ASSERT_BUFFER_REFCOUNT (buffer, "buffer", 1); + gst_buffer_unref (buffer); +} + +GST_END_TEST; + Suite * gst_test_suite (void) { @@ -281,6 +316,7 @@ gst_test_suite (void) 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); return s; }