Fix assertion in basetransform when the subclass chooses not to allocate a buffer...

Original commit message from CVS:
* libs/gst/base/gstbasetransform.c:
* plugins/elements/gstcapsfilter.c:
* tests/check/Makefile.am:
* tests/check/elements/.cvsignore:
* tests/check/elements/capsfilter.c:
Fix assertion in basetransform when the subclass chooses not to
allocate a buffer in prepare_buffer(), and make capsfilter error out
cleanly if requested to apply caps that don't completely specify the
buffer. Fixes #551509

diff --git a/ChangeLog b/ChangeLog
index 9d9d82f..86b1e29 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2008-09-28  Jan Schmidt  <jan.schmidt@sun.com>
+
+       * libs/gst/base/gstbasetransform.c:
+       * plugins/elements/gstcapsfilter.c:
+       * tests/check/Makefile.am:
+       * tests/check/elements/.cvsignore:
+       * tests/check/elements/capsfilter.c:
+       Fix assertion in basetransform when the subclass chooses not to
+       allocate a buffer in prepare_buffer(), and make capsfilter error out
+       cleanly if requested to apply caps that don't completely specify the
+       buffer. Fixes #551509
+
 2008-09-24  Wim Taymans  <wim.taymans@collabora.co.uk>
 
        * libs/gst/base/gstbasetransform.c:

diff --git a/libs/gst/base/gstbasetransform.c b/libs/gst/base/gstbasetransform.c
index d0c7a12..86a081b 100644 (file)
--- a/libs/gst/base/gstbasetransform.c
+++ b/libs/gst/base/gstbasetransform.c
@@ -1130,19 +1130,25 @@ gst_base_transform_prepare_output_buffer (GstBaseTransform * trans,
       gst_buffer_unref (in_buf);
 
     /* never discard the buffer from the prepare_buffer method */
-    discard = FALSE;
-  } else {
+    if (*out_buf != NULL)
+      discard = FALSE;
+  }
+
+  if (ret != GST_FLOW_OK)
+    goto alloc_failed;
+
+  if (*out_buf == NULL) {
     GST_DEBUG_OBJECT (trans, "doing alloc with caps %" GST_PTR_FORMAT, oldcaps);
 
     ret = gst_pad_alloc_buffer (trans->srcpad,
         GST_BUFFER_OFFSET (in_buf), outsize, oldcaps, out_buf);
+    if (ret != GST_FLOW_OK)
+      goto alloc_failed;
   }
 
-  if (ret != GST_FLOW_OK)
-    goto alloc_failed;
-
-  /* must always return a buffer */
-  g_assert (*out_buf != NULL);
+  /* must always have a buffer by now */
+  if (*out_buf == NULL)
+    goto no_buffer;
 
   /* check if we got different caps on this new output buffer */
   newcaps = GST_BUFFER_CAPS (*out_buf);
@@ -1273,6 +1279,12 @@ alloc_failed:
     GST_WARNING_OBJECT (trans, "pad-alloc failed: %s", gst_flow_get_name (ret));
     return ret;
   }
