Fix and new tests for Nalu prepend

lib/src/includes/signed_video_sign.h

@@ -133,6 +133,12 @@ signed_video_add_nalu_part_for_signing_with_timestamp(signed_video_t *self,
     const int64_t *timestamp,
     bool is_last_part);
 
+SignedVideoReturnCode
+signed_video_add_nalu_for_signing_with_timestamp_for_test(signed_video_t *self,
+    const uint8_t *nalu_data,
+    size_t nalu_data_size,
+    const int64_t *timestamp);
+
 /**
  * This API is identical to signed_video_add_nalu_part_for_signing_with_timestamp() where every call
  * has |is_last_part| = true, that is, every part is a complete NALU.
@@ -267,8 +273,7 @@ signed_video_get_nalu_to_prepend(signed_video_t *self,
  *     // The Second call is to  get the sei.
  *     uint8_t *sei = malloc(sei_size);
  *     status = signed_video_get_sei(sv, sei, &sei_size);
- *     while (status == SV_OK &&
- *         data_size != 0) {
+ *     while (status == SV_OK && sei_size != 0) {
  *         break;
  *       }
  *     }

lib/src/signed_video_h26x_sign.c

@@ -44,6 +44,13 @@ add_payload_to_buffer(signed_video_t *self, uint8_t *payload_ptr, uint8_t *paylo
 static svi_rc
 complete_sei_nalu_and_add_to_prepend(signed_video_t *self);
 
+SignedVideoReturnCode
+signed_video_add_nalu_part_for_signing_with_timestamp__for_test(signed_video_t *self,
+    const uint8_t *nalu_data,
+    size_t nalu_data_size,
+    const int64_t *timestamp,
+    bool is_last_part);
+
 /* Functions related to the list of NALUs to prepend. */
 static svi_rc
 generate_sei_nalu(signed_video_t *self, uint8_t **payload, uint8_t **payload_signature_ptr);
@@ -97,7 +104,26 @@ add_payload_to_buffer(signed_video_t *self, uint8_t *payload, uint8_t *payload_s
   self->sei_data_buffer[self->sei_data_buffer_idx].last_two_bytes = self->last_two_bytes;
   self->sei_data_buffer_idx += 1;
 }
+static void
+add_2_payload_to_buffer(signed_video_t *self, uint8_t *payload, uint8_t *payload_signature_ptr)
+{
+  assert(self);
 
+  if (self->sei_data_buffer_idx >= MAX_NALUS_TO_PREPEND) {
+    // Not enough space for this payload. Free the memory and return.
+    free(payload);
+    return;
+  }
+
+  self->sei_data_buffer[self->sei_data_buffer_idx].payload = payload;
+  self->sei_data_buffer[self->sei_data_buffer_idx].payload_signature_ptr = payload_signature_ptr;
+  self->sei_data_buffer[self->sei_data_buffer_idx].last_two_bytes = self->last_two_bytes;
+  self->sei_data_buffer_idx += 1;
+  self->sei_data_buffer[self->sei_data_buffer_idx].payload = payload;
+  self->sei_data_buffer[self->sei_data_buffer_idx].payload_signature_ptr = payload_signature_ptr;
+  self->sei_data_buffer[self->sei_data_buffer_idx].last_two_bytes = self->last_two_bytes;
+  self->sei_data_buffer_idx += 1;
+}
 /* Picks the oldest payload from the |sei_data_buffer| and completes it with the generated signature
  * and the stop byte. If we have no signature the SEI payload is freed and not added to the
  * video session. */
@@ -167,6 +193,18 @@ shift_sei_buffer_index(signed_video_t *self)
   self->sei_data_buffer_idx -= 1;
 }
 
