Skip to content

Commit

Permalink
[Style] Rename ValidateAndRead* ReadAndValidate*.
Browse files Browse the repository at this point in the history
  - Blanket `s/ValidateAndRead/ReadAndValidate`.
  - For style purposes we prefer function to be name based on the order of the operations they do.
  - These functions need to read in their binary payloads before they perform validation on what they read in.

PiperOrigin-RevId: 653661736
  • Loading branch information
jwcullen committed Jul 19, 2024
1 parent 1130013 commit 52eb5ee
Show file tree
Hide file tree
Showing 24 changed files with 93 additions and 94 deletions.
4 changes: 2 additions & 2 deletions iamf/obu/arbitrary_obu.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ absl::Status ArbitraryObu::ValidateAndWritePayload(WriteBitBuffer& wb) const {
: absl::OkStatus();
}

absl::Status ArbitraryObu::ValidateAndReadPayload(ReadBitBuffer& rb) {
absl::Status ArbitraryObu::ReadAndValidatePayload(ReadBitBuffer& rb) {
return absl::UnimplementedError(
"ArbitraryOBU ValidateAndReadPayload not yet implemented.");
"ArbitraryOBU ReadAndValidatePayload not yet implemented.");
}

void ArbitraryObu::PrintObu() const {
Expand Down
2 changes: 1 addition & 1 deletion iamf/obu/arbitrary_obu.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ class ArbitraryObu : public ObuBase {
* \return `absl::OkStatus()` if the payload is valid. A specific status on
* failure.
*/
absl::Status ValidateAndReadPayload(ReadBitBuffer& rb) override;
absl::Status ReadAndValidatePayload(ReadBitBuffer& rb) override;
};

} // namespace iamf_tools
Expand Down
8 changes: 4 additions & 4 deletions iamf/obu/audio_element.cc
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ absl::Status ValidateAndWriteScalableChannelLayout(
}

