From 542060fea71c13b8c34b0ea0deb8d39e564c2d5a Mon Sep 17 00:00:00 2001 From: Matthew Waters Date: Thu, 5 May 2022 18:27:50 +1000 Subject: [PATCH] ccconverter: fix framerate passthrough with malformed input If an input is malformed (only produces cea608 field 1 cc_data) then when in passthrough we would effectively be dropping every second cea608 on output as we would not store any unused cea608 data. Fix by having all code paths go through the framerate conversion code which will store and retrieve any relevant data across buffers. Part-of: --- .../ext/closedcaption/gstccconverter.c | 56 +++++++++------------ .../tests/check/elements/ccconverter.c | 57 ++++++++++++++++++++++ 2 files changed, 81 insertions(+), 32 deletions(-) diff --git a/subprojects/gst-plugins-bad/ext/closedcaption/gstccconverter.c b/subprojects/gst-plugins-bad/ext/closedcaption/gstccconverter.c index 66913ab..b22baa0 100644 --- a/subprojects/gst-plugins-bad/ext/closedcaption/gstccconverter.c +++ b/subprojects/gst-plugins-bad/ext/closedcaption/gstccconverter.c @@ -979,32 +979,32 @@ fit_and_scale_cc_data (GstCCConverter * self, /* This is slightly looser than checking for the exact framerate as the cdp * spec allow for 0.1% difference between framerates to be considered equal */ - if (in_fps_entry->max_cc_count == out_fps_entry->max_cc_count) { - if (tc && tc->config.fps_n != 0) - interpolate_time_code_with_framerate (self, tc, out_fps_entry->fps_n, - out_fps_entry->fps_d, 1, 1, &self->current_output_timecode); - - self->scratch_ccp_len = 0; - self->scratch_cea608_1_len = 0; - self->scratch_cea608_2_len = 0; - self->input_frames = 0; - self->output_frames = 0; - } else { + { int input_frame_n, input_frame_d, output_frame_n, output_frame_d; - int output_time_cmp, scale_n, scale_d, rate_cmp; + int output_time_cmp, scale_n, scale_d; /* TODO: handle input discont */ - /* compute the relative frame count for each */ - if (!gst_util_fraction_multiply (self->in_fps_d, self->in_fps_n, - self->input_frames, 1, &input_frame_n, &input_frame_d)) - /* we should never overflow */ - g_assert_not_reached (); + if (self->in_fps_n == 0) { + input_frame_n = self->input_frames; + input_frame_d = 1; + } else { + /* compute the relative frame count for each */ + if (!gst_util_fraction_multiply (self->in_fps_d, self->in_fps_n, + self->input_frames, 1, &input_frame_n, &input_frame_d)) + /* we should never overflow */ + g_assert_not_reached (); + } - if (!gst_util_fraction_multiply (self->out_fps_d, self->out_fps_n, - self->output_frames, 1, &output_frame_n, &output_frame_d)) - /* we should never overflow */ - g_assert_not_reached (); + if (self->in_fps_n == 0) { + output_frame_n = self->output_frames; + output_frame_d = 1; + } else { + if (!gst_util_fraction_multiply (self->out_fps_d, self->out_fps_n, + self->output_frames, 1, &output_frame_n, &output_frame_d)) + /* we should never overflow */ + g_assert_not_reached (); + } output_time_cmp = gst_util_fraction_compare (input_frame_n, input_frame_d, output_frame_n, output_frame_d); @@ -1012,18 +1012,12 @@ fit_and_scale_cc_data (GstCCConverter * self, /* compute the relative rates of the two framerates */ get_framerate_output_scale (self, in_fps_entry, &scale_n, &scale_d); - rate_cmp = gst_util_fraction_compare (scale_n, scale_d, 1, 1); - - GST_TRACE_OBJECT (self, "performing framerate conversion at scale %d/%d " + GST_TRACE_OBJECT (self, "performing conversion at scale %d/%d " "of cc data of with sizes, ccp:%u, cea608-1:%u, cea608-2:%u", scale_n, scale_d, VAL_OR_0 (ccp_data_len), VAL_OR_0 (cea608_1_len), VAL_OR_0 (cea608_2_len)); - if (rate_cmp == 0) { - /* we are not scaling. Should never happen with current conditions - * above */ - g_assert_not_reached (); - } else if (output_time_cmp < 0) { + if (output_time_cmp < 0) { /* we can't generate an output yet */ guint cd_len = ccp_data_len ? *ccp_data_len : 0; guint c1_len = cea608_1_len ? *cea608_1_len : 0; @@ -1038,7 +1032,7 @@ fit_and_scale_cc_data (GstCCConverter * self, if (cea608_2_len) *cea608_2_len = 0; return FALSE; - } else if (rate_cmp != 0) { + } else { /* we are changing the framerate and may overflow the max output packet * size. Split them where necessary. */ gint extra_ccp = 0, extra_cea608_1 = 0, extra_cea608_2 = 0; @@ -1130,8 +1124,6 @@ fit_and_scale_cc_data (GstCCConverter * self, self->scratch_cea608_1_len = 0; self->scratch_cea608_2_len = 0; } - } else { - g_assert_not_reached (); } if (tc && tc->config.fps_n != 0) diff --git a/subprojects/gst-plugins-bad/tests/check/elements/ccconverter.c b/subprojects/gst-plugins-bad/tests/check/elements/ccconverter.c index 127fb97..4d4d38e 100644 --- a/subprojects/gst-plugins-bad/tests/check/elements/ccconverter.c +++ b/subprojects/gst-plugins-bad/tests/check/elements/ccconverter.c @@ -1017,6 +1017,62 @@ GST_START_TEST (convert_cea708_cdp_cea708_cc_data_double_input_data) GST_END_TEST; +GST_START_TEST (convert_cea708_cc_data_cea708_cdp_double_input_data) +{ + /* caps say 60fps, but every buffer is cea608 field 1. Ensure data is taken + * alternatatively from each field even if there is too much input data */ + const guint8 in1[] = { 0xfc, 0x81, 0x82 }; + const guint8 in2[] = { 0xfc, 0x83, 0x84 }; + const guint8 in3[] = { 0xfc, 0x85, 0x86 }; + const guint8 *in[] = { in1, in2, in3, }; + guint in_len[] = { sizeof (in1), sizeof (in2), sizeof (in3), }; + /* two buffers from the first buffer, then the first half of the third input + * buffer */ + const guint8 out1[] = + { 0x96, 0x69, 0x2b, 0x8f, 0x43, 0x00, 0x00, 0x72, 0xea, 0xfc, 0x81, 0x82, + 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, + 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, + 0xfa, 0x00, 0x00, 0x74, 0x00, 0x00, 0x6b + }; + /* padding buffer */ + const guint8 out2[] = + { 0x96, 0x69, 0x2b, 0x8f, 0x43, 0x00, 0x01, 0x72, 0xea, 0xf9, 0x80, 0x80, + 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, + 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, + 0xfa, 0x00, 0x00, 0x74, 0x00, 0x01, 0x6f + }; + const guint8 out3[] = + { 0x96, 0x69, 0x2b, 0x8f, 0x43, 0x00, 0x02, 0x72, 0xea, 0xfc, 0x83, 0x84, + 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, + 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, + 0xfa, 0x00, 0x00, 0x74, 0x00, 0x02, 0x63 + }; + const guint8 out4[] = + { 0x96, 0x69, 0x2b, 0x8f, 0x43, 0x00, 0x03, 0x72, 0xea, 0xf9, 0x80, 0x80, + 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, + 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, + 0xfa, 0x00, 0x00, 0x74, 0x00, 0x03, 0x6b + }; + const guint8 out5[] = + { 0x96, 0x69, 0x2b, 0x8f, 0x43, 0x00, 0x04, 0x72, 0xea, 0xfc, 0x85, 0x86, + 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, + 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, + 0xfa, 0x00, 0x00, 0x74, 0x00, 0x04, 0x5b + }; + const guint8 *out[] = { out1, out2, out3, out4, out5 }; + guint out_len[] = + { sizeof (out1), sizeof (out2), sizeof (out3), sizeof (out4), + sizeof (out5) + }; + check_conversion_multiple (G_N_ELEMENTS (in_len), in, in_len, + G_N_ELEMENTS (out_len), out, out_len, + "closedcaption/x-cea-708,format=(string)cc_data,framerate=(fraction)60/1", + "closedcaption/x-cea-708,format=(string)cdp,framerate=(fraction)60/1", + NULL, NULL, FLAG_SEND_EOS); +} + +GST_END_TEST; + static Suite * ccextractor_suite (void) { @@ -1053,6 +1109,7 @@ ccextractor_suite (void) tcase_add_test (tc, convert_cea608_raw_cea708_cdp_double_framerate); tcase_add_test (tc, convert_cea608_s334_1a_cea708_cdp_double_framerate); tcase_add_test (tc, convert_cea708_cdp_cea708_cc_data_double_input_data); + tcase_add_test (tc, convert_cea708_cc_data_cea708_cdp_double_input_data); return s; } -- 2.7.4