`audio_element.proto`: Deprecate several num* fields.
- Similar to the rationale of AOMediaCodec/iamf_tools@d6e92ac9 and AOMediaCodec/iamf_tools@@822154612.
- The fields are useless and hold no real information and only make the proto more complicated.
PiperOrigin-RevId: 770193247
diff --git a/iamf/cli/proto/audio_element.proto b/iamf/cli/proto/audio_element.proto
index 9b29fa1..aaaeabe 100644
--- a/iamf/cli/proto/audio_element.proto
+++ b/iamf/cli/proto/audio_element.proto
@@ -24,7 +24,9 @@
}
message ParamDefinitionExtension {
- uint32 param_definition_size = 1;
+ // `param_definition_size` is ignored. The value in the bitstream is inferred
+ // based on the size of `param_definition_bytes`.
+ uint32 param_definition_size = 1 [deprecated = true];
bytes param_definition_bytes = 2;
}
@@ -96,7 +98,9 @@
}
message ScalableChannelLayoutConfig {
- uint32 num_layers = 1;
+ // `num_layers` is ignored. The value in the bitstream is inferred based on
+ // the number of `channel_audio_layer_configs`.
+ uint32 num_layers = 1 [deprecated = true];
uint32 reserved = 2;
repeated ChannelAudioLayerConfig channel_audio_layer_configs = 3;
}
@@ -130,7 +134,9 @@
}
message AudioElementConfigExtension {
- uint32 audio_element_config_size = 1;
+ // `audio_element_config_size` is ignored. The value in the bitstream is
+ // inferred based on the size of `audio_element_config_bytes`.
+ uint32 audio_element_config_size = 1 [deprecated = true];
bytes audio_element_config_bytes = 2;
}
@@ -139,9 +145,13 @@
AudioElementType audio_element_type = 2;
uint32 reserved = 3;
uint32 codec_config_id = 4;
- uint32 num_substreams = 5;
+ // `num_substreams` is ignored. The value in the bitstream is inferred based
+ // on the number of `audio_substream_ids`.
+ uint32 num_substreams = 5 [deprecated = true];
repeated uint32 audio_substream_ids = 6;
- uint32 num_parameters = 7;
+ // `num_parameters` is ignored. The value in the bitstream is inferred based
+ // on the number of `audio_element_params`.
+ uint32 num_parameters = 7 [deprecated = true];
repeated AudioElementParam audio_element_params = 8;
oneof config {
diff --git a/iamf/cli/proto_conversion/proto_to_obu/audio_element_generator.cc b/iamf/cli/proto_conversion/proto_to_obu/audio_element_generator.cc
index 37337c5..69c43be 100644
--- a/iamf/cli/proto_conversion/proto_to_obu/audio_element_generator.cc
+++ b/iamf/cli/proto_conversion/proto_to_obu/audio_element_generator.cc
@@ -88,42 +88,34 @@
}
}
-absl::Status GenerateAudioSubstreams(
+void GenerateAudioSubstreams(
const iamf_tools_cli_proto::AudioElementObuMetadata& audio_element_metadata,
AudioElementObu& audio_element_obu) {
- if (audio_element_metadata.num_substreams() !=
- audio_element_metadata.audio_substream_ids_size()) {
- return InvalidArgumentError(
- StrCat("User data has inconsistent `num_substreams` and "
- "`audio_substream_ids`. User provided ",
- audio_element_metadata.audio_substream_ids_size(),
- " substreams in `audio_substream_ids`, and `num_substreams`= ",
- audio_element_metadata.num_substreams()));
+ if (audio_element_metadata.has_num_substreams()) {
+ LOG(WARNING) << "Ignoring deprecated `num_substreams` field. "
+ "Please remove it.";
}
audio_element_obu.InitializeAudioSubstreams(
- audio_element_metadata.num_substreams());
- for (int i = 0; i < audio_element_metadata.num_substreams(); ++i) {
+ audio_element_metadata.audio_substream_ids_size());
+ for (int i = 0; i < audio_element_metadata.audio_substream_ids_size(); ++i) {
audio_element_obu.audio_substream_ids_[i] =
audio_element_metadata.audio_substream_ids(i);
}
- return absl::OkStatus();
}
absl::Status GenerateParameterDefinitions(
const iamf_tools_cli_proto::AudioElementObuMetadata& audio_element_metadata,
const CodecConfigObu& codec_config_obu,
AudioElementObu& audio_element_obu) {
- if (audio_element_metadata.num_parameters() !=
- audio_element_metadata.audio_element_params_size()) {
- return InvalidArgumentError(StrCat(
- "User data has inconsistent `num_parameters`. Found: ",
- audio_element_metadata.audio_element_params_size(),
- " parameters, expected: ", audio_element_metadata.num_parameters()));
+ if (audio_element_metadata.has_num_parameters()) {
+ LOG(WARNING) << "Ignoring deprecated `num_parameters` field. "
+ "Please remove it.";
}
- audio_element_obu.InitializeParams(audio_element_metadata.num_parameters());
- for (int i = 0; i < audio_element_metadata.num_parameters(); ++i) {
+ audio_element_obu.InitializeParams(
+ audio_element_metadata.audio_element_params_size());
+ for (int i = 0; i < audio_element_metadata.audio_element_params_size(); ++i) {
const auto& user_data_parameter =
audio_element_metadata.audio_element_params(i);
@@ -193,10 +185,14 @@
ExtendedParamDefinition extended_param_definition(
copied_param_definition_type);
// Copy the extension bytes.
+ if (user_param_definition.has_param_definition_size()) {
+ LOG(WARNING) << "Ignoring deprecated `param_definition_size` field. "
+ "Please remove it.";
+ }
extended_param_definition.param_definition_size_ =
- user_param_definition.param_definition_size();
+ user_param_definition.param_definition_bytes().size();
extended_param_definition.param_definition_bytes_.resize(
- user_param_definition.param_definition_size());
+ extended_param_definition.param_definition_size_);
RETURN_IF_NOT_OK(StaticCastSpanIfInRange(
"param_definition_bytes",
absl::MakeConstSpan(user_param_definition.param_definition_bytes()),
@@ -360,15 +356,15 @@
const auto& input_config =
audio_element_metadata.scalable_channel_layout_config();
+ if (input_config.has_num_layers()) {
+ LOG(WARNING) << "Ignoring deprecated `num_layers` field. Please remove it.";
+ }
+
RETURN_IF_NOT_OK(audio_element.obu.InitializeScalableChannelLayout(
- input_config.num_layers(), input_config.reserved()));
+ input_config.channel_audio_layer_configs_size(),
+ input_config.reserved()));
auto& config =
std::get<ScalableChannelLayoutConfig>(audio_element.obu.config_);
- if (config.num_layers != input_config.channel_audio_layer_configs_size()) {
- return InvalidArgumentError(StrCat(
- "Expected ", config.num_layers, " layers in the metadata. Found ",
- input_config.channel_audio_layer_configs_size(), " layers."));
- }
for (int i = 0; i < config.num_layers; ++i) {
ChannelAudioLayerConfig* const layer_config =
&config.channel_audio_layer_configs[i];
@@ -597,8 +593,7 @@
audio_element_id, audio_element_type, reserved, codec_config_id);
// Audio Substreams.
- RETURN_IF_NOT_OK(
- GenerateAudioSubstreams(audio_element_metadata, audio_element_obu));
+ GenerateAudioSubstreams(audio_element_metadata, audio_element_obu);
// Parameter definitions.
if (!codec_configs.contains(audio_element_metadata.codec_config_id())) {
diff --git a/iamf/cli/proto_conversion/proto_to_obu/tests/BUILD b/iamf/cli/proto_conversion/proto_to_obu/tests/BUILD
index 6fd1b9e..6dc17c9 100644
--- a/iamf/cli/proto_conversion/proto_to_obu/tests/BUILD
+++ b/iamf/cli/proto_conversion/proto_to_obu/tests/BUILD
@@ -21,6 +21,7 @@
"//iamf/cli:audio_element_with_data",
"//iamf/cli:channel_label",
"//iamf/cli/proto:audio_element_cc_proto",
+ "//iamf/cli/proto:param_definitions_cc_proto",
"//iamf/cli/proto_conversion/proto_to_obu:audio_element_generator",
"//iamf/cli/tests:cli_test_utils",
"//iamf/obu:audio_element",
@@ -29,6 +30,7 @@
"//iamf/obu:parameter_data",
"//iamf/obu:types",
"@com_google_absl//absl/container:flat_hash_map",
+ "@com_google_absl//absl/status",
"@com_google_absl//absl/status:status_matchers",
"@com_google_googletest//:gtest_main",
"@com_google_protobuf//:protobuf",
diff --git a/iamf/cli/proto_conversion/proto_to_obu/tests/audio_element_generator_test.cc b/iamf/cli/proto_conversion/proto_to_obu/tests/audio_element_generator_test.cc
index 1ad1be0..3a93b1c 100644
--- a/iamf/cli/proto_conversion/proto_to_obu/tests/audio_element_generator_test.cc
+++ b/iamf/cli/proto_conversion/proto_to_obu/tests/audio_element_generator_test.cc
@@ -17,12 +17,14 @@
#include <vector>
#include "absl/container/flat_hash_map.h"
+#include "absl/status/status.h"
#include "absl/status/status_matchers.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include "iamf/cli/audio_element_with_data.h"
#include "iamf/cli/channel_label.h"
#include "iamf/cli/proto/audio_element.pb.h"
+#include "iamf/cli/proto/param_definitions.pb.h"
#include "iamf/cli/tests/cli_test_utils.h"
#include "iamf/obu/audio_element.h"
#include "iamf/obu/codec_config.h"
@@ -48,6 +50,9 @@
constexpr DecodedUleb128 kAudioElementId = 300;
constexpr uint32_t kSampleRate = 48000;
+constexpr DecodedUleb128 kMonoSubstreamId = 99;
+constexpr DecodedUleb128 kL2SubstreamId = 100;
+
const ScalableChannelLayoutConfig kOneLayerStereoConfig{
.num_layers = 1,
.channel_audio_layer_configs = {
@@ -69,6 +74,70 @@
output_scalable_channel_layout_config);
}
+void AddFirstOrderAmbisonicsMetadata(
+ AudioElementObuMetadatas& audio_element_metadatas) {
+ ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString(
+ R"pb(
+ audio_element_id: 300
+ audio_element_type: AUDIO_ELEMENT_SCENE_BASED
+ reserved: 0
+ codec_config_id: 200
+ num_substreams: 4
+ audio_substream_ids: [ 0, 1, 2, 3 ]
+ num_parameters: 0
+ ambisonics_config {
+ ambisonics_mode: AMBISONICS_MODE_MONO
+ ambisonics_mono_config {
+ output_channel_count: 4
+ substream_count: 4
+ channel_mapping: [ 0, 1, 2, 3 ]
+ }
+ }
+ )pb",
+ audio_element_metadatas.Add()));
+}
+
+void AddTwoLayerStereoMetadata(
+ AudioElementObuMetadatas& audio_element_metadatas) {
+ auto* new_audio_element_metadata = audio_element_metadatas.Add();
+ ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString(
+ R"pb(
+ audio_element_id: 300
+ audio_element_type: AUDIO_ELEMENT_CHANNEL_BASED
+ reserved: 0
+ codec_config_id: 200
+ num_substreams: 2
+ num_parameters: 0
+ scalable_channel_layout_config {
+ num_layers: 2
+ reserved: 0
+ channel_audio_layer_configs {
+ loudspeaker_layout: LOUDSPEAKER_LAYOUT_MONO
+ output_gain_is_present_flag: 0
+ recon_gain_is_present_flag: 0
+ reserved_a: 0
+ substream_count: 1
+ coupled_substream_count: 0
+ }
+ channel_audio_layer_configs {
+ loudspeaker_layout: LOUDSPEAKER_LAYOUT_STEREO
+ output_gain_is_present_flag: 1
+ recon_gain_is_present_flag: 0
+ reserved_a: 0
+ substream_count: 1
+ coupled_substream_count: 0
+ output_gain_flag: 32
+ output_gain: 32767
+ }
+ }
+ )pb",
+ new_audio_element_metadata));
+ new_audio_element_metadata->mutable_audio_substream_ids()->Add(
+ kMonoSubstreamId);
+ new_audio_element_metadata->mutable_audio_substream_ids()->Add(
+ kL2SubstreamId);
+}
+
TEST(Generate, PopulatesExpandedLoudspeakerLayout) {
AudioElementObuMetadatas audio_element_metadatas;
ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString(
@@ -230,26 +299,7 @@
TEST_F(AudioElementGeneratorTest, NoAudioElementObus) { InitAndTestGenerate(); }
TEST_F(AudioElementGeneratorTest, FirstOrderMonoAmbisonicsNumericalOrder) {
- ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString(
- R"pb(
- audio_element_id: 300
- audio_element_type: AUDIO_ELEMENT_SCENE_BASED
- reserved: 0
- codec_config_id: 200
- num_substreams: 4
- audio_substream_ids: [ 0, 1, 2, 3 ]
- num_parameters: 0
- ambisonics_config {
- ambisonics_mode: AMBISONICS_MODE_MONO
- ambisonics_mono_config {
- output_channel_count: 4
- substream_count: 4
- channel_mapping: [ 0, 1, 2, 3 ]
- }
- }
- )pb",
- audio_element_metadata_.Add()));
-
+ AddFirstOrderAmbisonicsMetadata(audio_element_metadata_);
AddAmbisonicsMonoAudioElementWithSubstreamIds(
kAudioElementId, kCodecConfigId, {0, 1, 2, 3}, codec_config_obus_,
expected_obus_);
@@ -461,44 +511,12 @@
}
TEST_F(AudioElementGeneratorTest, FillsAudioElementWithDataFields) {
- const SubstreamIdLabelsMap kExpectedSubstreamIdToLabels = {{99, {kMono}},
- {100, {kL2}}};
+ const SubstreamIdLabelsMap kExpectedSubstreamIdToLabels = {
+ {kMonoSubstreamId, {kMono}}, {kL2SubstreamId, {kL2}}};
const std::vector<ChannelNumbers> kExpectedChannelNumbersForLayer = {
{.surround = 1, .lfe = 0, .height = 0},
{.surround = 2, .lfe = 0, .height = 0}};
- ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString(
- R"pb(
- audio_element_id: 300
- audio_element_type: AUDIO_ELEMENT_CHANNEL_BASED
- reserved: 0
- codec_config_id: 200
- num_substreams: 2
- audio_substream_ids: [ 99, 100 ]
- num_parameters: 0
- scalable_channel_layout_config {
- num_layers: 2
- reserved: 0
- channel_audio_layer_configs {
- loudspeaker_layout: LOUDSPEAKER_LAYOUT_MONO
- output_gain_is_present_flag: 0
- recon_gain_is_present_flag: 0
- reserved_a: 0
- substream_count: 1
- coupled_substream_count: 0
- }
- channel_audio_layer_configs {
- loudspeaker_layout: LOUDSPEAKER_LAYOUT_STEREO
- output_gain_is_present_flag: 1
- recon_gain_is_present_flag: 0
- reserved_a: 0
- substream_count: 1
- coupled_substream_count: 0
- output_gain_flag: 32
- output_gain: 32767
- }
- }
- )pb",
- audio_element_metadata_.Add()));
+ AddTwoLayerStereoMetadata(audio_element_metadata_);
AudioElementGenerator generator(audio_element_metadata_);
EXPECT_THAT(generator.Generate(codec_config_obus_, output_obus_), IsOk());
@@ -748,5 +766,107 @@
kExpectedAudioElementParam);
}
+TEST_F(AudioElementGeneratorTest, IgnoresDeprecatedNumSubstreamsField) {
+ AddFirstOrderAmbisonicsMetadata(audio_element_metadata_);
+ auto& first_order_ambisonics_metadata = audio_element_metadata_.at(0);
+ // Normally first-order ambisonics has four substreams.
+ constexpr DecodedUleb128 kExpectedNumSubstreams = 4;
+ // Corrupt the `num_substreams` field.
+ const auto kIgnoredNumSubstreams = 9999;
+ first_order_ambisonics_metadata.set_num_substreams(kIgnoredNumSubstreams);
+ AudioElementGenerator generator(audio_element_metadata_);
+
+ EXPECT_THAT(generator.Generate(codec_config_obus_, output_obus_), IsOk());
+
+ // The field is deprecatred and ignored, the actual number of substreams are
+ // set based on the `audio_substream_ids` field.
+ ASSERT_TRUE(output_obus_.contains(kAudioElementId));
+ EXPECT_EQ(output_obus_.at(kAudioElementId).obu.audio_substream_ids_.size(),
+ kExpectedNumSubstreams);
+ EXPECT_EQ(output_obus_.at(kAudioElementId).obu.num_substreams_,
+ kExpectedNumSubstreams);
+}
+
+TEST_F(AudioElementGeneratorTest, IgnoresDeprecatedNumParametersField) {
+ AddFirstOrderAmbisonicsMetadata(audio_element_metadata_);
+ auto& first_order_ambisonics_metadata = audio_element_metadata_.at(0);
+ constexpr DecodedUleb128 kExpectedNumParameters = 0;
+ // Corrupt the `num_parameters` field.
+ const auto kIgnoredNumParameters = 9999;
+ first_order_ambisonics_metadata.set_num_parameters(kIgnoredNumParameters);
+ AudioElementGenerator generator(audio_element_metadata_);
+
+ EXPECT_THAT(generator.Generate(codec_config_obus_, output_obus_), IsOk());
+
+ // The field is deprecatred and ignored, the actual number of parameters are
+ // set based on the `audio_element_params` field.
+ ASSERT_TRUE(output_obus_.contains(kAudioElementId));
+ EXPECT_EQ(output_obus_.at(kAudioElementId).obu.num_parameters_,
+ kExpectedNumParameters);
+ EXPECT_EQ(output_obus_.at(kAudioElementId).obu.audio_element_params_.size(),
+ kExpectedNumParameters);
+}
+
+TEST_F(AudioElementGeneratorTest, IgnoresDeprecatedParamDefinitionSizeField) {
+ AddFirstOrderAmbisonicsMetadata(audio_element_metadata_);
+ auto& first_order_ambisonics_metadata = audio_element_metadata_.at(0);
+ auto* audio_element_param =
+ first_order_ambisonics_metadata.mutable_audio_element_params()->Add();
+ audio_element_param->set_param_definition_type(
+ iamf_tools_cli_proto::PARAM_DEFINITION_TYPE_RESERVED_3);
+ // Corrupt the `num_parameters` field.
+ constexpr absl::string_view kParamDefinitionBytes = "abc";
+ constexpr DecodedUleb128 kExpectedParamDefinitionSize =
+ kParamDefinitionBytes.size();
+ const auto kInconsistentParamDefinitionSize = 9999;
+ audio_element_param->mutable_param_definition_extension()
+ ->set_param_definition_size(kInconsistentParamDefinitionSize);
+ audio_element_param->mutable_param_definition_extension()
+ ->set_param_definition_bytes(kParamDefinitionBytes);
+ AudioElementGenerator generator(audio_element_metadata_);
+
+ EXPECT_THAT(generator.Generate(codec_config_obus_, output_obus_), IsOk());
+
+ // The field is deprecatred and ignored, the actual number of parameters are
+ // set based on the `param_definition_bytes` field.
+ ASSERT_TRUE(output_obus_.contains(kAudioElementId));
+ ASSERT_FALSE(
+ output_obus_.at(kAudioElementId).obu.audio_element_params_.empty());
+ const auto* extended_param_definition = std::get_if<ExtendedParamDefinition>(
+ &output_obus_.at(kAudioElementId)
+ .obu.audio_element_params_.front()
+ .param_definition);
+ ASSERT_NE(extended_param_definition, nullptr);
+ EXPECT_EQ(extended_param_definition->param_definition_size_,
+ kExpectedParamDefinitionSize);
+ EXPECT_EQ(extended_param_definition->param_definition_bytes_.size(),
+ kExpectedParamDefinitionSize);
+}
+
+TEST_F(AudioElementGeneratorTest, IgnoresDeprecatedNumLayers) {
+ AddTwoLayerStereoMetadata(audio_element_metadata_);
+ auto& first_order_ambisonics_metadata = audio_element_metadata_.at(0);
+ // Two layers are set in the metadata.
+ constexpr uint8_t kExpectedNumLayers = 2;
+ // Corrupt the `num_layers` field.
+ const auto kIgnoredNumLayers = 7;
+ first_order_ambisonics_metadata.mutable_scalable_channel_layout_config()
+ ->set_num_layers(kIgnoredNumLayers);
+ AudioElementGenerator generator(audio_element_metadata_);
+
+ EXPECT_THAT(generator.Generate(codec_config_obus_, output_obus_), IsOk());
+
+ // The corrupted value is ignored, and the actual number of parameters are set
+ // correctly.
+ ASSERT_TRUE(output_obus_.contains(kAudioElementId));
+ const auto* scalable_channel_layout_config =
+ std::get_if<ScalableChannelLayoutConfig>(
+ &output_obus_.at(kAudioElementId).obu.config_);
+ ASSERT_NE(scalable_channel_layout_config, nullptr);
+ EXPECT_EQ(scalable_channel_layout_config->num_layers, kExpectedNumLayers);
+ EXPECT_EQ(scalable_channel_layout_config->channel_audio_layer_configs.size(),
+ kExpectedNumLayers);
+}
+
} // namespace
} // namespace iamf_tools
