Split the bit reader functionalities from the byte stream provider.

media/base/bit_reader.cc

Index: media/base/bit_reader.cc
diff --git a/media/base/bit_reader.cc b/media/base/bit_reader.cc
index e4d83af74107a09a2f745bd363ff1615af6fcd36..b6b1a496c27c9333449f47d95cf7cb8dfc1e51e0 100644
--- a/media/base/bit_reader.cc
+++ b/media/base/bit_reader.cc
@@ -4,80 +4,152 @@
 
 #include "media/base/bit_reader.h"
 
-#include <algorithm>
-
 namespace media {
 
-BitReader::BitReader(const uint8* data, off_t size)
-    : data_(data), bytes_left_(size), num_remaining_bits_in_curr_byte_(0) {
+namespace {
+
+class LinearByteStreamProvider : public BitReaderCore::ByteStreamProvider {
+ public:
+  LinearByteStreamProvider(const uint8* data, off_t size);
+  virtual ~LinearByteStreamProvider();
+
+  // BitReaderCore::ByteStreamProvider implementation.
+  virtual int GetBytes(int min_n,
+                       int max_n,
+                      const uint8** out) OVERRIDE;
+  virtual int GetBytesLeft() const OVERRIDE;
+
+ private:
+  // Pointer to the next unread byte in the stream.
+  // Does not include bits held in the bit registers.
+  const uint8* data_;
+
+  // Bytes left in the stream.
+  // Does not include bits held in the bit registers.
+  int bytes_left_;
+};
+
+LinearByteStreamProvider::LinearByteStreamProvider(
+    const uint8* data, off_t size)
+  : data_(data),
+    bytes_left_(size) {
   DCHECK(data_ != NULL && bytes_left_ > 0);
-
-  UpdateCurrByte();
 }
 
-BitReader::~BitReader() {}
+LinearByteStreamProvider::~LinearByteStreamProvider() {
+}
 
-bool BitReader::SkipBits(int num_bits) {
-  DCHECK_GE(num_bits, 0);
-  DVLOG_IF(0, num_bits > 100)
-      << "BitReader::SkipBits inefficient for large skips";
-
-  // Skip any bits in the current byte waiting to be processed, then
-  // process full bytes until less than 8 bits remaining.
-  while (num_bits > 0 && num_bits > num_remaining_bits_in_curr_byte_) {
-    num_bits -= num_remaining_bits_in_curr_byte_;
-    num_remaining_bits_in_curr_byte_ = 0;
-    UpdateCurrByte();
-
-    // If there is no more data remaining, only return true if we
-    // skipped all that were requested.
-    if (num_remaining_bits_in_curr_byte_ == 0)
-      return (num_bits == 0);
-  }
+int LinearByteStreamProvider::GetBytes(
+    int min_nbytes, int max_nbytes, const uint8** out) {
+  int nbytes = max_nbytes;
+  if (nbytes > bytes_left_)
+    nbytes = bytes_left_;
 
-  // Less than 8 bits remaining to skip. Use ReadBitsInternal to verify
-  // that the remaining bits we need exist, and adjust them as necessary
-  // for subsequent operations.
-  uint64 not_needed;
-  return ReadBitsInternal(num_bits, &not_needed);
+  *out = data_;
+  data_ += nbytes;
+  bytes_left_ -= nbytes;
+  return nbytes;
 }
 
-int BitReader::bits_available() const {
-  return 8 * bytes_left_ + num_remaining_bits_in_curr_byte_;
+int LinearByteStreamProvider::GetBytesLeft() const {
+  return bytes_left_;
 }
 
-bool BitReader::ReadBitsInternal(int num_bits, uint64* out) {
-  DCHECK_LE(num_bits, 64);
-
-  *out = 0;
-
-  while (num_remaining_bits_in_curr_byte_ != 0 && num_bits != 0) {
-    int bits_to_take = std::min(num_remaining_bits_in_curr_byte_, num_bits);
+class H264ByteStreamProvider : public BitReaderCore::ByteStreamProvider {

[acolwell GONE FROM CHROMIUM, 2013/12/27 20:35:21]

This should not be here. It should be with the H.264 code.

[damienv1, 2013/12/27 21:23:20]

Will do.

+ public:
+  H264ByteStreamProvider(const uint8* data, int size);
+  virtual ~H264ByteStreamProvider();
+
+  // BitReaderCore::ByteStreamProvider implementation.
+  virtual int GetBytes(int min_n,
+                       int max_n,
+                      const uint8** out) OVERRIDE;
+  virtual int GetBytesLeft() const OVERRIDE;
+
+ private:
+  // Pointer to the next unread byte in the stream.
+  // Does not include bits held in the bit registers.
+  const uint8* data_;
+
+  // Bytes left in the stream.
+  // Does not include bits held in the bit registers.
+  int bytes_left_;
+
+  // Array used to return some bytes when emulation prevention bytes
+  // are detected.
+  uint8 data_window_[8];
+
+  // Last two bytes read from the stream.
+  int prev_two_bytes_;
+};
+
+H264ByteStreamProvider::H264ByteStreamProvider(const uint8* data, int size)
+  : data_(data),
+    bytes_left_(size),
+    prev_two_bytes_(0xffff) {
+  DCHECK(data_ != NULL && bytes_left_ > 0);
+}
 
-    *out <<= bits_to_take;
-    *out += curr_byte_ >> (num_remaining_bits_in_curr_byte_ - bits_to_take);
-    num_bits -= bits_to_take;
-    num_remaining_bits_in_curr_byte_ -= bits_to_take;
-    curr_byte_ &= (1 << num_remaining_bits_in_curr_byte_) - 1;
+H264ByteStreamProvider::~H264ByteStreamProvider() {
+}
 
-    if (num_remaining_bits_in_curr_byte_ == 0)
-      UpdateCurrByte();
+int H264ByteStreamProvider::GetBytes(
+    int min_nbytes, int max_nbytes, const uint8** out) {
+  DCHECK_LE(max_nbytes, static_cast<int>(sizeof(data_window_)));
+
+  const uint8* start = data_;
+  bool epb = false;
+
+  int nbytes = 0;
+  *out = data_;
+
+  while (nbytes < max_nbytes && bytes_left_ > 0) {
+    // Emulation prevention three-byte detection.
+    // If a sequence of 0x000003 is found, skip (ignore) the last byte (0x03)
+    // and starts copying data to |data_window_| which is used as the output.
+    if (*data_ == 0x3 && (prev_two_bytes_ & 0xffff) == 0) {
+      if (!epb && nbytes > 0)
+        memcpy(data_window_, start, nbytes);
+      epb = true;
+      *out = data_window_;
+      // Need another full three bytes before we can detect the sequence again.
+      prev_two_bytes_ = 0xffff;
+      continue;
+    } else {
+      if (epb)
+        data_window_[nbytes] = *data_;
+      ++nbytes;
+    }
+
+    ++data_;
+    --bytes_left_;
   }
 
-  return num_bits == 0;
+  return nbytes;
+}
+
+int H264ByteStreamProvider::GetBytesLeft() const {
+  return bytes_left_;
 }
 
-void BitReader::UpdateCurrByte() {
-  DCHECK_EQ(num_remaining_bits_in_curr_byte_, 0);
+}  // namespace
+
+BitReader::BitReader(
+    const uint8* data, off_t size, bool use_h264_byte_provider)
+    : byte_stream_provider_(
+        use_h264_byte_provider ?
+        static_cast<BitReaderCore::ByteStreamProvider*>(
+            new H264ByteStreamProvider(data, size)) :
+        static_cast<BitReaderCore::ByteStreamProvider*>(
+            new LinearByteStreamProvider(data, size))),
+      bit_reader_core_(byte_stream_provider_.get()) {
+}
 
-  if (bytes_left_ == 0)
-    return;
+BitReader::~BitReader() {}
 
-  // Load a new byte and advance pointers.
-  curr_byte_ = *data_;
-  ++data_;
-  --bytes_left_;
-  num_remaining_bits_in_curr_byte_ = 8;
+int BitReader::bits_available() const {
+  return (byte_stream_provider_->GetBytesLeft() * CHAR_BIT +

[acolwell GONE FROM CHROMIUM, 2013/12/27 20:35:21]

Why not push this part of the computation down into the BitReaderCore? It has a
reference to the provider.

[damienv1, 2013/12/27 21:23:20]

You're right. I initially didn't have GetBytesLeft in the ByteStreamProvider
interface, that's why it was not part of the BitReaderCore.

+          bit_reader_core_.bits_available());
 }
 
 }  // namespace media