Fix AnnexB validation logic to work with encrypted content.

media/filters/h264_parser.cc

 // Copyright 2014 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 "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/stl_util.h"
 
+#include "media/base/decrypt_config.h"
 #include "media/filters/h264_parser.h"
 
 namespace media {
 
 bool H264SliceHeader::IsPSlice() const {
   return (slice_type % 5 == kPSlice);
 }
 
 bool H264SliceHeader::IsBSlice() const {
   return (slice_type % 5 == kBSlice);
@@ skipping 103 matching lines @@
 }
 
 H264Parser::~H264Parser() {
   STLDeleteValues(&active_SPSes_);
   STLDeleteValues(&active_PPSes_);
 }
 
 void H264Parser::Reset() {
   stream_ = NULL;
   bytes_left_ = 0;
+  clear_ranges_.clear();
+  clear_range_index_ = 0;
 }
 
 void H264Parser::SetStream(const uint8* stream, off_t stream_size) {
+  std::vector<SubsampleEntry> subsamples;
+  SetEncryptedStream(stream, stream_size, subsamples);
+}
+
+void H264Parser::SetEncryptedStream(
+    const uint8* stream, off_t stream_size,
+    const std::vector<SubsampleEntry>& subsamples) {
   DCHECK(stream);
   DCHECK_GT(stream_size, 0);
 
   stream_ = stream;
   bytes_left_ = stream_size;
+
+  clear_ranges_.clear();
+  clear_range_index_ = 0;
+  const uint8* start = stream;
+  const uint8* stream_end = stream_ + bytes_left_;
+  for (size_t i = 0; i < subsamples.size() && start < stream_end; ++i) {
+    const uint8* end = std::min(start + subsamples[i].clear_bytes, stream_end);

[xhwang, 2014/07/16 05:06:13]

It seems possible that
start + subsamples[i].clear_bytes > stream_end
?
Is that caused by appending partial buffer? In that case, how will the remaining
encrypted bytes be excluded? Is this what the clear_range_index_ is used for?

[acolwell GONE FROM CHROMIUM, 2014/07/28 19:43:12]

This is to protect this code from invalid subsample data in the file.
Part of this comment no longer applies. Basically we only keep information for
regions that actually exist inside the buffer. Anything outside of that doesn't
really matter since it would result in an invalid memory access if it was
allowed to occur.

+    clear_ranges_.Add(start, end);
+    start += subsamples[i].clear_bytes + subsamples[i].cypher_bytes;
+  }
 }
 
 const H264PPS* H264Parser::GetPPS(int pps_id) {
   return active_PPSes_[pps_id];
 }
 
 const H264SPS* H264Parser::GetSPS(int sps_id) {
   return active_SPSes_[sps_id];
 }
 
@@ skipping 34 matching lines @@
   // |*offset| is equal to 0 (valid offset).
   *offset = data_size - bytes_left;
   *start_code_size = 0;
   return false;
 }
 
 bool H264Parser::LocateNALU(off_t* nalu_size, off_t* start_code_size) {
   // Find the start code of next NALU.
   off_t nalu_start_off = 0;
   off_t annexb_start_code_size = 0;
-  if (!FindStartCode(stream_, bytes_left_,
-                     &nalu_start_off, &annexb_start_code_size)) {
+
+  if (!FindStartCodeInClearRanges(stream_, bytes_left_,
+                                  &nalu_start_off, &annexb_start_code_size)) {
     DVLOG(4) << "Could not find start code, end of stream?";
     return false;
   }
 
   // Move the stream to the beginning of the NALU (pointing at the start code).
   stream_ += nalu_start_off;
   bytes_left_ -= nalu_start_off;
 
   const uint8* nalu_data = stream_ + annexb_start_code_size;
   off_t max_nalu_data_size = bytes_left_ - annexb_start_code_size;
   if (max_nalu_data_size <= 0) {
     DVLOG(3) << "End of stream";
     return false;
   }
 
   // Find the start code of next NALU;
   // if successful, |nalu_size_without_start_code| is the number of bytes from
   // after previous start code to before this one;
   // if next start code is not found, it is still a valid NALU since there
   // are some bytes left after the first start code: all the remaining bytes
   // belong to the current NALU.
   off_t next_start_code_size = 0;
   off_t nalu_size_without_start_code = 0;
-  if (!FindStartCode(nalu_data, max_nalu_data_size,
-                     &nalu_size_without_start_code, &next_start_code_size)) {
+  if (!FindStartCodeInClearRanges(nalu_data, max_nalu_data_size,
+                                  &nalu_size_without_start_code,
+                                  &next_start_code_size)) {
     nalu_size_without_start_code = max_nalu_data_size;
   }
   *nalu_size = nalu_size_without_start_code + annexb_start_code_size;
   *start_code_size = annexb_start_code_size;
   return true;
 }
 
+bool H264Parser::FindStartCodeInClearRanges(

[damienv1, 2014/07/16 14:33:17]

The likelihood of a start code emulation (although it can exist) is pretty low.
So why not having a linear search (just like FindStartCode) and when we get a
start code, we could have a small utility function which checks given the offset
of the start code whether the start code overlaps with some encrypted content
(using the subsamples vector, no need to have additional clear ranges).

I agree that if there was a fake start codes, this would increase the complexity
but in this case, it seems that code simplicity takes precedence.

[acolwell GONE FROM CHROMIUM, 2014/07/28 19:43:12]

I've restored the code to what I had in Patch Set 1. This is a linear scan like
you suggested and I think is easier to follow.

+    const uint8* data,
+    off_t data_size,
+    off_t* offset,
+    off_t* start_code_size) {
+  if (clear_ranges_.size() == 0)
+    return FindStartCode(data, data_size, offset, start_code_size);

[xhwang, 2014/07/16 05:06:13]

If there's no clear ranges, we don't need to search?

[acolwell GONE FROM CHROMIUM, 2014/07/28 19:43:12]

Yes. A lack of clear ranges, or now encrypted ranges means that none of the data
is encrypted. Now that this is encrypted_ranges_ again, I think that is a little
more intuitive.

+
+  if (clear_range_index_ >= clear_ranges_.size())
+    return false;
+
+  DCHECK_GE(data_size, 0);
+  const uint8* start = data;
+  const uint8* end = data + data_size;
+  while (start < end) {
+    while (start >= clear_ranges_.end(clear_range_index_)) {
+      clear_range_index_++;
+      if (clear_range_index_ >= clear_ranges_.size())
+        return false;
+      if (start < clear_ranges_.start(clear_range_index_))
+        start = clear_ranges_.start(clear_range_index_);
+    }

[xhwang, 2014/07/16 05:06:13]

This seems complicated. Can you add some comments explaining the basics how it
works?

[acolwell GONE FROM CHROMIUM, 2014/07/28 19:43:12]

Code is no longer here anymore.

+
+    DCHECK_LT(start, end);
+    DCHECK_GE(start, clear_ranges_.start(clear_range_index_));
+    DCHECK_LT(start, clear_ranges_.end(clear_range_index_));
+    off_t clear_bytes_left =
+        clear_ranges_.end(clear_range_index_) - start;
+    off_t bytes_left = std::min(end - start, clear_bytes_left);
+
+    if (FindStartCode(start, bytes_left, offset, start_code_size))
+      break;
+
+    start += bytes_left;
+  }
+
+  if (start >= end)
+    return false;
+
+  // Update |*offset| to include the data we skipped over.
+  *offset += start - data;
+  return true;
+}
+
 H264Parser::Result H264Parser::ReadUE(int* val) {
   int num_bits = -1;
   int bit;
   int rest;
 
   // Count the number of contiguous zero bits.
   do {
     READ_BITS_OR_RETURN(1, &bit);
     num_bits++;
   } while (bit == 0);
@@ skipping 1013 matching lines @@
 
     default:
       DVLOG(4) << "Unsupported SEI message";
       break;
   }
 
   return kOk;
 }
 
 }  // namespace media