ccconverter: write the cdp timecode data correctly

We were mixing up the tens part with the unit parts all over the place.

e.g. 12 seconds would be encoded as 0x21 instead of the correct 0x12

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

diff --git a/ext/closedcaption/gstccconverter.c b/ext/closedcaption/gstccconverter.c
index 6177c5b..2a39433 100644 (file)
--- a/ext/closedcaption/gstccconverter.c
+++ b/ext/closedcaption/gstccconverter.c
@@ -1016,25 +1016,42 @@ convert_cea708_cc_data_cea708_cdp_internal (GstCCConverter * self,
   gst_byte_writer_put_uint16_be_unchecked (&bw, self->cdp_hdr_sequence_cntr);
 
   if (tc && tc->config.fps_n > 0) {
-    gst_byte_writer_put_uint8_unchecked (&bw, 0x71);
-    gst_byte_writer_put_uint8_unchecked (&bw, 0xc0 |
-        (((tc->hours % 10) & 0x3) << 4) |
-        ((tc->hours - (tc->hours % 10)) & 0xf));
-
-    gst_byte_writer_put_uint8_unchecked (&bw, 0x80 |
-        (((tc->minutes % 10) & 0x7) << 4) |
-        ((tc->minutes - (tc->minutes % 10)) & 0xf));
-
-    gst_byte_writer_put_uint8_unchecked (&bw,
-        (tc->field_count <
-            2 ? 0x00 : 0x80) | (((tc->seconds %
-                    10) & 0x7) << 4) | ((tc->seconds -
-                (tc->seconds % 10)) & 0xf));
+    guint8 u8;
 
-    gst_byte_writer_put_uint8_unchecked (&bw,
-        ((tc->config.flags & GST_VIDEO_TIME_CODE_FLAGS_DROP_FRAME) ? 0x80 :
-            0x00) | (((tc->frames % 10) & 0x3) << 4) | ((tc->frames -
-                (tc->frames % 10)) & 0xf));
+    gst_byte_writer_put_uint8_unchecked (&bw, 0x71);
+    /* reserved 11 - 2 bits */
+    u8 = 0xc0;
+    /* tens of hours - 2 bits */
+    u8 |= ((tc->hours / 10) & 0x3) << 4;
+    /* units of hours - 4 bits */
+    u8 |= (tc->hours % 10) & 0xf;
+    gst_byte_writer_put_uint8_unchecked (&bw, u8);
+
+    /* reserved 1 - 1 bit */
+    u8 = 0x80;
+    /* tens of minutes - 3 bits */
+    u8 |= ((tc->minutes / 10) & 0x7) << 4;
+    /* units of minutes - 4 bits */
+    u8 |= (tc->minutes % 10) & 0xf;
+    gst_byte_writer_put_uint8_unchecked (&bw, u8);
+
+    /* field flag - 1 bit */
+    u8 = tc->field_count < 2 ? 0x00 : 0x80;
+    /* tens of seconds - 3 bits */
+    u8 |= ((tc->seconds / 10) & 0x7) << 4;
+    /* units of seconds - 4 bits */
+    u8 |= (tc->seconds % 10) & 0xf;
+    gst_byte_writer_put_uint8_unchecked (&bw, u8);
+
+    /* drop frame flag - 1 bit */
+    u8 = (tc->config.flags & GST_VIDEO_TIME_CODE_FLAGS_DROP_FRAME) ? 0x80 :
+        0x00;
+    /* reserved0 - 1 bit */
+    /* tens of frames - 2 bits */
+    u8 |= ((tc->frames / 10) & 0x3) << 4;
+    /* units of frames 4 bits */
+    u8 |= (tc->frames % 10) & 0xf;
+    gst_byte_writer_put_uint8_unchecked (&bw, u8);
   }
 
   gst_byte_writer_put_uint8_unchecked (&bw, 0x72);
@@ -2103,10 +2120,9 @@ gst_cc_converter_transform (GstCCConverter * self, GstBuffer * inbuf,
       else
         get_framerate_output_scale (self, in_fps_entry, &scale_n, &scale_d);
 
-      if (tc_meta)
-        interpolate_time_code_with_framerate (self, &tc_meta->tc,
-            self->out_fps_n, self->out_fps_d, scale_n, scale_d,
-            &self->current_output_timecode);
+      interpolate_time_code_with_framerate (self, &tc_meta->tc,
+          self->out_fps_n, self->out_fps_d, scale_n, scale_d,
+          &self->current_output_timecode);
     }
   }
 

