Fix SPS/PPS insertion logic in MP4StreamParser.

The code that inserts SPS & PPS NAL units into keyframes wasn't properly
handling frames that start with AUD NAL units. This patch adds code to
make sure the configuration data is inserted after the AUD NAL units as
the H.264 spec mandates.

I've also added AVC::IsValidAnnexB() DCHECKS to verify that our
code generates proper Annex B data that conforms to the NAL unit
ordering rules outlined in the spec. These DCHECKS caught the old bad
behavior and should help prevent future regressions.

BUG=364925
TEST=AVCConversionTest.NALUSizeTooLarge, AVCConversionTest.NALUSizeIsZero, AVCConversionTest.ValidAnnexBConstructs, AVCConversionTest.InvalidAnnexBConstructs, AVCConversionTest.InsertParamSetsAnnexB

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

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

5 files changed, 530 insertions, 16 deletions

diff --git a/media/formats/mp4/avc.cc b/media/formats/mp4/avc.cc
index 6b670fe..ed7e5aa 100644
--- a/media/formats/mp4/avc.cc
+++ b/media/formats/mp4/avc.cc
@@ -7,6 +7,9 @@
 #include <algorithm>
 #include <vector>
 
+#include "base/logging.h"
+#include "media/base/decrypt_config.h"
+#include "media/filters/h264_parser.h"
 #include "media/formats/mp4/box_definitions.h"
 #include "media/formats/mp4/box_reader.h"
 
