Skip to content

Commit

Permalink
Move constraints to their own struct and use new Validate methods
Browse files Browse the repository at this point in the history 
This CL moves the values used to validate the FlacMetadataBlockStreamInfo into its own struct to organize them by purpose.

It also uses the new Validate methods to simplify some of the validation.

PiperOrigin-RevId: 721378564
  • Loading branch information
@trevorknight @jwcullen
trevorknight authored and jwcullen committed Feb 3, 2025
1 parent cd9a685 commit def6168
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 109 deletions.
10 changes: 4 additions & 6 deletions iamf/cli/proto_to_obu/tests/codec_config_generator_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -886,16 +886,14 @@ TEST_F(CodecConfigGeneratorTest, ObeysInvalidFlacStreamInfo) {
// IAMF requires several fields in the Stream Info block are fixed. The
// generator does not validate OBU requirements.
const uint32_t kInvalidMinimumFrameSize = 99;
ASSERT_NE(kInvalidMinimumFrameSize,
FlacMetaBlockStreamInfo::kMinimumFrameSize);
ASSERT_NE(kInvalidMinimumFrameSize, FlacStreamInfoConstraints::kMinFrameSize);
const uint32_t kInvalidMaximumFrameSize = 98;
ASSERT_NE(kInvalidMaximumFrameSize,
FlacMetaBlockStreamInfo::kMaximumFrameSize);
ASSERT_NE(kInvalidMaximumFrameSize, FlacStreamInfoConstraints::kMaxFrameSize);
const uint8_t kInvalidNumberOfChannels = 97;
ASSERT_NE(kInvalidNumberOfChannels,
FlacMetaBlockStreamInfo::kNumberOfChannels);
FlacStreamInfoConstraints::kNumberOfChannels);
const std::array<uint8_t, 16> kInvalidMd5Signature = {1};
ASSERT_NE(kInvalidMd5Signature, FlacMetaBlockStreamInfo::kMd5Signature);
ASSERT_NE(kInvalidMd5Signature, FlacStreamInfoConstraints::kMd5Signature);
InitMetadataForFlac(codec_config_metadata_);
auto* stream_info_metadata = codec_config_metadata_.at(0)
.mutable_codec_config()
Expand Down
58 changes: 22 additions & 36 deletions iamf/obu/decoder_config/flac_decoder_config.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "iamf/obu/decoder_config/flac_decoder_config.h"

#include <cstdint>
#include <functional>
#include <utility>
#include <vector>

Expand Down Expand Up @@ -43,41 +44,28 @@ absl::Status GetStreamInfo(const FlacDecoderConfig& decoder_config,
return absl::OkStatus();
}

absl::Status ValidateSampleRate(uint32_t sample_rate) {
// Validate restrictions from the FLAC specification.
if (sample_rate < FlacMetaBlockStreamInfo::kMinSampleRate ||
sample_rate > FlacMetaBlockStreamInfo::kMaxSampleRate) {
return absl::InvalidArgumentError(
absl::StrCat("Invalid sample rate= ", sample_rate));
}
using Cons = FlacStreamInfoConstraints;

return absl::OkStatus();
absl::Status ValidateSampleRate(uint32_t sample_rate) {
return ValidateInRange(
sample_rate, {Cons::kMinSampleRate, Cons::kMaxSampleRate}, "sample_rate");
}

absl::Status ValidateBitsPerSample(uint8_t bits_per_sample) {
// Validate restrictions from the FLAC specification.
if (bits_per_sample < FlacMetaBlockStreamInfo::kMinBitsPerSample ||
bits_per_sample > FlacMetaBlockStreamInfo::kMaxBitsPerSample) {
return absl::InvalidArgumentError(
absl::StrCat("Invalid bits_per_sample= ", bits_per_sample));
}

return absl::OkStatus();
return ValidateInRange(bits_per_sample,
{Cons::kMinBitsPerSample, Cons::kMaxBitsPerSample},
"bits_per_sample");
}

absl::Status ValidateTotalSamplesInStream(uint64_t total_samples_in_stream) {
// The FLAC specification treats this as a 36-bit value which is always valid,
// but in `iamf_tools` it could be out of bounds because it is stored as a
// `uint64_t`.
if (total_samples_in_stream <
FlacMetaBlockStreamInfo::kMinTotalSamplesInStream ||
total_samples_in_stream >
FlacMetaBlockStreamInfo::kMaxTotalSamplesInStream) {
return absl::InvalidArgumentError(absl::StrCat(
"Invalid total_samples_in_stream= ", total_samples_in_stream));
}

return absl::OkStatus();
return ValidateInRange(
total_samples_in_stream,
{Cons::kMinTotalSamplesInStream, Cons::kMaxTotalSamplesInStream},
"total_samples_in_stream");
}

// Validates the `FlacDecoderConfig`.
Expand All @@ -103,12 +91,12 @@ absl::Status ValidatePayload(uint32_t num_samples_per_frame,
RETURN_IF_NOT_OK(ValidateSampleRate(stream_info->sample_rate));
RETURN_IF_NOT_OK(ValidateBitsPerSample(stream_info->bits_per_sample));

if (stream_info->minimum_block_size < 16 ||
stream_info->maximum_block_size < 16) {
return absl::InvalidArgumentError(absl::StrCat(
"Invalid minimum_block_size= ", stream_info->minimum_block_size,
" or invalid maximum_block_size=", stream_info->maximum_block_size));
}
RETURN_IF_NOT_OK(Validate(stream_info->minimum_block_size,
std::greater_equal{}, Cons::kMinMinAndMaxBlockSize,
"minimum_block_size >="));
RETURN_IF_NOT_OK(Validate(stream_info->maximum_block_size,
std::greater_equal{}, Cons::kMinMinAndMaxBlockSize,
"maximum_block_size >="));

// IAMF restricts some fields.
RETURN_IF_NOT_OK(
Expand All @@ -118,19 +106,17 @@ absl::Status ValidatePayload(uint32_t num_samples_per_frame,
ValidateEqual(static_cast<uint32_t>(stream_info->minimum_block_size),
num_samples_per_frame, "minimum_block_size"));
RETURN_IF_NOT_OK(ValidateEqual(stream_info->minimum_frame_size,
FlacMetaBlockStreamInfo::kMinimumFrameSize,
"minimum_frame_size"));
Cons::kMinFrameSize, "minimum_frame_size"));
RETURN_IF_NOT_OK(ValidateEqual(stream_info->maximum_frame_size,
FlacMetaBlockStreamInfo::kMaximumFrameSize,
"maximum_frame_size"));
Cons::kMaxFrameSize, "maximum_frame_size"));

