Skip to content

Commit

Permalink
Remove dead code from when WriteBitBuffer was not always resizable.
Browse files Browse the repository at this point in the history
  - A long time ago the buffer had an option to prevent resizing. This option is long gone.
  - The internal function is only called with `allow_resizing == true`. Returning `ResourceExhaustedError` is dead code.
  - Remove the dead code. The class never returns `ResourceExhaustedError` nor was the error tested. Clean up interface documents to reflect this reality.
  - Also reduce some log spam when the buffer is flushed to file.

PiperOrigin-RevId: 691471124
  • Loading branch information
jwcullen committed Nov 1, 2024
1 parent 399bd5c commit b764b0a
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 38 deletions.
19 changes: 5 additions & 14 deletions iamf/common/write_bit_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,27 +31,18 @@ namespace iamf_tools {

namespace {

absl::Status CanWriteBits(const bool allow_resizing, const int num_bits,
const int64_t bit_offset,
std::vector<uint8_t>& bit_buffer) {
void MaybeResizeBufferToFitNumBits(int num_bits, int64_t bit_offset,
std::vector<uint8_t>& bit_buffer) {
const int64_t size = static_cast<int64_t>(bit_buffer.size());
if (bit_offset + num_bits <= size * 8) {
return absl::OkStatus();
}

if (!allow_resizing) {
return absl::ResourceExhaustedError(
"The buffer does not have enough capacity to write and cannot be "
"resized.");
return;
}

const int64_t required_bytes = ((bit_offset + num_bits) / 8) +
((bit_offset + num_bits) % 8 == 0 ? 0 : 1);

// Adjust the size of the buffer.
bit_buffer.resize(required_bytes, 0);

return absl::OkStatus();
}

// Write one bit to the buffer using an AND or OR mask. All unwritten bits are
Expand Down Expand Up @@ -107,7 +98,7 @@ absl::Status InternalWriteUnsigned(int max_bits, uint64_t data, int num_bits,
}

// Expand the buffer and pad the input data with zeroes.
RETURN_IF_NOT_OK(CanWriteBits(true, num_bits, bit_offset, bit_buffer));
MaybeResizeBufferToFitNumBits(num_bits, bit_offset, bit_buffer);

if (bit_offset % 8 == 0 && num_bits % 8 == 0) {
// Short-circuit the common case of writing a byte-aligned input to a
Expand Down Expand Up @@ -259,7 +250,7 @@ absl::Status WriteBitBuffer::FlushAndWriteToFile(
RETURN_IF_NOT_OK(WriteBufferToFile(bit_buffer_, *output_file));
}

LOG(INFO) << "Flushing " << bit_offset_ / 8 << " bytes";
LOG_EVERY_POW_2(INFO) << "Flushing " << bit_offset_ / 8 << " bytes";
Reset();
return absl::OkStatus();
}
Expand Down
37 changes: 13 additions & 24 deletions iamf/common/write_bit_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,7 @@ class WriteBitBuffer {
* 32.
* \return `absl::OkStatus()` on success. `absl::InvalidArgumentError()` if
* `num_bits > 32` or if `data >= 2^(num_bits)`.
* `absl::Status::kResourceExhausted` if there is not enough room in
* the write buffer. `absl::UnknownError()` if the `wb->bit_offset` is
* negative.
* `absl::UnknownError()` if the `wb->bit_offset` is negative.
*/
absl::Status WriteUnsignedLiteral(uint32_t data, int num_bits);

Expand All @@ -58,27 +56,24 @@ class WriteBitBuffer {
* 64.
* \return `absl::OkStatus()` on success. `absl::InvalidArgumentError()` if
* `num_bits > 64` or if `data >= 2^(num_bits)`.
* `absl::Status::kResourceExhausted` if there is not enough room in
* the write buffer. `absl::UnknownError()` if the `wb->bit_offset` is
* `absl::UnknownError()` if the `wb->bit_offset` is
* negative.
*/
absl::Status WriteUnsignedLiteral64(uint64_t data, int num_bits);

/*!\brief Writes specified signed 8 bit integer to the write buffer.
*
* \param data Data to write in standard two's complement form.
* \return `absl::OkStatus()` on success. `absl::Status::kResourceExhausted`
* if there is not enough room in the write buffer.
* `absl::UnknownError()` if the `wb->bit_offset` is negative.
* \return `absl::OkStatus()` on success. `absl::UnknownError()` if the
* `wb->bit_offset` is negative.
*/
absl::Status WriteSigned8(int8_t data);

/*!\brief Writes the signed 16 bit integer to the write buffer.
*
* \param data Data to write in standard two's complement form.
* \return `absl::OkStatus()` on success. `absl::Status::kResourceExhausted`
* if there is not enough room in the write buffer.
* `absl::UnknownError()` if the `wb->bit_offset` is negative.
* \return `absl::OkStatus()` on success. `absl::UnknownError()` if the
* `wb->bit_offset` is negative.
*/
absl::Status WriteSigned16(int16_t data);

Expand All @@ -87,37 +82,31 @@ class WriteBitBuffer {
* \param data Data to write.
* \return `absl::OkStatus()` on success. `absl::InvalidArgumentError()` if
* the string is not terminated within `kIamfMaxStringSize` bytes.
* `absl::Status::kResourceExhausted` if there is not enough room in
* the write buffer. Other specific statuses on failure.
* Other specific statuses on failure.
*/
absl::Status WriteString(const std::string& data);

/*!\brief Writes a `std::vector<uint8_t>` to the write buffer.
*
* \param data Data to write.
* \return `absl::OkStatus()` on success. `absl::Status::kResourceExhausted`
* if there is not enough room in the write buffer.
* `absl::UnknownError()` if the `wb->bit_offset` is negative.
* \return `absl::OkStatus()` on success. `absl::UnknownError()` if the
* `wb->bit_offset` is negative.
*/
absl::Status WriteUint8Vector(const std::vector<uint8_t>& data);

/*!\brief Writes a ULEB128 to the buffer using an implicit generator.
*
* \param data Data to write using the member `leb_generator_`.
* \return `absl::OkStatus()` on success. `absl::Status::kResourceExhausted`
* if there is not enough room in the write buffer.
* `absl::InvalidArgumentError()` if the generation fails. Other
* specific statuses on failure.
* \return `absl::OkStatus()` on success. `absl::InvalidArgumentError()` if
* the generation fails. Other specific statuses on failure.
*/
absl::Status WriteUleb128(DecodedUleb128 data);

/*!\brief Writes the expandable size according to ISO 14496-1.
*
* \param size_of_instance Size of the instance.
* \return `absl::OkStatus()` on success. `absl::Status::kResourceExhausted`
* if there is not enough room in the write buffer.
* `absl::InvalidArgumentError()` if the generation fails. Other
* specific statuses on failure.
* \return `absl::OkStatus()` on success. `absl::InvalidArgumentError()` if
* the generation fails. Other specific statuses on failure.
*/
absl::Status WriteIso14496_1Expanded(uint32_t size_of_instance);

Expand Down

0 comments on commit b764b0a

Please sign in to comment.