@@ -20,10 +23,16 @@ static bool ConvertAVCToAnnexBInPlaceForLengthSize4(std::vector<uint8>* buf) {
   const int kLengthSize = 4;
   size_t pos = 0;
   while (pos + kLengthSize < buf->size()) {
-    int nal_size = (*buf)[pos];
+    uint32 nal_size = (*buf)[pos];
     nal_size = (nal_size << 8) + (*buf)[pos+1];
     nal_size = (nal_size << 8) + (*buf)[pos+2];
     nal_size = (nal_size << 8) + (*buf)[pos+3];
+
+    if (nal_size == 0) {
+      DVLOG(1) << "nal_size is 0";
+      return false;
+    }
+
     std::copy(kAnnexBStartCode, kAnnexBStartCode + kAnnexBStartCodeSize,
               buf->begin() + pos);
     pos += kLengthSize + nal_size;
@@ -48,6 +57,11 @@ bool AVC::ConvertFrameToAnnexB(int length_size, std::vector<uint8>* buffer) {
     if (length_size == 2) nal_size = (nal_size << 8) + temp[pos+1];
     pos += length_size;
 
+    if (nal_size == 0) {
+      DVLOG(1) << "nal_size is 0";
+      return false;
+    }
+
     RCHECK(pos + nal_size <= temp.size());
     buffer->insert(buffer->end(), kAnnexBStartCode,
                    kAnnexBStartCode + kAnnexBStartCodeSize);
@@ -59,9 +73,68 @@ bool AVC::ConvertFrameToAnnexB(int length_size, std::vector<uint8>* buffer) {
 }
 
 // static
+bool AVC::InsertParamSetsAnnexB(const AVCDecoderConfigurationRecord& avc_config,
+                                std::vector<uint8>* buffer,
+                                std::vector<SubsampleEntry>* subsamples) {
+  DCHECK(AVC::IsValidAnnexB(*buffer));
+
+  scoped_ptr<H264Parser> parser(new H264Parser());
+  const uint8* start = &(*buffer)[0];
+  parser->SetStream(start, buffer->size());
+
+  H264NALU nalu;
+  if (parser->AdvanceToNextNALU(&nalu) != H264Parser::kOk)
+    return false;
+
+  std::vector<uint8>::iterator config_insert_point = buffer->begin();
+  std::vector<SubsampleEntry>::iterator subsamples_insert_point =
+      subsamples->begin();
+
+  if (nalu.nal_unit_type == H264NALU::kAUD) {
+    // Move insert point to just after the AUD.
+    config_insert_point += (nalu.data + nalu.size) - start;
+
+    if (!subsamples->empty()) {
+      int64 first_subsample_size =
+          (*subsamples)[0].clear_bytes + (*subsamples)[0].cypher_bytes;
+
+      if (first_subsample_size != (config_insert_point - buffer->begin()))
+        return false;
+
+      subsamples_insert_point++;
+    }
+
+  }
+
+  // Clear |parser| and |start| since they aren't needed anymore and
+  // will hold stale pointers once the insert happens.
+  parser.reset();
+  start = NULL;
+
+  std::vector<uint8> param_sets;
+  std::vector<SubsampleEntry> config_subsamples;
+  RCHECK(AVC::ConvertConfigToAnnexB(avc_config,
+                                    &param_sets,
+                                    &config_subsamples));
+
+  if (!subsamples->empty()) {
+    subsamples->insert(subsamples_insert_point,
+                       config_subsamples.begin(),
+                       config_subsamples.end());
+  }
+
+  buffer->insert(config_insert_point,
+                 param_sets.begin(), param_sets.end());
+
+  DCHECK(AVC::IsValidAnnexB(*buffer));
+  return true;
+}
+
+// static
 bool AVC::ConvertConfigToAnnexB(
     const AVCDecoderConfigurationRecord& avc_config,
-    std::vector<uint8>* buffer) {
+    std::vector<uint8>* buffer,
+    std::vector<SubsampleEntry>* subsamples) {
   DCHECK(buffer->empty());
   buffer->clear();
   int total_size = 0;
@@ -76,6 +149,11 @@ bool AVC::ConvertConfigToAnnexB(
                 kAnnexBStartCode + kAnnexBStartCodeSize);
     buffer->insert(buffer->end(), avc_config.sps_list[i].begin(),
                 avc_config.sps_list[i].end());
+
+    SubsampleEntry entry;
+    entry.clear_bytes = kAnnexBStartCodeSize + avc_config.sps_list[i].size();
+    entry.cypher_bytes = 0;
+    subsamples->push_back(entry);
   }
 
   for (size_t i = 0; i < avc_config.pps_list.size(); i++) {
@@ -83,9 +161,145 @@ bool AVC::ConvertConfigToAnnexB(
                    kAnnexBStartCode + kAnnexBStartCodeSize);
     buffer->insert(buffer->end(), avc_config.pps_list[i].begin(),
                    avc_config.pps_list[i].end());
+
+    SubsampleEntry entry;
+    entry.clear_bytes = kAnnexBStartCodeSize + avc_config.pps_list[i].size();
+    entry.cypher_bytes = 0;
+    subsamples->push_back(entry);
   }
   return true;
 }
 
+// Verifies AnnexB NALU order according to ISO/IEC 14496-10 Section 7.4.1.2.3
+bool AVC::IsValidAnnexB(const std::vector<uint8>& buffer) {
+  DVLOG(1) << __FUNCTION__;
+
+  if (buffer.empty())
+    return true;
+
+  H264Parser parser;
+  parser.SetStream(&buffer[0], buffer.size());
+
+  typedef enum {
+    kAUDAllowed,
+    kBeforeFirstVCL,  // VCL == nal_unit_types 1-5
+    kAfterFirstVCL,
+    kEOStreamAllowed,
+    kNoMoreDataAllowed,
+  } NALUOrderState;
+
+  H264NALU nalu;
+  NALUOrderState order_state = kAUDAllowed;
+  int last_nalu_type = H264NALU::kUnspecified;
+  bool done = false;
+  while (!done) {
+    switch (parser.AdvanceToNextNALU(&nalu)) {
+      case H264Parser::kOk:
+        DVLOG(1) << "nal_unit_type " << nalu.nal_unit_type;
+
+        switch (nalu.nal_unit_type) {
+          case H264NALU::kAUD:
+            if (order_state > kAUDAllowed) {
+              DVLOG(1) << "Unexpected AUD in order_state " << order_state;
+              return false;
+            }
+            order_state = kBeforeFirstVCL;
+            break;
+
+          case H264NALU::kSEIMessage:
+          case H264NALU::kReserved14:
+          case H264NALU::kReserved15:
+          case H264NALU::kReserved16:
+          case H264NALU::kReserved17:
+          case H264NALU::kReserved18:
+          case H264NALU::kPPS:
+          case H264NALU::kSPS:
+            if (order_state > kBeforeFirstVCL) {
+              DVLOG(1) << "Unexpected NALU type " << nalu.nal_unit_type
+                       << " in order_state " << order_state;
+              return false;
+            }
+            order_state = kBeforeFirstVCL;
+            break;
+
+          case H264NALU::kSPSExt:
+            if (last_nalu_type != H264NALU::kSPS) {
+              DVLOG(1) << "SPS extension does not follow an SPS.";
+              return false;
+            }
+            break;
+
+          case H264NALU::kNonIDRSlice:
+          case H264NALU::kSliceDataA:
+          case H264NALU::kSliceDataB:
+          case H264NALU::kSliceDataC:
+          case H264NALU::kIDRSlice:
+            if (order_state > kAfterFirstVCL) {
+              DVLOG(1) << "Unexpected VCL in order_state " << order_state;
+              return false;
+            }
+            order_state = kAfterFirstVCL;
+            break;
+
+          case H264NALU::kCodedSliceAux:
+            if (order_state != kAfterFirstVCL) {
+              DVLOG(1) << "Unexpected extension in order_state " << order_state;
+              return false;
+            }
+            break;
+
+          case H264NALU::kEOSeq:
+            if (order_state != kAfterFirstVCL) {
+              DVLOG(1) << "Unexpected EOSeq in order_state " << order_state;
+              return false;
+            }
+            order_state = kEOStreamAllowed;
+            break;
+
+          case H264NALU::kEOStream:
+            if (order_state < kAfterFirstVCL) {
+              DVLOG(1) << "Unexpected EOStream in order_state " << order_state;
+              return false;
+            }
+            order_state = kNoMoreDataAllowed;
+            break;
+
+          case H264NALU::kFiller:
+          case H264NALU::kUnspecified:
+            if (!(order_state >= kAfterFirstVCL &&
+                  order_state < kEOStreamAllowed)) {
+              DVLOG(1) << "Unexpected NALU type " << nalu.nal_unit_type
+                       << " in order_state " << order_state;
+              return false;
+            }
+            break;
+
+          default:
+            DCHECK_GE(nalu.nal_unit_type, 20);
+            if (nalu.nal_unit_type >= 20 && nalu.nal_unit_type <= 31 &&
+                order_state != kAfterFirstVCL) {
+              DVLOG(1) << "Unexpected NALU type " << nalu.nal_unit_type
+                       << " in order_state " << order_state;
+              return false;
+            }
+        }
+        last_nalu_type = nalu.nal_unit_type;
+        break;
+
+      case H264Parser::kInvalidStream:
+        return false;
+
+      case H264Parser::kUnsupportedStream:
+        NOTREACHED() << "AdvanceToNextNALU() returned kUnsupportedStream!";
+        return false;
+
+      case H264Parser::kEOStream:
+        done = true;
+    }
+  }
+
+  return order_state >= kAfterFirstVCL;
+}
+
 }  // namespace mp4
 }  // namespace media