diff --git a/tests/check/elements/ccconverter.c b/tests/check/elements/ccconverter.c
index 868113e..556e26d 100644 (file)
--- a/tests/check/elements/ccconverter.c
+++ b/tests/check/elements/ccconverter.c
@@ -526,17 +526,17 @@ GST_START_TEST (convert_cea708_cdp_cea708_cdp_double_framerate)
   GstVideoTimeCode in_tc1;
   const GstVideoTimeCode *in_tc[] = { &in_tc1 };
 
-  const guint8 out1[] = { 0x96, 0x69, 0x30, 0x8f, 0xc3, 0x00, 0x00, 0x71, 0xd0,
-    0xa0, 0x30, 0x00, 0x72, 0xea, 0xfc, 0x01, 0x02, 0xfe, 0x05, 0x06, 0xfe,
+  const guint8 out1[] = { 0x96, 0x69, 0x30, 0x8f, 0xc3, 0x00, 0x00, 0x71, 0xc1,
+    0x82, 0x03, 0x08, 0x72, 0xea, 0xfc, 0x01, 0x02, 0xfe, 0x05, 0x06, 0xfe,
     0x07, 0x08, 0xfe, 0x09, 0x0a, 0xfe, 0x0b, 0x0c, 0xfe, 0x0d, 0x0e, 0xfe,
     0x0f, 0x10, 0xfe, 0x11, 0x12, 0xfe, 0x13, 0x14, 0xfe, 0x15, 0x16, 0x74,
-    0x00, 0x00, 0xbe
+    0x00, 0x00, 0x10
   };
-  const guint8 out2[] = { 0x96, 0x69, 0x30, 0x8f, 0xc3, 0x00, 0x01, 0x71, 0xd0,
-    0xa0, 0x30, 0x10, 0x72, 0xea, 0xfc, 0x03, 0x04, 0xfe, 0x17, 0x18, 0xfe,
+  const guint8 out2[] = { 0x96, 0x69, 0x30, 0x8f, 0xc3, 0x00, 0x01, 0x71, 0xc1,
+    0x82, 0x03, 0x09, 0x72, 0xea, 0xfc, 0x03, 0x04, 0xfe, 0x17, 0x18, 0xfe,
     0x19, 0x1a, 0xfe, 0x1b, 0x1c, 0xfe, 0x1d, 0x1e, 0xfe, 0x1f, 0x20, 0xfe,
     0x21, 0x22, 0xfe, 0x23, 0x24, 0xfe, 0x25, 0x26, 0xfe, 0x27, 0x28, 0x74,
-    0x00, 0x01, 0x64
+    0x00, 0x01, 0xc5
   };
   const guint8 *out[] = { out1, out2 };
   guint out_len[] = { sizeof (out1), sizeof (out2) };
@@ -587,13 +587,13 @@ GST_START_TEST (convert_cea708_cdp_cea708_cdp_half_framerate)
   const GstVideoTimeCode *in_tc[] = { &in_tc1, &in_tc2 };
 
   const guint8 out1[] =
-      { 0x96, 0x69, 0x4e, 0x5f, 0xc3, 0x00, 0x00, 0x71, 0xd0, 0xa0, 0x30, 0x00,
+      { 0x96, 0x69, 0x4e, 0x5f, 0xc3, 0x00, 0x00, 0x71, 0xc1, 0x82, 0x03, 0x04,
     0x72, 0xf4, 0xfc, 0x01, 0x02, 0xfc, 0x14, 0x15, 0xfe, 0x03, 0x04, 0xfe,
     0x05, 0x06, 0xfe, 0x07, 0x08, 0xfe, 0x09, 0x0a, 0xfe, 0x0b, 0x0c, 0xfe,
     0x0d, 0x0e, 0xfe, 0x0f, 0x10, 0xfe, 0x11, 0x12, 0xfe, 0x13, 0x14, 0xfe,
     0x16, 0x17, 0xfe, 0x18, 0x19, 0xfe, 0x1a, 0x1b, 0xfe, 0x1c, 0x1d, 0xfe,
     0x1e, 0x1f, 0xfe, 0x20, 0x21, 0xfe, 0x22, 0x23, 0xfe, 0x24, 0x25, 0xfe,
-    0x26, 0x27, 0x74, 0x00, 0x00, 0xb2
+    0x26, 0x27, 0x74, 0x00, 0x00, 0x08
   };
   const guint8 *out[] = { out1 };
   guint out_len[] = { sizeof (out1) };
@@ -776,42 +776,48 @@ GST_START_TEST (convert_cea708_cdp_cea708_cdp_from_drop_frame_scaling)
   const GstVideoTimeCode *in_tc[] = { &in_tc1, &in_tc2 };
 
   const guint8 out1[] =
-      { 0x96, 0x69, 0x30, 0x8f, 0xc3, 0x00, 0x00, 0x71, 0xc0, 0x90, 0x12, 0x12,
-    0x72, 0xea, 0xfc, 0x80, 0x80, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa,
+      { 0x96, 0x69, 0x4e, 0x5f, 0xc3, 0x00, 0x00, 0x71, 0xc0, 0x81, 0x59, 0x29,
+    0x72, 0xf4, 0xfc, 0x80, 0x80, 0xf8, 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, 0x74, 0x00, 0x00, 0x04
+    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, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa,
+    0x00, 0x00, 0x74, 0x00, 0x00, 0xfb
   };
   const guint8 out2[] =
-      { 0x96, 0x69, 0x30, 0x8f, 0xc3, 0x00, 0x01, 0x71, 0xc0, 0xa0, 0x00, 0x00,
-    0x72, 0xea, 0xfc, 0x80, 0x80, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa,
+      { 0x96, 0x69, 0x4e, 0x5f, 0xc3, 0x00, 0x01, 0x71, 0xc0, 0x82, 0x00, 0x00,
+    0x72, 0xf4, 0xfc, 0x80, 0x80, 0xf8, 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, 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, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0x74, 0x00, 0x01, 0x16
+    0x00, 0x00, 0x74, 0x00, 0x01, 0x7a
   };
   const guint8 *out[] = { out1, out2 };
   guint out_len[] = { sizeof (out1), sizeof (out2) };
   GstVideoTimeCode out_tc1, out_tc2;
   const GstVideoTimeCode *out_tc[] = { &out_tc1, &out_tc2 };
 
-  gst_video_time_code_init (&in_tc1, 60000, 1001, NULL,
-      GST_VIDEO_TIME_CODE_FLAGS_DROP_FRAME, 0, 1, 59, 59, 0);
+  gst_video_time_code_init (&in_tc1, 30000, 1001, NULL,
+      GST_VIDEO_TIME_CODE_FLAGS_DROP_FRAME, 0, 1, 59, 29, 0);
   fail_unless (gst_video_time_code_is_valid (&in_tc1));
 
-  gst_video_time_code_init (&in_tc2, 60000, 1001, NULL,
+  gst_video_time_code_init (&in_tc2, 30000, 1001, NULL,
       GST_VIDEO_TIME_CODE_FLAGS_DROP_FRAME, 0, 2, 0, 4, 0);
   fail_unless (gst_video_time_code_is_valid (&in_tc2));
 
-  gst_video_time_code_init (&out_tc1, 60, 1, NULL,
-      GST_VIDEO_TIME_CODE_FLAGS_NONE, 0, 1, 59, 59, 0);
+  gst_video_time_code_init (&out_tc1, 30, 1, NULL,
+      GST_VIDEO_TIME_CODE_FLAGS_NONE, 0, 1, 59, 29, 0);
   fail_unless (gst_video_time_code_is_valid (&out_tc1));
 
-  gst_video_time_code_init (&out_tc2, 60, 1, NULL,
+  gst_video_time_code_init (&out_tc2, 30, 1, NULL,
       GST_VIDEO_TIME_CODE_FLAGS_NONE, 0, 2, 0, 0, 0);
   fail_unless (gst_video_time_code_is_valid (&out_tc2));
 
   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=(fraction)60000/1001",
-      "closedcaption/x-cea-708,format=(string)cdp,framerate=(fraction)60/1",
+      "closedcaption/x-cea-708,format=(string)cdp,framerate=(fraction)30000/1001",
+      "closedcaption/x-cea-708,format=(string)cdp,framerate=(fraction)30/1",
       in_tc, out_tc, FLAG_SEND_EOS);
 
   gst_video_time_code_clear (&in_tc1);