diff --git a/logging/BUILD.gn b/logging/BUILD.gn index 0700403d3b..200a37795e 100644 --- a/logging/BUILD.gn +++ b/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", diff --git a/logging/rtc_event_log/encoder/delta_encoding.cc b/logging/rtc_event_log/encoder/delta_encoding.cc index b62ce29bb3..437392f929 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 @@ 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 reader, + FixedLengthDeltaDecoder(BitstreamReader reader, const FixedLengthEncodingParameters& params, absl::optional 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> 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 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(&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(encoding_type_bits); return encoding_type == EncodingType::kFixedSizeUnsignedDeltasNoEarlyWrapNoOpt || @@ -719,18 +704,13 @@ std::unique_ptr FixedLengthDeltaDecoder::Create( const std::string& input, absl::optional 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( - reinterpret_cast(&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(encoding_type_bits); if (encoding != EncodingType::kFixedSizeUnsignedDeltasNoEarlyWrapNoOpt && encoding != @@ -739,15 +719,10 @@ std::unique_ptr FixedLengthDeltaDecoder::Create( 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 @@ std::unique_ptr FixedLengthDeltaDecoder::Create( values_optional = kDefaultValuesOptional; value_width_bits = kDefaultValueWidthBits; } else { - // signed_deltas - if (!reader->ReadBits(kBitsInHeaderForSignedDeltas, read_buffer)) { - return nullptr; - } - signed_deltas = rtc::dchecked_cast(read_buffer); + signed_deltas = reader.Read(); + values_optional = reader.Read(); + // 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(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 @@ std::unique_ptr FixedLengthDeltaDecoder::Create( 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 reader, + BitstreamReader reader, const FixedLengthEncodingParameters& params, absl::optional 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> FixedLengthDeltaDecoder::Decode() { - RTC_DCHECK(reader_); - + RTC_DCHECK(reader_.Ok()); std::vector 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>(); - } - RTC_DCHECK_LE(exists, 1u); - existing_values[i] = (exists == 1); + existing_values[i] = reader_.Read(); } } else { std::fill(existing_values.begin(), existing_values.end(), true); @@ -837,66 +795,22 @@ std::vector> 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; - if (!ParseVarInt(&first_value)) { - RTC_LOG(LS_WARNING) << "Failed to read first value."; - return std::vector>(); - } - values[i] = first_value; + values[i] = DecodeVarInt(reader_); } else { - uint64_t delta; - if (!ParseDelta(&delta)) { - return std::vector>(); - } - 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(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(lower_bits); - const uint64_t higher_bits_64 = static_cast(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 f2819c0c73..a84a233d6b 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 @@ std::pair 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) { - 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(); decoded += (static_cast(byte & 0x7f) << static_cast(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 dbe1f1103f..4624e046ba 100644 --- a/logging/rtc_event_log/encoder/var_int.h +++ b/logging/rtc_event_log/encoder/var_int.h @@ -18,7 +18,7 @@ #include #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 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