diff --git a/media/formats/mp4/avc.h b/media/formats/mp4/avc.h
index 731e401..8155883 100644
--- a/media/formats/mp4/avc.h
+++ b/media/formats/mp4/avc.h
@@ -11,6 +11,9 @@
 #include "media/base/media_export.h"
 
 namespace media {
+
+struct SubsampleEntry;
+
 namespace mp4 {
 
 struct AVCDecoderConfigurationRecord;
@@ -19,9 +22,25 @@ class MEDIA_EXPORT AVC {
  public:
   static bool ConvertFrameToAnnexB(int length_size, std::vector<uint8>* buffer);
 
+  // Inserts the SPS & PPS data from |avc_config| into |buffer|.
+  // |buffer| is expected to contain AnnexB conformant data.
+  // |subsamples| contains the SubsampleEntry info if |buffer| contains
+  // encrypted data.
+  // Returns true if the param sets were successfully inserted.
+  static bool InsertParamSetsAnnexB(
+      const AVCDecoderConfigurationRecord& avc_config,
+      std::vector<uint8>* buffer,
+      std::vector<SubsampleEntry>* subsamples);
+
   static bool ConvertConfigToAnnexB(
       const AVCDecoderConfigurationRecord& avc_config,
-      std::vector<uint8>* buffer);
+      std::vector<uint8>* buffer,
+      std::vector<SubsampleEntry>* subsamples);
+
+  // Verifies that the contents of |buffer| conform to
+  // Section 7.4.1.2.3 of ISO/IEC 14496-10.
+  // Returns true if |buffer| contains conformant AnnexB data.
+  static bool IsValidAnnexB(const std::vector<uint8>& buffer);
 };
 
 }  // namespace mp4
