From 18b54f8d347030c73d8afbeaf55df57aa5acbb96 Mon Sep 17 00:00:00 2001
From: Jan Schmidt <jan@centricular.com>
Date: Wed, 31 Jul 2019 00:38:44 +1000
Subject: [PATCH] h265parser: Skip unused SEI bits differently

3-byte emulation bytes can confuse the current code that skips
bits at the end of an SEI. Use a simpler method that's also
quicker because it skips all remaining bits in one go instead
of 1 bit at a time.
---
 gst-libs/gst/codecparsers/gsth265parser.c | 59 +++++++++--------------
 1 file changed, 24 insertions(+), 35 deletions(-)

diff --git a/gst-libs/gst/codecparsers/gsth265parser.c b/gst-libs/gst/codecparsers/gsth265parser.c
index e87dd0d77a..df8e99dbab 100644
--- a/gst-libs/gst/codecparsers/gsth265parser.c
+++ b/gst-libs/gst/codecparsers/gsth265parser.c
@@ -2426,28 +2426,17 @@ error:
   return GST_H265_PARSER_ERROR;
 }
 
-static gboolean
-nal_reader_has_more_data_in_payload (NalReader * nr,
-    guint32 payload_start_pos_bit, guint32 payloadSize)
-{
-  if (nal_reader_is_byte_aligned (nr) &&
-      (nal_reader_get_pos (nr) == (payload_start_pos_bit + 8 * payloadSize)))
-    return FALSE;
-
-  return TRUE;
-}
-
 static GstH265ParserResult
 gst_h265_parser_parse_sei_message (GstH265Parser * parser,
     guint8 nal_type, NalReader * nr, GstH265SEIMessage * sei)
 {
-  guint32 payloadSize;
+  guint32 payloadSizeBytes;
   guint8 payload_type_byte, payload_size_byte;
-  guint remaining, payload_size;
-  guint32 payload_start_pos_bit;
+  guint32 remaining, payload_size_bits, next;
   GstH265ParserResult res = GST_H265_PARSER_OK;
 
-  GST_DEBUG ("parsing \"Sei message\"");
+  GST_DEBUG ("parsing \"SEI message\" %u bits available",
+      nal_reader_get_remaining (nr));
 
   memset (sei, 0, sizeof (*sei));
 
@@ -2455,20 +2444,21 @@ gst_h265_parser_parse_sei_message (GstH265Parser * parser,
     READ_UINT8 (nr, payload_type_byte, 8);
     sei->payloadType += payload_type_byte;
   } while (payload_type_byte == 0xff);
-  payloadSize = 0;
+  payloadSizeBytes = 0;
   do {
     READ_UINT8 (nr, payload_size_byte, 8);
-    payloadSize += payload_size_byte;
+    payloadSizeBytes += payload_size_byte;
   }
   while (payload_size_byte == 0xff);
 
   remaining = nal_reader_get_remaining (nr);
-  payload_size = payloadSize * 8 < remaining ? payloadSize * 8 : remaining;
+  payload_size_bits =
+      payloadSizeBytes * 8 < remaining ? payloadSizeBytes * 8 : remaining;
+  next = nal_reader_get_pos (nr) + payload_size_bits;
 
-  payload_start_pos_bit = nal_reader_get_pos (nr);
   GST_DEBUG
-      ("SEI message received: payloadType  %u, payloadSize = %u bytes",
-      sei->payloadType, payload_size);
+      ("SEI message received: payloadType  %u, payloadSize = %u bits",
+      sei->payloadType, payload_size_bits);
 
   if (nal_type == GST_H265_NAL_PREFIX_SEI) {
     switch (sei->payloadType) {
@@ -2501,7 +2491,7 @@ gst_h265_parser_parse_sei_message (GstH265Parser * parser,
       default:
         /* Just consume payloadSize bytes, which does not account for
            emulation prevention bytes */
-        if (!nal_reader_skip_long (nr, payload_size))
+        if (!nal_reader_skip_long (nr, payload_size_bits))
           goto error;
         res = GST_H265_PARSER_OK;
         break;
@@ -2511,7 +2501,7 @@ gst_h265_parser_parse_sei_message (GstH265Parser * parser,
       default:
         /* Just consume payloadSize bytes, which does not account for
            emulation prevention bytes */
-        if (!nal_reader_skip_long (nr, payload_size))
+        if (!nal_reader_skip_long (nr, payload_size_bits))
           goto error;
         res = GST_H265_PARSER_OK;
         break;
@@ -2524,19 +2514,18 @@ gst_h265_parser_parse_sei_message (GstH265Parser * parser,
    * it is present, the size will be less than total PayloadSize since the
    * size of reserved_payload_extension is supposed to be
    * 8 * payloadSize - nEarlierBits - nPayloadZeroBits -1 which means the
-   * the current implementation will still skip all unnecessary bits correctly.
-   * In theory, we can have a more optimized implementation by skipping the
-   * data left in PayLoadSize without out individually checking for each bits,
-   * since the totoal size will be always less than payloadSize*/
-  if (nal_reader_has_more_data_in_payload (nr, payload_start_pos_bit,
-          payloadSize)) {
-    /* Skip the byte alignment bits */
-    if (!nal_reader_skip (nr, 1))
+   * the current implementation will still skip all unnecessary bits correctly. */
+
+  /* Always make sure all the advertised SEI bits
+   * were consumed during parsing. This is sufficient to skip to the next
+   * byte aligned position after the SEI payload because we start
+   * at a byte-aligned position and calculate the 'next' position as a
+   * multiple of 8 bits, and this correctly skips any three-byte emulation
+   * bytes encountered without getting confused. */
+  if (next > nal_reader_get_pos (nr)) {
+    GST_LOG ("Skipping %u unused SEI bits", next - nal_reader_get_pos (nr));
+    if (!nal_reader_skip_long (nr, next - nal_reader_get_pos (nr)))
       goto error;
-    while (!nal_reader_is_byte_aligned (nr)) {
-      if (!nal_reader_skip (nr, 1))
-        goto error;
-    }
   }
 
   return res;