diff --git a/iamf/cli/BUILD b/iamf/cli/BUILD index c9f849d..cc781d7 100644 --- a/iamf/cli/BUILD +++ b/iamf/cli/BUILD @@ -392,7 +392,6 @@ cc_library( "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/log", "@com_google_absl//absl/status", - "@com_google_absl//absl/strings", ], ) diff --git a/iamf/cli/adm_to_user_metadata/iamf/BUILD b/iamf/cli/adm_to_user_metadata/iamf/BUILD index 8dbef08..7c1833d 100644 --- a/iamf/cli/adm_to_user_metadata/iamf/BUILD +++ b/iamf/cli/adm_to_user_metadata/iamf/BUILD @@ -11,6 +11,7 @@ cc_library( "//iamf/cli/proto:audio_element_cc_proto", "//iamf/cli/proto:ia_sequence_header_cc_proto", "//iamf/cli/proto:user_metadata_cc_proto", + "//iamf/common:obu_util", "@com_google_absl//absl/base:no_destructor", "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/status", @@ -29,6 +30,7 @@ cc_library( ":iamf_input_layout", "//iamf/cli/proto:audio_frame_cc_proto", "//iamf/cli/proto:user_metadata_cc_proto", + "//iamf/common:obu_util", "@com_google_absl//absl/base:no_destructor", "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/status", @@ -88,9 +90,9 @@ cc_library( srcs = ["iamf_input_layout.cc"], hdrs = ["iamf_input_layout.h"], deps = [ + "//iamf/common:obu_util", "@com_google_absl//absl/base:no_destructor", "@com_google_absl//absl/container:flat_hash_map", - "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings", ], diff --git a/iamf/cli/adm_to_user_metadata/iamf/audio_element_handler.cc b/iamf/cli/adm_to_user_metadata/iamf/audio_element_handler.cc index 5ed943a..619d466 100644 --- a/iamf/cli/adm_to_user_metadata/iamf/audio_element_handler.cc +++ b/iamf/cli/adm_to_user_metadata/iamf/audio_element_handler.cc @@ -22,6 +22,7 @@ #include "iamf/cli/adm_to_user_metadata/iamf/iamf_input_layout.h" #include "iamf/cli/proto/audio_element.pb.h" #include "iamf/cli/proto/user_metadata.pb.h" +#include "iamf/common/obu_util.h" namespace iamf_tools { namespace adm_to_user_metadata { @@ -49,12 +50,8 @@ absl::StatusOr LookupNumSubstreamsFromInputLayout( {kAmbisonicsOrder3, 16}, }); - auto it = kInputLayoutToNumSubstreams->find(input_layout); - if (it == kInputLayoutToNumSubstreams->end()) { - return absl::NotFoundError(absl::StrCat( - "Num substreams not found for input_layout= ", input_layout)); - } - return it->second; + return LookupInMap(*kInputLayoutToNumSubstreams, input_layout, + "Number of channels for `IamfInputLayout`"); } absl::StatusOr LookupCoupledSubstreamCountFromInputLayout( @@ -74,12 +71,8 @@ absl::StatusOr LookupCoupledSubstreamCountFromInputLayout( {kBinaural, 1}, }); - auto it = kInputLayoutToCoupledSubstreamCount->find(input_layout); - if (it == kInputLayoutToCoupledSubstreamCount->end()) { - return absl::NotFoundError(absl::StrCat( - "Coupled substream count not found for input_layout= ", input_layout)); - } - return it->second; + return LookupInMap(*kInputLayoutToCoupledSubstreamCount, input_layout, + "Coupled substream count for `IamfInputLayout`"); } absl::StatusOr @@ -102,12 +95,8 @@ LookupLoudspeakerLayoutFromInputLayout(IamfInputLayout input_layout) { {kBinaural, LOUDSPEAKER_LAYOUT_BINAURAL}, }); - auto it = KInputLayoutToLoudspeakerLayout->find(input_layout); - if (it == KInputLayoutToLoudspeakerLayout->end()) { - return absl::NotFoundError(absl::StrCat( - "Loudspeaker layout not found for input_layout= ", input_layout)); - } - return it->second; + return LookupInMap(*KInputLayoutToLoudspeakerLayout, input_layout, + "Proto `LoudspeakerLayout` for `IamfInputLayout`"); } absl::StatusOr @@ -133,12 +122,8 @@ LookupAudioElementTypeFromInputLayout(IamfInputLayout input_layout) { {kAmbisonicsOrder3, AUDIO_ELEMENT_SCENE_BASED}, }); - auto it = KInputLayoutToAudioElementType->find(input_layout); - if (it == KInputLayoutToAudioElementType->end()) { - return absl::NotFoundError(absl::StrCat( - "Loudspeaker layout not found for input_layout= ", input_layout)); - } - return it->second; + return LookupInMap(*KInputLayoutToAudioElementType, input_layout, + "Proto `AudioElementType` for `IamfInputLayout`"); } absl::Status PopulateChannelBasedAudioElementMetadata( diff --git a/iamf/cli/adm_to_user_metadata/iamf/audio_frame_handler.cc b/iamf/cli/adm_to_user_metadata/iamf/audio_frame_handler.cc index 7fc8466..f0b443b 100644 --- a/iamf/cli/adm_to_user_metadata/iamf/audio_frame_handler.cc +++ b/iamf/cli/adm_to_user_metadata/iamf/audio_frame_handler.cc @@ -25,6 +25,7 @@ #include "iamf/cli/adm_to_user_metadata/iamf/iamf_input_layout.h" #include "iamf/cli/proto/audio_frame.pb.h" #include "iamf/cli/proto/user_metadata.pb.h" +#include "iamf/common/obu_util.h" namespace iamf_tools { namespace adm_to_user_metadata { @@ -59,12 +60,8 @@ absl::StatusOr> LookupLabelsFromInputLayout( "A11", "A12", "A13", "A14", "A15"}}, }); - auto it = kInputLayoutToLabels->find(input_layout); - if (it == kInputLayoutToLabels->end()) { - return absl::NotFoundError( - absl::StrCat("Labels not found for input_layout= ", input_layout)); - } - return it->second; + return LookupInMap(*kInputLayoutToLabels, input_layout, + "String-based channels labels for `IamfInputLayout`"); } } // namespace diff --git a/iamf/cli/adm_to_user_metadata/iamf/iamf_input_layout.cc b/iamf/cli/adm_to_user_metadata/iamf/iamf_input_layout.cc index 0ece10d..a8d73e3 100644 --- a/iamf/cli/adm_to_user_metadata/iamf/iamf_input_layout.cc +++ b/iamf/cli/adm_to_user_metadata/iamf/iamf_input_layout.cc @@ -12,14 +12,11 @@ #include "iamf/cli/adm_to_user_metadata/iamf/iamf_input_layout.h" -#include - #include "absl/base/no_destructor.h" #include "absl/container/flat_hash_map.h" -#include "absl/status/status.h" #include "absl/status/statusor.h" -#include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" +#include "iamf/common/obu_util.h" namespace iamf_tools { namespace adm_to_user_metadata { @@ -30,7 +27,7 @@ absl::StatusOr LookupInputLayoutFromAudioPackFormatId( // loudspeaker layout in IAMF. using enum IamfInputLayout; static const absl::NoDestructor< - absl::flat_hash_map> + absl::flat_hash_map> kAudioPackFormatIdToInputLayout({ {"AP_00010001", IamfInputLayout::kMono}, {"AP_00010002", IamfInputLayout::kStereo}, @@ -45,13 +42,8 @@ absl::StatusOr LookupInputLayoutFromAudioPackFormatId( {"AP_00040003", IamfInputLayout::kAmbisonicsOrder3}, }); - auto it = kAudioPackFormatIdToInputLayout->find(audio_pack_format_id); - if (it == kAudioPackFormatIdToInputLayout->end()) { - return absl::NotFoundError( - absl::StrCat("Input layout not found for audio_pack_format_id= ", - audio_pack_format_id)); - } - return it->second; + return LookupInMap(*kAudioPackFormatIdToInputLayout, audio_pack_format_id, + "`IamfInputLayout` for `audio_pack_format_id`"); } } // namespace adm_to_user_metadata diff --git a/iamf/cli/adm_to_user_metadata/iamf/mix_presentation_handler.cc b/iamf/cli/adm_to_user_metadata/iamf/mix_presentation_handler.cc index f13d385..9dc4d20 100644 --- a/iamf/cli/adm_to_user_metadata/iamf/mix_presentation_handler.cc +++ b/iamf/cli/adm_to_user_metadata/iamf/mix_presentation_handler.cc @@ -53,11 +53,8 @@ LookupSoundSystemFromInputLayout(IamfInputLayout layout) { {k7_1_4, SOUND_SYSTEM_J_4_7_0}, }); - auto it = kInputLayoutToSoundSystem->find(layout); - if (it == kInputLayoutToSoundSystem->end()) { - return absl::NotFoundError("Sound system not found for input_layout"); - } - return it->second; + return LookupInMap(*kInputLayoutToSoundSystem, layout, + "`SoundSystem` for `IamfInputLayout`"); } absl::Status CopyLoudness(const LoudnessMetadata& loudness_metadata, diff --git a/iamf/cli/channel_label.cc b/iamf/cli/channel_label.cc index 4223147..2585b02 100644 --- a/iamf/cli/channel_label.cc +++ b/iamf/cli/channel_label.cc @@ -13,7 +13,6 @@ #include #include -#include #include #include "absl/base/no_destructor.h" @@ -60,12 +59,8 @@ LookupEarChannelOrderFromNonExpandedLoudspeakerLayout( {kLayoutBinaural, {kL2, kR2}}, }); - auto it = kSoundSystemToLoudspeakerLayout->find(loudspeaker_layout); - if (it == kSoundSystemToLoudspeakerLayout->end()) { - return absl::InvalidArgumentError(absl::StrCat( - "Channel order not found for layout= ", loudspeaker_layout)); - } - return it->second; + return LookupInMap(*kSoundSystemToLoudspeakerLayout, loudspeaker_layout, + "`ChannelLabel::Label` for `LoudspeakerLayout`"); } void SetLabelsToOmittedExceptFor( @@ -180,7 +175,7 @@ absl::StatusOr ChannelLabel::StringToLabel( absl::string_view label) { using enum ChannelLabel::Label; static const absl::NoDestructor< - absl::flat_hash_map> + absl::flat_hash_map> kStringToChannelLabel({ {"Omitted", kOmitted}, {"M", kMono}, @@ -265,12 +260,8 @@ absl::StatusOr ChannelLabel::StringToLabel( {"A24", kA24}, }); - auto it = kStringToChannelLabel->find(label); - if (it == kStringToChannelLabel->end()) { - return absl::InvalidArgumentError( - absl::StrCat("Unknown string-based label= ", label)); - } - return it->second; + return LookupInMap(*kStringToChannelLabel, label, + "`Channel::Label` for string-based label"); } absl::StatusOr ChannelLabel::ProtoToLabel( @@ -607,13 +598,8 @@ absl::StatusOr ChannelLabel::GetDemixedLabel( {kR7, kDemixedR7}, {kLrs7, kDemixedLrs7}, {kRrs7, kDemixedRrs7}}); - - auto it = kChannelLabelToDemixedLabel->find(label); - if (it == kChannelLabelToDemixedLabel->end()) { - return absl::InvalidArgumentError(absl::StrCat( - "Demixed label is not known or allowed for label= ", label)); - } - return it->second; + return LookupInMap(*kChannelLabelToDemixedLabel, label, + "Demixed label for `ChannelLabel::Label`"); } absl::StatusOr> diff --git a/iamf/cli/obu_to_proto/BUILD b/iamf/cli/obu_to_proto/BUILD index 44709af..335d234 100644 --- a/iamf/cli/obu_to_proto/BUILD +++ b/iamf/cli/obu_to_proto/BUILD @@ -21,6 +21,5 @@ cc_library( "@com_google_absl//absl/log:check", "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", - "@com_google_absl//absl/strings", ], ) diff --git a/iamf/cli/obu_to_proto/parameter_block_metadata_generator.cc b/iamf/cli/obu_to_proto/parameter_block_metadata_generator.cc index 8ab9727..282d65f 100644 --- a/iamf/cli/obu_to_proto/parameter_block_metadata_generator.cc +++ b/iamf/cli/obu_to_proto/parameter_block_metadata_generator.cc @@ -20,7 +20,6 @@ #include "absl/log/log.h" #include "absl/status/status.h" #include "absl/status/statusor.h" -#include "absl/strings/str_cat.h" #include "iamf/cli/proto/parameter_block.pb.h" #include "iamf/cli/proto/parameter_data.pb.h" #include "iamf/common/macros.h" @@ -113,14 +112,9 @@ absl::Status CopyDMixPMode(DemixingInfoParameterData::DMixPMode obu_dmixp_mode, {kDMixPMode3_n, DMIXP_MODE_3_N}, {kDMixPModeReserved2, DMIXP_MODE_RESERVED_B}, }); - if (!LookupInMap(*kObuDmixPModeToMetadataDMixPMode, obu_dmixp_mode, - dmixp_mode) - .ok()) { - return absl::InvalidArgumentError( - absl::StrCat("Unknown dmixp_mode = ", obu_dmixp_mode)); - } - return absl::OkStatus(); + return CopyFromMap(*kObuDmixPModeToMetadataDMixPMode, obu_dmixp_mode, + "Proto version of internal `DMixPMode`", dmixp_mode); } // Gets the proto representation of the input `mix_gain_parameter_data`. diff --git a/iamf/cli/proto_to_obu/arbitrary_obu_generator.cc b/iamf/cli/proto_to_obu/arbitrary_obu_generator.cc index e96bbcf..fdf4b71 100644 --- a/iamf/cli/proto_to_obu/arbitrary_obu_generator.cc +++ b/iamf/cli/proto_to_obu/arbitrary_obu_generator.cc @@ -73,13 +73,9 @@ absl::Status CopyArbitraryObuType( {OBU_IA_SEQUENCE_HEADER, kObuIaSequenceHeader}, }); - if (!LookupInMap(*kArbitraryObuTypeToObuType, arbitrary_obu_type, - output_obu_type) - .ok()) { - return absl::InvalidArgumentError( - absl::StrCat("Unknown arbitrary_obu_type= ", arbitrary_obu_type)); - } - return absl::OkStatus(); + return CopyFromMap(*kArbitraryObuTypeToObuType, arbitrary_obu_type, + "Internal version of proto `ArbitraryObuType`", + output_obu_type); } } // namespace diff --git a/iamf/cli/proto_to_obu/audio_element_generator.cc b/iamf/cli/proto_to_obu/audio_element_generator.cc index 0f7288c..3fafa66 100644 --- a/iamf/cli/proto_to_obu/audio_element_generator.cc +++ b/iamf/cli/proto_to_obu/audio_element_generator.cc @@ -721,16 +721,10 @@ absl::Status CopyLoudspeakerLayout( {LOUDSPEAKER_LAYOUT_EXPANDED, kLayoutExpanded}, }); - if (!LookupInMap(*kInputLoudspeakerLayoutToOutputLoudspeakerLayout, - input_channel_audio_layer_config.loudspeaker_layout(), - output_loudspeaker_layout) - .ok()) { - return InvalidArgumentError( - StrCat("Unknown loudspeaker_layout= ", - input_channel_audio_layer_config.loudspeaker_layout())); - } - - return absl::OkStatus(); + return CopyFromMap(*kInputLoudspeakerLayoutToOutputLoudspeakerLayout, + input_channel_audio_layer_config.loudspeaker_layout(), + "Internal version of proto `LoudspeakerLayout`= ", + output_loudspeaker_layout); } // Copies the `ExpandedLoudspeakerLayout` based on the input data. @@ -760,15 +754,10 @@ absl::Status CopyExpandedLoudspeakerLayout( {EXPANDED_LOUDSPEAKER_LAYOUT_TOP_6_CH, kExpandedLayoutTop6Ch}, }); - if (!LookupInMap(*kInputLoudspeakerLayoutToOutputLoudspeakerLayout, - input_expanded_loudspeaker_layout, - output_expanded_loudspeaker_layout) - .ok()) { - return InvalidArgumentError(StrCat("Unknown expanded_loudspeaker_layout= ", - input_expanded_loudspeaker_layout)); - } - - return absl::OkStatus(); + return CopyFromMap(*kInputLoudspeakerLayoutToOutputLoudspeakerLayout, + input_expanded_loudspeaker_layout, + "Internal version of proto `ExpandedLoudspeakerLayout`= ", + output_expanded_loudspeaker_layout); } // Copies the `LoudspeakerLayout` and `ExpandedLoudspeakerLayout` based on the diff --git a/iamf/cli/proto_to_obu/codec_config_generator.cc b/iamf/cli/proto_to_obu/codec_config_generator.cc index ca80b22..164dde7 100644 --- a/iamf/cli/proto_to_obu/codec_config_generator.cc +++ b/iamf/cli/proto_to_obu/codec_config_generator.cc @@ -66,13 +66,9 @@ absl::Status CopyCodecId( {CODEC_ID_LPCM, kCodecIdLpcm}, }); - if (!LookupInMap(*kInputCodecIdToOutputCodecId, input_codec_config.codec_id(), - output_codec_id) - .ok()) { - return absl::InvalidArgumentError(absl::StrCat( - "Unknown codec with codec_id= ", input_codec_config.codec_id())); - } - return absl::OkStatus(); + return CopyFromMap(*kInputCodecIdToOutputCodecId, + input_codec_config.codec_id(), + "Internal version of proto `CodecId`= ", output_codec_id); } absl::Status CopyFlacBlockType( @@ -91,13 +87,9 @@ absl::Status CopyFlacBlockType( {FLAC_BLOCK_TYPE_CUESHEET, kFlacCuesheet}, {FLAC_BLOCK_TYPE_PICTURE, kFlacPicture}}); - if (!LookupInMap(*kInputFlacBlockTypeToOutputFlacBlockType, - input_flac_block_type, output_flac_block_type) - .ok()) { - return absl::InvalidArgumentError( - absl::StrCat("Unknown input_flac_block_type= ", input_flac_block_type)); - } - return absl::OkStatus(); + return CopyFromMap( + *kInputFlacBlockTypeToOutputFlacBlockType, input_flac_block_type, + "Internal version of proto `FlacBlockType`", output_flac_block_type); } absl::Status CopySampleFrequencyIndex( @@ -131,14 +123,10 @@ absl::Status CopySampleFrequencyIndex( kSampleFrequencyIndexEscapeValue}, }); - if (!LookupInMap(*kInputSampleFrequencyIndexToOutputSampleFrequencyIndex, - input_sample_frequency_index, output_sample_frequency_index) - .ok()) { - return absl::InvalidArgumentError( - absl::StrCat("Unknown input_sample_frequency_index= ", - input_sample_frequency_index)); - } - return absl::OkStatus(); + return CopyFromMap(*kInputSampleFrequencyIndexToOutputSampleFrequencyIndex, + input_sample_frequency_index, + "Internal version of proto `SampleFrequencyIndex`", + output_sample_frequency_index); } absl::Status GenerateLpcmDecoderConfig( diff --git a/iamf/cli/proto_to_obu/mix_presentation_generator.cc b/iamf/cli/proto_to_obu/mix_presentation_generator.cc index 7e4ba2d..b9702b0 100644 --- a/iamf/cli/proto_to_obu/mix_presentation_generator.cc +++ b/iamf/cli/proto_to_obu/mix_presentation_generator.cc @@ -338,13 +338,9 @@ absl::Status MixPresentationGenerator::CopySoundSystem( {SOUND_SYSTEM_13_6_9_0, kSoundSystem13_6_9_0}, }); - if (!LookupInMap(*kInputSoundSystemToOutputSoundSystem, input_sound_system, - output_sound_system) - .ok()) { - return absl::InvalidArgumentError( - absl::StrCat("Unknown input_sound_system= ", input_sound_system)); - } - return absl::OkStatus(); + return CopyFromMap( + *kInputSoundSystemToOutputSoundSystem, input_sound_system, + "Internal version of proto `SoundSystem`= ", output_sound_system); } absl::Status MixPresentationGenerator::CopyInfoType( @@ -384,14 +380,12 @@ absl::Status MixPresentationGenerator::CopyInfoType( uint8_t accumulated_info_type_bitmask = 0; for (int i = 0; i < input_loudness_info.info_type_bit_masks_size(); ++i) { LoudnessInfo::InfoTypeBitmask user_output_bit_mask; - if (!LookupInMap( - *kInputLoudnessInfoTypeBitMaskToOutputLoudnessInfoTypeBitMask, - input_loudness_info.info_type_bit_masks(i), user_output_bit_mask) - .ok()) { - return absl::InvalidArgumentError( - absl::StrCat("Unknown info_type_bit_masks(", i, - ")= ", input_loudness_info.info_type_bit_masks(i))); - } + RETURN_IF_NOT_OK(CopyFromMap( + *kInputLoudnessInfoTypeBitMaskToOutputLoudnessInfoTypeBitMask, + input_loudness_info.info_type_bit_masks(i), + absl::StrCat("Internal version of proto `LoudnessInfoTypeBitMask(", i, + ")= "), + user_output_bit_mask)); // Track the accumulated bit mask. accumulated_info_type_bitmask |= static_cast(user_output_bit_mask); diff --git a/iamf/cli/proto_to_obu/tests/arbitrary_obu_generator_test.cc b/iamf/cli/proto_to_obu/tests/arbitrary_obu_generator_test.cc index eddfa06..b48619d 100644 --- a/iamf/cli/proto_to_obu/tests/arbitrary_obu_generator_test.cc +++ b/iamf/cli/proto_to_obu/tests/arbitrary_obu_generator_test.cc @@ -216,20 +216,18 @@ class ArbitraryObuGeneratorTest : public testing::Test { public: ArbitraryObuGeneratorTest() {} - void InitAndTestGenerate() { + void InitAndTestGenerateExpectOk() { // Generate the OBUs. std::list output_obus; ArbitraryObuGenerator generator(arbitrary_obu_metadata_); - EXPECT_EQ(generator.Generate(output_obus).code(), - expected_generate_status_code_); + EXPECT_THAT(generator.Generate(output_obus), IsOk()); + EXPECT_EQ(output_obus, expected_obus_); } protected: ArbitraryObuMetadatas arbitrary_obu_metadata_; - absl::StatusCode expected_generate_status_code_ = absl::StatusCode::kOk; - std::list expected_obus_; }; @@ -250,7 +248,7 @@ TEST_F(ArbitraryObuGeneratorTest, ReservedObu) { expected_obus_.emplace_back(kObuIaReserved24, ObuHeader(), std::vector({'a', 'b', 'c'}), ArbitraryObu::kInsertionHookBeforeDescriptors); - InitAndTestGenerate(); + InitAndTestGenerateExpectOk(); } TEST_F(ArbitraryObuGeneratorTest, ObuWithExtensionHeader) { @@ -276,7 +274,7 @@ TEST_F(ArbitraryObuGeneratorTest, ObuWithExtensionHeader) { .extension_header_bytes = {'e', 'x', 't', 'r', 'a'}}, std::vector({'i', 'a', 'm', 'f', '\0', '\0'}), ArbitraryObu::kInsertionHookAfterDescriptors); - InitAndTestGenerate(); + InitAndTestGenerateExpectOk(); } TEST_F(ArbitraryObuGeneratorTest, InvalidObuType) { @@ -292,9 +290,10 @@ TEST_F(ArbitraryObuGeneratorTest, InvalidObuType) { payload: "" )pb", arbitrary_obu_metadata_.Add())); - expected_generate_status_code_ = absl::StatusCode::kInvalidArgument; + std::list output_obus; + ArbitraryObuGenerator generator(arbitrary_obu_metadata_); - InitAndTestGenerate(); + EXPECT_FALSE(generator.Generate(output_obus).ok()); } } // namespace diff --git a/iamf/cli/recon_gain_generator.cc b/iamf/cli/recon_gain_generator.cc index fdd69e8..9a750a3 100644 --- a/iamf/cli/recon_gain_generator.cc +++ b/iamf/cli/recon_gain_generator.cc @@ -18,7 +18,6 @@ #include "absl/container/flat_hash_map.h" #include "absl/log/log.h" #include "absl/status/status.h" -#include "absl/strings/str_cat.h" #include "iamf/cli/channel_label.h" #include "iamf/cli/demixing_module.h" #include "iamf/common/macros.h" @@ -67,11 +66,10 @@ absl::Status FindRelevantMixedSamples( {kDemixedR2, kMono}}); ChannelLabel::Label relevant_mixed_label; - if (!LookupInMap(*kLabelToRelevantMixedLabel, label, relevant_mixed_label) - .ok()) { - return absl::InvalidArgumentError(absl::StrCat( - "Failed to find relevant mixed label associated with label= ", label)); - } + RETURN_IF_NOT_OK( + CopyFromMap(*kLabelToRelevantMixedLabel, label, + "`relevant_mixed_label` for demixed `ChannelLabel::Label`", + relevant_mixed_label)); LOG_IF(INFO, additional_logging) << "Relevant mixed samples has label: " << relevant_mixed_label; diff --git a/iamf/cli/renderer/BUILD b/iamf/cli/renderer/BUILD index a2e8964..69b9c32 100644 --- a/iamf/cli/renderer/BUILD +++ b/iamf/cli/renderer/BUILD @@ -58,6 +58,7 @@ cc_library( "//iamf/cli:channel_label", "//iamf/cli:demixing_module", "//iamf/common:macros", + "//iamf/common:obu_util", "//iamf/obu:mix_presentation", "//iamf/obu:types", "@com_google_absl//absl/base:no_destructor", diff --git a/iamf/cli/renderer/audio_element_renderer_passthrough.cc b/iamf/cli/renderer/audio_element_renderer_passthrough.cc index 198cdd2..a4ae4c7 100644 --- a/iamf/cli/renderer/audio_element_renderer_passthrough.cc +++ b/iamf/cli/renderer/audio_element_renderer_passthrough.cc @@ -15,7 +15,6 @@ #include #include -#include #include #include "absl/base/no_destructor.h" @@ -60,14 +59,13 @@ absl::StatusOr IsLoudspeakerLayoutEquivalentToSoundSystem( {kSoundSystem11_2_3_0, kLayout3_1_2_ch}, }); - const auto equivalent_loudspeaker_layout_iter = - kSoundSystemToLoudspeakerLayout->find(layout); - if (equivalent_loudspeaker_layout_iter == - kSoundSystemToLoudspeakerLayout->end()) { - return absl::InvalidArgumentError( - absl::StrCat("Sound system not found for layout= ", layout)); - } - return equivalent_loudspeaker_layout_iter->second == loudspeaker_layout; + ChannelAudioLayerConfig::LoudspeakerLayout equivalent_loudspeaker_layout; + RETURN_IF_NOT_OK( + CopyFromMap(*kSoundSystemToLoudspeakerLayout, layout, + "`LoudspeakerLayout` equivalent to `SoundSystem`", + equivalent_loudspeaker_layout)); + + return equivalent_loudspeaker_layout == loudspeaker_layout; } // Several expanded layouts are defined as being based on a particular sound diff --git a/iamf/cli/renderer/renderer_utils.cc b/iamf/cli/renderer/renderer_utils.cc index c865f6d..c281425 100644 --- a/iamf/cli/renderer/renderer_utils.cc +++ b/iamf/cli/renderer/renderer_utils.cc @@ -21,6 +21,7 @@ #include "iamf/cli/channel_label.h" #include "iamf/cli/demixing_module.h" #include "iamf/common/macros.h" +#include "iamf/common/obu_util.h" #include "iamf/obu/mix_presentation.h" #include "iamf/obu/types.h" @@ -147,12 +148,8 @@ absl::StatusOr LookupOutputKeyFromPlaybackLayout( {kSoundSystem13_6_9_0, "9.1.6"}, }); - auto it = kSoundSystemToOutputKey->find(sound_system); - if (it == kSoundSystemToOutputKey->end()) { - return absl::InvalidArgumentError(absl::StrCat( - "Output key not found for sound_system= ", sound_system)); - } - return it->second; + return LookupInMap(*kSoundSystemToOutputKey, sound_system, + "Output key for `SoundSystem`"); } case kLayoutTypeBinaural: diff --git a/iamf/common/obu_util.cc b/iamf/common/obu_util.cc index a57563a..ef2946f 100644 --- a/iamf/common/obu_util.cc +++ b/iamf/common/obu_util.cc @@ -221,14 +221,14 @@ bool IsNativeBigEndian() { absl::Status ValidateVectorSizeEqual(const std::string& field_name, size_t vector_size, DecodedUleb128 obu_reported_size) { - if (vector_size != obu_reported_size) { - auto error_message = absl::StrCat( - "Found inconsistency with `", field_name, ".size()`= ", vector_size, - ". Expected a value of ", obu_reported_size, "."); - LOG(ERROR) << error_message; - return absl::InvalidArgumentError(error_message); - } - return absl::OkStatus(); + if (vector_size == obu_reported_size) [[likely]] { + return absl::OkStatus(); + } + auto error_message = absl::StrCat( + "Found inconsistency with `", field_name, ".size()`= ", vector_size, + ". Expected a value of ", obu_reported_size, "."); + LOG(ERROR) << error_message; + return absl::InvalidArgumentError(error_message); } } // namespace iamf_tools diff --git a/iamf/common/obu_util.h b/iamf/common/obu_util.h index 75527d2..7f7d667 100644 --- a/iamf/common/obu_util.h +++ b/iamf/common/obu_util.h @@ -217,16 +217,64 @@ absl::Status ValidateVectorSizeEqual(const std::string& field_name, size_t vector_size, DecodedUleb128 obu_reported_size); +/*!\brief Looks up a key in a map and returns a status or value. + * + * When lookup fails the error message will contain the `context` string + * followed by "= $KEY", where $KEY is the stringified `key`. + * + * Some mapping have sufficient context in the typenames, for example: + * - Input Map: A map of `PersonName` to `Birthday`. + * - Typename-based context: "`Birthday` for `PersonName`". + * - Output message: "`Birthday` for `PersonName`= John was not found in the + * map.". + * + * Some mappings provide insufficient context in the typenames. Or the typenames + * would be easily confused. Variable names or phrases should be used as + * context: + * - Input Map: A map of `absl::string_view` names to `int` ages. + * - Variable-based context: "`age` for `name`". + * - Phrase-based context: or "Age for name". + * Or: + * - Input Map: A map of `proto::Type` to `iamf_tools::Type`. + * - Phrase-based context: "Internal version of proto `Type`". + * + * \param map Map to search. + * \param key Key to search for. + * \param context Context to insert into the error message for debugging + * purposes. + * \return Associated value if lookup is successful. `absl::NotFoundError()` + * when lookup fails. + */ template -absl::Status LookupInMap(const absl::flat_hash_map& map, T key, - U& value) { +absl::StatusOr LookupInMap(const absl::flat_hash_map& map, + const T& key, absl::string_view context) { auto iter = map.find(key); - if (iter == map.end()) { - return absl::InvalidArgumentError( - absl::StrCat("Key = ", key, " not found in the map.")); + if (iter != map.end()) [[likely]] { + return iter->second; } - value = iter->second; - return absl::OkStatus(); + return absl::NotFoundError( + absl::StrCat(context, "= ", key, " was not found in the map.")); +} + +/*!\brief Looks up a key in a map and copies the value to the output argument. + * + * \param map Map to search. + * \param key Key to search for. + * \param context Context to insert into the error message for debugging + * purposes. Forwared to `LookupInMap` which has detailed documentation + * on usage. + * \param value Output argument to write the value to. + * \return `absl::OkStatus()` if lookup is successful. `absl::NotFoundError()` + * when lookup fails. + */ +template +absl::Status CopyFromMap(const absl::flat_hash_map& map, const T& key, + absl::string_view context, U& value) { + const auto& result = LookupInMap(map, key, context); + if (result.ok()) [[likely]] { + value = *result; + } + return result.status(); } /*!\brief Returns `absl::OkStatus()` if the arguments are equal. @@ -240,8 +288,8 @@ absl::Status LookupInMap(const absl::flat_hash_map& map, T key, */ template absl::Status ValidateEqual(const T& lhs, const T& rhs, - const std::string& context) { - if (lhs == rhs) { + absl::string_view context) { + if (lhs == rhs) [[likely]] { return absl::OkStatus(); } @@ -260,8 +308,8 @@ absl::Status ValidateEqual(const T& lhs, const T& rhs, */ template absl::Status ValidateNotEqual(const T& lhs, const T& rhs, - const std::string& context) { - if (lhs != rhs) { + absl::string_view context) { + if (lhs != rhs) [[likely]] { return absl::OkStatus(); } @@ -279,8 +327,8 @@ absl::Status ValidateNotEqual(const T& lhs, const T& rhs, */ template absl::Status ValidateHasValue(const std::optional& argument, - const absl::string_view context) { - if (argument.has_value()) { + absl::string_view context) { + if (argument.has_value()) [[likely]] { return absl::OkStatus(); } @@ -299,7 +347,7 @@ absl::Status ValidateHasValue(const std::optional& argument, */ template absl::Status ValidateUnique(InputIt first, InputIt last, - const std::string& context) { + absl::string_view context) { absl::flat_hash_set seen_values; for (auto iter = first; iter != last; ++iter) { diff --git a/iamf/common/tests/BUILD b/iamf/common/tests/BUILD index ad9940e..c1e9477 100644 --- a/iamf/common/tests/BUILD +++ b/iamf/common/tests/BUILD @@ -19,6 +19,7 @@ cc_test( "//iamf/cli/tests:cli_test_utils", "//iamf/common:obu_util", "//iamf/common:write_bit_buffer", + "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/status", "@com_google_absl//absl/status:status_matchers", "@com_google_absl//absl/strings:string_view", diff --git a/iamf/common/tests/obu_util_test.cc b/iamf/common/tests/obu_util_test.cc index d112828..b6dcdb6 100644 --- a/iamf/common/tests/obu_util_test.cc +++ b/iamf/common/tests/obu_util_test.cc @@ -20,6 +20,7 @@ #include #include +#include "absl/container/flat_hash_map.h" #include "absl/status/status.h" #include "absl/status/status_matchers.h" #include "absl/strings/string_view.h" @@ -33,6 +34,10 @@ namespace iamf_tools { namespace { using ::absl_testing::IsOk; +using ::absl_testing::IsOkAndHolds; +using ::absl_testing::StatusIs; + +constexpr absl::string_view kOmitContext = ""; TEST(AddUint32CheckOverflow, SmallInput) { uint32_t result; @@ -792,45 +797,81 @@ TEST(WritePcmSample, InvalidOver32Bits) { absl::StatusCode::kInvalidArgument); } +TEST(CopyFromMap, ReturnsOkWhenLookupSucceeds) { + const absl::flat_hash_map kIntegerToIsPrime = { + {1, false}, {2, true}, {3, true}, {4, false}}; + + bool result; + EXPECT_THAT(CopyFromMap(kIntegerToIsPrime, 3, kOmitContext, result), IsOk()); + + EXPECT_TRUE(result); +} + +TEST(CopyFromMap, ReturnsStatusNotFoundWhenLookupFails) { + const absl::flat_hash_map kIntegerToIsPrime = { + {1, false}, {2, true}, {3, true}, {4, false}}; + + bool undefined_result; + EXPECT_THAT( + CopyFromMap(kIntegerToIsPrime, -1, kOmitContext, undefined_result), + StatusIs(absl::StatusCode::kNotFound)); +} + +TEST(LookupInMapStatusOr, OkIfLookupSucceeds) { + const absl::flat_hash_map kIntegerToIsPrime = { + {1, false}, {2, true}, {3, true}, {4, false}}; + + EXPECT_THAT(LookupInMap(kIntegerToIsPrime, 3, kOmitContext), + IsOkAndHolds(true)); +} + +TEST(LookupInMapStatusOr, ReturnsStatusNotFoundWhenLookupFails) { + const absl::flat_hash_map kIntegerToIsPrime = { + {1, false}, {2, true}, {3, true}, {4, false}}; + + EXPECT_THAT(LookupInMap(kIntegerToIsPrime, -1, kOmitContext), + StatusIs(absl::StatusCode::kNotFound)); +} + TEST(ValidateEqual, OkIfArgsAreEqual) { const auto kLeftArg = 123; const auto kRightArg = 123; - EXPECT_THAT(ValidateEqual(kLeftArg, kRightArg, ""), IsOk()); + EXPECT_THAT(ValidateEqual(kLeftArg, kRightArg, kOmitContext), IsOk()); } TEST(ValidateEqual, NotOkIfArgsAreNotEqual) { const auto kLeftArg = 123; const auto kUnequalRightArg = 223; - EXPECT_FALSE(ValidateEqual(kLeftArg, kUnequalRightArg, "").ok()); + EXPECT_FALSE(ValidateEqual(kLeftArg, kUnequalRightArg, kOmitContext).ok()); } TEST(ValidateNotEqual, OkIfArgsAreNotEqual) { const auto kLeftArg = 123; const auto kRightArg = 124; - EXPECT_THAT(ValidateNotEqual(kLeftArg, kRightArg, ""), IsOk()); + EXPECT_THAT(ValidateNotEqual(kLeftArg, kRightArg, kOmitContext), IsOk()); } TEST(ValidateNotEqual, NotOkIfArgsAreEqual) { const auto kLeftArg = 123; const auto kEqualRightArg = 123; - EXPECT_FALSE(ValidateNotEqual(kLeftArg, kEqualRightArg, "").ok()); + EXPECT_FALSE(ValidateNotEqual(kLeftArg, kEqualRightArg, kOmitContext).ok()); } TEST(ValidateHasValue, OkIfArgHasValue) { constexpr std::optional kArg = 123; - EXPECT_THAT(ValidateHasValue(kArg, ""), IsOk()); + EXPECT_THAT(ValidateHasValue(kArg, kOmitContext), IsOk()); } TEST(ValidateHasValue, NotOkIfArgDoesNotHaveValue) { constexpr std::optional kArg = std::nullopt; - EXPECT_FALSE(ValidateHasValue(kArg, "").ok()); + EXPECT_FALSE(ValidateHasValue(kArg, kOmitContext).ok()); } TEST(ValidateUnique, OkIfArgsAreUnique) { const std::vector kVectorWithUniqueValues = {1, 2, 3, 99}; EXPECT_THAT(ValidateUnique(kVectorWithUniqueValues.begin(), - kVectorWithUniqueValues.end(), ""), + kVectorWithUniqueValues.end(), kOmitContext), IsOk()); } @@ -838,7 +879,7 @@ TEST(ValidateUnique, NotOkIfArgsAreNotUnique) { const std::vector kVectorWithDuplicateValues = {1, 2, 3, 99, 1}; EXPECT_FALSE(ValidateUnique(kVectorWithDuplicateValues.begin(), - kVectorWithDuplicateValues.end(), "") + kVectorWithDuplicateValues.end(), kOmitContext) .ok()); } diff --git a/iamf/obu/mix_presentation.cc b/iamf/obu/mix_presentation.cc index 546bced..c1deda3 100644 --- a/iamf/obu/mix_presentation.cc +++ b/iamf/obu/mix_presentation.cc @@ -432,14 +432,8 @@ absl::Status MixPresentationObu::GetNumChannelsFromLayout( loudness_layout.specific_layout) .sound_system; - if (!LookupInMap(*kSoundSystemToNumChannels, sound_system, num_channels) - .ok()) { - return absl::InvalidArgumentError( - absl::StrCat("Unknown number of channels for reserved or out of " - "bounds sound_system= ", - sound_system)); - } - return absl::OkStatus(); + return CopyFromMap(*kSoundSystemToNumChannels, sound_system, + "Number of channels for `SoundSystem`", num_channels); } case kLayoutTypeReserved0: case kLayoutTypeReserved1: