Make H264BitReader ignore trailing zero bytes and stop bit.

content/common/gpu/media/h264_bit_reader.cc

 // Copyright (c) 2012 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.
 
 #include "content/common/gpu/media/h264_bit_reader.h"
 #include "base/logging.h"
 
 namespace content {
 
-H264BitReader::H264BitReader()
-    : data_(NULL), bytes_left_(0), curr_byte_(0),
-      num_remaining_bits_in_curr_byte_(0), prev_two_bytes_(0) {
-}
+H264BitReader::H264BitReader() {}
 
 H264BitReader::~H264BitReader() {}
 
 bool H264BitReader::Initialize(const uint8* data, off_t size) {
   DCHECK(data);
-
-  if (size < 1)
-    return false;
+  DCHECK_GE(size, 0);
 
   data_ = data;
   bytes_left_ = size;
+
+  if (!RemoveTrailingZeroAndStopBit())
+    return false;
+
   num_remaining_bits_in_curr_byte_ = 0;
   // Initially set to 0xffff to accept all initial two-byte sequences.
   prev_two_bytes_ = 0xffff;
 
   return true;
 }
 
 bool H264BitReader::UpdateCurrByte() {
   if (bytes_left_ < 1)
     return false;
 
   // Emulation prevention three-byte detection.
   // If a sequence of 0x000003 is found, skip (ignore) the last byte (0x03).
   if (*data_ == 0x03 && (prev_two_bytes_ & 0xffff) == 0) {
     // Detected 0x000003, skip last byte.
     ++data_;
     --bytes_left_;
     // Need another full three bytes before we can detect the sequence again.
     prev_two_bytes_ = 0xffff;
 
     if (bytes_left_ < 1)
       return false;
   }
 
   // Load a new byte and advance pointers.
   curr_byte_ = *data_++ & 0xff;
   --bytes_left_;
-  num_remaining_bits_in_curr_byte_ = 8;
+
+  if (bytes_left_ == 0) {
+    curr_byte_ >>= 8 - data_bits_in_last_byte_;
+    num_remaining_bits_in_curr_byte_ = data_bits_in_last_byte_;
+  } else {
+    num_remaining_bits_in_curr_byte_ = 8;
+  }
 
   prev_two_bytes_ = (prev_two_bytes_ << 8) | curr_byte_;
 
   return true;
 }
 
-// Read |num_bits| (1 to 31 inclusive) from the stream and return them
-// in |out|, with first bit in the stream as MSB in |out| at position
-// (|num_bits| - 1).
 bool H264BitReader::ReadBits(int num_bits, int *out) {
   int bits_left = num_bits;
   *out = 0;
-  DCHECK(num_bits <= 31);
+  DCHECK_LE(num_bits, 31);
 
   while (num_remaining_bits_in_curr_byte_ < bits_left) {
     // Take all that's left in current byte, shift to make space for the rest.
-    *out = (curr_byte_ << (bits_left - num_remaining_bits_in_curr_byte_));
+    *out |= (curr_byte_ << (bits_left - num_remaining_bits_in_curr_byte_));

[Ami GONE FROM CHROMIUM, 2012/08/02 19:49:48]

I think this bugfix belongs in its own CL.
How did the ReadBits(31) test pass before this CL?

[xiaomings, 2012/08/03 00:37:58]

Done.

     bits_left -= num_remaining_bits_in_curr_byte_;
 
     if (!UpdateCurrByte())
       return false;
   }
 
   *out |= (curr_byte_ >> (num_remaining_bits_in_curr_byte_ - bits_left));
   *out &= ((1 << num_bits) - 1);
   num_remaining_bits_in_curr_byte_ -= bits_left;
 
   return true;
 }
 
-off_t H264BitReader::NumBitsLeft() {
-  return (num_remaining_bits_in_curr_byte_ + bytes_left_ * 8);
+off_t H264BitReader::NumBitsLeft() const {
+  off_t bits_left = num_remaining_bits_in_curr_byte_ +
+      (bytes_left_ + trailing_zero_bytes_) * 8;
+  if (bytes_left_ == 0)
+    bits_left += 8 - data_bits_in_last_byte_;
+  return bits_left;
 }
 
-bool H264BitReader::HasMoreRBSPData() {
-  // Make sure we have more bits, if we are at 0 bits in current byte
-  // and updating current byte fails, we don't have more data anyway.
-  if (num_remaining_bits_in_curr_byte_ == 0 && !UpdateCurrByte())
-      return false;
+bool H264BitReader::RemoveTrailingZeroAndStopBit() {
+  // First remove all trailing zero bytes.
+  trailing_zero_bytes_ = 0;
 
-  // On last byte?
-  if (bytes_left_)
-    return true;
+  while (bytes_left_ != 0) {
+    if (data_[bytes_left_ - 1] == 0) {
+      --bytes_left_;
+      ++trailing_zero_bytes_;
+      continue;
+    }
+    if (bytes_left_ >= 3 && data_[bytes_left_ - 1] == 0x03 &&
+        data_[bytes_left_ - 2] == 0x00 && data_[bytes_left_ - 3] == 0x00) {
+      bytes_left_ -= 3;
+      trailing_zero_bytes_ += 3;
+      continue;
+    }
 
-  // Last byte, look for stop bit;
-  // We have more RBSP data if the last non-zero bit we find is not the
-  // first available bit.
-  return (curr_byte_ &
-          ((1 << (num_remaining_bits_in_curr_byte_ - 1)) - 1)) != 0;
+    break;
+  }
+
+  if (bytes_left_ == 0)
+    return false;
+
+  // Then remove the trailing zero bits of the last byte.
+  uint8 last_byte = data_[bytes_left_ - 1];
+
+  data_bits_in_last_byte_ = 8;

[Ami GONE FROM CHROMIUM, 2012/08/02 19:49:48]

l.118-125 can be replaced with:
data_bits_in_last_byte_ = 8 - ffs(last_byte);

[xiaomings, 2012/08/03 00:37:58]

Done.

+
+  while ((last_byte & (1 << (8 - data_bits_in_last_byte_))) == 0)
+    --data_bits_in_last_byte_;
+
+  --data_bits_in_last_byte_;  // Remove the stop bit
+
+  if (data_bits_in_last_byte_ == 0) {

[Ami GONE FROM CHROMIUM, 2012/08/02 19:49:48]

I see now what you were doing before, and I think that was better than this
(esp. if you take my ffs suggestion above).  
You probably want to restore l.128-134 of patchset 2 and delete this.

[xiaomings, 2012/08/03 00:37:58]

Done.

+    --bytes_left_;
+    data_bits_in_last_byte_ = 8;
+    ++trailing_zero_bytes_;
+  }
+
+  return bytes_left_ != 0;
 }
 
 }  // namespace content