RETURN_IF_NOT_OK(ValidateEqual(stream_info->number_of_channels,
FlacMetaBlockStreamInfo::kNumberOfChannels,
Cons::kNumberOfChannels,
"number_of_channels"));

RETURN_IF_NOT_OK(ValidateTotalSamplesInStream(stream_info->bits_per_sample));

if (stream_info->md5_signature != FlacMetaBlockStreamInfo::kMd5Signature) {
if (stream_info->md5_signature != Cons::kMd5Signature) {
return absl::InvalidArgumentError("Invalid md5_signature.");
}

Expand Down
55 changes: 38 additions & 17 deletions iamf/obu/decoder_config/flac_decoder_config.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,35 +23,54 @@

namespace iamf_tools {

struct FlacMetaBlockStreamInfo {
// IAMF requires some fields to have fixed values.
static constexpr uint32_t kMinimumFrameSize = 0;
static constexpr uint32_t kMaximumFrameSize = 0;
// In IAMF the field is fixed to `1`, but ignored. The actual number of
// channels is determined on a per-substream basis based on the audio element.
struct FlacStreamInfoConstraints {
// Required 0 audio_roll_distance as per IAMF spec.
static constexpr int16_t kAudioRollDistance = 0;

// Block size must be equal to num_samples_per_frame and at least 16, as per
// FLAC spec.
static constexpr uint16_t kMinMinAndMaxBlockSize = 16;

// IAMF requires frame_size fields to have fixed values.
static constexpr uint32_t kMinFrameSize = 0;
static constexpr uint32_t kMaxFrameSize = 0;

// In IAMF the number_of_channels is fixed to `1`, but can be ignored when
// reading / decoding. The actual number of channels is determined on a
// per-substream basis based on the audio element.
static constexpr uint8_t kNumberOfChannels = 1;

// Required signature, as per IAMF spec.
static constexpr std::array<uint8_t, 16> kMd5Signature = {0};
// Constants for restrictions from the FLAC documentation.

// Acceptable ranges for sample_rate, bits_per_sample, and
// totals_samples_in_stream from the FLAC documentation.
static constexpr uint32_t kMinSampleRate = 1;
static constexpr uint32_t kMaxSampleRate = 655350;
static constexpr uint32_t kMinBitsPerSample = 3;
static constexpr uint32_t kMaxBitsPerSample = 31;
static constexpr uint8_t kMinBitsPerSample = 3;
static constexpr uint8_t kMaxBitsPerSample = 31;
// FLAC allows a value of 0 to represent an unknown total number of samples.
static constexpr uint64_t kMinTotalSamplesInStream = 0;
static constexpr uint64_t kMaxTotalSamplesInStream = 0xfffffffff;
};

struct FlacMetaBlockStreamInfo {
friend bool operator==(const FlacMetaBlockStreamInfo& lhs,
const FlacMetaBlockStreamInfo& rhs) = default;

uint16_t minimum_block_size;
uint16_t maximum_block_size;
uint32_t minimum_frame_size = kMinimumFrameSize; // 24 bits.
uint32_t maximum_frame_size = kMaximumFrameSize; // 24 bits.
uint32_t sample_rate; // 20 bits.
uint8_t number_of_channels = kNumberOfChannels; // 3 bits.
uint8_t bits_per_sample; // 5 bits.
uint64_t total_samples_in_stream; // 36 bits.
std::array<uint8_t, 16> md5_signature = kMd5Signature;
uint32_t minimum_frame_size =
FlacStreamInfoConstraints::kMinFrameSize; // 24 bits.
uint32_t maximum_frame_size =
FlacStreamInfoConstraints::kMaxFrameSize; // 24 bits.
uint32_t sample_rate; // 20 bits.
uint8_t number_of_channels =
FlacStreamInfoConstraints::kNumberOfChannels; // 3 bits.
uint8_t bits_per_sample; // 5 bits.
uint64_t total_samples_in_stream; // 36 bits.
std::array<uint8_t, 16> md5_signature =
FlacStreamInfoConstraints::kMd5Signature;
};

/*!\brief The header portion of a metadata block described in the FLAC spec. */
Expand Down Expand Up @@ -100,7 +119,9 @@ class FlacDecoderConfig {
*
* \return Audio roll distance required by the IAMF spec.
*/
static int16_t GetRequiredAudioRollDistance() { return 0; }
static int16_t GetRequiredAudioRollDistance() {
return FlacStreamInfoConstraints::kAudioRollDistance;
}

/*!\brief Validates and writes the `FlacDecoderConfig` to a buffer.
*
Expand Down
Loading

0 comments on commit def6168

Please sign in to comment.