appsink: Reuse sample object in pull_sample if possible
authorMathieu Duponchelle <mathieu@centricular.com>
Tue, 10 Apr 2018 22:57:43 +0000 (00:57 +0200)
committerMathieu Duponchelle <mathieu@centricular.com>
Thu, 19 Apr 2018 14:14:12 +0000 (16:14 +0200)
Simple optimization to reduce memory allocations.

https://bugzilla.gnome.org/show_bug.cgi?id=795145

gst-libs/gst/app/gstappsink.c
tests/check/elements/appsink.c

index 87ae643..ab662f5 100644 (file)
@@ -107,6 +107,8 @@ struct _GstAppSinkPrivate
   GstAppSinkCallbacks callbacks;
   gpointer user_data;
   GDestroyNotify notify;
+
+  GstSample *sample;
 };
 
 GST_DEBUG_CATEGORY_STATIC (app_sink_debug);
@@ -461,6 +463,7 @@ gst_app_sink_init (GstAppSink * appsink)
   g_mutex_init (&priv->mutex);
   g_cond_init (&priv->cond);
   priv->queue = gst_queue_array_new (16);
+  priv->sample = gst_sample_new (NULL, NULL, NULL, NULL);
 
   priv->emit_signals = DEFAULT_PROP_EMIT_SIGNALS;
   priv->max_buffers = DEFAULT_PROP_MAX_BUFFERS;
@@ -496,6 +499,10 @@ gst_app_sink_dispose (GObject * obj)
   gst_buffer_replace (&priv->preroll_buffer, NULL);
   gst_caps_replace (&priv->preroll_caps, NULL);
   gst_caps_replace (&priv->last_caps, NULL);
+  if (priv->sample) {
+    gst_sample_unref (priv->sample);
+    priv->sample = NULL;
+  }
   g_mutex_unlock (&priv->mutex);
 
   G_OBJECT_CLASS (parent_class)->dispose (obj);
@@ -646,6 +653,11 @@ gst_app_sink_start (GstBaseSink * psink)
   priv->started = TRUE;
   gst_segment_init (&priv->preroll_segment, GST_FORMAT_TIME);
   gst_segment_init (&priv->last_segment, GST_FORMAT_TIME);
+  priv->sample = gst_sample_make_writable (priv->sample);
+  gst_sample_set_buffer (priv->sample, NULL);
+  gst_sample_set_buffer_list (priv->sample, NULL);
+  gst_sample_set_caps (priv->sample, NULL);
+  gst_sample_set_segment (priv->sample, NULL);
   g_mutex_unlock (&priv->mutex);
 
   return TRUE;
@@ -818,10 +830,14 @@ dequeue_buffer (GstAppSink * appsink)
           gst_event_parse_caps (event, &caps);
           GST_DEBUG_OBJECT (appsink, "activating caps %" GST_PTR_FORMAT, caps);
           gst_caps_replace (&priv->last_caps, caps);
+          priv->sample = gst_sample_make_writable (priv->sample);
+          gst_sample_set_caps (priv->sample, priv->last_caps);
           break;
         }
         case GST_EVENT_SEGMENT:
           gst_event_copy_segment (event, &priv->last_segment);
+          priv->sample = gst_sample_make_writable (priv->sample);
+          gst_sample_set_segment (priv->sample, &priv->last_segment);
           GST_DEBUG_OBJECT (appsink, "activated segment %" GST_SEGMENT_FORMAT,
               &priv->last_segment);
           break;
@@ -854,6 +870,7 @@ restart:
   if (G_UNLIKELY (!priv->last_caps &&
           gst_pad_has_current_caps (GST_BASE_SINK_PAD (psink)))) {
     priv->last_caps = gst_pad_get_current_caps (GST_BASE_SINK_PAD (psink));
+    gst_sample_set_caps (priv->sample, priv->last_caps);
     GST_DEBUG_OBJECT (appsink, "activating pad caps %" GST_PTR_FORMAT,
         priv->last_caps);
   }
@@ -1622,12 +1639,16 @@ gst_app_sink_try_pull_sample (GstAppSink * appsink, GstClockTime timeout)
   obj = dequeue_buffer (appsink);
   if (GST_IS_BUFFER (obj)) {
     GST_DEBUG_OBJECT (appsink, "we have a buffer %p", obj);
-    sample = gst_sample_new (GST_BUFFER_CAST (obj), priv->last_caps,
-        &priv->last_segment, NULL);
+    priv->sample = gst_sample_make_writable (priv->sample);
+    gst_sample_set_buffer_list (priv->sample, NULL);
+    gst_sample_set_buffer (priv->sample, GST_BUFFER_CAST (obj));
+    sample = gst_sample_ref (priv->sample);
   } else {
     GST_DEBUG_OBJECT (appsink, "we have a list %p", obj);
-    sample = gst_sample_new (NULL, priv->last_caps, &priv->last_segment, NULL);
-    gst_sample_set_buffer_list (sample, GST_BUFFER_LIST_CAST (obj));
+    priv->sample = gst_sample_make_writable (priv->sample);
+    gst_sample_set_buffer (priv->sample, NULL);
+    gst_sample_set_buffer_list (priv->sample, GST_BUFFER_LIST_CAST (obj));
+    sample = gst_sample_ref (priv->sample);
   }
   gst_mini_object_unref (obj);
 
index 03adf06..9acbdcb 100644 (file)
@@ -666,6 +666,54 @@ GST_START_TEST (test_query_drain)
 
 GST_END_TEST;
 
+GST_START_TEST (test_pull_sample_refcounts)
+{
+  GstElement *sink;
+  GstBuffer *buffer;
+  GstSample *s1, *s2, *s3;
+
+  sink = setup_appsink ();
+
+  ASSERT_SET_STATE (sink, GST_STATE_PLAYING, GST_STATE_CHANGE_ASYNC);
+
+  buffer = gst_buffer_new_and_alloc (4);
+  fail_unless (gst_pad_push (mysrcpad, buffer) == GST_FLOW_OK);
+
+  s1 = gst_app_sink_pull_sample (GST_APP_SINK (sink));
+  fail_unless (s1 != NULL);
+  fail_unless (gst_buffer_get_size (gst_sample_get_buffer (s1)) == 4);
+  gst_sample_unref (s1);
+
+  buffer = gst_buffer_new_and_alloc (6);
+  fail_unless (gst_pad_push (mysrcpad, buffer) == GST_FLOW_OK);
+  s2 = gst_app_sink_pull_sample (GST_APP_SINK (sink));
+  fail_unless (s2 != NULL);
+  fail_unless (gst_buffer_get_size (gst_sample_get_buffer (s2)) == 6);
+
+  /* We unreffed s1, appsink should thus reuse the same sample,
+   * avoiding an extra allocation */
+  fail_unless (s1 == s2);
+
+  buffer = gst_buffer_new_and_alloc (8);
+  fail_unless (gst_pad_push (mysrcpad, buffer) == GST_FLOW_OK);
+  s3 = gst_app_sink_pull_sample (GST_APP_SINK (sink));
+  fail_unless (s3 != NULL);
+  fail_unless (gst_buffer_get_size (gst_sample_get_buffer (s2)) == 6);
+  fail_unless (gst_buffer_get_size (gst_sample_get_buffer (s3)) == 8);
+
+
+  /* We didn't unref s2, appsink should thus have created a new sample */
+  fail_unless (s2 != s3);
+
+  gst_sample_unref (s2);
+  gst_sample_unref (s3);
+
+  ASSERT_SET_STATE (sink, GST_STATE_NULL, GST_STATE_CHANGE_SUCCESS);
+  cleanup_appsink (sink);
+}
+
+GST_END_TEST;
+
 static Suite *
 appsink_suite (void)
 {
@@ -686,6 +734,7 @@ appsink_suite (void)
   tcase_add_test (tc_chain, test_query_drain);
   tcase_add_test (tc_chain, test_pull_preroll);
   tcase_add_test (tc_chain, test_do_not_care_preroll);
+  tcase_add_test (tc_chain, test_pull_sample_refcounts);
 
   return s;
 }