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}

logging/BUILD.gn

@@ -210,6 +210,7 @@ rtc_library("rtc_event_log_impl_encoder") {
     "../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",

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 @@ class FixedLengthDeltaDecoder final {
   // 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 @@ class FixedLengthDeltaDecoder final {
   // 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 @@ class FixedLengthDeltaDecoder final {
 
   // 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 @@ class FixedLengthDeltaDecoder final {
 };
 
 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 @@ std::unique_ptr<FixedLengthDeltaDecoder> FixedLengthDeltaDecoder::Create(
     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 @@ std::unique_ptr<FixedLengthDeltaDecoder> FixedLengthDeltaDecoder::Create(
     return nullptr;
   }
 
-  uint32_t read_buffer;
+  // See encoding for +1's rationale.
-
-  // delta_width_bits
-  if (!reader->ReadBits(kBitsInHeaderForDeltaWidthBits, read_buffer)) {
-    return nullptr;
-  }
-  RTC_DCHECK_LE(read_buffer, 64 - 1);  // 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,26 +733,16 @@ std::unique_ptr<FixedLengthDeltaDecoder> FixedLengthDeltaDecoder::Create(
     values_optional = kDefaultValuesOptional;
     value_width_bits = kDefaultValueWidthBits;
   } else {
-    // signed_deltas
+    signed_deltas = reader.Read<bool>();
-    if (!reader->ReadBits(kBitsInHeaderForSignedDeltas, read_buffer)) {
+    values_optional = reader.Read<bool>();
-      return nullptr;
+    // See encoding for +1's rationale.
+    value_width_bits = reader.ReadBits(kBitsInHeaderForValueWidthBits) + 1;
+    RTC_DCHECK_LE(value_width_bits, 64);
   }
-    signed_deltas = rtc::dchecked_cast<bool>(read_buffer);
 
-    // values_optional
+  if (!reader.Ok()) {
-    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.
-  }
 
   // Note: Because of the way the parameters are read, it is not possible
   // for illegal values to be read. We check nevertheless, in case the code
@@ -790,35 +755,28 @@ std::unique_ptr<FixedLengthDeltaDecoder> FixedLengthDeltaDecoder::Create(
 
   FixedLengthEncodingParameters params(delta_width_bits, signed_deltas,
                                        values_optional, value_width_bits);
-  return absl::WrapUnique(new FixedLengthDeltaDecoder(std::move(reader), params,
+  return absl::WrapUnique(
-                                                      base, num_of_deltas));
+      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;
+      existing_values[i] = reader_.Read<bool>();
-      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);
     }
   } else {
     std::fill(existing_values.begin(), existing_values.end(), true);
@@ -837,66 +795,22 @@ std::vector<absl::optional<uint64_t>> FixedLengthDeltaDecoder::Decode() {
       // 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;
+      values[i] = DecodeVarInt(reader_);
-      if (!ParseVarInt(&first_value)) {
-        RTC_LOG(LS_WARNING) << "Failed to read first value.";
-        return std::vector<absl::optional<uint64_t>>();
-      }
-      values[i] = first_value;
     } else {
-      uint64_t delta;
+      uint64_t delta = reader_.ReadBits(params_.delta_width_bits());
-      if (!ParseDelta(&delta)) {
+      values[i] = ApplyDelta(*previous, delta);
-        return std::vector<absl::optional<uint64_t>>();
-      }
-      values[i] = ApplyDelta(previous.value(), 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()));

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 @@ std::pair<bool, absl::string_view> DecodeVarInt(absl::string_view input,
 
 // 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) {
+uint64_t DecodeVarInt(BitstreamReader& input) {
-  RTC_DCHECK(output);
-
   uint64_t decoded = 0;
   for (size_t i = 0; i < kMaxVarIntLengthBytes; ++i) {
-    uint8_t byte;
+    uint8_t byte = input.Read<uint8_t>();
-    if (!input->ReadUInt8(byte)) {
-      return 0;
-    }
     decoded +=
         (static_cast<uint64_t>(byte & 0x7f) << static_cast<uint64_t>(7 * i));
     if (!(byte & 0x80)) {
-      *output = decoded;
+      return decoded;
-      return i + 1;
     }
   }
 
+  input.Invalidate();
   return 0;
 }
 

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::string EncodeVarInt(uint64_t input);
 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.
+// Same as other version, but uses a BitstreamReader for input.
-// If decoding is successful, a non-zero number is returned, indicating the
+// If decoding is successful returns the decoded varint.
-// number of bytes read from `input`, and the decoded varint is written
+// If not successful, `input` reader is set into the failure state, return value
-// into `output`.
+// is unspecified.
-// If not successful, 0 is returned, and `output` is not modified.
+uint64_t DecodeVarInt(BitstreamReader& input);
-// Some bits may be consumed even if a varint fails to be read.
-size_t DecodeVarInt(rtc::BitBuffer* input, uint64_t* output);
 
 }  // namespace webrtc