ccconverter: drop data when overflow on extracting cea608 from cc_data

If the buffer overflows, then drop rather than causing a failure and
fropping the output buffer indefinitely.  This may have caused downstream to
be waiting for data the will never arrive.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/3211>

diff --git a/subprojects/gst-plugins-bad/ext/closedcaption/gstccconverter.c b/subprojects/gst-plugins-bad/ext/closedcaption/gstccconverter.c
index b22baa0..1774364 100644 (file)
--- a/subprojects/gst-plugins-bad/ext/closedcaption/gstccconverter.c
+++ b/subprojects/gst-plugins-bad/ext/closedcaption/gstccconverter.c
@@ -647,6 +647,9 @@ compact_cc_data (guint8 * cc_data, guint cc_data_len)
   return out_len;
 }
 
+#define CC_DATA_EXTRACT_TOO_MANY_FIELD1 -2
+#define CC_DATA_EXTRACT_TOO_MANY_FIELD2 -3
+
 static gint
 cc_data_extract_cea608 (guint8 * cc_data, guint cc_data_len,
     guint8 * cea608_field1, guint * cea608_field1_len,
@@ -687,7 +690,7 @@ cc_data_extract_cea608 (guint8 * cc_data, guint cc_data_len,
         if (*cea608_field1_len + 2 > field_1_len) {
           GST_WARNING ("Too many cea608 input bytes %u for field 1",
               *cea608_field1_len + 2);
-          return -1;
+          return CC_DATA_EXTRACT_TOO_MANY_FIELD1;
         }
 
         if (byte1 != 0x80 || byte2 != 0x80) {
@@ -703,7 +706,7 @@ cc_data_extract_cea608 (guint8 * cc_data, guint cc_data_len,
         if (*cea608_field2_len + 2 > field_2_len) {
           GST_WARNING ("Too many cea608 input bytes %u for field 2",
               *cea608_field2_len + 2);
-          return -1;
+          return CC_DATA_EXTRACT_TOO_MANY_FIELD2;
         }
         if (byte1 != 0x80 || byte2 != 0x80) {
           cea608_field2[(*cea608_field2_len)++] = byte1;
@@ -1510,15 +1513,6 @@ cc_data_to_cea608_ccp (GstCCConverter * self, guint8 * cc_data,
     guint new_cea608_1_len = 0, new_cea608_2_len = 0;
     guint8 *new_cea608_1 = cea608_1, *new_cea608_2 = cea608_2;
 
-    if (cea608_1_len) {
-      new_cea608_1_len = cea608_1_in_size - *cea608_1_len;
-      new_cea608_1 = &cea608_1[*cea608_1_len];
-    }
-    if (cea608_2_len) {
-      new_cea608_2_len = cea608_2_in_size - *cea608_2_len;
-      new_cea608_2 = &cea608_2[*cea608_2_len];
-    }
-
     cc_data_len = compact_cc_data (cc_data, cc_data_len);
 
     if (cc_data_len / 3 > in_fps_entry->max_cc_count) {
@@ -1527,11 +1521,34 @@ cc_data_to_cea608_ccp (GstCCConverter * self, guint8 * cc_data,
       cc_data_len = 3 * in_fps_entry->max_cc_count;
     }
 
-    ccp_offset = cc_data_extract_cea608 (cc_data, cc_data_len, new_cea608_1,
-        &new_cea608_1_len, new_cea608_2, &new_cea608_2_len);
-    if (ccp_offset < 0) {
-      GST_WARNING_OBJECT (self, "Failed to extract cea608 from cc_data");
-      goto fail;
+    while (TRUE) {
+      if (cea608_1_len) {
+        new_cea608_1_len = cea608_1_in_size - *cea608_1_len;
+        new_cea608_1 = &cea608_1[*cea608_1_len];
+      }
+      if (cea608_2_len) {
+        new_cea608_2_len = cea608_2_in_size - *cea608_2_len;
+        new_cea608_2 = &cea608_2[*cea608_2_len];
+      }
+
+      ccp_offset = cc_data_extract_cea608 (cc_data, cc_data_len, new_cea608_1,
+          &new_cea608_1_len, new_cea608_2, &new_cea608_2_len);
+
+      if (ccp_offset == CC_DATA_EXTRACT_TOO_MANY_FIELD1 && cea608_1_len) {
+        GST_WARNING_OBJECT (self, "cea608 field 1 overflow, dropping all "
+            "previously stored field 1 data and trying again");
+        *cea608_1_len = 0;
+      } else if (ccp_offset == CC_DATA_EXTRACT_TOO_MANY_FIELD2 && cea608_2_len) {
+        GST_WARNING_OBJECT (self, "cea608 field 2 overflow, dropping all "
+            "previously stored field 2 data and trying again");
+        *cea608_2_len = 0;
+      } else if (ccp_offset < 0) {
+        GST_WARNING_OBJECT (self, "Failed to extract cea608 from cc_data");
+        goto fail;
+      } else {
+        /* success */
+        break;
+      }
     }
 
     if ((new_cea608_1_len + new_cea608_2_len) / 2 >

diff --git a/subprojects/gst-plugins-bad/tests/check/elements/ccconverter.c b/subprojects/gst-plugins-bad/tests/check/elements/ccconverter.c
index 4d4d38e..a925087 100644 (file)
--- a/subprojects/gst-plugins-bad/tests/check/elements/ccconverter.c
+++ b/subprojects/gst-plugins-bad/tests/check/elements/ccconverter.c
@@ -1073,6 +1073,93 @@ GST_START_TEST (convert_cea708_cc_data_cea708_cdp_double_input_data)
 
 GST_END_TEST;
 
+static guint8
+calculate_cdp_checksum (guint8 * cdp, gsize len)
+{
+  guint8 checksum = 0;
+  gsize i;
+
+  for (i = 0; i < len; i++) {
+    checksum += cdp[i];
+  }
+  checksum &= 0xff;
+  return 256 - checksum;
+}
+
+GST_START_TEST (convert_cea708_cc_data_cea708_cdp_field1_overflow)
+{
+  /* 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.
+   * Also ensure that overflow does something sane, like dropping previous data */
+#define N_INPUTS 100
+  guint8 in_data[N_INPUTS * 3];
+  guint in_len[N_INPUTS];
+  guint8 *in[N_INPUTS];
+  guint i;
+
+#define N_OUTPUTS 100
+  guint8 out_data[N_OUTPUTS * 43];
+  guint out_len[N_OUTPUTS];
+  guint8 *out[N_OUTPUTS];
+
+  const guint8 out_template[] =
+      { 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
+  };
+
+  G_STATIC_ASSERT (sizeof (out_template) == 43);
+
+  /* generate input data */
+  for (i = 0; i < N_INPUTS; i++) {
+    in_len[i] = 3;
+    in_data[i * 3 + 0] = 0xfc;
+    in_data[i * 3 + 1] = 0x81 + i * 2;
+    in_data[i * 3 + 2] = 0x81 + i * 2 + 1;
+    in[i] = &in_data[i * 3];
+  }
+
+  for (i = 0; i < N_OUTPUTS; i++) {
+    out_len[i] = 43;
+    memcpy (&out_data[i * 43], out_template, sizeof (out_template));
+    /* write correct counters */
+    out_data[i * 43 + 6] = i;
+    out_data[i * 43 + 41] = i;
+    /* write the correct cea608 data */
+    if (i % 2 == 0) {
+      gsize in_data_offset;
+      /* take frames sequentially from the input */
+      gsize in_idx = i / 2;
+      /* take the first 16 input frames, then skip the next 16 frames and take
+       * the next 16 frames etc.
+       * 32 is the byte size of the internal cea608 field buffers that we are
+       * overflowing but every second buffer will have cea608 field 1 in it.
+       * 16 frames is 32 bytes stored and is enough to cause overflow */
+      in_idx = (in_idx / 16) * 32 + in_idx % 16;
+      in_data_offset = in_idx * 3;
+
+      out_data[i * 43 + 9] = in_data[in_data_offset + 0];
+      out_data[i * 43 + 10] = in_data[in_data_offset + 1];
+      out_data[i * 43 + 11] = in_data[in_data_offset + 2];
+    } else {
+      out_data[i * 43 + 9] = 0xf9;
+      out_data[i * 43 + 10] = 0x80;
+      out_data[i * 43 + 11] = 0x80;
+    }
+    out_data[i * 43 + 42] = calculate_cdp_checksum (&out_data[i * 43], 42);
+    out[i] = &out_data[i * 43];
+  }
+
+  check_conversion_multiple (G_N_ELEMENTS (in_len), (const guint8 **) in,
+      in_len, G_N_ELEMENTS (out_len), (const guint8 **) 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)
 {
@@ -1110,6 +1197,7 @@ ccextractor_suite (void)
   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);
+  tcase_add_test (tc, convert_cea708_cc_data_cea708_cdp_field1_overflow);
 
   return s;
 }