From 91e8331b52eb9bf989658b41fc46ac3363e1a1cd Mon Sep 17 00:00:00 2001 From: Matthew Waters Date: Fri, 22 Mar 2024 16:35:54 +1100 Subject: [PATCH] ccconverter: fix cdp->cea608-raw field 1 60fps conversion There was a potential busy loop occuring because when we were taking data from the internal ccbuffer, we were not resetting which field had written data. This would mean that the next time data was retrieved from ccbuffer, it was always from field 0 and never from field 1. This only affects usage of cc_buffer_take_separated() which is only used by cdp->raw cea608. Part-of: --- .../ext/closedcaption/ccutils.c | 14 +++++---- .../ext/closedcaption/gstccconverter.c | 10 +++++-- .../tests/check/elements/ccconverter.c | 29 +++++++++++++++++++ 3 files changed, 45 insertions(+), 8 deletions(-) diff --git a/subprojects/gst-plugins-bad/ext/closedcaption/ccutils.c b/subprojects/gst-plugins-bad/ext/closedcaption/ccutils.c index a4e00c798b..959b767e71 100644 --- a/subprojects/gst-plugins-bad/ext/closedcaption/ccutils.c +++ b/subprojects/gst-plugins-bad/ext/closedcaption/ccutils.c @@ -784,7 +784,7 @@ cc_buffer_get_out_sizes (CCBuffer * buf, const struct cdp_fps_entry *fps_entry, *field1_padding = 0; *field2_padding = 0; - wrote_first = !buf->last_cea608_written_was_field1; + wrote_first = buf->last_cea608_written_was_field1; /* try to push data into the packets. Anything 'extra' will be * stored for later */ while (TRUE) { @@ -795,7 +795,7 @@ cc_buffer_get_out_sizes (CCBuffer * buf, const struct cdp_fps_entry *fps_entry, if (avail_1 + avail_2 >= 2 * fps_entry->max_cea608_count) break; - if (wrote_first) { + if (!wrote_first) { if (extra_cea608_1 > 0) { extra_cea608_1 -= 2; g_assert_cmpint (extra_cea608_1, >=, 0); @@ -821,7 +821,7 @@ cc_buffer_get_out_sizes (CCBuffer * buf, const struct cdp_fps_entry *fps_entry, * requested to start with field2 */ *field2_padding += 2; } - wrote_first = TRUE; + wrote_first = FALSE; } // don't write padding if not requested @@ -868,11 +868,13 @@ cc_buffer_take_separated (CCBuffer * buf, memcpy (cea608_1, buf->cea608_1->data, write_cea608_1_size); memset (&cea608_1[write_cea608_1_size], 0x80, field1_padding); if (write_cea608_1_size == 0) { - buf->field1_padding_written_count += write_cea608_1_size / 2; + buf->field1_padding_written_count += field1_padding / 2; } else { buf->field1_padding_written_count = 0; } *cea608_1_len = write_cea608_1_size + field1_padding; + if (*cea608_1_len > 0) + buf->last_cea608_written_was_field1 = TRUE; } else { *cea608_1_len = 0; } @@ -886,11 +888,13 @@ cc_buffer_take_separated (CCBuffer * buf, memcpy (cea608_2, buf->cea608_2->data, write_cea608_2_size); memset (&cea608_2[write_cea608_2_size], 0x80, field2_padding); if (write_cea608_2_size == 0) { - buf->field2_padding_written_count += write_cea608_2_size / 2; + buf->field2_padding_written_count += field2_padding / 2; } else { buf->field2_padding_written_count = 0; } *cea608_2_len = write_cea608_2_size + field2_padding; + if (*cea608_2_len > 0) + buf->last_cea608_written_was_field1 = FALSE; } else { *cea608_2_len = 0; } diff --git a/subprojects/gst-plugins-bad/ext/closedcaption/gstccconverter.c b/subprojects/gst-plugins-bad/ext/closedcaption/gstccconverter.c index d012e995b1..1a0c4d5e88 100644 --- a/subprojects/gst-plugins-bad/ext/closedcaption/gstccconverter.c +++ b/subprojects/gst-plugins-bad/ext/closedcaption/gstccconverter.c @@ -1255,11 +1255,15 @@ convert_cea708_cdp_cea608_raw (GstCCConverter * self, GstBuffer * inbuf, gst_buffer_map (outbuf, &out, GST_MAP_WRITE); cea608_len = out.size; if (self->out_field == 0) { + guint8 unused_field[MAX_CEA608_LEN]; + guint unused_len = MAX_CEA608_LEN; cc_buffer_take_separated (self->cc_buffer, out_fps_entry, out.data, - &cea608_len, NULL, 0, NULL, 0); + &cea608_len, unused_field, &unused_len, NULL, 0); } else { - cc_buffer_take_separated (self->cc_buffer, out_fps_entry, NULL, 0, out.data, - &cea608_len, NULL, 0); + guint8 unused_field[MAX_CEA608_LEN]; + guint unused_len = MAX_CEA608_LEN; + cc_buffer_take_separated (self->cc_buffer, out_fps_entry, unused_field, + &unused_len, out.data, &cea608_len, NULL, 0); } gst_buffer_unmap (outbuf, &out); self->output_frames++; diff --git a/subprojects/gst-plugins-bad/tests/check/elements/ccconverter.c b/subprojects/gst-plugins-bad/tests/check/elements/ccconverter.c index d74fccb4fe..3d7946cdf7 100644 --- a/subprojects/gst-plugins-bad/tests/check/elements/ccconverter.c +++ b/subprojects/gst-plugins-bad/tests/check/elements/ccconverter.c @@ -1285,6 +1285,34 @@ GST_START_TEST (convert_cea608_raw_field1_cea708_cdp_60fps) GST_END_TEST; +GST_START_TEST (convert_cea708_cdp_cea608_raw_field_one_60fps) +{ + const guint8 in1[] = + { 0x96, 0x69, 0x2b, 0x8f, 0x43, 0x00, 0x00, 0x72, 0xea, 0xfc, 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, 0x00, 0x6e, + }; + const guint8 in2[] = + { 0x96, 0x69, 0x2b, 0x8f, 0x43, 0x00, 0x01, 0x72, 0xea, 0xfd, 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, 0x01, 0x68, + }; + const guint8 *in[] = { in1, in2 }; + guint in_len[] = { sizeof (in1), sizeof (in2) }; + const guint8 out1[] = { 0x81, 0x82 }; + const guint8 *out[] = { out1 }; + guint out_len[] = { sizeof (out1) }; + 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)cdp,framerate=60/1", + "closedcaption/x-cea-608,format=(string)raw,framerate=60/1,field=1", NULL, + NULL, 0); +} + +GST_END_TEST; + static Suite * ccextractor_suite (void) { @@ -1330,6 +1358,7 @@ ccextractor_suite (void) tcase_add_test (tc, convert_cea608_s334_1a_cea608_raw_field_one); tcase_add_test (tc, convert_cea708_cc_data_cea608_raw_field_one); tcase_add_test (tc, convert_cea708_cdp_cea608_raw_field_one); + tcase_add_test (tc, convert_cea708_cdp_cea608_raw_field_one_60fps); tcase_add_test (tc, convert_cea608_raw_cea608_raw_field_one); tcase_add_test (tc, convert_cea608_raw_field1_cea708_cdp_60fps);