From 27350019e2650ba1031a54545a781465d146e2e1 Mon Sep 17 00:00:00 2001 From: Xavier Claessens Date: Thu, 9 Mar 2023 22:18:12 -0800 Subject: [PATCH] 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: --- subprojects/gstreamer/gst/gstbuffer.c | 18 +++---- .../gstreamer/tests/check/gst/gstbufferpool.c | 53 +++++++++++++++++++ 2 files changed, 62 insertions(+), 9 deletions(-) diff --git a/subprojects/gstreamer/gst/gstbuffer.c b/subprojects/gstreamer/gst/gstbuffer.c index 13bf9c87e6..441fc82710 100644 --- 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 9f25c0d859..d1d87cddae 100644 --- 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; } -- 2.34.1