audioresample: fix negotiation so that upstream can actually fixate to downstream's rate

If one side has a preference for a particular sample rate or set of sample rates, we
should honour this in the caps we advertise and transform to and from, so that elements
actually know about the other side's sample rate preference and can negotiate to it
if supported. Also add unit test for this.

2 files changed, 79 insertions, 5 deletions

diff --git a/gst/audioresample/gstaudioresample.c b/gst/audioresample/gstaudioresample.c
index 2acbc5578..1ce736467 100644
--- a/gst/audioresample/gstaudioresample.c
+++ b/gst/audioresample/gstaudioresample.c
@@ -295,14 +295,34 @@ static GstCaps *
 gst_audio_resample_transform_caps (GstBaseTransform * base,
     GstPadDirection direction, GstCaps * caps)
 {
+  const GValue *val;
+  GstStructure *s;
   GstCaps *res;
-  GstStructure *structure;
 
-  /* transform caps gives one single caps so we can just replace
-   * the rate property with our range. */
+  /* transform single caps into input_caps + input_caps with the rate
+   * field set to our supported range. This ensures that upstream knows
+   * about downstream's prefered rate(s) and can negotiate accordingly. */
   res = gst_caps_copy (caps);
-  structure = gst_caps_get_structure (res, 0);
-  gst_structure_set (structure, "rate", GST_TYPE_INT_RANGE, 1, G_MAXINT, NULL);
+
+  /* first, however, check if the caps contain a range for the rate field, in
+   * which case that side isn't going to care much about the exact sample rate
+   * chosen and we should just assume things will get fixated to something sane
+   * and we may just as well offer our full range instead of the range in the
+   * caps. If the rate is not an int range value, it's likely to express a
+   * real preference or limitation and we should maintain that structure as
+   * preference by putting it first into the transformed caps, and only add
+   * our full rate range as second option  */
+  s = gst_caps_get_structure (res, 0);
+  val = gst_structure_get_value (s, "rate");
+  if (val == NULL || GST_VALUE_HOLDS_INT_RANGE (val)) {
+    /* overwrite existing range, or add field if it doesn't exist yet */
+    gst_structure_set (s, "rate", GST_TYPE_INT_RANGE, 1, G_MAXINT, NULL);
+  } else {
+    /* append caps with full range to existing caps with non-range rate field */
+    s = gst_structure_copy (s);
+    gst_structure_set (s, "rate", GST_TYPE_INT_RANGE, 1, G_MAXINT, NULL);
+    gst_caps_append_structure (res, s);
+  }
 
   return res;
 }
diff --git a/tests/check/elements/audioresample.c b/tests/check/elements/audioresample.c
index 7b8408d41..24f77a140 100644
--- a/tests/check/elements/audioresample.c
+++ b/tests/check/elements/audioresample.c
@@ -674,6 +674,59 @@ GST_START_TEST (test_pipelines)
 }
 
 GST_END_TEST;
+
+GST_START_TEST (test_preference_passthrough)
+{
+  GstStateChangeReturn ret;
+  GstElement *pipeline, *src;
+  GstStructure *s;
+  GstMessage *msg;
+  GstCaps *caps;
+  GstPad *pad;
+  GstBus *bus;
+  GError *error = NULL;
+  gint rate = 0;
+
+  pipeline = gst_parse_launch ("audiotestsrc num-buffers=1 name=src ! "
+      "audioresample ! audio/x-raw-int,channels=1,width=16,depth=16,"
+      "endianness=1234,signed=true,rate=8000 ! "
+      "fakesink can-activate-pull=false", &error);
+  fail_unless (pipeline != NULL, "Error parsing pipeline: %s",
+      error ? error->message : "(invalid error)");
+
+  ret = gst_element_set_state (pipeline, GST_STATE_PLAYING);
+  fail_unless_equals_int (ret, GST_STATE_CHANGE_ASYNC);
+
+  /* run until we receive EOS */
+  bus = gst_element_get_bus (pipeline);
+  fail_if (bus == NULL);
+  msg = gst_bus_timed_pop_filtered (bus, -1, GST_MESSAGE_EOS);
+  gst_message_unref (msg);
+  gst_object_unref (bus);
+
+  src = gst_bin_get_by_name (GST_BIN (pipeline), "src");
+  fail_unless (src != NULL);
+  pad = gst_element_get_static_pad (src, "src");
+  fail_unless (pad != NULL);
+  caps = gst_pad_get_negotiated_caps (pad);
+  GST_LOG ("negotiated audiotestsrc caps: %" GST_PTR_FORMAT, caps);
+  fail_unless (caps != NULL);
+  s = gst_caps_get_structure (caps, 0);
+  fail_unless (gst_structure_get_int (s, "rate", &rate));
+  /* there's no need to resample, audiotestsrc supports any rate, so make
+   * sure audioresample provided upstream with the right caps to negotiate
+   * this correctly */
+  fail_unless_equals_int (rate, 8000);
+  gst_caps_unref (caps);
+  gst_object_unref (pad);
+  gst_object_unref (src);
+
+  gst_element_set_state (pipeline, GST_STATE_NULL);
+  gst_object_unref (pipeline);
+}
+
+GST_END_TEST;
+
 #endif
 
 static Suite *
@@ -692,6 +745,7 @@ audioresample_suite (void)
 #ifndef GST_DISABLE_PARSE
   tcase_set_timeout (tc_chain, 360);
   tcase_add_test (tc_chain, test_pipelines);
+  tcase_add_test (tc_chain, test_preference_passthrough);
 #endif
 
   return s;