Skip to content

Commit

Permalink
Switch to MemoryBasedReadBitBuffer::CreateFromSpan instead of from …
Browse files Browse the repository at this point in the history
…a vector.

  - Update interface to be `absl::Span` based for flexibility.
  - This version, and the prior version copy in the data; there are no ownership concerns.
  - Maybe in the future, we can migrate to more uses of `constexpr` arrays for "stock" binary data to read.

PiperOrigin-RevId: 712901564
  • Loading branch information
jwcullen committed Jan 7, 2025
1 parent 3af5518 commit 3d423d0
Show file tree
Hide file tree
Showing 25 changed files with 306 additions and 168 deletions.
8 changes: 4 additions & 4 deletions iamf/common/read_bit_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -371,8 +371,8 @@ absl::Status ReadBitBuffer::ReadUnsignedLiteralInternal(const int num_bits,
// ----- MemoryBasedReadBitBuffer -----

std::unique_ptr<MemoryBasedReadBitBuffer>
MemoryBasedReadBitBuffer::CreateFromVector(int64_t capacity,
const std::vector<uint8_t>& source) {
MemoryBasedReadBitBuffer::CreateFromSpan(int64_t capacity,
absl::Span<const uint8_t> source) {
if (capacity < 0) {
LOG(ERROR) << "MemoryBasedReadBitBuffer capacity must be >= 0.";
return nullptr;
Expand All @@ -395,9 +395,9 @@ absl::Status MemoryBasedReadBitBuffer::LoadBytesToBuffer(int64_t starting_byte,
}

MemoryBasedReadBitBuffer::MemoryBasedReadBitBuffer(
size_t capacity, const std::vector<uint8_t>& source)
size_t capacity, absl::Span<const uint8_t> source)
: ReadBitBuffer(capacity, static_cast<int64_t>(source.size()) * 8),
source_vector_(source) {}
source_vector_(source.begin(), source.end()) {}

// ----- FileBasedReadBitBuffer -----

Expand Down
16 changes: 7 additions & 9 deletions iamf/common/read_bit_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -272,14 +272,13 @@ class MemoryBasedReadBitBuffer : public ReadBitBuffer {
/*!\brief Creates an instance of a memory-based read bit buffer.
*
* \param capacity Capacity of the internal buffer in bytes.
* \param source Reference of the source vector from which the buffer will
* load data. The entire content will be copied into the constructed
* instance.
* \param source Source span from which the buffer will load data. The
* entire contents will be copied into the constructed instance.
* \return Unique pointer of the created instance. `nullptr` if the creation
* fails.
*/
static std::unique_ptr<MemoryBasedReadBitBuffer> CreateFromVector(
int64_t capacity, const std::vector<uint8_t>& source);
static std::unique_ptr<MemoryBasedReadBitBuffer> CreateFromSpan(
int64_t capacity, absl::Span<const uint8_t> source);

/*!\brief Destructor.*/
~MemoryBasedReadBitBuffer() override = default;
Expand All @@ -288,11 +287,10 @@ class MemoryBasedReadBitBuffer : public ReadBitBuffer {
/*!\brief Private constructor. Called by the factory method only.
*
* \param capacity Capacity of the internal buffer in bytes.
* \param source Reference of the source vector from which the buffer will
* load data. The entire content will be copied into the constructed
* instance.
* \param source Source span from which the buffer will load data. The
* entire contents will be copied into the constructed instance.
*/
MemoryBasedReadBitBuffer(size_t capacity, const std::vector<uint8_t>& source);
MemoryBasedReadBitBuffer(size_t capacity, absl::Span<const uint8_t> source);

/*!\brief Load bytes from the source vector to the buffer.
*
Expand Down
20 changes: 10 additions & 10 deletions iamf/common/tests/read_bit_buffer_fuzz_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ namespace {
void ReadUnsignedLiteral64NoUndefinedBehavior(std::vector<uint8_t> data,
int num_bits) {
std::unique_ptr<MemoryBasedReadBitBuffer> rb =
MemoryBasedReadBitBuffer::CreateFromVector(256, data);
MemoryBasedReadBitBuffer::CreateFromSpan(256, absl::MakeConstSpan(data));
uint64_t read_data;
absl::Status status = rb->ReadUnsignedLiteral(num_bits, read_data);
}
Expand All @@ -26,7 +26,7 @@ FUZZ_TEST(ReadBitBufferFuzzTest, ReadUnsignedLiteral64NoUndefinedBehavior);
void ReadUnsignedLiteral32NoUndefinedBehavior(std::vector<uint8_t> data,
int num_bits) {
std::unique_ptr<MemoryBasedReadBitBuffer> rb =
MemoryBasedReadBitBuffer::CreateFromVector(256, data);
MemoryBasedReadBitBuffer::CreateFromSpan(256, absl::MakeConstSpan(data));
uint32_t read_data;
absl::Status status = rb->ReadUnsignedLiteral(num_bits, read_data);
}
Expand All @@ -36,7 +36,7 @@ FUZZ_TEST(ReadBitBufferFuzzTest, ReadUnsignedLiteral32NoUndefinedBehavior);
void ReadUnsignedLiteral16NoUndefinedBehavior(std::vector<uint8_t> data,
int num_bits) {
std::unique_ptr<MemoryBasedReadBitBuffer> rb =
MemoryBasedReadBitBuffer::CreateFromVector(256, data);
MemoryBasedReadBitBuffer::CreateFromSpan(256, absl::MakeConstSpan(data));
uint16_t read_data;
absl::Status status = rb->ReadUnsignedLiteral(num_bits, read_data);
}
Expand All @@ -46,7 +46,7 @@ FUZZ_TEST(ReadBitBufferFuzzTest, ReadUnsignedLiteral16NoUndefinedBehavior);
void ReadUnsignedLiteral8NoUndefinedBehavior(std::vector<uint8_t> data,
int num_bits) {
std::unique_ptr<MemoryBasedReadBitBuffer> rb =
MemoryBasedReadBitBuffer::CreateFromVector(256, data);
MemoryBasedReadBitBuffer::CreateFromSpan(256, absl::MakeConstSpan(data));
uint8_t read_data;
absl::Status status = rb->ReadUnsignedLiteral(num_bits, read_data);
}
Expand All @@ -55,7 +55,7 @@ FUZZ_TEST(ReadBitBufferFuzzTest, ReadUnsignedLiteral8NoUndefinedBehavior);

void ReadSigned16NoUndefinedBehavior(std::vector<uint8_t> data) {
std::unique_ptr<MemoryBasedReadBitBuffer> rb =
MemoryBasedReadBitBuffer::CreateFromVector(256, data);
MemoryBasedReadBitBuffer::CreateFromSpan(256, absl::MakeConstSpan(data));
int16_t read_data;
absl::Status status = rb->ReadSigned16(read_data);
}
Expand All @@ -64,7 +64,7 @@ FUZZ_TEST(ReadBitBufferFuzzTest, ReadSigned16NoUndefinedBehavior);

void ReadStringNoUndefinedBehavior(std::vector<uint8_t> data) {
std::unique_ptr<MemoryBasedReadBitBuffer> rb =
MemoryBasedReadBitBuffer::CreateFromVector(256, data);
MemoryBasedReadBitBuffer::CreateFromSpan(256, absl::MakeConstSpan(data));
std::string read_data;
absl::Status status = rb->ReadString(read_data);
}
Expand All @@ -73,7 +73,7 @@ FUZZ_TEST(ReadBitBufferFuzzTest, ReadStringNoUndefinedBehavior);

void ReadULeb128NoUndefinedBehavior(std::vector<uint8_t> data) {
std::unique_ptr<MemoryBasedReadBitBuffer> rb =
MemoryBasedReadBitBuffer::CreateFromVector(256, data);
MemoryBasedReadBitBuffer::CreateFromSpan(256, absl::MakeConstSpan(data));
DecodedUleb128 read_data;
absl::Status status = rb->ReadULeb128(read_data);
}
Expand All @@ -83,7 +83,7 @@ FUZZ_TEST(ReadBitBufferFuzzTest, ReadULeb128NoUndefinedBehavior);
void ReadIso14496_1ExpandedNoUndefinedBehavior(std::vector<uint8_t> data,
uint32_t max_class_size) {
std::unique_ptr<MemoryBasedReadBitBuffer> rb =
MemoryBasedReadBitBuffer::CreateFromVector(256, data);
MemoryBasedReadBitBuffer::CreateFromSpan(256, absl::MakeConstSpan(data));
uint32_t read_data;
absl::Status status = rb->ReadIso14496_1Expanded(max_class_size, read_data);
}
Expand All @@ -92,7 +92,7 @@ FUZZ_TEST(ReadBitBufferFuzzTest, ReadIso14496_1ExpandedNoUndefinedBehavior);

void ReadUint8SpanNoUndefinedBehavior(std::vector<uint8_t> data) {
std::unique_ptr<MemoryBasedReadBitBuffer> rb =
MemoryBasedReadBitBuffer::CreateFromVector(256, data);
MemoryBasedReadBitBuffer::CreateFromSpan(256, absl::MakeConstSpan(data));
std::vector<uint8_t> read_data(data.size());
absl::Span<uint8_t> span(read_data);
absl::Status status = rb->ReadUint8Span(span);
Expand All @@ -102,7 +102,7 @@ FUZZ_TEST(ReadBitBufferFuzzTest, ReadUint8SpanNoUndefinedBehavior);

void ReadBooleanNoUndefinedBehavior(std::vector<uint8_t> data) {
std::unique_ptr<MemoryBasedReadBitBuffer> rb =
MemoryBasedReadBitBuffer::CreateFromVector(256, data);
MemoryBasedReadBitBuffer::CreateFromSpan(256, absl::MakeConstSpan(data));
bool read_data;
absl::Status status = rb->ReadBoolean(read_data);
}
Expand Down
6 changes: 4 additions & 2 deletions iamf/common/tests/read_bit_buffer_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ constexpr int kMaxUint32 = std::numeric_limits<uint32_t>::max();

TEST(MemoryBasedReadBitBufferTest, CreateFromVectorFailsWithNegativeCapacity) {
const std::vector<uint8_t> source_data = {0x01, 0x23, 0x45, 0x68, 0x89};
EXPECT_THAT(MemoryBasedReadBitBuffer::CreateFromVector(-1, source_data),
EXPECT_THAT(MemoryBasedReadBitBuffer::CreateFromSpan(
-1, absl::MakeConstSpan(source_data)),
::testing::IsNull());
}

Expand Down Expand Up @@ -82,7 +83,8 @@ std::unique_ptr<BufferReaderType> CreateConcreteReadBitBuffer(
template <>
std::unique_ptr<MemoryBasedReadBitBuffer> CreateConcreteReadBitBuffer(
int64_t capacity, std::vector<uint8_t>& source_data) {
return MemoryBasedReadBitBuffer::CreateFromVector(capacity, source_data);
return MemoryBasedReadBitBuffer::CreateFromSpan(
capacity, absl::MakeConstSpan(source_data));
}

template <>
Expand Down
6 changes: 4 additions & 2 deletions iamf/common/tests/write_read_fuzz_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ void WriteReadString(const std::string& data) {
if (write_status.ok()) {
std::vector<uint8_t> source_data = wb.bit_buffer();
std::unique_ptr<MemoryBasedReadBitBuffer> rb =
MemoryBasedReadBitBuffer::CreateFromVector(256, source_data);
MemoryBasedReadBitBuffer::CreateFromSpan(
256, absl::MakeConstSpan(source_data));

std::string read_data;
EXPECT_THAT(rb->ReadString(read_data), IsOk());
Expand All @@ -41,7 +42,8 @@ void WriteReadUint8Vector(const std::vector<uint8_t>& data) {
if (write_status.ok()) {
std::vector<uint8_t> source_data = wb.bit_buffer();
std::unique_ptr<MemoryBasedReadBitBuffer> rb =
MemoryBasedReadBitBuffer::CreateFromVector(256, source_data);
MemoryBasedReadBitBuffer::CreateFromSpan(
256, absl::MakeConstSpan(source_data));

std::vector<uint8_t> read_data(data.size());
EXPECT_THAT(rb->ReadUint8Span(absl::MakeSpan(read_data)), IsOk());
Expand Down
4 changes: 4 additions & 0 deletions iamf/obu/decoder_config/tests/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ cc_test(
"//iamf/obu/decoder_config:aac_decoder_config",
"@com_google_absl//absl/status",
"@com_google_absl//absl/status:status_matchers",
"@com_google_absl//absl/types:span",
"@com_google_googletest//:gtest_main",
],
)
Expand All @@ -24,6 +25,7 @@ cc_test(
"//iamf/obu/decoder_config:flac_decoder_config",
"@com_google_absl//absl/status",
"@com_google_absl//absl/status:status_matchers",
"@com_google_absl//absl/types:span",
"@com_google_googletest//:gtest_main",
],
)
Expand All @@ -38,6 +40,7 @@ cc_test(
"//iamf/obu/decoder_config:lpcm_decoder_config",
"@com_google_absl//absl/status",
"@com_google_absl//absl/status:status_matchers",
"@com_google_absl//absl/types:span",
"@com_google_googletest//:gtest_main",
],
)
Expand All @@ -52,6 +55,7 @@ cc_test(
"//iamf/obu/decoder_config:opus_decoder_config",
"@com_google_absl//absl/status",
"@com_google_absl//absl/status:status_matchers",
"@com_google_absl//absl/types:span",
"@com_google_googletest//:gtest_main",
],
)
22 changes: 15 additions & 7 deletions iamf/obu/decoder_config/tests/aac_decoder_config_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#include "absl/status/status.h"
#include "absl/status/status_matchers.h"
#include "absl/types/span.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include "iamf/common/read_bit_buffer.h"
Expand Down Expand Up @@ -195,7 +196,8 @@ TEST(AudioSpecificConfig, ReadsWithImplicitSampleFrequency) {
kLowerByteSerializedSamplingFrequencyIndex64000 |
kChannelConfigurationAndGaSpecificConfigMask};
AudioSpecificConfig audio_specific_config;
auto rb = MemoryBasedReadBitBuffer::CreateFromVector(1024, data);
auto rb =
MemoryBasedReadBitBuffer::CreateFromSpan(1024, absl::MakeConstSpan(data));

EXPECT_THAT(audio_specific_config.Read(*rb), IsOk());

Expand Down Expand Up @@ -229,7 +231,8 @@ TEST(AudioSpecificConfig, ReadsWithExplicitSampleFrequency) {
// `frame_length_flag`, `depends_on_core_coder`, `extension_flag`.
((kSampleFrequency & 1)) | kChannelConfigurationAndGaSpecificConfigMask};
AudioSpecificConfig audio_specific_config;
auto rb = MemoryBasedReadBitBuffer::CreateFromVector(1024, data);
auto rb =
MemoryBasedReadBitBuffer::CreateFromSpan(1024, absl::MakeConstSpan(data));

EXPECT_THAT(audio_specific_config.Read(*rb), IsOk());

Expand Down Expand Up @@ -267,7 +270,8 @@ TEST(AacDecoderConfig, ReadAndValidateReadsAllFields) {
kLowerByteSerializedSamplingFrequencyIndex64000 |
kChannelConfigurationAndGaSpecificConfigMask};
AacDecoderConfig decoder_config;
auto rb = MemoryBasedReadBitBuffer::CreateFromVector(1024, data);
auto rb =
MemoryBasedReadBitBuffer::CreateFromSpan(1024, absl::MakeConstSpan(data));

EXPECT_THAT(decoder_config.ReadAndValidate(kAudioRollDistance, *rb), IsOk());

Expand Down Expand Up @@ -323,7 +327,8 @@ TEST(AacDecoderConfig, ReadAndValidateWithExplicitSampleFrequency) {
// `frame_length_flag`, `depends_on_core_coder`, `extension_flag`.
((48000 & 1)) | kChannelConfigurationAndGaSpecificConfigMask};
AacDecoderConfig decoder_config;
auto rb = MemoryBasedReadBitBuffer::CreateFromVector(1024, data);
auto rb =
MemoryBasedReadBitBuffer::CreateFromSpan(1024, absl::MakeConstSpan(data));

EXPECT_THAT(decoder_config.ReadAndValidate(kAudioRollDistance, *rb), IsOk());

Expand Down Expand Up @@ -361,7 +366,8 @@ TEST(AacDecoderConfig, FailsIfDecoderConfigDescriptorExpandableSizeIsTooSmall) {
kLowerByteSerializedSamplingFrequencyIndex64000 |
kChannelConfigurationAndGaSpecificConfigMask};
AacDecoderConfig decoder_config;
auto rb = MemoryBasedReadBitBuffer::CreateFromVector(1024, data);
auto rb =
MemoryBasedReadBitBuffer::CreateFromSpan(1024, absl::MakeConstSpan(data));

EXPECT_FALSE(decoder_config.ReadAndValidate(kAudioRollDistance, *rb).ok());
}
Expand Down Expand Up @@ -396,7 +402,8 @@ TEST(AacDecoderConfig, ReadExtensions) {
kChannelConfigurationAndGaSpecificConfigMask,
'd', 'e', 'f', 'a', 'b', 'c'};
AacDecoderConfig decoder_config;
auto rb = MemoryBasedReadBitBuffer::CreateFromVector(1024, data);
auto rb =
MemoryBasedReadBitBuffer::CreateFromSpan(1024, absl::MakeConstSpan(data));

EXPECT_THAT(decoder_config.ReadAndValidate(kAudioRollDistance, *rb), IsOk());

Expand Down Expand Up @@ -436,7 +443,8 @@ TEST(AacDecoderConfig, ValidatesAudioRollDistance) {
kLowerByteSerializedSamplingFrequencyIndex64000 |
kChannelConfigurationAndGaSpecificConfigMask};
AacDecoderConfig decoder_config;
auto rb = MemoryBasedReadBitBuffer::CreateFromVector(1024, data);
auto rb =
MemoryBasedReadBitBuffer::CreateFromSpan(1024, absl::MakeConstSpan(data));

EXPECT_FALSE(
decoder_config.ReadAndValidate(kInvalidAudioRollDistance, *rb).ok());
Expand Down
7 changes: 5 additions & 2 deletions iamf/obu/decoder_config/tests/flac_decoder_config_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

#include "absl/status/status.h"
#include "absl/status/status_matchers.h"
#include "absl/types/span.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include "iamf/common/read_bit_buffer.h"
Expand Down Expand Up @@ -737,7 +738,8 @@ TEST(ReadAndValidateTest, ReadAndValidateStreamInfoSuccess) {
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00};
;
auto rb = MemoryBasedReadBitBuffer::CreateFromVector(1024, payload);
auto rb = MemoryBasedReadBitBuffer::CreateFromSpan(
1024, absl::MakeConstSpan(payload));
FlacDecoderConfig decoder_config;
EXPECT_THAT(decoder_config.ReadAndValidate(
/*num_samples_per_frame=*/64, /*audio_roll_distance=*/0, *rb),
Expand Down Expand Up @@ -787,7 +789,8 @@ TEST(ReadAndValidateTest, ReadAndValidateStreamInfoFailsOnInvalidMd5Signature) {
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x01};
;
auto rb = MemoryBasedReadBitBuffer::CreateFromVector(1024, payload);
auto rb = MemoryBasedReadBitBuffer::CreateFromSpan(
1024, absl::MakeConstSpan(payload));
FlacDecoderConfig decoder_config;
EXPECT_FALSE(
decoder_config
Expand Down
7 changes: 5 additions & 2 deletions iamf/obu/decoder_config/tests/lpcm_decoder_config_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#include "absl/status/status.h"
#include "absl/status/status_matchers.h"
#include "absl/types/span.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include "iamf/common/read_bit_buffer.h"
Expand Down Expand Up @@ -275,7 +276,8 @@ TEST(ReadAndValidateTest, ReadAllFields) {
0x00, 0x00, 0xbb, 0x80 // sample_rate
};
int16_t audio_roll_distance = 0;
auto read_buffer = MemoryBasedReadBitBuffer::CreateFromVector(1024, source);
auto read_buffer = MemoryBasedReadBitBuffer::CreateFromSpan(
1024, absl::MakeConstSpan(source));
LpcmDecoderConfig lpcm_decoder_config;
EXPECT_THAT(
lpcm_decoder_config.ReadAndValidate(audio_roll_distance, *read_buffer),
Expand All @@ -292,7 +294,8 @@ TEST(ReadAndValidateTest, RejectInvalidAudioRollDistance) {
0x00, 0x00, 0xbb, 0x80 // sample_rate
};
int16_t audio_roll_distance = 1;
auto read_buffer = MemoryBasedReadBitBuffer::CreateFromVector(1024, source);
auto read_buffer = MemoryBasedReadBitBuffer::CreateFromSpan(
1024, absl::MakeConstSpan(source));
LpcmDecoderConfig lpcm_decoder_config;
EXPECT_FALSE(
lpcm_decoder_config.ReadAndValidate(audio_roll_distance, *read_buffer)
Expand Down
Loading

0 comments on commit 3d423d0

Please sign in to comment.