Skip to content

Commit

Permalink
Write out MixPresentationTags in the OBU. Enforce content_language
Browse files Browse the repository at this point in the history
…is plausibly a ISO 639-2 code.

  - Add missing call to `MixPresentationTags::ValidateAndWrite` to actually write this out with the OBU.
  - Enforce `content_language` is a three-character code at write time (i.e. plausibly ISO 639-2).
    - At read time we will ignore this validation to be robust to bad bitstreams.
    - The restriction is loosely enforced, a strict one could check vs. the set of ISO 639-2.
    - ISO 639-2 is not to be confused with BCP-47 (see `annotations_language`).
    - Null terminator is automatically handled and guarded by `MixPresentationTagsWriteAndValidate/WritesContentLanguageTag`.
  - Update test vectors to use valid ISO 639-2 codes.
  - `encoder_main_lib_test`: For debugging, also write out the test file name when a test fails.

PiperOrigin-RevId: 665038667
  • Loading branch information
jwcullen committed Aug 19, 2024
1 parent 0cb2194 commit 051cc03
Show file tree
Hide file tree
Showing 10 changed files with 98 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -388,11 +388,11 @@ TEST(Generate, CopiesDuplicateContentLanguageTags) {
auto* first_tag =
mix_presentation.mutable_mix_presentation_tags()->add_tags();
first_tag->set_tag_name("content_language");
first_tag->set_tag_value("en-us");
first_tag->set_tag_value("eng");
auto* second_tag =
mix_presentation.mutable_mix_presentation_tags()->add_tags();
second_tag->set_tag_name("content_language");
second_tag->set_tag_value("en-uk");
second_tag->set_tag_value("kor");

MixPresentationGenerator generator(mix_presentation_metadata);

Expand All @@ -405,10 +405,10 @@ TEST(Generate, CopiesDuplicateContentLanguageTags) {
ASSERT_EQ(first_obu.mix_presentation_tags_->tags.size(), 2);
EXPECT_EQ(first_obu.mix_presentation_tags_->tags[0].tag_name,
"content_language");
EXPECT_EQ(first_obu.mix_presentation_tags_->tags[0].tag_value, "en-us");
EXPECT_EQ(first_obu.mix_presentation_tags_->tags[0].tag_value, "eng");
EXPECT_EQ(first_obu.mix_presentation_tags_->tags[1].tag_name,
"content_language");
EXPECT_EQ(first_obu.mix_presentation_tags_->tags[1].tag_value, "en-uk");
EXPECT_EQ(first_obu.mix_presentation_tags_->tags[1].tag_value, "kor");
}

TEST(Generate, IgnoresTagsWhenSetIncludeMixPresentationTagsIsFalse) {
Expand Down
2 changes: 1 addition & 1 deletion iamf/cli/testdata/test_000704.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ mix_presentation_metadata {
num_tags: 1
tags: {
tag_name: "content_language"
tag_value: "en-us"
tag_value: "eng"
}
}
}
Expand Down
12 changes: 6 additions & 6 deletions iamf/cli/testdata/test_000705.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ mix_presentation_metadata [
mix_presentation_id: 42
count_label: 1
annotations_language: ["en-us"]
localized_presentation_annotations: ["Simulated US English Dialog (-0dB)."]
localized_presentation_annotations: ["Simulated English Dialog (-0dB)."]
num_sub_mixes: 1
sub_mixes {
num_audio_elements: 2
Expand Down Expand Up @@ -221,15 +221,15 @@ mix_presentation_metadata [
num_tags: 1
tags: {
tag_name: "content_language"
tag_value: "en-us"
tag_value: "eng"
}
}
},
{
mix_presentation_id: 43
count_label: 1
annotations_language: ["en-us"]
localized_presentation_annotations: ["Simulated UK English Dialog (-5dB)."]
localized_presentation_annotations: ["Simulated Korean Dialog (-5dB)."]
num_sub_mixes: 1
sub_mixes {
num_audio_elements: 2
Expand Down Expand Up @@ -297,15 +297,15 @@ mix_presentation_metadata [
num_tags: 1
tags: {
tag_name: "content_language"
tag_value: "en-uk"
tag_value: "kor"
}
}
},
{
mix_presentation_id: 44
count_label: 1
annotations_language: ["en-us"]
localized_presentation_annotations: ["Simulated AU English Dialog (-10dB)."]
localized_presentation_annotations: ["Simulated Esperanto Dialog (-10dB)."]
num_sub_mixes: 1
sub_mixes {
num_audio_elements: 2
Expand Down Expand Up @@ -373,7 +373,7 @@ mix_presentation_metadata [
num_tags: 1
tags: {
tag_name: "content_language"
tag_value: "en-au"
tag_value: "epo"
}
}
}
Expand Down
12 changes: 6 additions & 6 deletions iamf/cli/testdata/test_000706.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ mix_presentation_metadata [
mix_presentation_id: 42
count_label: 1
annotations_language: ["en-us"]
localized_presentation_annotations: ["Simulated US English Dialog (-0dB)."]
localized_presentation_annotations: ["Simulated English Dialog (-0dB)."]
num_sub_mixes: 1
sub_mixes {
num_audio_elements: 2
Expand Down Expand Up @@ -216,15 +216,15 @@ mix_presentation_metadata [
num_tags: 1
tags: {
tag_name: "content_language"
tag_value: "en-us"
tag_value: "eng"
}
}
},
{
mix_presentation_id: 43
count_label: 1
annotations_language: ["en-us"]
localized_presentation_annotations: ["Simulated UK English Dialog (-5dB)."]
localized_presentation_annotations: ["Simulated Korean Dialog (-5dB)."]
num_sub_mixes: 1
sub_mixes {
num_audio_elements: 2
Expand Down Expand Up @@ -292,15 +292,15 @@ mix_presentation_metadata [
num_tags: 1
tags: {
tag_name: "content_language"
tag_value: "en-uk"
tag_value: "kor"
}
}
},
{
mix_presentation_id: 44
count_label: 1
annotations_language: ["en-us"]
localized_presentation_annotations: ["Simulated AU English Dialog (-10dB)."]
localized_presentation_annotations: ["Simulated Esperanto Dialog (-10dB)."]
num_sub_mixes: 1
sub_mixes {
num_audio_elements: 2
Expand Down Expand Up @@ -368,7 +368,7 @@ mix_presentation_metadata [
num_tags: 1
tags: {
tag_name: "content_language"
tag_value: "en-au"
tag_value: "epo"
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion iamf/cli/testdata/test_000707.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -1188,7 +1188,7 @@ mix_presentation_metadata {
num_tags: 1
tags: {
tag_name: "content_language"
tag_value: "en-us"
tag_value: "eng"
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions iamf/cli/testdata/test_000710.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -1232,7 +1232,7 @@ mix_presentation_metadata [
num_tags: 1
tags: {
tag_name: "content_language"
tag_value: "en-us"
tag_value: "eng"
}
}
},
Expand Down Expand Up @@ -1308,7 +1308,7 @@ mix_presentation_metadata [
num_tags: 1
tags: {
tag_name: "content_language"
tag_value: "en-us"
tag_value: "eng"
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions iamf/cli/tests/encoder_main_lib_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -219,9 +219,9 @@ TEST_P(TestVector, ValidateTestSuite) {

// Check if the result matches the expected value in the protos.
if (user_metadata.test_vector_metadata().is_valid()) {
EXPECT_THAT(result, IsOk());
EXPECT_THAT(result, IsOk()) << "File= " << test_case.textproto_filename;
} else {
EXPECT_FALSE(result.ok());
EXPECT_FALSE(result.ok()) << " File= " << test_case.textproto_filename;
}
}

Expand Down
21 changes: 21 additions & 0 deletions iamf/obu/mix_presentation.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "absl/log/log.h"
#include "absl/status/status.h"
#include "absl/strings/str_cat.h"
#include "absl/strings/string_view.h"
#include "iamf/common/macros.h"
#include "iamf/common/obu_util.h"
#include "iamf/common/read_bit_buffer.h"
Expand Down Expand Up @@ -327,6 +328,20 @@ absl::Status MixPresentationSubMix::ReadAndValidate(const int32_t& count_label,
return absl::OkStatus();
}

absl::Status ValidateCompliesWithIso639_2(absl::string_view string) {
if (string.size() == 3) {
// Consider any any three character string valid. A stricter
// implementation could check it actually is present in the list of valid
// ISO-639-2 code.
return absl::OkStatus();
} else {
return absl::InvalidArgumentError(absl::StrCat(
"Expected an ISO-639-2 code. ISO-639-2 codes should have three "
"characters. string= ",
string));
}
}

absl::Status MixPresentationTags::ValidateAndWrite(WriteBitBuffer& wb) const {
RETURN_IF_NOT_OK(wb.WriteUnsignedLiteral(num_tags, 8));
RETURN_IF_NOT_OK(ValidateVectorSizeEqual("tags", tags.size(), num_tags));
Expand All @@ -335,6 +350,8 @@ absl::Status MixPresentationTags::ValidateAndWrite(WriteBitBuffer& wb) const {

for (const auto& tag : tags) {
if (tag.tag_name == "content_language") {
RETURN_IF_NOT_OK(ValidateCompliesWithIso639_2(tag.tag_value));

count_content_language_tag++;
}
RETURN_IF_NOT_OK(wb.WriteString(tag.tag_name));
Expand Down Expand Up @@ -474,6 +491,10 @@ absl::Status MixPresentationObu::ValidateAndWritePayload(
RETURN_IF_NOT_OK(ValidateAndWriteSubMix(count_label_, sub_mix, wb));
}

if (mix_presentation_tags_.has_value()) {
RETURN_IF_NOT_OK(mix_presentation_tags_->ValidateAndWrite(wb));
}

return absl::OkStatus();
}

Expand Down
1 change: 1 addition & 0 deletions iamf/obu/tests/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ cc_test(
"//iamf/common:bit_buffer_util",
"//iamf/common:read_bit_buffer",
"//iamf/common:write_bit_buffer",
"//iamf/common/tests:test_utils",
"//iamf/obu:leb128",
"//iamf/obu:mix_presentation",
"//iamf/obu:obu_header",
Expand Down
59 changes: 54 additions & 5 deletions iamf/obu/tests/mix_presentation_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "iamf/cli/leb_generator.h"
#include "iamf/common/bit_buffer_util.h"
#include "iamf/common/read_bit_buffer.h"
#include "iamf/common/tests/test_utils.h"
#include "iamf/common/write_bit_buffer.h"
#include "iamf/obu/leb128.h"
#include "iamf/obu/obu_header.h"
Expand Down Expand Up @@ -105,7 +106,8 @@ class MixPresentationObuTest : public ObuTestBase, public testing::Test {
.digital_peak = 19,
.true_peak = 20}}}}}),
dynamic_sub_mix_args_({{.element_mix_gain_subblocks = {{}},
.output_mix_gain_subblocks = {}}}) {
.output_mix_gain_subblocks = {}}}),
mix_presentation_tags_(std::nullopt) {
MixGainParamDefinition element_mix_gain;
element_mix_gain.parameter_id_ = 12;
element_mix_gain.parameter_rate_ = 13;
Expand Down Expand Up @@ -147,6 +149,8 @@ class MixPresentationObuTest : public ObuTestBase, public testing::Test {
// Length `num_sub_mixes`.
std::vector<DynamicSubMixArguments> dynamic_sub_mix_args_;

std::optional<MixPresentationTags> mix_presentation_tags_;

private:
// Copies over data into the output argument `obu->sub_mix[i]`.
void InitSubMixDynamicMemory(int i) {
Expand Down Expand Up @@ -787,6 +791,40 @@ TEST_F(MixPresentationObuTest, ValidateAndWriteFailsWithInvalidMissingStero) {
EXPECT_FALSE(obu_->ValidateAndWriteObu(unused_wb).ok());
}

TEST_F(MixPresentationObuTest, WritesMixPresentionTags) {
expected_header_ = {kObuIaMixPresentation << 3, 58};
expected_payload_ = {
// Start Mix OBU.
10, 1, 'e', 'n', '-', 'u', 's', '\0', 'M', 'i', 'x', ' ', '1', '\0', 1,
// Start Submix 1
1, 11, 'S', 'u', 'b', 'm', 'i', 'x', ' ', '1', '\0',
// Start RenderingConfig.
RenderingConfig::kHeadphonesRenderingModeStereo << 6, 0,
// End RenderingConfig.
12, 13, 0x80, 0, 14, 15, 16, 0x80, 0, 17, 1,
// Start Layout 1 (of Submix 1).
(Layout::kLayoutTypeLoudspeakersSsConvention << 6) |
LoudspeakersSsConventionLayout::kSoundSystemA_0_2_0,
LoudnessInfo::kTruePeak, 0, 18, 0, 19, 0, 20,
// Start Mix Presentation Tags.
// `num_tags`.
1,
// `tag_name[0]`.
't', 'a', 'g', '\0',
// `tag_value[0]`.
'v', 'a', 'l', 'u', 'e', '\0',
// End Mix OBU.
};
InitExpectOk();
obu_->mix_presentation_tags_ =
MixPresentationTags{.num_tags = 1, .tags = {{"tag", "value"}}};

WriteBitBuffer wb(1024);
EXPECT_THAT(obu_->ValidateAndWriteObu(wb), IsOk());

ValidateObuWriteResults(wb, expected_header_, expected_payload_);
}

class GetNumChannelsFromLayoutTest : public testing::Test {
public:
GetNumChannelsFromLayoutTest()
Expand Down Expand Up @@ -1329,15 +1367,15 @@ TEST(MixPresentationTagsWriteAndValidate, WritesWithZeroTags) {
TEST(MixPresentationTagsWriteAndValidate, WritesContentLanguageTag) {
constexpr uint8_t kOneTag = 1;
const MixPresentationTags kMixPresentationTagsWithContentLanguageTag = {
.num_tags = kOneTag, .tags = {{"content_language", "en-us"}}};
.num_tags = kOneTag, .tags = {{"content_language", "eng"}}};
const std::vector<uint8_t> kExpectedBuffer = {
// `num_tags`.
kOneTag,
// `tag_name[0]`.
'c', 'o', 'n', 't', 'e', 'n', 't', '_', 'l', 'a', 'n', 'g', 'u', 'a', 'g',
'e', '\0',
// `tag_value[0]`.
'e', 'n', '-', 'u', 's', '\0'};
'e', 'n', 'g', '\0'};
WriteBitBuffer wb(1024);

EXPECT_THAT(kMixPresentationTagsWithContentLanguageTag.ValidateAndWrite(wb),
Expand All @@ -1346,6 +1384,18 @@ TEST(MixPresentationTagsWriteAndValidate, WritesContentLanguageTag) {
EXPECT_EQ(wb.bit_buffer(), kExpectedBuffer);
}

TEST(MixPresentationTagsWriteAndValidate,
InvalidWhenContentLanguageTagNotThreeCharacters) {
constexpr uint8_t kOneTag = 1;
const MixPresentationTags kMixPresentationTagsWithContentLanguageTag = {
.num_tags = kOneTag, .tags = {{"content_language", "en-us"}}};

WriteBitBuffer wb(0);

EXPECT_FALSE(
kMixPresentationTagsWithContentLanguageTag.ValidateAndWrite(wb).ok());
}

TEST(MixPresentationTagsWriteAndValidate, WritesArbitraryTags) {
constexpr uint8_t kNumTags = 1;
const MixPresentationTags kMixPresentationTagsWithArbitraryTag = {
Expand Down Expand Up @@ -1391,8 +1441,7 @@ TEST(MixPresentationTagsWriteAndValidate, InvalidForDuplicateContentIdTag) {
const MixPresentationTags
kMixPresentationTagsWithDuplicateContentLanguageTag = {
.num_tags = kTwoTags,
.tags = {{"content_language", "en-us"},
{"content_language", "en-gb"}}};
.tags = {{"content_language", "eng"}, {"content_language", "kor"}}};

WriteBitBuffer wb(1024);

Expand Down

0 comments on commit 051cc03

Please sign in to comment.