gstbuffer: Unref memories before metas

gst_buffer_add_parent_buffer_meta() is used when a GstBuffer uses
GstMemory from another buffer that was allocated from a pool. In that
case we want to make sure the buffer returns to the pool when the memory
is writable again, otherwise a copy of the memory is created. That means
the child buffer must drop its ref to the memory first, then drop the
ref to parent buffer so it can return to the pool when it is the only
owner of the memory.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/5696>

diff --git a/subprojects/gstreamer/gst/gstbuffer.c b/subprojects/gstreamer/gst/gstbuffer.c
index 13bf9c87e608a7238f1ce68dbf260117fc079e27..441fc82710039e6a691aa97cf1760b3693241c35 100644 (file)
--- a/subprojects/gstreamer/gst/gstbuffer.c
+++ b/subprojects/gstreamer/gst/gstbuffer.c
@@ -789,6 +789,15 @@ _gst_buffer_free (GstBuffer * buffer)
 
   GST_CAT_LOG (GST_CAT_BUFFER, "finalize %p", buffer);
 
+  /* free our memory */
+  len = GST_BUFFER_MEM_LEN (buffer);
+  for (i = 0; i < len; i++) {
+    gst_memory_unlock (GST_BUFFER_MEM_PTR (buffer, i), GST_LOCK_FLAG_EXCLUSIVE);
+    gst_mini_object_remove_parent (GST_MINI_OBJECT_CAST (GST_BUFFER_MEM_PTR
+            (buffer, i)), GST_MINI_OBJECT_CAST (buffer));
+    gst_memory_unref (GST_BUFFER_MEM_PTR (buffer, i));
+  }
+
   /* free metadata */
   for (walk = GST_BUFFER_META (buffer); walk; walk = next) {
     GstMeta *meta = &walk->meta;
@@ -807,15 +816,6 @@ _gst_buffer_free (GstBuffer * buffer)
    * itself */
   msize = GST_BUFFER_SLICE_SIZE (buffer);
 
-  /* free our memory */
-  len = GST_BUFFER_MEM_LEN (buffer);
-  for (i = 0; i < len; i++) {
-    gst_memory_unlock (GST_BUFFER_MEM_PTR (buffer, i), GST_LOCK_FLAG_EXCLUSIVE);
-    gst_mini_object_remove_parent (GST_MINI_OBJECT_CAST (GST_BUFFER_MEM_PTR
-            (buffer, i)), GST_MINI_OBJECT_CAST (buffer));
-    gst_memory_unref (GST_BUFFER_MEM_PTR (buffer, i));
-  }
-
   /* we set msize to 0 when the buffer is part of the memory block */
   if (msize) {
 #ifdef USE_POISONING

diff --git a/subprojects/gstreamer/tests/check/gst/gstbufferpool.c b/subprojects/gstreamer/tests/check/gst/gstbufferpool.c
index 9f25c0d85981b886500a615a71a501151243020f..d1d87cddae83b241b1d07fac0457369e6adc216e 100644 (file)
--- a/subprojects/gstreamer/tests/check/gst/gstbufferpool.c
+++ b/subprojects/gstreamer/tests/check/gst/gstbufferpool.c
@@ -336,6 +336,58 @@ GST_START_TEST (test_no_deadlock_for_buffer_discard)
 
 GST_END_TEST;
 
+GST_START_TEST (test_parent_meta)
+{
+  GstBufferPool *pool;
+  GstBuffer *buf1, *buf2, *buf3;
+  GstMemory *mem;
+  gint buf1_dcount = 0;
+  gint buf2_dcount = 0;
+
+  pool = create_pool (1, 0, 0);
+  fail_unless (pool);
+  gst_buffer_pool_set_active (pool, TRUE);
+
+  fail_unless (gst_buffer_pool_acquire_buffer (pool, &buf1,
+          NULL) == GST_FLOW_OK);
+  buffer_track_destroy (buf1, &buf1_dcount);
+
+  /* Create a 2nd buffer reffing the same memory. Set parent meta to make sure
+   * buf1 does not return to pool until buf2 is destroyed. */
+  mem = gst_buffer_get_memory (buf1, 0);
+  buf2 = gst_buffer_new ();
+  gst_buffer_append_memory (buf2, mem);
+  gst_buffer_add_parent_buffer_meta (buf2, buf1);
+  buffer_track_destroy (buf2, &buf2_dcount);
+
+  /* buf1 is still reffed by the meta */
+  gst_buffer_unref (buf1);
+  fail_unless_equals_int (buf1_dcount, 0);
+  fail_unless_equals_int (buf2_dcount, 0);
+
+  /* buf2 gets destroyed and buf1 returns to pool */
+  gst_buffer_unref (buf2);
+  fail_unless_equals_int (buf1_dcount, 0);
+  fail_unless_equals_int (buf2_dcount, 1);
+
+  /* buf1 should be recycled with the same memory */
+  fail_unless (gst_buffer_pool_acquire_buffer (pool, &buf3,
+          NULL) == GST_FLOW_OK);
+  fail_unless_equals_pointer (buf1, buf3);
+  fail_unless_equals_pointer (mem, gst_buffer_peek_memory (buf3, 0));
+
+  gst_buffer_unref (buf3);
+  fail_unless_equals_int (buf1_dcount, 0);
+  fail_unless_equals_int (buf2_dcount, 1);
+
+  gst_buffer_pool_set_active (pool, FALSE);
+  gst_object_unref (pool);
+  fail_unless_equals_int (buf1_dcount, 1);
+  fail_unless_equals_int (buf2_dcount, 1);
+}
+
+GST_END_TEST;
+
 static Suite *
 gst_buffer_pool_suite (void)
 {
@@ -355,6 +407,7 @@ gst_buffer_pool_suite (void)
   tcase_add_test (tc_chain, test_pool_config_validate);
   tcase_add_test (tc_chain, test_flushing_pool_returns_flushing);
   tcase_add_test (tc_chain, test_no_deadlock_for_buffer_discard);
+  tcase_add_test (tc_chain, test_parent_meta);
 
   return s;
 }