diff --git a/media/formats/mp4/avc_unittest.cc b/media/formats/mp4/avc_unittest.cc
index f6a1d569..d0ddc66 100644
--- a/media/formats/mp4/avc_unittest.cc
+++ b/media/formats/mp4/avc_unittest.cc
@@ -5,7 +5,10 @@
 #include <string.h>
 
 #include "base/basictypes.h"
+#include "base/strings/string_util.h"
+#include "media/base/decrypt_config.h"
 #include "media/base/stream_parser_buffer.h"
+#include "media/filters/h264_parser.h"
 #include "media/formats/mp4/avc.h"
 #include "media/formats/mp4/box_definitions.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -24,28 +27,199 @@ static const uint8 kExpectedParamSets[] = {
   0x00, 0x00, 0x00, 0x01, 0x67, 0x34,
   0x00, 0x00, 0x00, 0x01, 0x68, 0x56, 0x78};
 
+static H264NALU::Type StringToNALUType(const std::string& name) {
+  if (name == "P")
+    return H264NALU::kNonIDRSlice;
+
+  if (name == "I")
+    return H264NALU::kIDRSlice;
+
+  if (name == "SEI")
+    return H264NALU::kSEIMessage;
+
+  if (name == "SPS")
+    return H264NALU::kSPS;
+
+  if (name == "SPSExt")
+    return H264NALU::kSPSExt;
+
+  if (name == "PPS")
+    return H264NALU::kPPS;
+
+  if (name == "AUD")
+    return H264NALU::kAUD;
+
+  if (name == "EOSeq")
+    return H264NALU::kEOSeq;
+
+  if (name == "EOStr")
+    return H264NALU::kEOStream;
+
+  if (name == "FILL")
+    return H264NALU::kFiller;
+
+  if (name == "R14")
+    return H264NALU::kReserved14;
+
+  CHECK(false) << "Unexpected name: " << name;
+  return H264NALU::kUnspecified;
+}
+
+static std::string NALUTypeToString(int type) {
+  switch (type) {
+    case H264NALU::kNonIDRSlice:
+      return "P";
+    case H264NALU::kSliceDataA:
+      return "SDA";
+    case H264NALU::kSliceDataB:
+      return "SDB";
+    case H264NALU::kSliceDataC:
+      return "SDC";
+    case H264NALU::kIDRSlice:
+      return "I";
+    case H264NALU::kSEIMessage:
+      return "SEI";
+    case H264NALU::kSPS:
+      return "SPS";
+    case H264NALU::kSPSExt:
+      return "SPSExt";
+    case H264NALU::kPPS:
+      return "PPS";
+    case H264NALU::kAUD:
+      return "AUD";
+    case H264NALU::kEOSeq:
+      return "EOSeq";
+    case H264NALU::kEOStream:
+      return "EOStr";
+    case H264NALU::kFiller:
+      return "FILL";
+    case H264NALU::kReserved14:
+      return "R14";
+
+    case H264NALU::kUnspecified:
+    case H264NALU::kReserved15:
+    case H264NALU::kReserved16:
+    case H264NALU::kReserved17:
+    case H264NALU::kReserved18:
+    case H264NALU::kCodedSliceAux:
+    case H264NALU::kCodedSliceExtension:
+      CHECK(false) << "Unexpected type: " << type;
+      break;
+  };
+
+  return "UnsupportedType";
+}
+
+void StringToAnnexB(const std::string& str, std::vector<uint8>* buffer,
+                    std::vector<SubsampleEntry>* subsamples) {
+  DCHECK(!str.empty());
+
+  std::vector<std::string> tokens;
+  EXPECT_GT(Tokenize(str, " ", &tokens), 0u);
+
+  buffer->clear();
+  for (size_t i = 0; i < tokens.size(); ++i) {
+    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]));
+
+    entry.clear_bytes = buffer->size() - start;
+
+    // Write junk for the payload since the current code doesn't
+    // actually look at it.
+    buffer->push_back(0x32);
+    buffer->push_back(0x12);
+    buffer->push_back(0x67);
+
+    entry.cypher_bytes = buffer->size() - start - entry.clear_bytes;
+
+    if (subsamples) {
+      subsamples->push_back(entry);
+    }
+  }
+}
+
+std::string AnnexBToString(const std::vector<uint8>& buffer) {
+  std::stringstream ss;
+
+  H264Parser parser;
+  parser.SetStream(&buffer[0], buffer.size());
+
+  H264NALU nalu;
+  bool first = true;
+  while (parser.AdvanceToNextNALU(&nalu) == H264Parser::kOk) {
+    if (!first)
+      ss << " ";
+    else
+      first = false;
+
+    ss << NALUTypeToString(nalu.nal_unit_type);
+  }
+  return ss.str();
+}
+
 class AVCConversionTest : public testing::TestWithParam<int> {
  protected:
-  void MakeInputForLength(int length_size, std::vector<uint8>* buf) {
-    buf->clear();
+  void WriteLength(int length_size, int length, std::vector<uint8>* buf) {
+    DCHECK_GE(length, 0);
+    DCHECK_LE(length, 255);
+
     for (int i = 1; i < length_size; i++)
       buf->push_back(0);
-    buf->push_back(sizeof(kNALU1));
+    buf->push_back(length);
+  }
+
+  void MakeInputForLength(int length_size, std::vector<uint8>* buf) {
+    buf->clear();
+
+    WriteLength(length_size, sizeof(kNALU1), buf);
     buf->insert(buf->end(), kNALU1, kNALU1 + sizeof(kNALU1));
 
-    for (int i = 1; i < length_size; i++)
-      buf->push_back(0);
-    buf->push_back(sizeof(kNALU2));
+    WriteLength(length_size, sizeof(kNALU2), buf);
     buf->insert(buf->end(), kNALU2, kNALU2 + sizeof(kNALU2));
   }
+
 };
 
 TEST_P(AVCConversionTest, ParseCorrectly) {
   std::vector<uint8> buf;
   MakeInputForLength(GetParam(), &buf);
   EXPECT_TRUE(AVC::ConvertFrameToAnnexB(GetParam(), &buf));
+  EXPECT_TRUE(AVC::IsValidAnnexB(buf));
   EXPECT_EQ(buf.size(), sizeof(kExpected));
   EXPECT_EQ(0, memcmp(kExpected, &buf[0], sizeof(kExpected)));
+  EXPECT_EQ("P SDC", AnnexBToString(buf));
+}
+
+// Intentionally write NALU sizes that are larger than the buffer.
+TEST_P(AVCConversionTest, NALUSizeTooLarge) {
+  std::vector<uint8> buf;
+  WriteLength(GetParam(), 10 * sizeof(kNALU1), &buf);
+  buf.insert(buf.end(), kNALU1, kNALU1 + sizeof(kNALU1));
+  EXPECT_FALSE(AVC::ConvertFrameToAnnexB(GetParam(), &buf));
+}
+
+TEST_P(AVCConversionTest, NALUSizeIsZero) {
+  std::vector<uint8> buf;
+  WriteLength(GetParam(), 0, &buf);
+
+  WriteLength(GetParam(), sizeof(kNALU1), &buf);
+  buf.insert(buf.end(), kNALU1, kNALU1 + sizeof(kNALU1));
+
+  WriteLength(GetParam(), 0, &buf);
+
+  WriteLength(GetParam(), sizeof(kNALU2), &buf);
+  buf.insert(buf.end(), kNALU2, kNALU2 + sizeof(kNALU2));
+
+  EXPECT_FALSE(AVC::ConvertFrameToAnnexB(GetParam(), &buf));
 }
 
 TEST_P(AVCConversionTest, ParsePartial) {
@@ -85,9 +259,113 @@ TEST_F(AVCConversionTest, ConvertConfigToAnnexB) {
   avc_config.pps_list[0].push_back(0x78);
 
   std::vector<uint8> buf;
-  EXPECT_TRUE(AVC::ConvertConfigToAnnexB(avc_config, &buf));
+  std::vector<SubsampleEntry> subsamples;
+  EXPECT_TRUE(AVC::ConvertConfigToAnnexB(avc_config, &buf, &subsamples));
   EXPECT_EQ(0, memcmp(kExpectedParamSets, &buf[0],
                       sizeof(kExpectedParamSets)));
+  EXPECT_EQ("SPS SPS PPS", AnnexBToString(buf));
+}
+
+// Verify that we can round trip string -> Annex B -> string.
+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));
+
+  EXPECT_EQ(str, AnnexBToString(buf));
+}
+
+TEST_F(AVCConversionTest, ValidAnnexBConstructs) {
+  const char* test_cases[] = {
+    "I",
+    "I I I I",
+    "AUD I",
+    "AUD SPS PPS I",
+    "I EOSeq",
+    "I EOSeq EOStr",
+    "I EOStr",
+    "P",
+    "P P P P",
+    "AUD SPS PPS P",
+    "SEI SEI I",
+    "SEI SEI R14 I",
+    "SPS SPSExt SPS PPS I P",
+    "R14 SEI I",
+  };
+
+  for (size_t i = 0; i < arraysize(test_cases); ++i) {
+    std::vector<uint8> buf;
+    StringToAnnexB(test_cases[i], &buf, NULL);
+    EXPECT_TRUE(AVC::IsValidAnnexB(buf)) << "'" << test_cases[i] << "' failed";
+  }
+}
+
+TEST_F(AVCConversionTest, InvalidAnnexBConstructs) {
+  static const char* test_cases[] = {
+    "AUD",  // No VCL present.
+    "SPS PPS",  // No VCL present.
+    "SPS PPS AUD I",  // Parameter sets must come after AUD.
+    "SPSExt SPS P",  // SPS must come before SPSExt.
+    "SPS PPS SPSExt P",  // SPSExt must follow an SPS.
+    "EOSeq",  // EOSeq must come after a VCL.
+    "EOStr",  // EOStr must come after a VCL.
+    "I EOStr EOSeq",  // EOSeq must come before EOStr.
+    "I R14",  // Reserved14-18 must come before first VCL.
+    "I SEI",  // SEI must come before first VCL.
+    "P SPS P", // SPS after first VCL would indicate a new access unit.
+  };
+
+  for (size_t i = 0; i < arraysize(test_cases); ++i) {
+    std::vector<uint8> buf;
+    StringToAnnexB(test_cases[i], &buf, NULL);
+    EXPECT_FALSE(AVC::IsValidAnnexB(buf)) << "'" << test_cases[i] << "' failed";
+  }
+}
+
+typedef struct {
+  const char* input;
+  const char* expected;
+} InsertTestCases;
+
+TEST_F(AVCConversionTest, InsertParamSetsAnnexB) {
+  static const InsertTestCases test_cases[] = {
+    { "I", "SPS SPS PPS I" },
+    { "AUD I", "AUD SPS SPS PPS I" },
+
+    // Cases where param sets in |avc_config| are placed before
+    // the existing ones.
+    { "SPS PPS I", "SPS SPS PPS SPS PPS I" },
+    { "AUD SPS PPS I", "AUD SPS SPS PPS SPS PPS I" },  // Note: params placed
+                                                       // after AUD.
+  };
+
+  AVCDecoderConfigurationRecord avc_config;
+  avc_config.sps_list.resize(2);
+  avc_config.sps_list[0].push_back(0x67);
+  avc_config.sps_list[0].push_back(0x12);
+  avc_config.sps_list[1].push_back(0x67);
+  avc_config.sps_list[1].push_back(0x34);
+  avc_config.pps_list.resize(1);
+  avc_config.pps_list[0].push_back(0x68);
+  avc_config.pps_list[0].push_back(0x56);
+  avc_config.pps_list[0].push_back(0x78);
+
+  for (size_t i = 0; i < arraysize(test_cases); ++i) {
+    std::vector<uint8> buf;
+    std::vector<SubsampleEntry> subsamples;
+
+    StringToAnnexB(test_cases[i].input, &buf, &subsamples);
+
+    EXPECT_TRUE(AVC::InsertParamSetsAnnexB(avc_config, &buf, &subsamples))
+        << "'" << test_cases[i].input << "' insert failed.";
+    EXPECT_TRUE(AVC::IsValidAnnexB(buf))
+        << "'" << test_cases[i].input << "' created invalid AnnexB.";
+    EXPECT_EQ(test_cases[i].expected, AnnexBToString(buf))
+        << "'" << test_cases[i].input << "' generated unexpected output.";
+  }
 }
 
 }  // namespace mp4