// Reads the `ScalableChannelLayoutConfig` of an `AudioElementObu`.
absl::Status ValidateAndReadScalableChannelLayout(
absl::Status ReadAndValidateScalableChannelLayout(
ScalableChannelLayoutConfig& layout, const DecodedUleb128 num_substreams,
ReadBitBuffer& rb) {
// Read the main portion of the `ScalableChannelLayoutConfig`.
Expand Down Expand Up @@ -573,7 +573,7 @@ AudioElementObu::AudioElementObu(const ObuHeader& header,
absl::StatusOr<AudioElementObu> AudioElementObu::CreateFromBuffer(
const ObuHeader& header, ReadBitBuffer& rb) {
AudioElementObu audio_element_obu(header);
RETURN_IF_NOT_OK(audio_element_obu.ValidateAndReadPayload(rb));
RETURN_IF_NOT_OK(audio_element_obu.ReadAndValidatePayload(rb));
return audio_element_obu;
}

Expand Down Expand Up @@ -788,7 +788,7 @@ absl::Status AudioElementObu::ValidateAndWritePayload(
return absl::OkStatus();
}

absl::Status AudioElementObu::ValidateAndReadPayload(ReadBitBuffer& rb) {
absl::Status AudioElementObu::ReadAndValidatePayload(ReadBitBuffer& rb) {
RETURN_IF_NOT_OK(rb.ReadULeb128(audio_element_id_));
uint8_t audio_element_type;
RETURN_IF_NOT_OK(rb.ReadUnsignedLiteral(3, audio_element_type));
Expand Down Expand Up @@ -822,7 +822,7 @@ absl::Status AudioElementObu::ValidateAndReadPayload(ReadBitBuffer& rb) {
switch (audio_element_type_) {
case kAudioElementChannelBased:
config_ = ScalableChannelLayoutConfig();
return ValidateAndReadScalableChannelLayout(
return ReadAndValidateScalableChannelLayout(
std::get<ScalableChannelLayoutConfig>(config_), num_substreams_, rb);
case kAudioElementSceneBased:
config_ = AmbisonicsConfig();
Expand Down
4 changes: 2 additions & 2 deletions iamf/obu/audio_element.h
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ class AudioElementObu : public ObuBase {
/*!\brief Creates a `AudioElementObu` from a `ReadBitBuffer`.
*
* This function is designed to be used from the perspective of the decoder.
* It will call `ValidateAndReadPayload` in order to read from the buffer;
* It will call `ReadAndValidatePayload` in order to read from the buffer;
* therefore it can fail.
*
* \param header `ObuHeader` of the OBU.
Expand Down Expand Up @@ -383,7 +383,7 @@ class AudioElementObu : public ObuBase {
* \return `absl::OkStatus()` if the payload is valid. A specific status on
* failure.
*/
absl::Status ValidateAndReadPayload(ReadBitBuffer& rb) override;
absl::Status ReadAndValidatePayload(ReadBitBuffer& rb) override;
};

} // namespace iamf_tools
Expand Down
4 changes: 2 additions & 2 deletions iamf/obu/audio_frame.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ absl::StatusOr<AudioFrameObu> AudioFrameObu::CreateFromBuffer(
ReadBitBuffer& rb) {
AudioFrameObu audio_frame_obu(header);
audio_frame_obu.payload_serialized_size_ = obu_payload_serialized_size;
RETURN_IF_NOT_OK(audio_frame_obu.ValidateAndReadPayload(rb));
RETURN_IF_NOT_OK(audio_frame_obu.ReadAndValidatePayload(rb));
return audio_frame_obu;
}

Expand All @@ -67,7 +67,7 @@ absl::Status AudioFrameObu::ValidateAndWritePayload(WriteBitBuffer& wb) const {
return absl::OkStatus();
}

absl::Status AudioFrameObu::ValidateAndReadPayload(ReadBitBuffer& rb) {
absl::Status AudioFrameObu::ReadAndValidatePayload(ReadBitBuffer& rb) {
int8_t encoded_uleb128_size = 0;
if (header_.obu_type == kObuIaAudioFrame) {
// The ID is explicitly in the bitstream when `kObuIaAudioFrame`. Otherwise
Expand Down
4 changes: 2 additions & 2 deletions iamf/obu/audio_frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class AudioFrameObu : public ObuBase {
/*!\brief Creates an `AudioFrameObu` from a `ReadBitBuffer`.
*
* This function is designed to be used from the perspective of the decoder.
* It will call `ValidateAndReadPayload` in order to read from the buffer;
* It will call `ReadAndValidatePayload` in order to read from the buffer;
* therefore it can fail.
*
* \param header `ObuHeader` of the OBU.
Expand Down Expand Up @@ -107,7 +107,7 @@ class AudioFrameObu : public ObuBase {
* \return `absl::OkStatus()` if the payload is valid. A specific status on
* failure.
*/
absl::Status ValidateAndReadPayload(ReadBitBuffer& rb) override;
absl::Status ReadAndValidatePayload(ReadBitBuffer& rb) override;
};

} // namespace iamf_tools
Expand Down
10 changes: 5 additions & 5 deletions iamf/obu/codec_config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ CodecConfigObu::CodecConfigObu(const ObuHeader& header,
absl::StatusOr<CodecConfigObu> CodecConfigObu::CreateFromBuffer(
const ObuHeader& header, ReadBitBuffer& rb) {
CodecConfigObu codec_config_obu(header);
RETURN_IF_NOT_OK(codec_config_obu.ValidateAndReadPayload(rb));
RETURN_IF_NOT_OK(codec_config_obu.ReadAndValidatePayload(rb));
RETURN_IF_NOT_OK(codec_config_obu.Initialize());
return codec_config_obu;
}
Expand Down Expand Up @@ -161,15 +161,15 @@ absl::Status CodecConfigObu::ValidateAndWritePayload(WriteBitBuffer& wb) const {
return absl::OkStatus();
}

absl::Status CodecConfigObu::ValidateAndReadDecoderConfig(ReadBitBuffer& rb) {
absl::Status CodecConfigObu::ReadAndValidateDecoderConfig(ReadBitBuffer& rb) {
const int16_t audio_roll_distance = codec_config_.audio_roll_distance;
const uint32_t num_samples_per_frame = codec_config_.num_samples_per_frame;
// Read the `decoder_config` struct portion. This is codec specific.
switch (codec_config_.codec_id) {
using enum CodecConfig::CodecId;
case kCodecIdOpus:
return std::get<OpusDecoderConfig>(codec_config_.decoder_config)
.ValidateAndRead(num_samples_per_frame, audio_roll_distance, rb);
.ReadAndValidate(num_samples_per_frame, audio_roll_distance, rb);
case kCodecIdLpcm:
LpcmDecoderConfig lpcm_decoder_config;
RETURN_IF_NOT_OK(
Expand All @@ -187,7 +187,7 @@ absl::Status CodecConfigObu::ValidateAndReadDecoderConfig(ReadBitBuffer& rb) {
return absl::OkStatus();
}

absl::Status CodecConfigObu::ValidateAndReadPayload(ReadBitBuffer& rb) {
absl::Status CodecConfigObu::ReadAndValidatePayload(ReadBitBuffer& rb) {
RETURN_IF_NOT_OK(rb.ReadULeb128(codec_config_id_));
uint64_t codec_id;
RETURN_IF_NOT_OK(rb.ReadUnsignedLiteral(32, codec_id));
Expand All @@ -198,7 +198,7 @@ absl::Status CodecConfigObu::ValidateAndReadPayload(ReadBitBuffer& rb) {
RETURN_IF_NOT_OK(rb.ReadSigned16(codec_config_.audio_roll_distance));

// Read the `decoder_config_`. This is codec specific.
RETURN_IF_NOT_OK(ValidateAndReadDecoderConfig(rb));
RETURN_IF_NOT_OK(ReadAndValidateDecoderConfig(rb));
return absl::OkStatus();
}

Expand Down
6 changes: 3 additions & 3 deletions iamf/obu/codec_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class CodecConfigObu : public ObuBase {
/*!\brief Creates a `CodecConfigObu` from a `ReadBitBuffer`.
*
* This function is designed to be used from the perspective of the decoder.
* It will call `ValidateAndReadPayload` in order to read from the buffer;
* It will call `ReadAndValidatePayload` in order to read from the buffer;
* therefore it can fail.
*
* \param header `ObuHeader` of the OBU.
Expand Down Expand Up @@ -115,7 +115,7 @@ class CodecConfigObu : public ObuBase {
* \param rb Buffer to read from.
* \return `absl::OkStatus()` on success. A specific status on failure.
*/
absl::Status ValidateAndReadDecoderConfig(ReadBitBuffer& rb);
absl::Status ReadAndValidateDecoderConfig(ReadBitBuffer& rb);

/*!\brief Gets the output sample rate associated with the OBU.
*
Expand Down Expand Up @@ -199,7 +199,7 @@ class CodecConfigObu : public ObuBase {
* \return `absl::OkStatus()` if the payload is valid. A specific status on
* failure.
*/
absl::Status ValidateAndReadPayload(ReadBitBuffer& rb) override;
absl::Status ReadAndValidatePayload(ReadBitBuffer& rb) override;
};

} // namespace iamf_tools
Expand Down
2 changes: 1 addition & 1 deletion iamf/obu/decoder_config/opus_decoder_config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ absl::Status OpusDecoderConfig::ValidateAndWrite(uint32_t num_samples_per_frame,
return absl::OkStatus();
}

absl::Status OpusDecoderConfig::ValidateAndRead(uint32_t num_samples_per_frame,
absl::Status OpusDecoderConfig::ReadAndValidate(uint32_t num_samples_per_frame,
int16_t audio_roll_distance,
ReadBitBuffer& rb) {
RETURN_IF_NOT_OK(
Expand Down
2 changes: 1 addition & 1 deletion iamf/obu/decoder_config/opus_decoder_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class OpusDecoderConfig {
* \return `absl::OkStatus()` if the decoder config is valid. A specific error
* code on failure.
*/
absl::Status ValidateAndRead(uint32_t num_samples_per_frame,
absl::Status ReadAndValidate(uint32_t num_samples_per_frame,
int16_t audio_roll_distance, ReadBitBuffer& rb);

/*!\brief Gets the output sample rate represented within the decoder config.
Expand Down
42 changes: 21 additions & 21 deletions iamf/obu/decoder_config/tests/opus_decoder_config_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -272,9 +272,9 @@ TEST_F(OpusTest, IllegalMappingFamilyNotZero) {
TestWriteDecoderConfig();
}

// --- Begin ValidateAndRead Tests ---
// --- Begin ReadAndValidate Tests ---

TEST(ValidateAndRead, VaryAllLegalFields) {
TEST(ReadAndValidate, VaryAllLegalFields) {
OpusDecoderConfig opus_decoder_config;
uint32_t num_samples_per_frame = 960;
int16_t audio_roll_distance = -4;
Expand All @@ -291,7 +291,7 @@ TEST(ValidateAndRead, VaryAllLegalFields) {
// `mapping_family`.
OpusDecoderConfig::kMappingFamily};
ReadBitBuffer read_buffer(1024, &source);
EXPECT_THAT(opus_decoder_config.ValidateAndRead(
EXPECT_THAT(opus_decoder_config.ReadAndValidate(
num_samples_per_frame, audio_roll_distance, read_buffer),
IsOk());

Expand All @@ -300,7 +300,7 @@ TEST(ValidateAndRead, VaryAllLegalFields) {
EXPECT_EQ(opus_decoder_config.input_sample_rate_, 4);
}

TEST(ValidateAndRead, MaxAllLegalFields) {
TEST(ReadAndValidate, MaxAllLegalFields) {
OpusDecoderConfig opus_decoder_config;
uint32_t num_samples_per_frame = 960;
int16_t audio_roll_distance = -4;
Expand All @@ -317,7 +317,7 @@ TEST(ValidateAndRead, MaxAllLegalFields) {
// `mapping_family`.
OpusDecoderConfig::kMappingFamily};
ReadBitBuffer read_buffer(1024, &source);
EXPECT_THAT(opus_decoder_config.ValidateAndRead(
EXPECT_THAT(opus_decoder_config.ReadAndValidate(
num_samples_per_frame, audio_roll_distance, read_buffer),
IsOk());

Expand All @@ -326,7 +326,7 @@ TEST(ValidateAndRead, MaxAllLegalFields) {
EXPECT_EQ(opus_decoder_config.input_sample_rate_, 0xffffffff);
}

TEST(ValidateAndRead, MinorVersion) {
TEST(ReadAndValidate, MinorVersion) {
OpusDecoderConfig opus_decoder_config;
uint32_t num_samples_per_frame = 960;
int16_t audio_roll_distance = -4;
Expand All @@ -343,14 +343,14 @@ TEST(ValidateAndRead, MinorVersion) {
// `mapping_family`.
OpusDecoderConfig::kMappingFamily};
ReadBitBuffer read_buffer(1024, &source);
EXPECT_THAT(opus_decoder_config.ValidateAndRead(
EXPECT_THAT(opus_decoder_config.ReadAndValidate(
num_samples_per_frame, audio_roll_distance, read_buffer),
IsOk());

EXPECT_EQ(opus_decoder_config.version_, 2);
}

TEST(ValidateAndRead, IllegalVersionZero) {
TEST(ReadAndValidate, IllegalVersionZero) {
OpusDecoderConfig opus_decoder_config;
uint32_t num_samples_per_frame = 960;
int16_t audio_roll_distance = -4;
Expand All @@ -368,12 +368,12 @@ TEST(ValidateAndRead, IllegalVersionZero) {
OpusDecoderConfig::kMappingFamily};
ReadBitBuffer read_buffer(1024, &source);
EXPECT_FALSE(opus_decoder_config
.ValidateAndRead(num_samples_per_frame, audio_roll_distance,
.ReadAndValidate(num_samples_per_frame, audio_roll_distance,
read_buffer)
.ok());
}

TEST(ValidateAndRead, IllegalVersionFuture) {
TEST(ReadAndValidate, IllegalVersionFuture) {
OpusDecoderConfig opus_decoder_config;
uint32_t num_samples_per_frame = 960;
int16_t audio_roll_distance = -4;
Expand All @@ -391,12 +391,12 @@ TEST(ValidateAndRead, IllegalVersionFuture) {
OpusDecoderConfig::kMappingFamily};
ReadBitBuffer read_buffer(1024, &source);
EXPECT_FALSE(opus_decoder_config
.ValidateAndRead(num_samples_per_frame, audio_roll_distance,
.ReadAndValidate(num_samples_per_frame, audio_roll_distance,
read_buffer)
.ok());
}

TEST(ValidateAndRead, IllegalVersionmax) {
TEST(ReadAndValidate, IllegalVersionmax) {
OpusDecoderConfig opus_decoder_config;
uint32_t num_samples_per_frame = 960;
int16_t audio_roll_distance = -4;
Expand All @@ -414,12 +414,12 @@ TEST(ValidateAndRead, IllegalVersionmax) {
OpusDecoderConfig::kMappingFamily};
ReadBitBuffer read_buffer(1024, &source);
EXPECT_FALSE(opus_decoder_config
.ValidateAndRead(num_samples_per_frame, audio_roll_distance,
.ReadAndValidate(num_samples_per_frame, audio_roll_distance,
read_buffer)
.ok());
}

TEST(ValidateAndRead, IllegalChannelCountZero) {
TEST(ReadAndValidate, IllegalChannelCountZero) {
OpusDecoderConfig opus_decoder_config;
uint32_t num_samples_per_frame = 960;
int16_t audio_roll_distance = -4;
Expand All @@ -437,12 +437,12 @@ TEST(ValidateAndRead, IllegalChannelCountZero) {
OpusDecoderConfig::kMappingFamily};
ReadBitBuffer read_buffer(1024, &source);
EXPECT_FALSE(opus_decoder_config
.ValidateAndRead(num_samples_per_frame, audio_roll_distance,
.ReadAndValidate(num_samples_per_frame, audio_roll_distance,
read_buffer)
.ok());
}

TEST(ValidateAndRead, ReadPreSkip312) {
TEST(ReadAndValidate, ReadPreSkip312) {
OpusDecoderConfig opus_decoder_config;
uint32_t num_samples_per_frame = 960;
int16_t audio_roll_distance = -4;
Expand All @@ -459,15 +459,15 @@ TEST(ValidateAndRead, ReadPreSkip312) {
// `mapping_family`.
OpusDecoderConfig::kMappingFamily};
ReadBitBuffer read_buffer(1024, &source);
EXPECT_THAT(opus_decoder_config.ValidateAndRead(
EXPECT_THAT(opus_decoder_config.ReadAndValidate(
num_samples_per_frame, audio_roll_distance, read_buffer),
IsOk());

EXPECT_EQ(opus_decoder_config.version_, 1);
EXPECT_EQ(opus_decoder_config.pre_skip_, 312);
}

TEST(ValidateAndRead, ReadSampleRate48kHz) {
TEST(ReadAndValidate, ReadSampleRate48kHz) {
OpusDecoderConfig opus_decoder_config;
uint32_t num_samples_per_frame = 960;
int16_t audio_roll_distance = -4;
Expand All @@ -484,15 +484,15 @@ TEST(ValidateAndRead, ReadSampleRate48kHz) {
// `mapping_family`.
OpusDecoderConfig::kMappingFamily};
ReadBitBuffer read_buffer(1024, &source);
EXPECT_THAT(opus_decoder_config.ValidateAndRead(
EXPECT_THAT(opus_decoder_config.ReadAndValidate(
num_samples_per_frame, audio_roll_distance, read_buffer),
IsOk());

EXPECT_EQ(opus_decoder_config.version_, 1);
EXPECT_EQ(opus_decoder_config.input_sample_rate_, 48000);
}

TEST(ValidateAndRead, ReadSampleRate192kHz) {
TEST(ReadAndValidate, ReadSampleRate192kHz) {
OpusDecoderConfig opus_decoder_config;
uint32_t num_samples_per_frame = 960;
int16_t audio_roll_distance = -4;
Expand All @@ -509,7 +509,7 @@ TEST(ValidateAndRead, ReadSampleRate192kHz) {
// `mapping_family`.
OpusDecoderConfig::kMappingFamily};
ReadBitBuffer read_buffer(1024, &source);
EXPECT_THAT(opus_decoder_config.ValidateAndRead(
EXPECT_THAT(opus_decoder_config.ReadAndValidate(
num_samples_per_frame, audio_roll_distance, read_buffer),
IsOk());

Expand Down
4 changes: 2 additions & 2 deletions iamf/obu/ia_sequence_header.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ absl::Status IASequenceHeaderObu::Validate() const {
absl::StatusOr<IASequenceHeaderObu> IASequenceHeaderObu::CreateFromBuffer(
const ObuHeader& header, ReadBitBuffer& rb) {
IASequenceHeaderObu ia_sequence_header_obu(header);
RETURN_IF_NOT_OK(ia_sequence_header_obu.ValidateAndReadPayload(rb));
RETURN_IF_NOT_OK(ia_sequence_header_obu.ReadAndValidatePayload(rb));
return ia_sequence_header_obu;
}

Expand All @@ -75,7 +75,7 @@ absl::Status IASequenceHeaderObu::ValidateAndWritePayload(
return absl::OkStatus();
}

absl::Status IASequenceHeaderObu::ValidateAndReadPayload(ReadBitBuffer& rb) {
absl::Status IASequenceHeaderObu::ReadAndValidatePayload(ReadBitBuffer& rb) {
RETURN_IF_NOT_OK(rb.ReadUnsignedLiteral(32, ia_code_));
uint8_t primary_profile;
RETURN_IF_NOT_OK(rb.ReadUnsignedLiteral(8, primary_profile));
Expand Down
Loading

0 comments on commit 52eb5ee

Please sign in to comment.