From 22613b9baf49c71a18767e44f09a38820dea3b6e 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 772e7526b4..ce526715a3 100644 --- a/subprojects/gstreamer/gst/gstbuffer.c +++ b/subprojects/gstreamer/gst/gstbuffer.c @@ -785,6 +785,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; @@ -799,15 +808,6 @@ _gst_buffer_free (GstBuffer * buffer) g_free (walk); } - /* 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)); - } - #ifdef USE_POISONING memset (buffer, 0xff, sizeof (GstBufferImpl)); #endif 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; }