+no_buffer:
+  {
+    GST_ELEMENT_ERROR (trans, STREAM, NOT_IMPLEMENTED,
+        ("Sub-class failed to provide an output buffer"), (NULL));
+    return GST_FLOW_ERROR;
+  }
 unknown_size:
   {
     GST_ERROR_OBJECT (trans, "unknown output size");

diff --git a/plugins/elements/gstcapsfilter.c b/plugins/elements/gstcapsfilter.c
index 70cdbc2..bcef6ae 100644 (file)
--- a/plugins/elements/gstcapsfilter.c
+++ b/plugins/elements/gstcapsfilter.c
@@ -291,6 +291,8 @@ static GstFlowReturn
 gst_capsfilter_prepare_buf (GstBaseTransform * trans, GstBuffer * input,
     gint size, GstCaps * caps, GstBuffer ** buf)
 {
+  GstFlowReturn ret = GST_FLOW_OK;
+
   if (GST_BUFFER_CAPS (input) != NULL) {
     /* Output buffer already has caps */
     GST_DEBUG_OBJECT (trans,
@@ -332,11 +334,19 @@ gst_capsfilter_prepare_buf (GstBaseTransform * trans, GstBuffer * input,
       if (GST_PAD_CAPS (trans->srcpad) == NULL)
         gst_pad_set_caps (trans->srcpad, out_caps);
     } else {
-      GST_DEBUG_OBJECT (trans, "Have unfixed output caps %" GST_PTR_FORMAT,
-          out_caps);
+      gchar *caps_str = gst_caps_to_string (out_caps);
+
+      GST_DEBUG_OBJECT (trans, "Cannot choose caps. Have unfixed output caps %"
+          GST_PTR_FORMAT, out_caps);
       gst_caps_unref (out_caps);
+
+      ret = GST_FLOW_ERROR;
+      GST_ELEMENT_ERROR (trans, STREAM, FORMAT,
+          ("Filter caps do not completely specify the output format"),
+          ("Output caps are unfixed: %s", caps_str));
+      g_free (caps_str);
     }
   }
 
-  return GST_FLOW_OK;
+  return ret;
 }

diff --git a/tests/check/Makefile.am b/tests/check/Makefile.am
index aedc16b..2dddc93 100644 (file)
--- a/tests/check/Makefile.am
+++ b/tests/check/Makefile.am
@@ -58,6 +58,7 @@ REGISTRY_CHECKS =                             \
        gst/gsturi                              \
        gst/gstutils                            \
        generic/sinks                           \
+       elements/capsfilter                     \
        elements/fakesink                       \
        elements/fakesrc                        \
        elements/fdsrc                          \

diff --git a/tests/check/elements/.gitignore b/tests/check/elements/.gitignore
index 4dd5bf0..8b6d06c 100644 (file)
--- a/tests/check/elements/.gitignore
+++ b/tests/check/elements/.gitignore
@@ -1,4 +1,5 @@
 .dirstamp
+capsfilter
 fakesrc
 fakesink
 fdsrc

diff --git a/tests/check/elements/capsfilter.c b/tests/check/elements/capsfilter.c
new file mode 100644 (file)
index 0000000..97a6f09
--- /dev/null
+++ b/tests/check/elements/capsfilter.c
@@ -0,0 +1,93 @@
+/* GStreamer unit test for capsfilter
+ * Copyright (C) <2008> Tim-Philipp Müller <tim centricular net>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public
+ * License along with this library; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 02111-1307, USA.
+ */
+
+#include <gst/check/gstcheck.h>
+
+#define CAPS_TEMPLATE_STRING            \
+    "audio/x-raw-int, "                 \
+    "channels = (int) [ 1, 2], "        \
+    "rate = (int) [ 1,  MAX ]"
+
+static GstStaticPadTemplate sinktemplate = GST_STATIC_PAD_TEMPLATE ("sink",
+    GST_PAD_SINK,
+    GST_PAD_ALWAYS,
+    GST_STATIC_CAPS (CAPS_TEMPLATE_STRING)
+    );
+
+GST_START_TEST (test_unfixed_downstream_caps)
+{
+  GstElement *pipe, *src, *filter;
+  GstCaps *filter_caps;
+  GstPad *mysinkpad;
+  GstMessage *msg;
+
+  pipe = gst_check_setup_element ("pipeline");
+
+  src = gst_check_setup_element ("fakesrc");
+  g_object_set (src, "sizetype", 2, "sizemax", 1024, "num-buffers", 1, NULL);
+
+  filter = gst_check_setup_element ("capsfilter");
+  filter_caps = gst_caps_from_string ("audio/x-raw-int, rate=(int)44100");
+  fail_unless (filter_caps != NULL);
+  g_object_set (filter, "caps", filter_caps, NULL);
+
+  gst_bin_add_many (GST_BIN (pipe), src, filter, NULL);
+  fail_unless (gst_element_link (src, filter));
+
+  mysinkpad = gst_check_setup_sink_pad (filter, &sinktemplate, NULL);
+  gst_pad_set_active (mysinkpad, TRUE);
+
+  fail_unless_equals_int (gst_element_set_state (pipe, GST_STATE_PLAYING),
+      GST_STATE_CHANGE_SUCCESS);
+
+  /* wait for error on bus */
+  msg = gst_bus_poll (GST_ELEMENT_BUS (pipe),
+      GST_MESSAGE_EOS | GST_MESSAGE_ERROR, -1);
+
+  fail_if (GST_MESSAGE_TYPE (msg) != GST_MESSAGE_ERROR,
+      "Expected ERROR message, got EOS message");
+  gst_message_unref (msg);
+
+  /* We don't expect any output buffers unless the check fails */
+  fail_unless (buffers == NULL);
+
+  /* cleanup */
+  GST_DEBUG ("cleanup");
+
+  gst_pad_set_active (mysinkpad, FALSE);
+  gst_check_teardown_sink_pad (filter);
+  gst_check_teardown_element (pipe);
+  gst_caps_unref (filter_caps);
+}
+
+GST_END_TEST;
+
+static Suite *
+capsfilter_suite (void)
+{
+  Suite *s = suite_create ("capsfilter");
+  TCase *tc_chain = tcase_create ("general");
+
+  suite_add_tcase (s, tc_chain);
+  tcase_add_test (tc_chain, test_unfixed_downstream_caps);
+
+  return s;
+}
+
+GST_CHECK_MAIN (capsfilter)