Fix AnnexB validation logic to work with encrypted content.

AVC::IsValidAnnexB() would fail if encrypted sections happened to
contain something that looked like an Annex B start code. This change
updates the H264Parser so that it will skip byte sequences that look
like start codes in the encrypted sections of the buffer.

BUG=372617
TEST=AVCConversionTests updated to include fake start codes in the
  encrypted sections.

Review URL: https://codereview.chromium.org/379983002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@286373 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 138 insertions, 38 deletions

diff --git a/media/filters/ffmpeg_demuxer_unittest.cc b/media/filters/ffmpeg_demuxer_unittest.cc
index bf72e29..76ccba2 100644
--- a/media/filters/ffmpeg_demuxer_unittest.cc
+++ b/media/filters/ffmpeg_demuxer_unittest.cc
@@ -875,8 +875,14 @@ static void ValidateAnnexB(DemuxerStream* stream,
     return;
   }
 
+  std::vector<SubsampleEntry> subsamples;
+
+  if (buffer->decrypt_config())
+    subsamples = buffer->decrypt_config()->subsamples();
+
   bool is_valid =
-      mp4::AVC::IsValidAnnexB(buffer->data(), buffer->data_size());
+      mp4::AVC::IsValidAnnexB(buffer->data(), buffer->data_size(),
+                              subsamples);
   EXPECT_TRUE(is_valid);
 
   if (!is_valid) {
diff --git a/media/filters/h264_parser.cc b/media/filters/h264_parser.cc
index ee21ab8..fd5646d 100644
--- a/media/filters/h264_parser.cc
+++ b/media/filters/h264_parser.cc
@@ -6,6 +6,7 @@
 #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 {
@@ -129,14 +130,33 @@ H264Parser::~H264Parser() {
 void H264Parser::Reset() {
   stream_ = NULL;
   bytes_left_ = 0;
+  encrypted_ranges_.clear();
 }
 
 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;
+
+  encrypted_ranges_.clear();
+  const uint8* start = stream;
+  const uint8* stream_end = stream_ + bytes_left_;
+  for (size_t i = 0; i < subsamples.size() && start < stream_end; ++i) {
+    start += subsamples[i].clear_bytes;
+
+    const uint8* end = std::min(start + subsamples[i].cypher_bytes, stream_end);
+    encrypted_ranges_.Add(start, end);
+    start = end;
+  }
 }
 
 const H264PPS* H264Parser::GetPPS(int pps_id) {
@@ -191,8 +211,9 @@ 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;
   }
@@ -216,8 +237,9 @@ bool H264Parser::LocateNALU(off_t* nalu_size, off_t* start_code_size) {
   // 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;
@@ -225,6 +247,42 @@ bool H264Parser::LocateNALU(off_t* nalu_size, off_t* start_code_size) {
   return true;
 }
 
+bool H264Parser::FindStartCodeInClearRanges(
+    const uint8* data,
+    off_t data_size,
+    off_t* offset,
+    off_t* start_code_size) {
+  if (encrypted_ranges_.size() == 0)
+    return FindStartCode(data, data_size, offset, start_code_size);
+
+  DCHECK_GE(data_size, 0);
+  const uint8* start = data;
+  do {
+    off_t bytes_left = data_size - (start - data);
+
+    if (!FindStartCode(start, bytes_left, offset, start_code_size))
+      return false;
+
+    // Construct a Ranges object that represents the region occupied
+    // by the start code and the 1 byte needed to read the NAL unit type.
+    const uint8* start_code = start + *offset;
+    const uint8* start_code_end = start_code + *start_code_size;
+    Ranges<const uint8*> start_code_range;
+    start_code_range.Add(start_code, start_code_end + 1);
+
+    if (encrypted_ranges_.IntersectionWith(start_code_range).size() > 0) {
+      // The start code is inside an encrypted section so we need to scan
+      // for another start code.
+      *start_code_size = 0;
+      start += std::min(*offset + 1, bytes_left);
+    }
+  } while (*start_code_size == 0);
+
+  // 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;
diff --git a/media/filters/h264_parser.h b/media/filters/h264_parser.h
index 45020af..e248db7 100644
--- a/media/filters/h264_parser.h
+++ b/media/filters/h264_parser.h
@@ -10,13 +10,17 @@
 #include <sys/types.h>
 
 #include <map>
+#include <vector>
 
 #include "base/basictypes.h"
 #include "media/base/media_export.h"
+#include "media/base/ranges.h"
 #include "media/filters/h264_bit_reader.h"
 
 namespace media {
 
+struct SubsampleEntry;
+
 // For explanations of each struct and its members, see H.264 specification
 // at http://www.itu.int/rec/T-REC-H.264.
 struct MEDIA_EXPORT H264NALU {
@@ -337,7 +341,11 @@ class MEDIA_EXPORT H264Parser {
   void Reset();
   // Set current stream pointer to |stream| of |stream_size| in bytes,
   // |stream| owned by caller.
+  // |subsamples| contains information about what parts of |stream| are
+  // encrypted.
   void SetStream(const uint8* stream, off_t stream_size);
+  void SetEncryptedStream(const uint8* stream, off_t stream_size,
+                          const std::vector<SubsampleEntry>& subsamples);
 
   // Read the stream to find the next NALU, identify it and return
   // that information in |*nalu|. This advances the stream to the beginning
@@ -392,6 +400,12 @@ class MEDIA_EXPORT H264Parser {
   // - the size in bytes of the start code is returned in |*start_code_size|.
   bool LocateNALU(off_t* nalu_size, off_t* start_code_size);
 
+  // Wrapper for FindStartCode() that skips over start codes that
+  // may appear inside of |encrypted_ranges_|.
+  // Returns true if a start code was found. Otherwise returns false.
+  bool FindStartCodeInClearRanges(const uint8* data, off_t data_size,
+                                  off_t* offset, off_t* start_code_size);
+
   // Exp-Golomb code parsing as specified in chapter 9.1 of the spec.
   // Read one unsigned exp-Golomb code from the stream and return in |*val|.
   Result ReadUE(int* val);
@@ -441,6 +455,10 @@ class MEDIA_EXPORT H264Parser {
   SPSById active_SPSes_;
   PPSById active_PPSes_;
 
+  // Ranges of encrypted bytes in the buffer passed to
+  // SetEncryptedStream().
+  Ranges<const uint8*> encrypted_ranges_;
+
   DISALLOW_COPY_AND_ASSIGN(H264Parser);
 };
 
diff --git a/media/formats/mp4/avc.cc b/media/formats/mp4/avc.cc
index 6c2bc2a..4a9a880 100644
--- a/media/formats/mp4/avc.cc
+++ b/media/formats/mp4/avc.cc
@@ -76,11 +76,11 @@ bool AVC::ConvertFrameToAnnexB(int length_size, std::vector<uint8>* buffer) {
 bool AVC::InsertParamSetsAnnexB(const AVCDecoderConfigurationRecord& avc_config,
                                 std::vector<uint8>* buffer,
                                 std::vector<SubsampleEntry>* subsamples) {
-  DCHECK(AVC::IsValidAnnexB(*buffer));
+  DCHECK(AVC::IsValidAnnexB(*buffer, *subsamples));
 
   scoped_ptr<H264Parser> parser(new H264Parser());
   const uint8* start = &(*buffer)[0];
-  parser->SetStream(start, buffer->size());
+  parser->SetEncryptedStream(start, buffer->size(), *subsamples);
 
   H264NALU nalu;
   if (parser->AdvanceToNextNALU(&nalu) != H264Parser::kOk)
@@ -126,7 +126,7 @@ bool AVC::InsertParamSetsAnnexB(const AVCDecoderConfigurationRecord& avc_config,
   buffer->insert(config_insert_point,
                  param_sets.begin(), param_sets.end());
 
-  DCHECK(AVC::IsValidAnnexB(*buffer));
+  DCHECK(AVC::IsValidAnnexB(*buffer, *subsamples));
   return true;
 }
 
@@ -171,11 +171,13 @@ bool AVC::ConvertConfigToAnnexB(
 }
 
 // Verifies AnnexB NALU order according to ISO/IEC 14496-10 Section 7.4.1.2.3
-bool AVC::IsValidAnnexB(const std::vector<uint8>& buffer) {
-  return IsValidAnnexB(&buffer[0], buffer.size());
+bool AVC::IsValidAnnexB(const std::vector<uint8>& buffer,
+                        const std::vector<SubsampleEntry>& subsamples) {
+  return IsValidAnnexB(&buffer[0], buffer.size(), subsamples);
 }
 
-bool AVC::IsValidAnnexB(const uint8* buffer, size_t size) {
+bool AVC::IsValidAnnexB(const uint8* buffer, size_t size,
+                        const std::vector<SubsampleEntry>& subsamples) {
   DVLOG(1) << __FUNCTION__;
   DCHECK(buffer);
 
@@ -183,7 +185,7 @@ bool AVC::IsValidAnnexB(const uint8* buffer, size_t size) {
     return true;
 
   H264Parser parser;
-  parser.SetStream(buffer, size);
+  parser.SetEncryptedStream(buffer, size, subsamples);
 
   typedef enum {
     kAUDAllowed,
@@ -305,6 +307,5 @@ bool AVC::IsValidAnnexB(const uint8* buffer, size_t size) {
 
   return order_state >= kAfterFirstVCL;
 }
-
 }  // namespace mp4
 }  // namespace media
diff --git a/media/formats/mp4/avc.h b/media/formats/mp4/avc.h
index 0d84eef..1740b8f 100644
--- a/media/formats/mp4/avc.h
+++ b/media/formats/mp4/avc.h
@@ -39,11 +39,15 @@ class MEDIA_EXPORT AVC {
 
   // Verifies that the contents of |buffer| conform to
   // Section 7.4.1.2.3 of ISO/IEC 14496-10.
+  // |subsamples| contains the information about what parts of the buffer are
+  // encrypted and which parts are clear.
   // Returns true if |buffer| contains conformant Annex B data
   // TODO(acolwell): Remove the std::vector version when we can use,
   // C++11's std::vector<T>::data() method.
-  static bool IsValidAnnexB(const std::vector<uint8>& buffer);
-  static bool IsValidAnnexB(const uint8* buffer, size_t size);
+  static bool IsValidAnnexB(const std::vector<uint8>& buffer,
+                            const std::vector<SubsampleEntry>& subsamples);
+  static bool IsValidAnnexB(const uint8* buffer, size_t size,
+                            const std::vector<SubsampleEntry>& subsamples);
 };
 
 }  // namespace mp4
diff --git a/media/formats/mp4/avc_unittest.cc b/media/formats/mp4/avc_unittest.cc
index d0ddc66..96a8a97 100644
--- a/media/formats/mp4/avc_unittest.cc
+++ b/media/formats/mp4/avc_unittest.cc
@@ -110,6 +110,15 @@ static std::string NALUTypeToString(int type) {
   return "UnsupportedType";
 }
 
+static void WriteStartCodeAndNALUType(std::vector<uint8>* buffer,
+                                      const std::string& nal_unit_type) {
+  buffer->push_back(0x00);
+  buffer->push_back(0x00);
+  buffer->push_back(0x00);
+  buffer->push_back(0x01);
+  buffer->push_back(StringToNALUType(nal_unit_type));
+}
+
 void StringToAnnexB(const std::string& str, std::vector<uint8>* buffer,
                     std::vector<SubsampleEntry>* subsamples) {
   DCHECK(!str.empty());
@@ -122,14 +131,7 @@ void StringToAnnexB(const std::string& str, std::vector<uint8>* buffer,
     SubsampleEntry entry;
     size_t start = buffer->size();
 
-    // Write the start code.
-    buffer->push_back(0x00);
-    buffer->push_back(0x00);
-    buffer->push_back(0x00);
-    buffer->push_back(0x01);
-
-    // Write NALU type.
-    buffer->push_back(StringToNALUType(tokens[i]));
+    WriteStartCodeAndNALUType(buffer, tokens[i]);
 
     entry.clear_bytes = buffer->size() - start;
 
@@ -139,6 +141,12 @@ void StringToAnnexB(const std::string& str, std::vector<uint8>* buffer,
     buffer->push_back(0x12);
     buffer->push_back(0x67);
 
+    if (subsamples) {
+      // Simulate the encrypted bits containing something that looks
+      // like a SPS NALU.
+      WriteStartCodeAndNALUType(buffer, "SPS");
+    }
+
     entry.cypher_bytes = buffer->size() - start - entry.clear_bytes;
 
     if (subsamples) {
@@ -147,11 +155,12 @@ void StringToAnnexB(const std::string& str, std::vector<uint8>* buffer,
   }
 }
 
-std::string AnnexBToString(const std::vector<uint8>& buffer) {
+std::string AnnexBToString(const std::vector<uint8>& buffer,
+                           const std::vector<SubsampleEntry>& subsamples) {
   std::stringstream ss;
 
   H264Parser parser;
-  parser.SetStream(&buffer[0], buffer.size());
+  parser.SetEncryptedStream(&buffer[0], buffer.size(), subsamples);
 
   H264NALU nalu;
   bool first = true;
@@ -191,12 +200,13 @@ class AVCConversionTest : public testing::TestWithParam<int> {
 
 TEST_P(AVCConversionTest, ParseCorrectly) {
   std::vector<uint8> buf;
+  std::vector<SubsampleEntry> subsamples;
   MakeInputForLength(GetParam(), &buf);
   EXPECT_TRUE(AVC::ConvertFrameToAnnexB(GetParam(), &buf));
-  EXPECT_TRUE(AVC::IsValidAnnexB(buf));
+  EXPECT_TRUE(AVC::IsValidAnnexB(buf, subsamples));
   EXPECT_EQ(buf.size(), sizeof(kExpected));
   EXPECT_EQ(0, memcmp(kExpected, &buf[0], sizeof(kExpected)));
-  EXPECT_EQ("P SDC", AnnexBToString(buf));
+  EXPECT_EQ("P SDC", AnnexBToString(buf, subsamples));
 }
 
 // Intentionally write NALU sizes that are larger than the buffer.
@@ -263,7 +273,7 @@ TEST_F(AVCConversionTest, ConvertConfigToAnnexB) {
   EXPECT_TRUE(AVC::ConvertConfigToAnnexB(avc_config, &buf, &subsamples));
   EXPECT_EQ(0, memcmp(kExpectedParamSets, &buf[0],
                       sizeof(kExpectedParamSets)));
-  EXPECT_EQ("SPS SPS PPS", AnnexBToString(buf));
+  EXPECT_EQ("SPS SPS PPS", AnnexBToString(buf, subsamples));
 }
 
 // Verify that we can round trip string -> Annex B -> string.
@@ -271,11 +281,11 @@ TEST_F(AVCConversionTest, StringConversionFunctions) {
   std::string str =
       "AUD SPS SPSExt SPS PPS SEI SEI R14 I P FILL EOSeq EOStr";
   std::vector<uint8> buf;
-  StringToAnnexB(str, &buf, NULL);
-
-  EXPECT_TRUE(AVC::IsValidAnnexB(buf));
+  std::vector<SubsampleEntry> subsamples;
+  StringToAnnexB(str, &buf, &subsamples);
+  EXPECT_TRUE(AVC::IsValidAnnexB(buf, subsamples));
 
-  EXPECT_EQ(str, AnnexBToString(buf));
+  EXPECT_EQ(str, AnnexBToString(buf, subsamples));
 }
 
 TEST_F(AVCConversionTest, ValidAnnexBConstructs) {
@@ -298,8 +308,10 @@ TEST_F(AVCConversionTest, ValidAnnexBConstructs) {
 
   for (size_t i = 0; i < arraysize(test_cases); ++i) {
     std::vector<uint8> buf;
+    std::vector<SubsampleEntry> subsamples;
     StringToAnnexB(test_cases[i], &buf, NULL);
-    EXPECT_TRUE(AVC::IsValidAnnexB(buf)) << "'" << test_cases[i] << "' failed";
+    EXPECT_TRUE(AVC::IsValidAnnexB(buf, subsamples)) << "'" << test_cases[i]
+                                                     << "' failed";
   }
 }
 
@@ -320,8 +332,10 @@ TEST_F(AVCConversionTest, InvalidAnnexBConstructs) {
 
   for (size_t i = 0; i < arraysize(test_cases); ++i) {
     std::vector<uint8> buf;
+    std::vector<SubsampleEntry> subsamples;
     StringToAnnexB(test_cases[i], &buf, NULL);
-    EXPECT_FALSE(AVC::IsValidAnnexB(buf)) << "'" << test_cases[i] << "' failed";
+    EXPECT_FALSE(AVC::IsValidAnnexB(buf, subsamples)) << "'" << test_cases[i]
+                                                      << "' failed";
   }
 }
 
@@ -361,9 +375,9 @@ TEST_F(AVCConversionTest, InsertParamSetsAnnexB) {
 
     EXPECT_TRUE(AVC::InsertParamSetsAnnexB(avc_config, &buf, &subsamples))
         << "'" << test_cases[i].input << "' insert failed.";
-    EXPECT_TRUE(AVC::IsValidAnnexB(buf))
+    EXPECT_TRUE(AVC::IsValidAnnexB(buf, subsamples))
         << "'" << test_cases[i].input << "' created invalid AnnexB.";
-    EXPECT_EQ(test_cases[i].expected, AnnexBToString(buf))
+    EXPECT_EQ(test_cases[i].expected, AnnexBToString(buf, subsamples))
         << "'" << test_cases[i].input << "' generated unexpected output.";
   }
 }
diff --git a/media/formats/mp4/mp4_stream_parser.cc b/media/formats/mp4/mp4_stream_parser.cc
index 8ec925c..7ad3ccc 100644
--- a/media/formats/mp4/mp4_stream_parser.cc
+++ b/media/formats/mp4/mp4_stream_parser.cc
@@ -383,8 +383,7 @@ bool MP4StreamParser::PrepareAVCBuffer(
     RCHECK(AVC::InsertParamSetsAnnexB(avc_config, frame_buf, subsamples));
   }
 
-  // TODO(acolwell): Improve IsValidAnnexB() so it can handle encrypted content.
-  DCHECK(runs_->is_encrypted() || AVC::IsValidAnnexB(*frame_buf));
+  DCHECK(AVC::IsValidAnnexB(*frame_buf, *subsamples));
   return true;
 }