ccconverter: fail negotiation when framerate conversion is not possible

Converting between anything but cdp will fail at converting
framerates and negotiation should reflect that.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/-/merge_requests/1393>

diff --git a/ext/closedcaption/gstccconverter.c b/ext/closedcaption/gstccconverter.c
index 8d23130..56591c0 100644 (file)
--- a/ext/closedcaption/gstccconverter.c
+++ b/ext/closedcaption/gstccconverter.c
@@ -111,7 +111,16 @@ gst_cc_converter_transform_caps (GstBaseTransform * base,
          * We can convert everything to CEA608.
          */
         res = gst_caps_merge (res, gst_static_caps_get (&cdp_caps_framerate));
-        res = gst_caps_merge (res, gst_static_caps_get (&non_cdp_caps));
+        if (framerate) {
+          /* we can only keep the same framerate for non-cdp */
+          GstCaps *tmp;
+
+          tmp = gst_caps_make_writable (gst_static_caps_get (&non_cdp_caps));
+          gst_caps_set_value (tmp, "framerate", framerate);
+          res = gst_caps_merge (res, tmp);
+        } else {
+          res = gst_caps_merge (res, gst_static_caps_get (&non_cdp_caps));
+        }
       } else {
         /* SINK: We produce downstream caps
          *
@@ -211,7 +220,16 @@ gst_cc_converter_transform_caps (GstBaseTransform * base,
         } else {
           /* Downstream wants not only CDP, we can do everything */
           res = gst_caps_merge (res, gst_static_caps_get (&cdp_caps_framerate));
-          res = gst_caps_merge (res, gst_static_caps_get (&non_cdp_caps));
+          if (framerate) {
+            /* we can only keep the same framerate for non-cdp */
+            GstCaps *tmp;
+
+            tmp = gst_caps_make_writable (gst_static_caps_get (&non_cdp_caps));
+            gst_caps_set_value (tmp, "framerate", framerate);
+            res = gst_caps_merge (res, tmp);
+          } else {
+            res = gst_caps_merge (res, gst_static_caps_get (&non_cdp_caps));
+          }
         }
       } else {
         GstCaps *tmp;
@@ -254,7 +272,16 @@ gst_cc_converter_transform_caps (GstBaseTransform * base,
           res = gst_caps_merge (res, tmp);
         }
         /* We can always convert CEA708 to all non-CDP formats */
-        res = gst_caps_merge (res, gst_static_caps_get (&non_cdp_caps));
+        if (framerate) {
+          /* we can only keep the same framerate for non-cdp */
+          GstCaps *tmp;
+
+          tmp = gst_caps_make_writable (gst_static_caps_get (&non_cdp_caps));
+          gst_caps_set_value (tmp, "framerate", framerate);
+          res = gst_caps_merge (res, tmp);
+        } else {
+          res = gst_caps_merge (res, gst_static_caps_get (&non_cdp_caps));
+        }
       }
     } else {
       g_assert_not_reached ();

diff --git a/tests/check/elements/ccconverter.c b/tests/check/elements/ccconverter.c
index e44466d..868113e 100644 (file)
--- a/tests/check/elements/ccconverter.c
+++ b/tests/check/elements/ccconverter.c
@@ -195,6 +195,60 @@ GST_START_TEST (framerate_changes)
 
 GST_END_TEST;
 
+GST_START_TEST (framerate_invalid_format)
+{
+  GstHarness *h;
+  GstBuffer *buffer;
+  GstMapInfo map;
+  guint i, j;
+
+  const gchar *failure_caps[] = {
+    /* all of these combinations should fail with different framerates */
+    "closedcaption/x-cea-608,format=(string)raw",
+    "closedcaption/x-cea-608,format=(string)s334-1a",
+    "closedcaption/x-cea-708,format=(string)cc_data",
+  };
+
+  h = gst_harness_new ("ccconverter");
+
+  buffer = gst_buffer_new_and_alloc (3);
+  gst_buffer_map (buffer, &map, GST_MAP_WRITE);
+  map.data[0] = 0x00;
+  map.data[1] = 0x80;
+  map.data[2] = 0x80;
+  gst_buffer_unmap (buffer, &map);
+
+  /* framerate conversion failure cases */
+  for (i = 0; i < G_N_ELEMENTS (failure_caps); i++) {
+    for (j = 0; j < G_N_ELEMENTS (failure_caps); j++) {
+      gchar *srccaps, *sinkcaps;
+
+      srccaps =
+          g_strdup_printf ("%s%s", failure_caps[i],
+          ",framerate=(fraction)30/1");
+      sinkcaps =
+          g_strdup_printf ("%s%s", failure_caps[i],
+          ",framerate=(fraction)60/1");
+
+      GST_INFO ("attempting conversion from %s", srccaps);
+      GST_INFO ("                        to %s", sinkcaps);
+
+      gst_harness_set_src_caps_str (h, srccaps);
+      gst_harness_set_sink_caps_str (h, sinkcaps);
+      fail_unless_equals_int (gst_harness_push (h, gst_buffer_ref (buffer)),
+          GST_FLOW_NOT_NEGOTIATED);
+
+      g_free (srccaps);
+      g_free (sinkcaps);
+    }
+  }
+
+  gst_buffer_unref (buffer);
+  gst_harness_teardown (h);
+}
+
+GST_END_TEST;
+
 static void
 check_conversion_multiple (guint n_in, const guint8 ** in, guint * in_len,
     guint n_out, const guint8 ** out, guint * out_len, const gchar * in_caps,
@@ -903,6 +957,7 @@ ccextractor_suite (void)
   tcase_add_test (tc, cdp_requires_framerate);
   tcase_add_test (tc, framerate_passthrough);
   tcase_add_test (tc, framerate_changes);
+  tcase_add_test (tc, framerate_invalid_format);
   tcase_add_test (tc, convert_cea608_raw_cea608_s334_1a);
   tcase_add_test (tc, convert_cea608_raw_cea708_cc_data);
   tcase_add_test (tc, convert_cea608_raw_cea708_cdp);