ccconverter: fix framerate caps negotiation from non-cdp to cdp

We can only convert from non-cdp to cdp within the confines of valid cdp
framerates.  The existing caps negotiation code was allowing any
framerate to convert to a cdp output which is incorrect and would hit an
assertion later.

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

diff --git a/ext/closedcaption/gstccconverter.c b/ext/closedcaption/gstccconverter.c
index 3b3ad92..c7a3ef5 100644 (file)
--- a/ext/closedcaption/gstccconverter.c
+++ b/ext/closedcaption/gstccconverter.c
@@ -138,6 +138,9 @@ gst_cc_converter_transform_caps (GstBaseTransform * base,
 
   templ = gst_pad_get_pad_template_caps (base->srcpad);
 
+  GST_DEBUG_OBJECT (self, "direction %s from caps %" GST_PTR_FORMAT,
+      direction == GST_PAD_SRC ? "src" : "sink", caps);
+
   res = gst_caps_new_empty ();
   n = gst_caps_get_size (caps);
   for (i = 0; i < n; i++) {
@@ -312,9 +315,13 @@ gst_cc_converter_transform_caps (GstBaseTransform * base,
           /* There's an intersection between the framerates so we can convert
            * into CDP with exactly those framerates */
           cdp_framerate = gst_structure_get_value (t, "framerate");
-          gst_caps_set_value (tmp, "framerate", cdp_framerate);
+          if (gst_value_intersect (NULL, cdp_framerate, framerate)) {
+            gst_caps_set_value (tmp, "framerate", cdp_framerate);
 
-          res = gst_caps_merge (res, tmp);
+            res = gst_caps_merge (res, tmp);
+          } else {
+            gst_clear_caps (&tmp);
+          }
         }
         /* We can always convert CEA708 to all non-CDP formats */
         if (framerate) {

diff --git a/tests/check/elements/ccconverter.c b/tests/check/elements/ccconverter.c
index 556e26d..2b0f6b1 100644 (file)
--- a/tests/check/elements/ccconverter.c
+++ b/tests/check/elements/ccconverter.c
@@ -34,7 +34,7 @@ enum CheckConversionFlags
   FLAG_SEND_EOS = 1,
 };
 
-GST_START_TEST (cdp_requires_framerate)
+GST_START_TEST (cdp_requires_valid_framerate)
 {
   GstHarness *h;
   GstBuffer *buffer;
@@ -74,7 +74,17 @@ GST_START_TEST (cdp_requires_framerate)
   gst_harness_set_src_caps_str (h,
       "closedcaption/x-cea-708,format=(string)cc_data,framerate=(fraction)30/1");
 
-  fail_unless_equals_int (gst_harness_push (h, buffer), GST_FLOW_OK);
+  fail_unless_equals_int (gst_harness_push (h, gst_buffer_ref (buffer)),
+      GST_FLOW_OK);
+
+  /* Then try with an invalid CDP framerate, this should fail */
+  gst_harness_set_sink_caps_str (h,
+      "closedcaption/x-cea-708,format=(string)cdp");
+  gst_harness_set_src_caps_str (h,
+      "closedcaption/x-cea-708,format=(string)cc_data,framerate=(fraction)29/1");
+
+  fail_unless_equals_int (gst_harness_push (h, buffer),
+      GST_FLOW_NOT_NEGOTIATED);
 
   gst_harness_teardown (h);
 }
@@ -960,7 +970,7 @@ ccextractor_suite (void)
 
   suite_add_tcase (s, tc);
 
-  tcase_add_test (tc, cdp_requires_framerate);
+  tcase_add_test (tc, cdp_requires_valid_framerate);
   tcase_add_test (tc, framerate_passthrough);
   tcase_add_test (tc, framerate_changes);
   tcase_add_test (tc, framerate_invalid_format);