appsrc: optimise caps changing when previously-set caps have not taken effect yet

Only negotiate/change caps once when setting caps twice and
the first-set caps have not been used yet.

Based on patch by Eunhae Choi.

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

diff --git a/gst-libs/gst/app/gstappsrc.c b/gst-libs/gst/app/gstappsrc.c
index 1124438..be27b3f 100644 (file)
--- a/gst-libs/gst/app/gstappsrc.c
+++ b/gst-libs/gst/app/gstappsrc.c
@@ -1239,6 +1239,9 @@ gst_app_src_set_caps (GstAppSrc * appsrc, const GstCaps * caps)
     GstCaps *new_caps;
     new_caps = caps ? gst_caps_copy (caps) : NULL;
     GST_DEBUG_OBJECT (appsrc, "setting caps to %" GST_PTR_FORMAT, caps);
+    if (priv->queue->tail != NULL && GST_IS_CAPS (priv->queue->tail->data)) {
+      gst_caps_unref (g_queue_pop_tail (priv->queue));
+    }
     g_queue_push_tail (priv->queue, new_caps);
     gst_caps_replace (&priv->last_caps, new_caps);
   }

diff --git a/tests/check/elements/appsrc.c b/tests/check/elements/appsrc.c
index 981b098..595ac3f 100644 (file)
--- a/tests/check/elements/appsrc.c
+++ b/tests/check/elements/appsrc.c
@@ -205,6 +205,135 @@ GST_START_TEST (test_appsrc_block_deadlock)
 
 GST_END_TEST;
 
