Split the bit reader functionalities from the byte stream provider.

media/base/bit_reader_h264.h

Index: media/base/bit_reader_h264.h
diff --git a/media/base/bit_reader_h264.h b/media/base/bit_reader_h264.h
new file mode 100644
index 0000000000000000000000000000000000000000..5cffb461ec772439c4c88d4aedf1e6cf4c4b697b
--- /dev/null
+++ b/media/base/bit_reader_h264.h
@@ -0,0 +1,77 @@
+// 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_H264_H_
+#define MEDIA_BASE_BIT_READER_H264_H_
+
+#include "base/basictypes.h"
+#include "base/compiler_specific.h"
+#include "base/memory/scoped_ptr.h"
+#include "media/base/bit_reader_core.h"
+#include "media/base/media_export.h"
+
+namespace media {
+
+class MEDIA_EXPORT BitReaderH264
+    : NON_EXPORTED_BASE(private BitReaderCore::ByteStreamProvider)  {
+ public:
+  // Initialize the reader to start reading at |data|, |size| being size
+  // of |data| in bytes.
+  BitReaderH264(const uint8* data, int size);
+  virtual ~BitReaderH264();
+
+  template<typename T> bool ReadBits(int num_bits, T* out) {
+    return bit_reader_core_.ReadBits(num_bits, out);
+  }
+
+  bool ReadFlag(bool* flag) {
+    return bit_reader_core_.ReadFlag(flag);
+  }
+
+  bool SkipBits(int num_bits) {
+    return bit_reader_core_.SkipBits(num_bits);
+  }
+
+  int GetBitCount() {

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

s/GetBitCount/bits_read/ for similar reasons as in the other file.

[damienv1, 2014/01/07 00:19:40]

OK I can do it since this is the chromium style.

However, I don't really like that convention because it makes the naming
dependent on what you are doing in the function (let's say later, I change the
code, and returning the number of bits read so far is not a simple accessor, we
have to change the name and all the code using this class).

+    return bit_reader_core_.GetBitCount();
+  }
+
+  // Read an unsigned exp-golomb value.
+  // Return a negative number if the value cannot be read.
+  // Return the number of bits used by the exp-golomb code.
+  // Note: if the number of bits is greater than |kMaxExpGolombCodeSize|,

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

This seems like a weird API. Why not simply make this bool ReadUE(uint32* out);
so it matches the style of the other methods? It would then just return false if
a code that was too long was detected. Why do you need to know how many bits
were actually used by the code?

[damienv1, 2014/01/07 00:19:40]

This allows the user of this class to differentiate between an "end of stream"
error VS a code which is using too many bits.
Another example: let's the user of this class just wants to skip an unsigned
exp-golomb value. It does not care about the value. It we just return false,
there is no way to check whether the error is the result of end of stream.

The API is more flexible to me. As opposed to other functions which knows how
many bits the code is, here we don't know in advance. This information might be
useful.

+  // then the value in |*out| is not valid.
+  int ReadUE(uint32* out);
+
+  // See the definition of more_rbsp_data() in spec.
+  bool HasMoreRBSPData();
+
+  static const int kMaxExpGolombCodeSize;
+
+ private:
+  // BitReaderCore::ByteStreamProvider implementation.
+  virtual int GetBytes(int max_n, const uint8** out) OVERRIDE;
+  virtual int GetBytesLeft() const OVERRIDE;
+
+  // Pointer to the next unread byte in the stream.
+  const uint8* data_;
+
+  // Bytes left in the stream.
+  int bytes_left_;
+
+  // Array used to return some bytes when some start code emulation prevention
+  // bytes are detected.
+  uint8 data_window_[8];
+
+  // Last two bytes read from the stream.
+  uint32 prev_two_bytes_;
+
+  BitReaderCore bit_reader_core_;
+
+  DISALLOW_COPY_AND_ASSIGN(BitReaderH264);
+};
+
+}  // namespace media
+
+#endif  // MEDIA_BASE_BIT_READER_H264_H_