+static void
+shift_sei_buffer_last_index(signed_video_t *self)
+{
+  const int num_of_completed_seis = self->num_of_completed_seis;
+  for (int j = num_of_completed_seis + 1; j > 0; j--) {
+    self->sei_data_buffer[j] = self->sei_data_buffer[j - 1];
+  }
+  self->sei_data_buffer[0].payload = NULL;
+  self->sei_data_buffer[0].payload_signature_ptr = NULL;
+  self->sei_data_buffer[0].last_two_bytes = LAST_TWO_BYTES_INIT_VALUE;
+}
+
 /* This function generates a SEI NALU of type "user data unregistered". The payload encoded in this
  * SEI is constructed using a set of TLVs. The TLVs are organized as follows;
  *  | metadata | maybe hash_list | signature |
@@ -302,7 +340,9 @@ generate_sei_nalu(signed_video_t *self, uint8_t **payload, uint8_t **payload_sig
 
     // Add reserved byte(s).
     uint8_t reserved_byte = self->sei_epb << 7;
+    reserved_byte |= self->is_start_stream << 6;
     *payload_ptr++ = reserved_byte;
+    self->reseved_byte = &reserved_byte;
 
     size_t written_size =
         tlv_list_encode_or_get_size(self, document_encoders, num_doc_encoders, payload_ptr);
@@ -463,6 +503,16 @@ signed_video_add_nalu_for_signing_with_timestamp(signed_video_t *self,
       self, nalu_data, nalu_data_size, timestamp, true);
 }
 
+SignedVideoReturnCode
+signed_video_add_nalu_for_signing_with_timestamp_for_test(signed_video_t *self,
+    const uint8_t *nalu_data,
+    size_t nalu_data_size,
+    const int64_t *timestamp)
+{
+  return signed_video_add_nalu_part_for_signing_with_timestamp__for_test(
+      self, nalu_data, nalu_data_size, timestamp, true);
+}
+
 SignedVideoReturnCode
 signed_video_add_nalu_part_for_signing_with_timestamp(signed_video_t *self,
     const uint8_t *nalu_data,
@@ -572,6 +622,115 @@ signed_video_add_nalu_part_for_signing_with_timestamp(signed_video_t *self,
   return svi_rc_to_signed_video_rc(status);
 }
 
+SignedVideoReturnCode
+signed_video_add_nalu_part_for_signing_with_timestamp__for_test(signed_video_t *self,
+    const uint8_t *nalu_data,
+    size_t nalu_data_size,
+    const int64_t *timestamp,
+    bool is_last_part)
+{
+  if (!self || !nalu_data || !nalu_data_size) {
+    DEBUG_LOG("Invalid input parameters: (%p, %p, %zu)", self, nalu_data, nalu_data_size);
+    return SV_INVALID_PARAMETER;
+  }
+
+  h26x_nalu_t nalu = {0};
+  // TODO: Consider moving this into parse_nalu_info().
+  if (self->last_nalu->is_last_nalu_part) {
+    // Only check for trailing zeros if this is the last part.
+    nalu = parse_nalu_info(nalu_data, nalu_data_size, self->codec, is_last_part, false);
+    nalu.is_last_nalu_part = is_last_part;
+    copy_nalu_except_pointers(self->last_nalu, &nalu);
+  } else {
+    self->last_nalu->is_first_nalu_part = false;
+    self->last_nalu->is_last_nalu_part = is_last_part;
+    copy_nalu_except_pointers(&nalu, self->last_nalu);
+    nalu.nalu_data = nalu_data;
+    nalu.hashable_data = nalu_data;
+    // Remove any trailing 0x00 bytes at the end of a NALU.
+    while (is_last_part && (nalu_data[nalu_data_size - 1] == 0x00)) {
+      nalu_data_size--;
+    }
+    nalu.hashable_data_size = nalu_data_size;
+  }
+
+  signature_info_t *signature_info = self->signature_info;
+  int signing_present = self->signing_present;
+
+  svi_rc status = SVI_UNKNOWN;
+  SVI_TRY()
+    SVI_THROW(prepare_for_nalus_to_prepend(self));
+
+    SVI_THROW_IF(nalu.is_valid < 0, SVI_INVALID_PARAMETER);
+
+    // Note that |recurrence| is counted in frames and not in NALUs, hence we only increment the
+    // counter for primary slices.
+    if (nalu.is_primary_slice && nalu.is_last_nalu_part) {
+      if ((self->frame_count % self->recurrence) == 0) {
+        self->has_recurrent_data = true;
+      }
+      self->frame_count++;  // It is ok for this variable to wrap around
+    }
+    //
+    SVI_THROW(hash_and_add(self, &nalu));
+    // Depending on the input NALU, we need to take different actions. If the input is an I-NALU we
+    // have a transition to a new GOP. Then we need to generate the necessary SEI-NALU(s) and put in
+    // prepend_list. For all other valid NALUs, simply hash and proceed.
+    if (nalu.is_first_nalu_in_gop && nalu.is_last_nalu_part) {
+      // An I-NALU indicates the start of a new GOP, hence prepend with SEI-NALUs. This also means
+      // that the signing feature is present.
+
+      // Store the timestamp for the first nalu in gop.
+      if (timestamp) {
+        self->gop_info->timestamp = *timestamp;
+        self->gop_info->has_timestamp = true;
+      }
+
+      uint8_t *payload = NULL;
+      uint8_t *payload_signature_ptr = NULL;
+      signing_present = 0;  // About to add SEI NALUs.
+
+      SVI_THROW(generate_sei_nalu(self, &payload, &payload_signature_ptr));
+      // Add |payload| to buffer. Will be picked up again when the signature has been generated.
+      add_2_payload_to_buffer(self, payload, payload_signature_ptr);
+      // Now we are done with the previous GOP. The gop_hash was reset right after signing and
+      // adding it to the SEI NALU. Now it is time to start a new GOP, that is, hash and add this
+      // first NALU of the GOP.
+      SVI_THROW(hash_and_add(self, &nalu));
+    }
+
+    // Only add a SEI if the current NALU is the primary picture NALU and of course if signing is
+    // completed.
+    if ((nalu.nalu_type == NALU_TYPE_I || nalu.nalu_type == NALU_TYPE_P) && nalu.is_primary_slice &&
+        signature_info->signature) {
+      SignedVideoReturnCode signature_error = SV_UNKNOWN_FAILURE;
+      while (sv_interface_get_signature(self->plugin_handle, signature_info->signature,
+          signature_info->max_signature_size, &signature_info->signature_size, &signature_error)) {
+        SVI_THROW(sv_rc_to_svi_rc(signature_error));
+#ifdef SIGNED_VIDEO_DEBUG
+        // TODO: This might not work for blocked signatures, that is if the hash in
+        // |signature_info| does not correspond to the copied |signature|.
+        // Verify the just signed hash.
+        int verified = -1;
+        SVI_THROW_WITH_MSG(sv_rc_to_svi_rc(openssl_verify_hash(signature_info, &verified)),
+            "Verification test had errors");
+        SVI_THROW_IF_WITH_MSG(verified != 1, SVI_EXTERNAL_FAILURE, "Verification test failed");
+#endif
+        SVI_THROW(complete_sei_nalu_and_add_to_prepend(self));
+        signing_present = 1;  // At least one SEI NALU present.
+      }
+    }
+
+  SVI_CATCH()
+  SVI_DONE(status)
+
+  free(nalu.nalu_data_wo_epb);
+
+  if (signing_present > self->signing_present) self->signing_present = signing_present;
+
+  return svi_rc_to_signed_video_rc(status);
+}
+
 SignedVideoReturnCode
 signed_video_get_sei(signed_video_t *self, uint8_t *sei, size_t *sei_size)
 {
@@ -602,7 +761,6 @@ signed_video_get_sei(signed_video_t *self, uint8_t *sei, size_t *sei_size)
 static SignedVideoReturnCode
 signed_video_get_lastest_sei(signed_video_t *self, uint8_t *sei, size_t *sei_size)
 {
-
   if (!self || !sei_size) return SV_INVALID_PARAMETER;
   *sei_size = 0;
   if (self->num_of_completed_seis < 1) {
@@ -611,18 +769,17 @@ signed_video_get_lastest_sei(signed_video_t *self, uint8_t *sei, size_t *sei_siz
   }
   if (!sei) {
     // Assign the SEI size to the provided pointers.
-    *sei_size = self->sei_data_buffer[self->num_of_completed_seis].completed_sei_size;
+    *sei_size = self->sei_data_buffer[self->num_of_completed_seis - 1].completed_sei_size;
     return SV_OK;
   }
   // Assign the SEI size and SEI data to the provided pointers.
-  *sei_size = self->sei_data_buffer[self->num_of_completed_seis].completed_sei_size;
-  memcpy(sei, self->sei_data_buffer[self->num_of_completed_seis].payload, *sei_size);
+  *sei_size = self->sei_data_buffer[self->num_of_completed_seis - 1].completed_sei_size;
+  memcpy(sei, self->sei_data_buffer[self->num_of_completed_seis - 1].payload, *sei_size);
 
   // Reset the fetched SEI information from the sei buffer.
-  signed_video_nalu_data_free(self->sei_data_buffer[self->num_of_completed_seis].payload);
 
   --(self->num_of_completed_seis);
-  shift_sei_buffer_index(self);
+  shift_sei_buffer_last_index(self);
   return SV_OK;
 }
 
@@ -634,7 +791,7 @@ signed_video_get_nalu_to_prepend(signed_video_t *self,
   // Directly pass the members of nalu_to_prepend as arguments to signed_video_get_sei().
   size_t sei_size = 0;
   // Get the size from signed_video_get_sei() and check if its success.
-  SignedVideoReturnCode status = signed_video_get_sei(self, NULL, &sei_size);
+  SignedVideoReturnCode status = signed_video_get_lastest_sei(self, NULL, &sei_size);
   if (SV_OK != status) return status;
   if (0 != sei_size) {
     nalu_to_prepend->nalu_data = malloc(sei_size);

lib/src/signed_video_internal.h

@@ -120,8 +120,9 @@ struct _signed_video_t {
   SignedVideoAuthenticityLevel authenticity_level;
   bool add_public_key_to_sei;
   bool sei_epb;  // Flag that tells whether to generate SEI frames w/wo emulation prevention bytes
-  size_t max_sei_payload_size;  // Default 0 = unlimited
 
+  size_t max_sei_payload_size;  // Default 0 = unlimited
+  uint8_t *reseved_byte;
   sei_data_t sei_data_buffer[MAX_NALUS_TO_PREPEND];
   int sei_data_buffer_idx;
   int num_of_completed_seis;

tests/check/check_h26xsigned_sign.c

@@ -54,7 +54,6 @@ pull_nalus(signed_video_t *sv, int num_nalus_to_pull, int *nalus_pulled)
   SignedVideoReturnCode sv_rc = SV_OK;
   size_t sei_size = 0;
   sv_rc = signed_video_get_sei(sv, NULL, &sei_size);
-  if (SV_OK != sv_rc) return sv_rc;
   int num_pulled_nalus = 0;
   while (sv_rc == SV_OK && 0 < sei_size) {
     uint8_t *sei = malloc(sei_size);
@@ -582,6 +581,53 @@ START_TEST(undefined_nalu_in_sequence)
 }
 END_TEST
 
+START_TEST(two_seis_in_buffer)
+{
+  // This test runs in a loop with loop index _i, corresponding to struct sv_setting _i in
+  // |settings|; See signed_video_helpers.h.
+
+  SignedVideoCodec codec = settings[_i].codec;
+  SignedVideoReturnCode sv_rc;
+  signed_video_nalu_to_prepend_t nalu_to_prepend = {0};
+  signed_video_t *sv = signed_video_create(codec);
+  ck_assert(sv);
+  char *private_key = NULL;
+  size_t private_key_size = 0;
+  nalu_list_item_t *i_nalu = nalu_list_item_create_and_set_id('I', 0, codec);
+
+  // Setup the key
+  sv_rc =
+      signed_video_generate_private_key(settings[_i].algo, "./", &private_key, &private_key_size);
+  ck_assert_int_eq(sv_rc, SV_OK);
+
+  sv_rc = signed_video_set_private_key(sv, settings[_i].algo, private_key, private_key_size);
+  ck_assert_int_eq(sv_rc, SV_OK);
+  sv_rc = signed_video_set_authenticity_level(sv, settings[_i].auth_level);
+  ck_assert_int_eq(sv_rc, SV_OK);
+
+  sv_rc = signed_video_add_nalu_for_signing_with_timestamp_for_test(
+      sv, i_nalu->data, i_nalu->data_size, NULL);
+  ck_assert_int_eq(sv_rc, SV_OK);
+  sv_rc = signed_video_get_nalu_to_prepend(sv, &nalu_to_prepend);
+  ck_assert_int_eq(sv_rc, SV_OK);
+
+  // Get the hashable data (includes the signature)
+  h26x_nalu_t nalu = parse_nalu_info(
+      nalu_to_prepend.nalu_data, nalu_to_prepend.nalu_data_size, codec, false, true);
+
+  // Remove the signature
+  update_hashable_data(&nalu);
+
+  // Verify that hashable data sizes and data contents are identical
+  ck_assert(nalu.hashable_data_size > 0);
+
+  free(nalu.nalu_data_wo_epb);
+  nalu_list_free_item(i_nalu);
+  signed_video_free(sv);
+  free(private_key);
+  free(nalu_to_prepend.nalu_data);
+}
+END_TEST
 /* Test description
  * Verify that the new API for adding a timestamp with the NALU for signing doesn't change the
  * result when the timestamp is not present (NULL) compared to the old API.
@@ -822,6 +868,7 @@ signed_video_suite(void)
   tcase_add_loop_test(tc, correct_multislice_nalu_sequence_without_eos, s, e);
   tcase_add_loop_test(tc, sei_increase_with_gop_length, s, e);
   tcase_add_loop_test(tc, fallback_to_gop_level, s, e);
+  tcase_add_loop_test(tc, two_seis_in_buffer, s, e);
   tcase_add_loop_test(tc, undefined_nalu_in_sequence, s, e);
   tcase_add_loop_test(tc, correct_timestamp, s, e);
   tcase_add_loop_test(tc, correct_signing_nalus_in_parts, s, e);