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
diff --git a/av1/av1_cx_iface.c b/av1/av1_cx_iface.c index 6205257..f6ef0ca 100644 --- a/av1/av1_cx_iface.c +++ b/av1/av1_cx_iface.c
@@ -3356,7 +3356,6 @@ 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 4f5ec12..7c02eeb 100644 --- a/av1/encoder/bitstream.c +++ b/av1/encoder/bitstream.c
@@ -3356,10 +3356,8 @@ 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 @@ 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 @@ // 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 @@ 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 @@ 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 @@ (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 @@ 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 @@ 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 232c430..a8f3cc5 100644 --- a/av1/encoder/bitstream.h +++ b/av1/encoder/bitstream.h
@@ -91,11 +91,10 @@ 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 1a132fe..fd293b1 100644 --- a/av1/encoder/encoder.c +++ b/av1/encoder/encoder.c
@@ -558,7 +558,6 @@ 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 @@ i++; } } + seq->has_nonzero_operating_point_idc = true; } } @@ -5393,7 +5393,6 @@ 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;