diff --git a/iamf/cli/encoder_main_lib.cc b/iamf/cli/encoder_main_lib.cc index a178e03c..8ad77df2 100644 --- a/iamf/cli/encoder_main_lib.cc +++ b/iamf/cli/encoder_main_lib.cc @@ -220,7 +220,7 @@ absl::Status GenerateObus( audio_elements, mix_presentation_obus, param_definitions)); // Global timing module. - GlobalTimingModule global_timing_module(user_metadata); + GlobalTimingModule global_timing_module; RETURN_IF_NOT_OK(global_timing_module.Initialize( audio_elements, codec_config_obus, param_definitions)); diff --git a/iamf/cli/global_timing_module.cc b/iamf/cli/global_timing_module.cc index 5be249e9..3c454c49 100644 --- a/iamf/cli/global_timing_module.cc +++ b/iamf/cli/global_timing_module.cc @@ -20,7 +20,6 @@ #include "iamf/cli/audio_element_with_data.h" #include "iamf/cli/cli_util.h" #include "iamf/cli/proto/parameter_block.pb.h" -#include "iamf/cli/proto/user_metadata.pb.h" #include "iamf/common/macros.h" #include "iamf/obu/audio_element.h" #include "iamf/obu/codec_config.h" @@ -38,6 +37,8 @@ absl::Status FindParameterRate( param_definitions, DecodedUleb128& parameter_rate) { auto iter = param_definitions.find(parameter_id); + // TODO(b/337184341): Simplify this further since we can now assume that + // stray parameter blocks are not allowed. if (iter == param_definitions.end()) { LOG(WARNING) << "Parameter ID: " << parameter_id << " is a stray parameter block. Safely ignoring, but this is " @@ -127,12 +128,6 @@ absl::Status GlobalTimingModule::Initialize( param_definitions) { parameter_ids.insert(parameter_id); } - // Add in all user metadata parameter blocks. Even if the are strays (i.e. - // there is no corresponding `ParamDefinition`. - for (const auto& parameter_block : - user_metadata_.parameter_block_metadata()) { - parameter_ids.insert(parameter_block.parameter_id()); - } // Initialize all parameter IDs to start with a timestamp 0. for (const auto parameter_id : parameter_ids) { diff --git a/iamf/cli/global_timing_module.h b/iamf/cli/global_timing_module.h index b58233eb..07796133 100644 --- a/iamf/cli/global_timing_module.h +++ b/iamf/cli/global_timing_module.h @@ -28,10 +28,8 @@ namespace iamf_tools { class GlobalTimingModule { public: /*\!brief Constructor. - * \param user_metadata Input user metadata. */ - GlobalTimingModule(const iamf_tools_cli_proto::UserMetadata& user_metadata) - : user_metadata_(user_metadata) {} + GlobalTimingModule() = default; /*\!brief Initializes a Global Timing Module. * @@ -111,7 +109,6 @@ class GlobalTimingModule { absl::flat_hash_map& id_to_timing_data, int32_t& start_timestamp, int32_t& end_timestamp); - const iamf_tools_cli_proto::UserMetadata user_metadata_; absl::flat_hash_map audio_frame_timing_data_; absl::flat_hash_map parameter_block_timing_data_; }; diff --git a/iamf/cli/parameter_block_generator.cc b/iamf/cli/parameter_block_generator.cc index 6f39471e..5118ab7c 100644 --- a/iamf/cli/parameter_block_generator.cc +++ b/iamf/cli/parameter_block_generator.cc @@ -153,6 +153,8 @@ absl::Status GetPerIdMetadata( // Initialize some fields that may not be set later. per_id_metadata.num_layers = 0; + // TODO(b/337184341): Simplify logic since stray parameter blocks in the + // metadata are not allowed. auto iter = param_definitions.find(target_parameter_id); if (iter == param_definitions.end()) { LOG(WARNING) << "Found a stray parameter block with id: " diff --git a/iamf/cli/tests/BUILD b/iamf/cli/tests/BUILD index 9879e4e7..5bc4b884 100644 --- a/iamf/cli/tests/BUILD +++ b/iamf/cli/tests/BUILD @@ -202,7 +202,6 @@ cc_test( "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/status", "@com_google_googletest//:gtest_main", - "@com_google_protobuf//:protobuf", ], ) @@ -333,7 +332,6 @@ cc_test( "//iamf/cli:parameter_block_with_data", "//iamf/cli/proto:parameter_block_cc_proto", "//iamf/cli/proto:user_metadata_cc_proto", - "//iamf/common:write_bit_buffer", "//iamf/obu:audio_element", "//iamf/obu:codec_config", "//iamf/obu:demixing_info_param_data", diff --git a/iamf/cli/tests/audio_frame_generator_test.cc b/iamf/cli/tests/audio_frame_generator_test.cc index 630c3093..4c487d61 100644 --- a/iamf/cli/tests/audio_frame_generator_test.cc +++ b/iamf/cli/tests/audio_frame_generator_test.cc @@ -107,7 +107,7 @@ void GenerateAudioFrameWithEightSamples( const std::string output_wav_directory = "/dev/null"; DemixingModule demixing_module(user_metadata, audio_elements); - GlobalTimingModule global_timing_module(user_metadata); + GlobalTimingModule global_timing_module; ASSERT_TRUE( global_timing_module .Initialize(audio_elements, codec_config_obus, param_definitions) diff --git a/iamf/cli/tests/global_timing_module_test.cc b/iamf/cli/tests/global_timing_module_test.cc index cf7999c9..234e2b08 100644 --- a/iamf/cli/tests/global_timing_module_test.cc +++ b/iamf/cli/tests/global_timing_module_test.cc @@ -26,7 +26,6 @@ #include "iamf/obu/codec_config.h" #include "iamf/obu/leb128.h" #include "iamf/obu/param_definitions.h" -#include "src/google/protobuf/text_format.h" namespace iamf_tools { namespace { @@ -54,8 +53,8 @@ class GlobalTimingModuleTest : public ::testing::Test { // Constructs and initializes `global_timing_module_`. absl::Status Initialize() { - global_timing_module_ = std::make_unique( - GlobalTimingModule(user_metadata_)); + global_timing_module_ = + std::make_unique(GlobalTimingModule()); // Normally the `ParamDefinitions` are stored in the descriptor OBUs. For // simplicity they are stored in the class. Transform it to a map of @@ -103,7 +102,6 @@ class GlobalTimingModuleTest : public ::testing::Test { EXPECT_EQ(end_timestamp, expected_end_timestamp); } - iamf_tools_cli_proto::UserMetadata user_metadata_ = {}; std::unique_ptr global_timing_module_ = nullptr; protected: @@ -189,46 +187,20 @@ TEST_F(GlobalTimingModuleTest, OneParameterId) { } TEST_F(GlobalTimingModuleTest, - SupportsStrayParameterBlocksWithOneCodecConfigObu) { + FailsWhenGettingTimestampForStrayParameterBlock) { AddLpcmCodecConfigWithIdAndSampleRate(kCodecConfigId, kSampleRate, codec_config_obus_); - // Stray parameters are represented by parameter blocks in the user metadata, - // without a corresponding `ParamDefinition` in the descriptor OBUs. - ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString( - R"pb( - parameter_id: 0 - duration: 64 - constant_subblock_duration: 64 - num_subblocks: 1 - start_timestamp: 0 - )pb", - user_metadata_.add_parameter_block_metadata())); - EXPECT_TRUE(Initialize().ok()); - // Timing can be generated as expected. It has an implicit `parameter_rate` - // matching the sampler rate of the Codec Config OBU. - TestGetNextParameterBlockTimestamps(kFirstParameterId, 0, 64, 0, 64); - TestGetNextParameterBlockTimestamps(kFirstParameterId, 64, 64, 64, 128); - TestGetNextParameterBlockTimestamps(kFirstParameterId, 128, 64, 128, 192); -} - -TEST_F(GlobalTimingModuleTest, - InvalidWhenThereAreStrayParameterBlocksWithoutCodecConfigObu) { - // Stray parameters are represented by parameter blocks in the user metadata, - // without a corresponding `ParamDefinition` in the descriptor OBUs. - ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString( - R"pb( - parameter_id: 0 - duration: 64 - constant_subblock_duration: 64 - num_subblocks: 1 - start_timestamp: 0 - )pb", - user_metadata_.add_parameter_block_metadata())); - - EXPECT_FALSE(Initialize().ok()); + int32_t start_timestamp; + int32_t end_timestamp; + const auto kStrayParameterBlockId = kFirstParameterId + 1; + EXPECT_FALSE(global_timing_module_ + ->GetNextParameterBlockTimestamps(kStrayParameterBlockId, 0, + 64, start_timestamp, + end_timestamp) + .ok()); } TEST_F(GlobalTimingModuleTest, InvalidWhenParameterRateIsZero) { diff --git a/iamf/cli/tests/parameter_block_generator_test.cc b/iamf/cli/tests/parameter_block_generator_test.cc index 5dbddaac..76e31d23 100644 --- a/iamf/cli/tests/parameter_block_generator_test.cc +++ b/iamf/cli/tests/parameter_block_generator_test.cc @@ -17,7 +17,6 @@ #include "iamf/cli/proto/parameter_block.pb.h" #include "iamf/cli/proto/user_metadata.pb.h" #include "iamf/cli/tests/cli_test_utils.h" -#include "iamf/common/write_bit_buffer.h" #include "iamf/obu/audio_element.h" #include "iamf/obu/codec_config.h" #include "iamf/obu/demixing_info_param_data.h" @@ -50,7 +49,7 @@ TEST(ParameterBlockGeneratorTest, NoParameterBlocks) { // Generate. std::list output_parameter_blocks; - GlobalTimingModule global_timing_module(user_metadata); + GlobalTimingModule global_timing_module; EXPECT_TRUE( generator.GenerateDemixing(global_timing_module, output_parameter_blocks) .ok()); @@ -170,7 +169,7 @@ TEST(ParameterBlockGeneratorTest, GenerateTwoDemixingParameterBlocks) { .ok()); // Global timing Module; needed when calling `GenerateDemixing()`. - GlobalTimingModule global_timing_module(user_metadata); + GlobalTimingModule global_timing_module; ASSERT_TRUE( global_timing_module .Initialize(audio_elements, codec_config_obus, param_definitions) @@ -296,7 +295,7 @@ TEST(ParameterBlockGeneratorTest, GenerateMixGainParameterBlocks) { .ok()); // Global timing Module; needed when calling `GenerateDemixing()`. - GlobalTimingModule global_timing_module(user_metadata); + GlobalTimingModule global_timing_module; ASSERT_TRUE( global_timing_module .Initialize(audio_elements, codec_config_obus, param_definitions) @@ -483,7 +482,7 @@ TEST(ParameterBlockGeneratorTest, GenerateReconGainParameterBlocks) { .ok()); // Global timing Module; needed when calling `GenerateDemixing()`. - GlobalTimingModule global_timing_module(user_metadata); + GlobalTimingModule global_timing_module; ASSERT_TRUE( global_timing_module .Initialize(audio_elements, codec_config_obus, param_definitions) @@ -511,7 +510,7 @@ TEST(ParameterBlockGeneratorTest, GenerateReconGainParameterBlocks) { /*expected_end_timestamps=*/{8, 16}); } -TEST(Initialize, GeneratesValidStrayParameterBlocks) { +TEST(Generate, FailsWhenGeneratingStrayParameterBlocks) { iamf_tools_cli_proto::UserMetadata user_metadata; absl::flat_hash_map parameter_id_to_metadata; @@ -536,21 +535,16 @@ TEST(Initialize, GeneratesValidStrayParameterBlocks) { .ok()); // Global timing Module; needed when calling `GenerateDemixing()`. - GlobalTimingModule global_timing_module(user_metadata); + GlobalTimingModule global_timing_module; ASSERT_TRUE( global_timing_module .Initialize(audio_elements, codec_config_obus, param_definitions) .ok()); std::list output_parameter_blocks; - EXPECT_TRUE( + EXPECT_FALSE( generator.GenerateDemixing(global_timing_module, output_parameter_blocks) .ok()); - ASSERT_FALSE(output_parameter_blocks.empty()); - - WriteBitBuffer wb(0); - EXPECT_TRUE( - output_parameter_blocks.front().obu->ValidateAndWriteObu(wb).ok()); } } // namespace