Skip to content

Commit

Permalink
Drop support for encoding stray parameter blocks.
Browse files Browse the repository at this point in the history
  - A decoder has no way to determine anything about a parameter block with an ID that is not present in descriptor OBUS.
  - Drop normal support for generating stray parameter blocks from the encoder - this will allow future simplifications to be made to some components.
  - For testing purposes stray parameter blocks can be generated with arbitrary OBUs (e.g. `test_000015`).
  - `GlobalTimingModule`: Modify behavior to prevent computing timestamp of stray parameter blocks.
  - `ParameterBlockGenerator`: Now fails to generate stray parameter blocks because it cannot determine the timestamps.

PiperOrigin-RevId: 631038978
  • Loading branch information
jwcullen committed May 6, 2024
1 parent a99daca commit 151093c
Show file tree
Hide file tree
Showing 8 changed files with 25 additions and 67 deletions.
2 changes: 1 addition & 1 deletion iamf/cli/encoder_main_lib.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down
9 changes: 2 additions & 7 deletions iamf/cli/global_timing_module.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 "
Expand Down Expand Up @@ -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) {
Expand Down
5 changes: 1 addition & 4 deletions iamf/cli/global_timing_module.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -111,7 +109,6 @@ class GlobalTimingModule {
absl::flat_hash_map<DecodedUleb128, TimingData>& id_to_timing_data,
int32_t& start_timestamp, int32_t& end_timestamp);

const iamf_tools_cli_proto::UserMetadata user_metadata_;
absl::flat_hash_map<DecodedUleb128, TimingData> audio_frame_timing_data_;
absl::flat_hash_map<DecodedUleb128, TimingData> parameter_block_timing_data_;
};
Expand Down
2 changes: 2 additions & 0 deletions iamf/cli/parameter_block_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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: "
Expand Down
2 changes: 0 additions & 2 deletions iamf/cli/tests/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
)

Expand Down Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion iamf/cli/tests/audio_frame_generator_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
50 changes: 11 additions & 39 deletions iamf/cli/tests/global_timing_module_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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>(
GlobalTimingModule(user_metadata_));
global_timing_module_ =
std::make_unique<GlobalTimingModule>(GlobalTimingModule());

// Normally the `ParamDefinitions` are stored in the descriptor OBUs. For
// simplicity they are stored in the class. Transform it to a map of
Expand Down Expand Up @@ -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<GlobalTimingModule> global_timing_module_ = nullptr;

protected:
Expand Down Expand Up @@ -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) {
Expand Down
20 changes: 7 additions & 13 deletions iamf/cli/tests/parameter_block_generator_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -50,7 +49,7 @@ TEST(ParameterBlockGeneratorTest, NoParameterBlocks) {

// Generate.
std::list<ParameterBlockWithData> output_parameter_blocks;
GlobalTimingModule global_timing_module(user_metadata);
GlobalTimingModule global_timing_module;
EXPECT_TRUE(
generator.GenerateDemixing(global_timing_module, output_parameter_blocks)
.ok());
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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<uint32_t, PerIdParameterMetadata>
parameter_id_to_metadata;
Expand All @@ -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<ParameterBlockWithData> 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
Expand Down

0 comments on commit 151093c

Please sign in to comment.