aom: Avoid the notion of layer-specific OBUs

From 14190db24892bf457e21b846efb74351a28bbbf1 Mon Sep 17 00:00:00 2001
From: Wan-Teh Chang <[EMAIL REDACTED]>
Date: Thu, 13 Jun 2024 11:55:05 -0700
Subject: [PATCH] Avoid the notion of layer-specific OBUs

The notion of layer-specific OBUs was introduced in a proposed change to
the AV1 spec: https://github.com/AOMediaCodec/av1-spec/commit/86fb0ac.
Avoid mentioning a term that is not defined in the current version of
the AV1 spec. (I also forgot to define the term in commit 782840b.)

Remove a confusing assertion in init_large_scale_tile_obu_header() that
was added in commit 782840b. I will fix the use of obu_extension=0 (the
reason for adding that assertion) in the function in a separate CL.

Bug: aomedia:3076, aomedia:3582
Change-Id: Ifbf534e4d2b57c18d46a955dc5b28e3e8e0a3f31
---
 av1/av1_cx_iface.c      |  1 -
 av1/encoder/bitstream.c | 43 ++++++++++++++++++++++++-----------------
 av1/encoder/bitstream.h |  3 +--
 av1/encoder/encoder.c   |  3 +--
 4 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/av1/av1_cx_iface.c b/av1/av1_cx_iface.c