diff --git a/media/formats/mp4/mp4_stream_parser.cc b/media/formats/mp4/mp4_stream_parser.cc
index ccb3ccc..76fdeaf 100644
--- a/media/formats/mp4/mp4_stream_parser.cc
+++ b/media/formats/mp4/mp4_stream_parser.cc
@@ -363,13 +363,10 @@ bool MP4StreamParser::PrepareAVCBuffer(
     // If this is a keyframe, we (re-)inject SPS and PPS headers at the start of
     // a frame. If subsample info is present, we also update the clear byte
     // count for that first subsample.
-    std::vector<uint8> param_sets;
-    RCHECK(AVC::ConvertConfigToAnnexB(avc_config, &param_sets));
-    frame_buf->insert(frame_buf->begin(),
-                      param_sets.begin(), param_sets.end());
-    if (!subsamples->empty())
-      (*subsamples)[0].clear_bytes += param_sets.size();
+    RCHECK(AVC::InsertParamSetsAnnexB(avc_config, frame_buf, subsamples));
   }
+
+  DCHECK(AVC::IsValidAnnexB(*frame_buf));
   return true;
 }
 
diff --git a/media/formats/mp4/mp4_stream_parser_unittest.cc b/media/formats/mp4/mp4_stream_parser_unittest.cc
index 34e6984..d19e47c 100644
--- a/media/formats/mp4/mp4_stream_parser_unittest.cc
+++ b/media/formats/mp4/mp4_stream_parser_unittest.cc
@@ -212,6 +212,12 @@ TEST_F(MP4StreamParserTest, MissingSampleAuxInfo) {
   ParseMP4File("bear-1280x720-a_frag-cenc_missing-saiz-saio.mp4", 512);
 }
 
+// Test a file where all video samples start with an Access Unit
+// Delimiter (AUD) NALU.
+TEST_F(MP4StreamParserTest, VideoSamplesStartWithAUDs) {
+  ParseMP4File("bear-1280x720-av_with-aud-nalus_frag.mp4", 512);
+}
+
 // TODO(strobe): Create and test media which uses CENC auxiliary info stored
 // inside a private box