Migrate rtc event log from rtc::BitBuffer to BitstreamReader
BitstreamReader allows to write easier to reader parser
Bug: None
Change-Id: I9da88c86ee04be4c0b06e181e409a915ba1a5123
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/231232
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#34939}
diff --git a/logging/BUILD.gn b/logging/BUILD.gn
index 0700403..200a377 100644
--- a/logging/BUILD.gn
+++ b/logging/BUILD.gn
@@ -210,6 +210,7 @@
"../api:rtp_headers",
"../api:rtp_parameters",
"../api/transport:network_control",
+ "../rtc_base:bitstream_reader",
"../rtc_base:checks",
"../rtc_base:ignore_wundef",
"../rtc_base:rtc_base_approved",
diff --git a/logging/rtc_event_log/encoder/delta_encoding.cc b/logging/rtc_event_log/encoder/delta_encoding.cc
index b62ce29..437392f 100644
--- a/logging/rtc_event_log/encoder/delta_encoding.cc
+++ b/logging/rtc_event_log/encoder/delta_encoding.cc
@@ -18,6 +18,7 @@
#include "absl/memory/memory.h"
#include "logging/rtc_event_log/encoder/var_int.h"
#include "rtc_base/bit_buffer.h"
+#include "rtc_base/bitstream_reader.h"
#include "rtc_base/checks.h"
#include "rtc_base/constructor_magic.h"
#include "rtc_base/logging.h"
@@ -636,7 +637,7 @@
// Therefore, it was deemed acceptable that `reader` does not own the buffer
// it reads, meaning the lifetime of `this` must not exceed the lifetime
// of `reader`'s underlying buffer.
- FixedLengthDeltaDecoder(std::unique_ptr<rtc::BitBuffer> reader,
+ FixedLengthDeltaDecoder(BitstreamReader reader,
const FixedLengthEncodingParameters& params,
absl::optional<uint64_t> base,
size_t num_of_deltas);
@@ -644,16 +645,6 @@
// Perform the decoding using the parameters given to the ctor.
std::vector<absl::optional<uint64_t>> Decode();
- // Decode a varint and write it to `output`. Return value indicates success
- // or failure. In case of failure, no guarantees are made about the contents
- // of `output` or the results of additional reads.
- bool ParseVarInt(uint64_t* output);
-
- // Attempt to parse a delta from the input reader.
- // Returns true/false for success/failure.
- // Writes the delta into `delta` if successful.
- bool ParseDelta(uint64_t* delta);
-
// Add `delta` to `base` to produce the next value in a sequence.
// The delta is applied as signed/unsigned depending on the parameters
// given to the ctor. Wrap-around is taken into account according to the
@@ -666,7 +657,7 @@
// Reader of the input stream to be decoded. Does not own that buffer.
// See comment above ctor for details.
- const std::unique_ptr<rtc::BitBuffer> reader_;
+ BitstreamReader reader_;
// The parameters according to which encoding will be done (width of
// fields, whether signed deltas should be used, etc.)
@@ -684,18 +675,12 @@
};
bool FixedLengthDeltaDecoder::IsSuitableDecoderFor(const std::string& input) {
- if (input.length() < kBitsInHeaderForEncodingType) {
+ BitstreamReader reader(input);
+ uint64_t encoding_type_bits = reader.ReadBits(kBitsInHeaderForEncodingType);
+ if (!reader.Ok()) {
return false;
}
- rtc::BitBuffer reader(reinterpret_cast<const uint8_t*>(&input[0]),
- kBitsInHeaderForEncodingType);
-
- uint32_t encoding_type_bits;
- const bool result =
- reader.ReadBits(kBitsInHeaderForEncodingType, encoding_type_bits);
- RTC_DCHECK(result);
-
const auto encoding_type = static_cast<EncodingType>(encoding_type_bits);
return encoding_type ==
EncodingType::kFixedSizeUnsignedDeltasNoEarlyWrapNoOpt ||
@@ -719,18 +704,13 @@
const std::string& input,
absl::optional<uint64_t> base,
size_t num_of_deltas) {
- if (input.length() < kBitsInHeaderForEncodingType) {
+ BitstreamReader reader(input);
+ // Encoding type
+ uint32_t encoding_type_bits = reader.ReadBits(kBitsInHeaderForEncodingType);
+ if (!reader.Ok()) {
return nullptr;
}
- auto reader = std::make_unique<rtc::BitBuffer>(
- reinterpret_cast<const uint8_t*>(&input[0]), input.length());
-
- // Encoding type
- uint32_t encoding_type_bits;
- const bool result =
- reader->ReadBits(kBitsInHeaderForEncodingType, encoding_type_bits);
- RTC_DCHECK(result);
const EncodingType encoding = static_cast<EncodingType>(encoding_type_bits);
if (encoding != EncodingType::kFixedSizeUnsignedDeltasNoEarlyWrapNoOpt &&
encoding !=
@@ -739,15 +719,10 @@
return nullptr;
}
- uint32_t read_buffer;
-
- // delta_width_bits
- if (!reader->ReadBits(kBitsInHeaderForDeltaWidthBits, read_buffer)) {
- return nullptr;
- }
- RTC_DCHECK_LE(read_buffer, 64 - 1); // See encoding for -1's rationale.
+ // See encoding for +1's rationale.
const uint64_t delta_width_bits =
- read_buffer + 1; // See encoding for +1's rationale.
+ reader.ReadBits(kBitsInHeaderForDeltaWidthBits) + 1;
+ RTC_DCHECK_LE(delta_width_bits, 64);
// signed_deltas, values_optional, value_width_bits
bool signed_deltas;
@@ -758,25 +733,15 @@
values_optional = kDefaultValuesOptional;
value_width_bits = kDefaultValueWidthBits;
} else {
- // signed_deltas
- if (!reader->ReadBits(kBitsInHeaderForSignedDeltas, read_buffer)) {
- return nullptr;
- }
- signed_deltas = rtc::dchecked_cast<bool>(read_buffer);
+ signed_deltas = reader.Read<bool>();
+ values_optional = reader.Read<bool>();
+ // See encoding for +1's rationale.
+ value_width_bits = reader.ReadBits(kBitsInHeaderForValueWidthBits) + 1;
+ RTC_DCHECK_LE(value_width_bits, 64);
+ }
- // values_optional
- if (!reader->ReadBits(kBitsInHeaderForValuesOptional, read_buffer)) {
- return nullptr;
- }
- RTC_DCHECK_LE(read_buffer, 1);
- values_optional = rtc::dchecked_cast<bool>(read_buffer);
-
- // value_width_bits
- if (!reader->ReadBits(kBitsInHeaderForValueWidthBits, read_buffer)) {
- return nullptr;
- }
- RTC_DCHECK_LE(read_buffer, 64 - 1); // See encoding for -1's rationale.
- value_width_bits = read_buffer + 1; // See encoding for +1's rationale.
+ if (!reader.Ok()) {
+ return nullptr;
}
// Note: Because of the way the parameters are read, it is not possible
@@ -790,35 +755,28 @@
FixedLengthEncodingParameters params(delta_width_bits, signed_deltas,
values_optional, value_width_bits);
- return absl::WrapUnique(new FixedLengthDeltaDecoder(std::move(reader), params,
- base, num_of_deltas));
+ return absl::WrapUnique(
+ new FixedLengthDeltaDecoder(reader, params, base, num_of_deltas));
}
FixedLengthDeltaDecoder::FixedLengthDeltaDecoder(
- std::unique_ptr<rtc::BitBuffer> reader,
+ BitstreamReader reader,
const FixedLengthEncodingParameters& params,
absl::optional<uint64_t> base,
size_t num_of_deltas)
- : reader_(std::move(reader)),
+ : reader_(reader),
params_(params),
base_(base),
num_of_deltas_(num_of_deltas) {
- RTC_DCHECK(reader_);
+ RTC_DCHECK(reader_.Ok());
}
std::vector<absl::optional<uint64_t>> FixedLengthDeltaDecoder::Decode() {
- RTC_DCHECK(reader_);
-
+ RTC_DCHECK(reader_.Ok());
std::vector<bool> existing_values(num_of_deltas_);
if (params_.values_optional()) {
for (size_t i = 0; i < num_of_deltas_; ++i) {
- uint32_t exists;
- if (!reader_->ReadBits(1u, exists)) {
- RTC_LOG(LS_WARNING) << "Failed to read existence-indicating bit.";
- return std::vector<absl::optional<uint64_t>>();
- }
- RTC_DCHECK_LE(exists, 1u);
- existing_values[i] = (exists == 1);
+ existing_values[i] = reader_.Read<bool>();
}
} else {
std::fill(existing_values.begin(), existing_values.end(), true);
@@ -837,66 +795,22 @@
// If the base is non-existent, the first existent value is encoded as
// a varint, rather than as a delta.
RTC_DCHECK(!base_.has_value());
- uint64_t first_value;
- if (!ParseVarInt(&first_value)) {
- RTC_LOG(LS_WARNING) << "Failed to read first value.";
- return std::vector<absl::optional<uint64_t>>();
- }
- values[i] = first_value;
+ values[i] = DecodeVarInt(reader_);
} else {
- uint64_t delta;
- if (!ParseDelta(&delta)) {
- return std::vector<absl::optional<uint64_t>>();
- }
- values[i] = ApplyDelta(previous.value(), delta);
+ uint64_t delta = reader_.ReadBits(params_.delta_width_bits());
+ values[i] = ApplyDelta(*previous, delta);
}
previous = values[i];
}
+ if (!reader_.Ok()) {
+ values = {};
+ }
+
return values;
}
-bool FixedLengthDeltaDecoder::ParseVarInt(uint64_t* output) {
- RTC_DCHECK(reader_);
- return DecodeVarInt(reader_.get(), output) != 0;
-}
-
-bool FixedLengthDeltaDecoder::ParseDelta(uint64_t* delta) {
- RTC_DCHECK(reader_);
-
- // BitBuffer and BitBufferWriter read/write higher bits before lower bits.
-
- const size_t lower_bit_count =
- std::min<uint64_t>(params_.delta_width_bits(), 32u);
- const size_t higher_bit_count = (params_.delta_width_bits() <= 32u)
- ? 0
- : params_.delta_width_bits() - 32u;
-
- uint32_t lower_bits;
- uint32_t higher_bits;
-
- if (higher_bit_count > 0) {
- if (!reader_->ReadBits(higher_bit_count, higher_bits)) {
- RTC_LOG(LS_WARNING) << "Failed to read higher half of delta.";
- return false;
- }
- } else {
- higher_bits = 0;
- }
-
- if (!reader_->ReadBits(lower_bit_count, lower_bits)) {
- RTC_LOG(LS_WARNING) << "Failed to read lower half of delta.";
- return false;
- }
-
- const uint64_t lower_bits_64 = static_cast<uint64_t>(lower_bits);
- const uint64_t higher_bits_64 = static_cast<uint64_t>(higher_bits);
-
- *delta = (higher_bits_64 << 32) | lower_bits_64;
- return true;
-}
-
uint64_t FixedLengthDeltaDecoder::ApplyDelta(uint64_t base,
uint64_t delta) const {
RTC_DCHECK_LE(base, MaxUnsignedValueOfBitWidth(params_.value_width_bits()));
diff --git a/logging/rtc_event_log/encoder/var_int.cc b/logging/rtc_event_log/encoder/var_int.cc
index f2819c0..a84a233 100644
--- a/logging/rtc_event_log/encoder/var_int.cc
+++ b/logging/rtc_event_log/encoder/var_int.cc
@@ -10,6 +10,7 @@
#include "logging/rtc_event_log/encoder/var_int.h"
+#include "rtc_base/bitstream_reader.h"
#include "rtc_base/checks.h"
// TODO(eladalon): Add unit tests.
@@ -58,23 +59,18 @@
// There is some code duplication between the flavors of this function.
// For performance's sake, it's best to just keep it.
-size_t DecodeVarInt(rtc::BitBuffer* input, uint64_t* output) {
- RTC_DCHECK(output);
-
+uint64_t DecodeVarInt(BitstreamReader& input) {
uint64_t decoded = 0;
for (size_t i = 0; i < kMaxVarIntLengthBytes; ++i) {
- uint8_t byte;
- if (!input->ReadUInt8(byte)) {
- return 0;
- }
+ uint8_t byte = input.Read<uint8_t>();
decoded +=
(static_cast<uint64_t>(byte & 0x7f) << static_cast<uint64_t>(7 * i));
if (!(byte & 0x80)) {
- *output = decoded;
- return i + 1;
+ return decoded;
}
}
+ input.Invalidate();
return 0;
}
diff --git a/logging/rtc_event_log/encoder/var_int.h b/logging/rtc_event_log/encoder/var_int.h
index dbe1f11..4624e04 100644
--- a/logging/rtc_event_log/encoder/var_int.h
+++ b/logging/rtc_event_log/encoder/var_int.h
@@ -18,7 +18,7 @@
#include <utility>
#include "absl/strings/string_view.h"
-#include "rtc_base/bit_buffer.h"
+#include "rtc_base/bitstream_reader.h"
namespace webrtc {
@@ -39,13 +39,11 @@
std::pair<bool, absl::string_view> DecodeVarInt(absl::string_view input,
uint64_t* output);
-// Same as other version, but uses a rtc::BitBuffer for input.
-// If decoding is successful, a non-zero number is returned, indicating the
-// number of bytes read from `input`, and the decoded varint is written
-// into `output`.
-// If not successful, 0 is returned, and `output` is not modified.
-// Some bits may be consumed even if a varint fails to be read.
-size_t DecodeVarInt(rtc::BitBuffer* input, uint64_t* output);
+// Same as other version, but uses a BitstreamReader for input.
+// If decoding is successful returns the decoded varint.
+// If not successful, `input` reader is set into the failure state, return value
+// is unspecified.
+uint64_t DecodeVarInt(BitstreamReader& input);
} // namespace webrtc
