Skip to content

Commit

Permalink
WavSampleProvider: Add a warning when channel_ids are duplicated.
Browse files Browse the repository at this point in the history
  - Duplicating channel IDs means the same audio channel from a wav file represents multiple labels.
    - E.g. the user provides a one channel wav file and maps the single channel to both L2 and R2.
    - This is strange, but we can and have safely handled it for a while. Log and warn when this occurs.
  - Add tests for existing behavior:
    - `channel_ids` may be duplicated.
    - `channel_labels` may not be duplicated.
  - Drive-by: Update some names that are really just testing `Initialize`.

PiperOrigin-RevId: 686896385
  • Loading branch information
jwcullen committed Oct 18, 2024
1 parent 309e6eb commit 2c6a59e
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 3 deletions.
1 change: 1 addition & 0 deletions iamf/cli/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,7 @@ cc_library(
":wav_reader",
"//iamf/cli/proto:audio_frame_cc_proto",
"//iamf/common:macros",
"//iamf/common:obu_util",
"//iamf/obu:types",
"@com_google_absl//absl/container:flat_hash_map",
"@com_google_absl//absl/log",
Expand Down
36 changes: 33 additions & 3 deletions iamf/cli/tests/wav_sample_provider_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,36 @@ TEST(Initialize, FailsForUnknownLabels) {
wav_sample_provider.Initialize(GetInputWavDir(), audio_elements).ok());
}

TEST(Initialize, SucceedsForDuplicateChannelIds) {
constexpr uint32_t kDuplicateChannelId = 0;
iamf_tools_cli_proto::UserMetadata user_metadata;
absl::flat_hash_map<DecodedUleb128, AudioElementWithData> audio_elements;
InitializeTestData(kSampleRate, user_metadata, audio_elements);
user_metadata.mutable_audio_frame_metadata(0)->set_channel_ids(
0, kDuplicateChannelId);
user_metadata.mutable_audio_frame_metadata(0)->set_channel_ids(
1, kDuplicateChannelId);
WavSampleProvider wav_sample_provider(user_metadata.audio_frame_metadata());

EXPECT_THAT(wav_sample_provider.Initialize(GetInputWavDir(), audio_elements),
IsOk());
}

TEST(Initialize, FailsForDuplicateChannelLabels) {
constexpr absl::string_view kDuplicateLabel = "L2";
iamf_tools_cli_proto::UserMetadata user_metadata;
absl::flat_hash_map<DecodedUleb128, AudioElementWithData> audio_elements;
InitializeTestData(kSampleRate, user_metadata, audio_elements);
user_metadata.mutable_audio_frame_metadata(0)->set_channel_labels(
0, kDuplicateLabel);
user_metadata.mutable_audio_frame_metadata(0)->set_channel_labels(
1, kDuplicateLabel);
WavSampleProvider wav_sample_provider(user_metadata.audio_frame_metadata());

EXPECT_FALSE(
wav_sample_provider.Initialize(GetInputWavDir(), audio_elements).ok());
}

TEST(Initialize, FailsForChannelIdTooLarge) {
iamf_tools_cli_proto::UserMetadata user_metadata;
absl::flat_hash_map<DecodedUleb128, AudioElementWithData> audio_elements;
Expand All @@ -114,7 +144,7 @@ TEST(Initialize, FailsForChannelIdTooLarge) {
wav_sample_provider.Initialize(GetInputWavDir(), audio_elements).ok());
}

TEST(WavSampleProviderTest, MismatchingChannelIdsAndLabels) {
TEST(Initialize, FailsForDifferentSizedChannelIdsAndChannelLabels) {
iamf_tools_cli_proto::UserMetadata user_metadata;
absl::flat_hash_map<DecodedUleb128, AudioElementWithData> audio_elements;
InitializeTestData(kSampleRate, user_metadata, audio_elements);
Expand All @@ -128,7 +158,7 @@ TEST(WavSampleProviderTest, MismatchingChannelIdsAndLabels) {
wav_sample_provider.Initialize(GetInputWavDir(), audio_elements).ok());
}

TEST(WavSampleProviderTest, BitDepthLowerThanFile) {
TEST(Initialize, FailsForBitDepthLowerThanFile) {
iamf_tools_cli_proto::UserMetadata user_metadata;
absl::flat_hash_map<DecodedUleb128, AudioElementWithData> audio_elements;
InitializeTestData(kSampleRate, user_metadata, audio_elements);
Expand All @@ -143,7 +173,7 @@ TEST(WavSampleProviderTest, BitDepthLowerThanFile) {
wav_sample_provider.Initialize(GetInputWavDir(), audio_elements).ok());
}

TEST(WavSampleProviderTest, MismatchingSampleRates) {
TEST(Initialize, FailsForMismatchingSampleRates) {
iamf_tools_cli_proto::UserMetadata user_metadata;
absl::flat_hash_map<DecodedUleb128, AudioElementWithData> audio_elements;

Expand Down
10 changes: 10 additions & 0 deletions iamf/cli/wav_sample_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "iamf/cli/demixing_module.h"
#include "iamf/cli/wav_reader.h"
#include "iamf/common/macros.h"
#include "iamf/common/obu_util.h"
#include "iamf/obu/types.h"

namespace iamf_tools {
Expand All @@ -45,6 +46,15 @@ absl::Status WavSampleProvider::Initialize(
audio_frame_metadata.channel_ids_size(), " vs ",
audio_frame_metadata.channel_labels_size(), ")"));
}
if (!ValidateUnique(audio_frame_metadata.channel_ids().begin(),
audio_frame_metadata.channel_ids().end(), "channel ids")
.ok()) {
// OK. The user is claiming some channel IDs are shared between labels.
// This is strange, but permitted.
LOG(WARNING) << "Usually channel labels should be unique. Did you use "
"the same channel ID for different channels?";
}

// Precompute the `ChannelLabel::Label` for each channel label string.
RETURN_IF_NOT_OK(ChannelLabel::ConvertAndFillLabels(
audio_frame_metadata.channel_labels(),
Expand Down

0 comments on commit 2c6a59e

Please sign in to comment.