+typedef struct
+{
+  GstCaps *caps1;
+  GstCaps *caps2;
+  GstCaps *expected_caps;
+} Helper;
+
+static void
+caps_notify_cb (GObject * obj, GObject * child, GParamSpec * pspec, Helper * h)
+{
+  GstCaps *caps = NULL;
+
+  g_object_get (child, "caps", &caps, NULL);
+  if (caps) {
+    GST_LOG_OBJECT (child, "expected caps: %" GST_PTR_FORMAT, h->expected_caps);
+    GST_LOG_OBJECT (child, "caps set to  : %" GST_PTR_FORMAT, caps);
+    fail_unless (gst_caps_is_equal (caps, h->expected_caps));
+    gst_caps_unref (caps);
+  }
+}
+
+static void
+handoff_cb (GstElement * sink, GstBuffer * buf, GstPad * pad, Helper * h)
+{
+  /* have our buffer, now the caps should change */
+  h->expected_caps = h->caps2;
+  GST_INFO ("got buffer, expect caps %" GST_PTR_FORMAT " next", h->caps2);
+}
+
+/* Make sure that if set_caps() is called twice before the source is started,
+ * the caps are just replaced and not put into the internal queue */
+GST_START_TEST (test_appsrc_set_caps_twice)
+{
+  GstElement *pipe, *src, *sink;
+  GstMessage *msg;
+  GstCaps *caps;
+  Helper h;
+
+  h.caps1 = gst_caps_new_simple ("foo/bar", "bleh", G_TYPE_INT, 2, NULL);
+  h.caps2 = gst_caps_new_simple ("bar/foo", "xyz", G_TYPE_INT, 3, NULL);
+
+  pipe = gst_pipeline_new ("pipeline");
+  src = gst_element_factory_make ("appsrc", NULL);
+  sink = gst_element_factory_make ("fakesink", NULL);
+  gst_bin_add_many (GST_BIN (pipe), src, sink, NULL);
+  gst_element_link (src, sink);
+
+  g_signal_connect (pipe, "deep-notify::caps", G_CALLBACK (caps_notify_cb), &h);
+
+  g_object_set (sink, "signal-handoffs", TRUE, NULL);
+  g_signal_connect (sink, "handoff", G_CALLBACK (handoff_cb), &h);
+
+  /* case 1: set caps to caps1, then set again to caps2, all this before
+   * appsrc is started and before any buffers are in the queue yet. We don't
+   * want to see any trace of caps1 during negotiation in this case. */
+  gst_app_src_set_caps (GST_APP_SRC (src), h.caps1);
+  caps = gst_app_src_get_caps (GST_APP_SRC (src));
+  fail_unless (gst_caps_is_equal (caps, h.caps1));
+  gst_caps_unref (caps);
+
+  gst_app_src_set_caps (GST_APP_SRC (src), h.caps2);
+  caps = gst_app_src_get_caps (GST_APP_SRC (src));
+  fail_unless (gst_caps_is_equal (caps, h.caps2));
+  gst_caps_unref (caps);
+
+  gst_app_src_end_of_stream (GST_APP_SRC (src));
+
+  h.expected_caps = h.caps2;
+
+  gst_element_set_state (pipe, GST_STATE_PLAYING);
+
+  msg =
+      gst_bus_timed_pop_filtered (GST_ELEMENT_BUS (pipe), -1, GST_MESSAGE_EOS);
+  gst_message_unref (msg);
+
+  gst_element_set_state (pipe, GST_STATE_NULL);
+  gst_object_unref (pipe);
+
+  GST_INFO ("Case #2");
+
+  /* case 2: set caps to caps1, then push a buffer and set to caps2, again
+   * before appsrc is started. In this case appsrc should negotiate to caps1
+   * first, and then caps2 after pushing the first buffer. */
+
+  /* We're creating a new pipeline/appsrc here because appsrc's behaviour
+   * change slightly after setting it to NULL/READY and then re-using it */
+  pipe = gst_pipeline_new ("pipeline");
+  src = gst_element_factory_make ("appsrc", NULL);
+  sink = gst_element_factory_make ("fakesink", NULL);
+  gst_bin_add_many (GST_BIN (pipe), src, sink, NULL);
+  gst_element_link (src, sink);
+
+  g_signal_connect (pipe, "deep-notify::caps", G_CALLBACK (caps_notify_cb), &h);
+
+  g_object_set (sink, "signal-handoffs", TRUE, NULL);
+  g_signal_connect (sink, "handoff", G_CALLBACK (handoff_cb), &h);
+
+  gst_app_src_set_caps (GST_APP_SRC (src), h.caps1);
+  caps = gst_app_src_get_caps (GST_APP_SRC (src));
+  fail_unless (gst_caps_is_equal (caps, h.caps1));
+  gst_caps_unref (caps);
+
+  /* first caps1, then buffer, then later caps2 */
+  h.expected_caps = h.caps1;
+
+  gst_element_set_state (pipe, GST_STATE_PLAYING);
+
+  gst_app_src_push_buffer (GST_APP_SRC (src), gst_buffer_new ());
+
+  gst_app_src_set_caps (GST_APP_SRC (src), h.caps2);
+  caps = gst_app_src_get_caps (GST_APP_SRC (src));
+  fail_unless (gst_caps_is_equal (caps, h.caps2));
+  gst_caps_unref (caps);
+
+  gst_app_src_end_of_stream (GST_APP_SRC (src));
+
+  msg =
+      gst_bus_timed_pop_filtered (GST_ELEMENT_BUS (pipe), -1, GST_MESSAGE_EOS);
+  gst_message_unref (msg);
+
+  gst_element_set_state (pipe, GST_STATE_NULL);
+  gst_object_unref (pipe);
+
+  gst_caps_unref (h.caps2);
+  gst_caps_unref (h.caps1);
+}
+
+GST_END_TEST;
+
 static Suite *
 appsrc_suite (void)
 {
@@ -212,6 +341,7 @@ appsrc_suite (void)
   TCase *tc_chain = tcase_create ("general");
 
   tcase_add_test (tc_chain, test_appsrc_non_null_caps);
+  tcase_add_test (tc_chain, test_appsrc_set_caps_twice);
 
   if (RUNNING_ON_VALGRIND)
     tcase_add_loop_test (tc_chain, test_appsrc_block_deadlock, 0, 5);