Skip to content

Commit

Permalink
Fix a bug that forbid temporal delimiters from base-enhanced profile.
Browse files Browse the repository at this point in the history
  - `test_000712`: `Base-enhanced` profile with temporal delimiter OBUs.
    - Would have failed in the past, and useful in the public suite, to clarify they are permitted in base-enhanced profile as well.
    - Update language in a sister test case, because it refers to the incorrect section number.
  - Remove useless abstraction that temporal delimiters can be forbidden:
    - In an older draft of the spec, some profiles forbid temporal delimiters.
    - In the modern drafts of the spec, they are always permitted.
    - The spec does not imply or promise any rules or conditions on when they would be forbidden in future versions.
    - The abstraction is useless under these conditions. And it would belong in `ProfileFilter` in the modern codebase.
  - Note: Since "Base-enhanced" was omitted from
 a table in `ProfileSupportsTemporalDelimiterObus`, now the code permits them.
  - b/302470464: Increase coverage of temporal delimiters.

PiperOrigin-RevId: 691004564
  • Loading branch information
jwcullen committed Nov 1, 2024
1 parent f154bd7 commit e021bfe
Show file tree
Hide file tree
Showing 10 changed files with 291 additions and 156 deletions.
5 changes: 1 addition & 4 deletions iamf/cli/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,10 @@ cc_library(
"//iamf/cli/proto:obu_header_cc_proto",
"//iamf/cli/proto:param_definitions_cc_proto",
"//iamf/cli/proto:parameter_data_cc_proto",
"//iamf/cli/proto:temporal_delimiter_cc_proto",
"//iamf/cli/proto:user_metadata_cc_proto",
"//iamf/common:macros",
"//iamf/common:obu_util",
"//iamf/obu:audio_element",
"//iamf/obu:codec_config",
"//iamf/obu:ia_sequence_header",
"//iamf/obu:mix_presentation",
"//iamf/obu:obu_header",
"//iamf/obu:param_definitions",
Expand Down Expand Up @@ -147,7 +144,6 @@ cc_library(
deps = [
":audio_element_with_data",
":audio_frame_with_data",
":cli_util",
":demixing_module",
":iamf_components",
":iamf_encoder",
Expand All @@ -157,6 +153,7 @@ cc_library(
":rendering_mix_presentation_finalizer",
":wav_sample_provider",
":wav_writer",
"//iamf/cli/proto:temporal_delimiter_cc_proto",
"//iamf/cli/proto:test_vector_metadata_cc_proto",
"//iamf/cli/proto:user_metadata_cc_proto",
"//iamf/cli/proto_to_obu:arbitrary_obu_generator",
Expand Down
38 changes: 0 additions & 38 deletions iamf/cli/cli_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,11 @@
#include "iamf/cli/proto/obu_header.pb.h"
#include "iamf/cli/proto/param_definitions.pb.h"
#include "iamf/cli/proto/parameter_data.pb.h"
#include "iamf/cli/proto/temporal_delimiter.pb.h"
#include "iamf/cli/proto/user_metadata.pb.h"
#include "iamf/common/macros.h"
#include "iamf/common/obu_util.h"
#include "iamf/obu/audio_element.h"
#include "iamf/obu/codec_config.h"
#include "iamf/obu/demixing_info_parameter_data.h"
#include "iamf/obu/ia_sequence_header.h"
#include "iamf/obu/mix_presentation.h"
#include "iamf/obu/obu_header.h"
#include "iamf/obu/param_definitions.h"
Expand Down Expand Up @@ -197,41 +194,6 @@ absl::Status CopyDMixPMode(DemixingInfoParameterData::DMixPMode obu_dmixp_mode,
"Proto version of internal `DMixPMode`", dmixp_mode);
}

// Returns `true` if the profile fully supports temporal delimiter OBUs.
// Although profile that do not support them can safely ignore them.
bool ProfileSupportsTemporalDelimiterObus(ProfileVersion profile) {
switch (profile) {
case ProfileVersion::kIamfSimpleProfile:
case ProfileVersion::kIamfBaseProfile:
return true;
default:
return false;
}
}

absl::Status GetIncludeTemporalDelimiterObus(
const iamf_tools_cli_proto::UserMetadata& user_metadata,
const IASequenceHeaderObu& ia_sequence_header_obu,
bool& include_temporal_delimiter) {
const bool input_include_temporal_delimiter =
user_metadata.temporal_delimiter_metadata().enable_temporal_delimiters();

// Allow Temporal Delimiter OBUs as long as at least one of the profiles
// supports them. If one of the profiles (e.g. simple profile) does not
// "support" them they can be safely ignored.
if (input_include_temporal_delimiter &&
(!ProfileSupportsTemporalDelimiterObus(
ia_sequence_header_obu.GetPrimaryProfile()) &&
!ProfileSupportsTemporalDelimiterObus(
ia_sequence_header_obu.GetAdditionalProfile()))) {
return absl::InvalidArgumentError(
"Temporal Delimiter OBUs need either `primary_profile` or "
"`additional_profile` to support them.");
}
include_temporal_delimiter = input_include_temporal_delimiter;
return absl::OkStatus();
}

absl::Status CollectAndValidateParamDefinitions(
const absl::flat_hash_map<DecodedUleb128, AudioElementWithData>&
audio_elements,
Expand Down
16 changes: 0 additions & 16 deletions iamf/cli/cli_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,8 @@
#include "iamf/cli/proto/obu_header.pb.h"
#include "iamf/cli/proto/param_definitions.pb.h"
#include "iamf/cli/proto/parameter_data.pb.h"
#include "iamf/cli/proto/user_metadata.pb.h"
#include "iamf/obu/codec_config.h"
#include "iamf/obu/demixing_info_parameter_data.h"
#include "iamf/obu/ia_sequence_header.h"
#include "iamf/obu/mix_presentation.h"
#include "iamf/obu/obu_header.h"
#include "iamf/obu/param_definitions.h"
Expand Down Expand Up @@ -77,20 +75,6 @@ absl::Status CopyDemixingInfoParameterData(
absl::Status CopyDMixPMode(DemixingInfoParameterData::DMixPMode obu_dmixp_mode,
iamf_tools_cli_proto::DMixPMode& dmixp_mode);

/*!\brief Checks whether temporal delimiters OBUs should be inserted.
*
* \param user_metadata User controlled metadata.
* \param ia_sequence_header_obu IA Sequence Header OBU in the IA sequence.
* \param include_temporal_delimiter_obus True when temporal delimiters should
* be included in the IA sequence.
* \return `absl::OkStatus()` on success. `absl::InvalidArgumentError()` if
* including temporal delimiters would be invalid.
*/
absl::Status GetIncludeTemporalDelimiterObus(
const iamf_tools_cli_proto::UserMetadata& user_metadata,
const IASequenceHeaderObu& ia_sequence_header_obu,
bool& include_temporal_delimiter_obus);

/*!\brief Collects and validates the parameter definitions against the spec.
*
* When `param_definition_mode = 0`, `duration`, `num_subblocks`,
Expand Down
7 changes: 3 additions & 4 deletions iamf/cli/encoder_main_lib.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@
#include "absl/strings/str_cat.h"
#include "iamf/cli/audio_element_with_data.h"
#include "iamf/cli/audio_frame_with_data.h"
#include "iamf/cli/cli_util.h"
#include "iamf/cli/demixing_module.h"
#include "iamf/cli/iamf_components.h"
#include "iamf/cli/iamf_encoder.h"
#include "iamf/cli/obu_sequencer.h"
#include "iamf/cli/parameter_block_partitioner.h"
#include "iamf/cli/parameter_block_with_data.h"
#include "iamf/cli/proto/temporal_delimiter.pb.h"
#include "iamf/cli/proto/test_vector_metadata.pb.h"
#include "iamf/cli/proto/user_metadata.pb.h"
#include "iamf/cli/proto_to_obu/arbitrary_obu_generator.h"
Expand Down Expand Up @@ -309,9 +309,8 @@ absl::Status WriteObus(
const std::list<AudioFrameWithData>& audio_frames,
const std::list<ParameterBlockWithData>& parameter_blocks,
const std::list<ArbitraryObu>& arbitrary_obus) {
bool include_temporal_delimiters;
RETURN_IF_NOT_OK(GetIncludeTemporalDelimiterObus(
user_metadata, ia_sequence_header_obu, include_temporal_delimiters));
const bool include_temporal_delimiters =
user_metadata.temporal_delimiter_metadata().enable_temporal_delimiters();

// TODO(b/349271859): Move the OBU sequencer inside `IamfEncoder`.
auto obu_sequencers = CreateObuSequencers(
Expand Down
6 changes: 3 additions & 3 deletions iamf/cli/testdata/test_000006.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@

test_vector_metadata {
human_readable_description:
"A stereo base profile IAMF stream with temporal "
"delimiters."
"A stereo Base profile IAMF stream with temporal "
" delimiters."
file_name_prefix: "test_000006"
is_valid: true
is_valid_to_decode: true
Expand All @@ -30,7 +30,7 @@ test_vector_metadata {
"4.2/temporal_delimiter_obu",
"5.1/IA Sequence",
"5.1.2/IA Data OBUs",
"6.2.4/temporal_delimiter_obu"
"4.2/Temporal Delimiter OBU"
]
base_test: "test_000005"
}
Expand Down
158 changes: 158 additions & 0 deletions iamf/cli/testdata/test_000712.textproto
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
# Copyright (c) 2024, Alliance for Open Media. All rights reserved
#
# This source code is subject to the terms of the BSD 3-Clause Clear License
# and the Alliance for Open Media Patent License 1.0. If the BSD 3-Clause Clear
# License was not distributed with this source code in the LICENSE file, you
# can obtain it at www.aomedia.org/license/software-license/bsd-3-c-c. If the
# Alliance for Open Media Patent License 1.0 was not distributed with this
# source code in the PATENTS file, you can obtain it at
# www.aomedia.org/license/patent.

# proto-file: iamf/cli/proto/user_metadata.proto
# proto-message: UserMetadata

test_vector_metadata {
human_readable_description:
"A Base-Enhanced IAMF stream with Temporal Delimiter"
" OBUs "
file_name_prefix: "test_000712"
is_valid: true
is_valid_to_decode: true
validate_user_loudness: true
mp4_fixed_timestamp: "2024-10-28 00:00:00"
test_repository_tags: [
"github/aomediacodec/libiamf/main"
]
primary_tested_spec_sections: ["3.10/Temporal Delimiter OBU"]
base_test: "test_000005"
}

ia_sequence_header_metadata {
primary_profile: PROFILE_VERSION_BASE_ENHANCED
additional_profile: PROFILE_VERSION_BASE_ENHANCED
}

codec_config_metadata {
codec_config_id: 200
codec_config {
codec_id: CODEC_ID_LPCM
num_samples_per_frame: 64
audio_roll_distance: 0
decoder_config_lpcm {
sample_format_flags: LPCM_LITTLE_ENDIAN
sample_size: 16
sample_rate: 16000
}
}
}

audio_element_metadata {
audio_element_id: 300
audio_element_type: AUDIO_ELEMENT_CHANNEL_BASED
reserved: 0
codec_config_id: 200
num_substreams: 1
audio_substream_ids: [0]
num_parameters: 0
scalable_channel_layout_config {
num_layers: 1
reserved: 0
channel_audio_layer_configs: [
{
loudspeaker_layout: LOUDSPEAKER_LAYOUT_EXPANDED
expanded_loudspeaker_layout: EXPANDED_LOUDSPEAKER_LAYOUT_STEREO_S
output_gain_is_present_flag: 0
recon_gain_is_present_flag: 0
reserved_a: 0
substream_count: 1
coupled_substream_count: 1
}
]
}
}

mix_presentation_metadata {
mix_presentation_id: 42
count_label: 1
annotations_language: ["en-us"]
localized_presentation_annotations: ["test_mix_pres"]
num_sub_mixes: 1
sub_mixes {
num_audio_elements: 1
audio_elements {
audio_element_id: 300
localized_element_annotations: ["test_sub_mix_0_audio_element_0"]
rendering_config {
headphones_rendering_mode: HEADPHONES_RENDERING_MODE_STEREO
}
element_mix_gain {
param_definition {
parameter_id: 100
parameter_rate: 16000
param_definition_mode: 1
reserved: 0
}
default_mix_gain: 0
}
}
output_mix_gain {
param_definition {
parameter_id: 100
parameter_rate: 16000
param_definition_mode: 1
reserved: 0
}
default_mix_gain: 0
}
num_layouts: 1
layouts {
loudness_layout {
layout_type: LAYOUT_TYPE_LOUDSPEAKERS_SS_CONVENTION
ss_layout {
sound_system: SOUND_SYSTEM_A_0_2_0
reserved: 0
}
}
loudness {
info_type_bit_masks: []
integrated_loudness: -13733
digital_peak: -12879
}
}
}
}

audio_frame_metadata {
wav_filename: "sawtooth_100_stereo.wav"
samples_to_trim_at_end: 0
samples_to_trim_at_start: 0
audio_element_id: 300
channel_metadatas: [
{ channel_id: 0 channel_label: CHANNEL_LABEL_LS_5 },
{ channel_id: 1 channel_label: CHANNEL_LABEL_RS_5 }
]
}
parameter_block_metadata {
parameter_id: 100
start_timestamp: 0
duration: 8000
num_subblocks: 1
constant_subblock_duration: 8000
subblocks: [
{
mix_gain_parameter_data {
animation_type: ANIMATE_STEP
param_data {
step {
start_point_value: 0
}
}
}
}
]
}
temporal_delimiter_metadata {
enable_temporal_delimiters: true
}
3 changes: 0 additions & 3 deletions iamf/cli/tests/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,10 @@ cc_test(
"//iamf/cli:cli_util",
"//iamf/cli/proto:obu_header_cc_proto",
"//iamf/cli/proto:parameter_data_cc_proto",
"//iamf/cli/proto:temporal_delimiter_cc_proto",
"//iamf/cli/proto:user_metadata_cc_proto",
"//iamf/cli/proto_to_obu:audio_element_generator",
"//iamf/obu:audio_element",
"//iamf/obu:audio_frame",
"//iamf/obu:codec_config",
"//iamf/obu:ia_sequence_header",
"//iamf/obu:mix_presentation",
"//iamf/obu:obu_header",
"//iamf/obu:param_definitions",
Expand Down
Loading

0 comments on commit e021bfe

Please sign in to comment.