From f068b3bf5cbe19f4901f3d910fd9970ab69184e2 Mon Sep 17 00:00:00 2001 From: Albert Sjolund Date: Fri, 15 Nov 2024 11:02:20 +0100 Subject: [PATCH] gstpad: specialize gst_pad_chain_list_default In the case where the bufferlist is writable, send the buffers immediately without adding to the refcount. This allows writable buffers to maintain their writability, even without implementing a chain_list function on the element. Adds a test to verify this property, where a writable list maintains refcount 1, but a readonly list increases it. Part-of: --- subprojects/gstreamer/gst/gstpad.c | 46 ++++++++--- .../gstreamer/tests/check/gst/gstpad.c | 79 +++++++++++++++++++ 2 files changed, 115 insertions(+), 10 deletions(-) diff --git a/subprojects/gstreamer/gst/gstpad.c b/subprojects/gstreamer/gst/gstpad.c index bf08b78ad0..39b31bd9ed 100644 --- a/subprojects/gstreamer/gst/gstpad.c +++ b/subprojects/gstreamer/gst/gstpad.c @@ -195,6 +195,15 @@ static GstFlowReturn gst_pad_push_event_unchecked (GstPad * pad, static gboolean activate_mode_internal (GstPad * pad, GstObject * parent, GstPadMode mode, gboolean active); +typedef struct +{ + GstPad *pad; + GstFlowReturn flow_res; +} PadChainListData; + +static gboolean list_process_buffer_writable (GstBuffer ** buffer, guint idx, + gpointer userdata); + static guint gst_pad_signals[LAST_SIGNAL] = { 0 }; static GParamSpec *pspec_caps = NULL; @@ -4656,6 +4665,16 @@ gst_pad_chain (GstPad * pad, GstBuffer * buffer) GST_PAD_PROBE_TYPE_BUFFER | GST_PAD_PROBE_TYPE_PUSH, buffer); } +static gboolean +list_process_buffer_writable (GstBuffer ** buffer, guint idx, gpointer userdata) +{ + PadChainListData *data = (PadChainListData *) (userdata); + data->flow_res = gst_pad_chain_data_unchecked (data->pad, + GST_PAD_PROBE_TYPE_BUFFER | GST_PAD_PROBE_TYPE_PUSH, *buffer); + *buffer = NULL; + return data->flow_res == GST_FLOW_OK; +} + static GstFlowReturn gst_pad_chain_list_default (GstPad * pad, GstObject * parent, GstBufferList * list) @@ -4666,18 +4685,25 @@ gst_pad_chain_list_default (GstPad * pad, GstObject * parent, GST_LOG_OBJECT (pad, "chaining each buffer in list individually"); - len = gst_buffer_list_length (list); - ret = GST_FLOW_OK; - for (i = 0; i < len; i++) { - buffer = gst_buffer_list_get (list, i); - ret = - gst_pad_chain_data_unchecked (pad, - GST_PAD_PROBE_TYPE_BUFFER | GST_PAD_PROBE_TYPE_PUSH, - gst_buffer_ref (buffer)); - if (ret != GST_FLOW_OK) - break; + + if (gst_buffer_list_is_writable (list)) { + PadChainListData data = {.pad = pad,.flow_res = GST_FLOW_OK }; + gst_buffer_list_foreach (list, list_process_buffer_writable, &data); + ret = data.flow_res; + } else { + len = gst_buffer_list_length (list); + for (i = 0; i < len; i++) { + buffer = gst_buffer_list_get (list, i); + ret = + gst_pad_chain_data_unchecked (pad, + GST_PAD_PROBE_TYPE_BUFFER | GST_PAD_PROBE_TYPE_PUSH, + gst_buffer_ref (buffer)); + if (ret != GST_FLOW_OK) + break; + } } + gst_buffer_list_unref (list); return ret; diff --git a/subprojects/gstreamer/tests/check/gst/gstpad.c b/subprojects/gstreamer/tests/check/gst/gstpad.c index 0b545eec91..ff142edced 100644 --- a/subprojects/gstreamer/tests/check/gst/gstpad.c +++ b/subprojects/gstreamer/tests/check/gst/gstpad.c @@ -3390,6 +3390,84 @@ GST_START_TEST (test_pad_offset_src) GST_END_TEST; +/* Pushing a writable list should not increase refcount. */ +static GstFlowReturn +verify_buffer_writable (GstPad * pad, GstObject * parent, GstBuffer * buffer) +{ + fail_unless (gst_buffer_is_writable (buffer)); + gst_buffer_unref (buffer); + return GST_FLOW_OK; +} + +static GstFlowReturn +verify_buffer_refcount_two (GstPad * pad, GstObject * parent, + GstBuffer * buffer) +{ + fail_unless_equals_int (GST_MINI_OBJECT_REFCOUNT (buffer), 2); + gst_buffer_unref (buffer); + return GST_FLOW_OK; +} + +GST_START_TEST (test_pad_chain_list_writable) +{ + GstPad *src, *sink; + GstPadLinkReturn plr; + GstCaps *caps; + + /* setup */ + sink = gst_pad_new ("sink", GST_PAD_SINK); + fail_if (sink == NULL); + gst_pad_set_chain_function (sink, verify_buffer_writable); + + src = gst_pad_new ("src", GST_PAD_SRC); + fail_if (src == NULL); + + caps = gst_caps_from_string ("foo/bar"); + + gst_pad_set_active (src, TRUE); + + gst_pad_push_event (src, gst_event_new_stream_start ("test")); + + gst_pad_set_active (sink, TRUE); + + gst_pad_set_caps (src, caps); + plr = gst_pad_link (src, sink); + + fail_unless (gst_pad_push_event (src, + gst_event_new_segment (&dummy_segment)) == TRUE); + + fail_unless (GST_PAD_LINK_SUCCESSFUL (plr)); + + /* Create a buffer list, that moves into gst_pad_push_list */ + GstBufferList *list = gst_buffer_list_new_sized (1); + + gst_buffer_list_add (list, gst_buffer_new ()); + gst_buffer_list_add (list, gst_buffer_new ()); + + fail_unless (gst_buffer_list_is_writable (list)); + /* this takes ownership of list, and will free it */ + gst_pad_push_list (src, list); + + gst_pad_set_chain_function (sink, verify_buffer_refcount_two); + list = gst_buffer_list_new_sized (1); + + gst_buffer_list_add (list, gst_buffer_new ()); + + /* ref the list one more time, so it is no longer writable */ + gst_buffer_list_ref (list); + + fail_unless (!gst_buffer_list_is_writable (list)); + gst_pad_push_list (src, list); + + gst_buffer_list_unref (list); + + gst_object_unref (sink); + gst_object_unref (src); + gst_caps_unref (caps); +} + +GST_END_TEST; + static Suite * gst_pad_suite (void) { @@ -3451,6 +3529,7 @@ gst_pad_suite (void) tcase_add_test (tc_chain, test_proxy_accept_caps_with_proxy); tcase_add_test (tc_chain, test_proxy_accept_caps_with_incompatible_proxy); tcase_add_test (tc_chain, test_pad_offset_src); + tcase_add_test (tc_chain, test_pad_chain_list_writable); return s; }