From 8fd2c68968ae922bcf52c628575afc212da35d3e Mon Sep 17 00:00:00 2001 From: Matthew Waters Date: Fri, 2 Jul 2021 13:10:25 +1000 Subject: [PATCH] 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: --- ext/closedcaption/gstccconverter.c | 11 +++++++++-- tests/check/elements/ccconverter.c | 16 +++++++++++++--- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/ext/closedcaption/gstccconverter.c b/ext/closedcaption/gstccconverter.c index 3b3ad92..c7a3ef5 100644 --- 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 --- 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); -- 2.7.4