index 6205257ad..f6ef0ca4e 100644
--- a/av1/av1_cx_iface.c
+++ b/av1/av1_cx_iface.c
@@ -3356,7 +3356,6 @@ static aom_codec_err_t encoder_encode(aom_codec_alg_priv_t *ctx,
         obu_header_size = av1_write_obu_header(
             &ppi->level_params, &cpi->frame_header_count,
             OBU_TEMPORAL_DELIMITER,
-            /*is_layer_specific_obu=*/false,
             ppi->seq_params.has_nonzero_operating_point_idc, 0, ctx->cx_data);
 
         // OBUs are preceded/succeeded by an unsigned leb128 coded integer.
diff --git a/av1/encoder/bitstream.c b/av1/encoder/bitstream.c
index 4f5ec12d7..7c02eeb42 100644
--- a/av1/encoder/bitstream.c
+++ b/av1/encoder/bitstream.c
@@ -3356,10 +3356,8 @@ static int remux_tiles(const CommonTileParams *const tiles, uint8_t *dst,
 
 uint32_t av1_write_obu_header(AV1LevelParams *const level_params,
                               int *frame_header_count, OBU_TYPE obu_type,
-                              bool is_layer_specific_obu,
                               bool has_nonzero_operating_point_idc,
                               int obu_extension, uint8_t *const dst) {
-  assert(IMPLIES(!is_layer_specific_obu, obu_extension == 0));
   assert(IMPLIES(!has_nonzero_operating_point_idc, obu_extension == 0));
 
   if (level_params->keep_level_stats &&
@@ -3368,8 +3366,30 @@ uint32_t av1_write_obu_header(AV1LevelParams *const level_params,
 
   struct aom_write_bit_buffer wb = { dst, 0 };
   uint32_t size = 0;
-  int obu_extension_flag =
-      has_nonzero_operating_point_idc && is_layer_specific_obu;
+
+  // The AV1 spec Version 1.0.0 with Errata 1 has the following requirements on
+  // the OBU extension header:
+  //
+  // 6.4.1. General sequence header OBU semantics:
+  //   It is a requirement of bitstream conformance that if OperatingPointIdc
+  //   is equal to 0, then obu_extension_flag is equal to 0 for all OBUs that
+  //   follow this sequence header until the next sequence header.
+  //
+  // 7.5. Ordering of OBUs:
+  //   If a coded video sequence contains at least one enhancement layer (OBUs
+  //   with spatial_id greater than 0 or temporal_id greater than 0) then all
+  //   frame headers and tile group OBUs associated with base (spatial_id
+  //   equals 0 and temporal_id equals 0) and enhancement layer (spatial_id
+  //   greater than 0 or temporal_id greater than 0) data must include the OBU
+  //   extension header.
+  //
+  // Set obu_extension_flag to satisfy these requirements.
+  int obu_extension_flag = 0;
+  if (has_nonzero_operating_point_idc) {
+    obu_extension_flag =
+        (obu_type == OBU_FRAME_HEADER || obu_type == OBU_TILE_GROUP ||
+         obu_type == OBU_FRAME || obu_type == OBU_REDUNDANT_FRAME_HEADER);
+  }
 
   aom_wb_write_literal(&wb, 0, 1);  // forbidden bit.
   aom_wb_write_literal(&wb, (int)obu_type, 4);
@@ -3544,14 +3564,9 @@ static uint32_t init_large_scale_tile_obu_header(
   // For large_scale_tile case, we always have only one tile group, so it can
   // be written as an OBU_FRAME.
   const OBU_TYPE obu_type = OBU_FRAME;
-  // We pass obu_extension=0 to av1_write_obu_header(), so
-  // has_nonzero_operating_point_idc must be false.
-  assert(!cpi->common.seq_params->has_nonzero_operating_point_idc);
   lst_obu->tg_hdr_size = av1_write_obu_header(
       level_params, &cpi->frame_header_count, obu_type,
-      /*is_layer_specific_obu=*/true,
-      cpi->common.seq_params->has_nonzero_operating_point_idc,
-      /*obu_extension=*/0, *data);
+      cpi->common.seq_params->has_nonzero_operating_point_idc, 0, *data);
   *data += lst_obu->tg_hdr_size;
 
   const uint32_t frame_header_size =
@@ -3749,7 +3764,6 @@ void av1_write_obu_tg_tile_headers(AV1_COMP *const cpi, MACROBLOCKD *const xd,
   const OBU_TYPE obu_type = (cpi->num_tg == 1) ? OBU_FRAME : OBU_TILE_GROUP;
   *curr_tg_hdr_size = av1_write_obu_header(
       &cpi->ppi->level_params, &cpi->frame_header_count, obu_type,
-      /*is_layer_specific_obu=*/true,
       cm->seq_params->has_nonzero_operating_point_idc,
       pack_bs_params->obu_extn_header, pack_bs_params->tile_data_curr);
   pack_bs_params->obu_header_size = *curr_tg_hdr_size;
@@ -3852,7 +3866,6 @@ void av1_write_last_tile_info(
     av1_write_obu_header(
         &cpi->ppi->level_params, &cpi->frame_header_count,
         OBU_REDUNDANT_FRAME_HEADER,
-        /*is_layer_specific_obu=*/true,
         cpi->common.seq_params->has_nonzero_operating_point_idc,
         obu_extn_header, &curr_tg_start[fh_info->obu_header_byte_offset]);
 
@@ -4153,12 +4166,8 @@ static size_t av1_write_metadata_array(AV1_COMP *const cpi, uint8_t *dst) {
           (cm->current_frame.frame_type != KEY_FRAME &&
            current_metadata->insert_flag == AOM_MIF_NON_KEY_FRAME) ||
           current_metadata->insert_flag == AOM_MIF_ANY_FRAME) {
-        // Whether METADATA_TYPE_ITUT_T35 is layer-specific or not is
-        // payload-specific. Other metadata types are not layer-specific.
-        const bool is_layer_specific_obu = false;
         obu_header_size = av1_write_obu_header(
             &cpi->ppi->level_params, &cpi->frame_header_count, OBU_METADATA,
-            is_layer_specific_obu,
             cm->seq_params->has_nonzero_operating_point_idc, 0, dst);
         obu_payload_size =
             av1_write_metadata_obu(current_metadata, dst + obu_header_size);
@@ -4209,7 +4218,6 @@ int av1_pack_bitstream(AV1_COMP *const cpi, uint8_t *dst, size_t *size,
       cm->current_frame.frame_type == KEY_FRAME) {
     obu_header_size = av1_write_obu_header(
         level_params, &cpi->frame_header_count, OBU_SEQUENCE_HEADER,
-        /*is_layer_specific_obu=*/false,
         cm->seq_params->has_nonzero_operating_point_idc, 0, data);
     obu_payload_size =
         av1_write_sequence_header_obu(cm->seq_params, data + obu_header_size);
@@ -4235,7 +4243,6 @@ int av1_pack_bitstream(AV1_COMP *const cpi, uint8_t *dst, size_t *size,
     fh_info.frame_header = data;
     obu_header_size = av1_write_obu_header(
         level_params, &cpi->frame_header_count, OBU_FRAME_HEADER,
-        /*is_layer_specific_obu=*/true,
         cm->seq_params->has_nonzero_operating_point_idc, obu_extension_header,
         data);
     obu_payload_size = write_frame_header_obu(cpi, &cpi->td.mb.e_mbd, &saved_wb,
diff --git a/av1/encoder/bitstream.h b/av1/encoder/bitstream.h
index 232c43040..a8f3cc541 100644
--- a/av1/encoder/bitstream.h
+++ b/av1/encoder/bitstream.h
@@ -91,11 +91,10 @@ uint32_t av1_write_sequence_header_obu(const SequenceHeader *seq_params,
                                        uint8_t *const dst);
 
 // Writes the OBU header byte, and the OBU header extension byte when
-// has_nonzero_operating_point_idc is true and the OBU is layer-specific.
+// has_nonzero_operating_point_idc is true and the OBU is part of a frame.
 // Returns number of bytes written to 'dst'.
 uint32_t av1_write_obu_header(AV1LevelParams *const level_params,
                               int *frame_header_count, OBU_TYPE obu_type,
-                              bool is_layer_specific_obu,
                               bool has_nonzero_operating_point_idc,
                               int obu_extension, uint8_t *const dst);
 
diff --git a/av1/encoder/encoder.c b/av1/encoder/encoder.c
index 1a132fe88..fd293b194 100644
--- a/av1/encoder/encoder.c
+++ b/av1/encoder/encoder.c
@@ -558,7 +558,6 @@ void av1_init_seq_coding_tools(AV1_PRIMARY *const ppi,
     int i = 0;
     assert(seq->operating_points_cnt_minus_1 ==
            (int)(ppi->number_spatial_layers * ppi->number_temporal_layers - 1));
-    seq->has_nonzero_operating_point_idc = true;
     for (unsigned int sl = 0; sl < ppi->number_spatial_layers; sl++) {
       for (unsigned int tl = 0; tl < ppi->number_temporal_layers; tl++) {
         seq->operating_point_idc[i] =
@@ -568,6 +567,7 @@ void av1_init_seq_coding_tools(AV1_PRIMARY *const ppi,
         i++;
       }
     }
+    seq->has_nonzero_operating_point_idc = true;
   }
 }
 
@@ -5393,7 +5393,6 @@ aom_fixed_buf_t *av1_get_global_headers(AV1_PRIMARY *ppi) {
 
   if (av1_write_obu_header(&ppi->level_params, &ppi->cpi->frame_header_count,
                            OBU_SEQUENCE_HEADER,
-                           /*is_layer_specific_obu=*/false,
                            ppi->seq_params.has_nonzero_operating_point_idc, 0,
                            &header_buf[0]) != obu_header_size) {
     return NULL;