Split the bit reader functionalities from the byte stream provider.

media/base/bit_reader_core.h

Index: media/base/bit_reader_core.h
diff --git a/media/base/bit_reader_core.h b/media/base/bit_reader_core.h
new file mode 100644
index 0000000000000000000000000000000000000000..d8a391155f13df7ee3400dfce59ededcf2fcf192
--- /dev/null
+++ b/media/base/bit_reader_core.h
@@ -0,0 +1,108 @@
+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef MEDIA_BASE_BIT_READER_CORE_H_
+#define MEDIA_BASE_BIT_READER_CORE_H_
+
+#include "base/basictypes.h"
+#include "base/logging.h"
+#include "media/base/media_export.h"
+
+namespace media {
+
+class MEDIA_EXPORT BitReaderCore {
+ public:
+  class ByteStreamProvider {
+   public:
+    ByteStreamProvider();
+    virtual ~ByteStreamProvider();
+
+    // Request n number of bytes where:
+    // - n should be equal to or greater than |min_n|,
+    // - n must be less than or equal to |max_n|.
+    // Return the actual number of bytes available in |*array|.
+    virtual int GetBytes(int min_n, int max_n, const uint8** array) = 0;

[acolwell GONE FROM CHROMIUM, 2013/12/28 01:54:42]

nit: Add text about the expected lifetime of |*array|.

[damienv1, 2013/12/28 02:26:17]

Will do.

+
+    // Return the number of bytes left in the stream.
+    virtual int GetBytesLeft() const = 0;
+  };
+
+  explicit BitReaderCore(ByteStreamProvider* byte_stream_provider);

[damienv1, 2013/12/28 01:32:25]

Add a comment to say the lifetime of the ByteStreamProvider should be longer
than the lifetime of the BitReaderCore.

[damienv1, 2013/12/28 02:26:17]

Done.

+  ~BitReaderCore();
+
+  // Read |num_bits| next bits from stream and return in |*out|, first bit
+  // from the stream starting at |num_bits| position in |*out|.
+  // |num_bits| cannot be larger than the bits the type can hold.
+  // Return false if the given number of bits cannot be read (not enough
+  // bits in the stream), true otherwise. When return false, the stream will
+  // enter a state where further ReadBits/SkipBits operations will always
+  // return false unless |num_bits| is 0. The type |T| has to be a primitive
+  // integer type.
+  // T can especially not be a boolean since it generates
+  // some optimization warnings during compilation on windows platforms,
+  // use ReadFlag instead.
+  template<typename T> bool ReadBits(int num_bits, T *out) {

[acolwell GONE FROM CHROMIUM, 2013/12/28 01:54:42]

nit: s/T *out/T* out/

[damienv1, 2013/12/28 02:26:17]

Will do.

+    DCHECK_LE(num_bits, static_cast<int>(sizeof(T) * 8));
+    uint64 temp;
+    bool ret = ReadBitsInternal(num_bits, &temp);
+    *out = static_cast<T>(temp);
+    return ret;
+  }
+
+  // Read a boolean.
+  bool ReadFlag(bool* flag);
+
+  // Read an unsigned exp-golomb value.
+  bool ReadUE(uint32* out);

[acolwell GONE FROM CHROMIUM, 2013/12/28 01:54:42]

nit: s/ReadUE/ReadExpGolomb/ ?

[damienv1, 2013/12/28 02:26:17]

That was my original naming but I noticed the current H264 bit reader (in
content/common/gpu) is using ReadUE instead (which is the abbreviation used in
the H264 spec for Exp-Golomb values).

[acolwell GONE FROM CHROMIUM, 2014/01/06 22:44:06]

That is why I suggested ExpGolumb over UE because it is a more descriptive name
and not tied to the H.264 spec. :)

+
+  // Skip |num_bits| next bits from stream. Return false if the given number of
+  // bits cannot be skipped (not enough bits in the stream), true otherwise.
+  // When return false, the stream will enter a state where further ReadBits/
+  // SkipBits operations will always return false unless |num_bits| is 0.

[acolwell GONE FROM CHROMIUM, 2013/12/28 01:54:42]

nit: ReadFlag & ReadUE should be added or the comment should be reworded to
include all bit reading and skipping methods in general.

[damienv1, 2013/12/28 04:02:23]

Done.

+  bool SkipBits(int num_bits);
+
+  // Returns the number of bits read so far.
+  int GetBitCount() const;
+
+  // Returns the number of bits available.
+  int bits_available() const;
+
+  // RBSP stop bit.
+  bool HasMoreRBSPData();

[acolwell GONE FROM CHROMIUM, 2013/12/28 01:54:42]

This H.264 concept should not be here. Please move it to the H.264 code. IIUC
you'll probably need to add a PeekBits() method here to implement it though.

[damienv1, 2013/12/28 02:26:17]

Agree, I actually already started to do that (using ViewBits but PeekBits might
be a better name).

+
+ private:
+  typedef uint64 RegType;
+  typedef int64 SignedRegType;
+
+  // Help function used by ReadBits to avoid inlining the bit reading logic.
+  bool ReadBitsInternal(int num_bits, uint64* out);
+
+  // Refill bit registers to have at least |min_nbits| bits available.
+  // Return true if the mininimum bit count condition is met after the refill.
+  bool Refill(int min_nbits);
+
+  // Refill the current bit register from the next bit register.
+  void RefillCurrentRegister();
+
+  ByteStreamProvider* const byte_stream_provider_;
+
+  // Number of bits read so far.
+  int bit_count_;
+
+  // Number of valid bits left in |reg|.
+  // Note: bits are consumed from MSB to LSB.
+  int nbits_;
+  RegType reg_;
+
+  // Number of valid bits left in |reg_next_|.
+  // Note: bits are consumed from MSB to LSB.
+  int nbits_next_;
+  RegType reg_next_;
+
+  DISALLOW_COPY_AND_ASSIGN(BitReaderCore);
+};
+
+}  // namespace media
+
+#endif  // MEDIA_BASE_BIT_READER_CORE_H_