Add support for 3-byte headers in VideoToolbox NALU parser.

webrtc/modules/video_coding/codecs/h264/h264_video_toolbox_nalu.cc

Index: webrtc/modules/video_coding/codecs/h264/h264_video_toolbox_nalu.cc
diff --git a/webrtc/modules/video_coding/codecs/h264/h264_video_toolbox_nalu.cc b/webrtc/modules/video_coding/codecs/h264/h264_video_toolbox_nalu.cc
index c6c0406128f05d2c818b0a94212ea5cb1377bc60..cf20537bdce5dae5c18c08965e068f6a2a0bc8b0 100644
--- a/webrtc/modules/video_coding/codecs/h264/h264_video_toolbox_nalu.cc
+++ b/webrtc/modules/video_coding/codecs/h264/h264_video_toolbox_nalu.cc
@@ -19,7 +19,6 @@
 
 #include "webrtc/base/checks.h"
 #include "webrtc/base/logging.h"
-#include "webrtc/common_video/h264/h264_common.h"
 
 namespace webrtc {
 
@@ -27,6 +26,7 @@ using H264::NaluType;
 using H264::kAud;
 using H264::kSps;
 using H264::ParseNaluType;
+using H264::NaluIndex;

[tkchin_webrtc, 2016/09/27 11:29:35]

nit: consider alphabetizing

[kthelgason, 2016/09/28 08:48:27]

Done.

 
 const char kAnnexBHeaderBytes[4] = {0, 0, 0, 1};
 const size_t kAvccHeaderByteSize = sizeof(uint32_t);
@@ -57,8 +57,6 @@ bool H264CMSampleBufferToAnnexBBuffer(
     LOG(LS_ERROR) << "Failed to get parameter set.";
     return false;
   }
-  // TODO(tkchin): handle other potential sizes.
-  RTC_DCHECK_EQ(nalu_header_size, 4);

[tkchin_webrtc, 2016/09/27 11:29:35]

why is this removed? Perhaps should be checked that it is equal to
kAvccHeaderByteSize instead?

[kthelgason, 2016/09/28 08:48:27]

That doesn't make sense to me, won't kAvccHeaderByteSize always be 4?
nalu_header_size is supposed to be allowed to be either 3 or 4, right?

[tkchin_webrtc, 2016/09/28 11:33:24]

iiuc nalu_header_size here is not the 0001/001. It is the length byte for avcc
format. We expect it to only 4 bytes (the size of a 32-bit int) but who knows if
what's returned by the CMSampleBuffer changes.

   RTC_DCHECK_EQ(param_set_count, 2u);
 
   // Truncate any previous data in the buffer without changing its capacity.
@@ -140,7 +138,7 @@ bool H264CMSampleBufferToAnnexBBuffer(
     frag_lengths.push_back(packet_size);
     nalu_offset += sizeof(kAnnexBHeaderBytes) + packet_size;
 
-    size_t bytes_written = packet_size + nalu_header_size;
+    size_t bytes_written = packet_size + sizeof(kAnnexBHeaderBytes);

[tkchin_webrtc, 2016/09/27 11:29:35]

good catch

     bytes_remaining -= bytes_written;
     data_ptr += bytes_written;
   }
@@ -312,11 +310,10 @@ CMVideoFormatDescriptionRef CreateVideoFormatDescription(
 
 AnnexBBufferReader::AnnexBBufferReader(const uint8_t* annexb_buffer,
                                        size_t length)
-    : start_(annexb_buffer), offset_(0), next_offset_(0), length_(length) {
+    : start_(annexb_buffer), length_(length) {
   RTC_DCHECK(annexb_buffer);
-  offset_ = FindNextNaluHeader(start_, length_, 0);
-  next_offset_ =
-      FindNextNaluHeader(start_, length_, offset_ + sizeof(kAnnexBHeaderBytes));
+  offsets_ = H264::FindNaluIndices(annexb_buffer, length);

[tkchin_webrtc, 2016/09/27 11:29:34]

How large do we expect these buffers to be? Is it safe to put potentially heavy
work into the ctor like this?

[kthelgason, 2016/09/28 08:48:27]

I have no idea, didn't consider that. In what context would it represent an
issue? The work has to be done at some point.

[tkchin_webrtc, 2016/09/28 11:33:24]

It probably doesn't matter in this particular case. It would only be an issue if
the buffer was extremely large. Just following the general paradigm of not
putting heavy work into ctors.

+  offset_ = offsets_.begin();
 }
 
 bool AnnexBBufferReader::ReadNalu(const uint8_t** out_nalu,
@@ -326,46 +323,20 @@ bool AnnexBBufferReader::ReadNalu(const uint8_t** out_nalu,
   *out_nalu = nullptr;
   *out_length = 0;
 
-  size_t data_offset = offset_ + sizeof(kAnnexBHeaderBytes);
-  if (data_offset > length_) {
+  if (offset_ == offsets_.end()) {
     return false;
   }
-  *out_nalu = start_ + data_offset;
-  *out_length = next_offset_ - data_offset;
-  offset_ = next_offset_;
-  next_offset_ =
-      FindNextNaluHeader(start_, length_, offset_ + sizeof(kAnnexBHeaderBytes));
+  *out_nalu = start_ + offset_->payload_start_offset;
+  *out_length = offset_->payload_size;
+  ++offset_;
   return true;
 }
 
 size_t AnnexBBufferReader::BytesRemaining() const {
-  return length_ - offset_;
-}
-
-size_t AnnexBBufferReader::FindNextNaluHeader(const uint8_t* start,
-                                              size_t length,
-                                              size_t offset) const {
-  RTC_DCHECK(start);
-  if (offset + sizeof(kAnnexBHeaderBytes) > length) {
-    return length;
-  }
-  // NALUs are separated by an 00 00 00 01 header. Scan the byte stream
-  // starting from the offset for the next such sequence.
-  const uint8_t* current = start + offset;
-  // The loop reads sizeof(kAnnexBHeaderBytes) at a time, so stop when there
-  // aren't enough bytes remaining.
-  const uint8_t* const end = start + length - sizeof(kAnnexBHeaderBytes);
-  while (current < end) {
-    if (current[3] > 1) {
-      current += 4;
-    } else if (current[3] == 1 && current[2] == 0 && current[1] == 0 &&
-               current[0] == 0) {
-      return current - start;
-    } else {
-      ++current;
-    }
+  if (offset_ == offsets_.end()) {
+    return 0;
   }
-  return length;
+  return length_ - offset_->start_offset;
 }
 
 AvccBufferWriter::AvccBufferWriter(uint8_t* const avcc_buffer, size_t length)