H264SpsPpsTracker.InsertSpsPpsNalus() should accept Nalus with header.

webrtc/modules/video_coding/h264_sps_pps_tracker_unittest.cc

 /*
  *  Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 243 matching lines @@
   expected.insert(expected.end(), {H264::NaluType::kSps, 13});
   expected.insert(expected.end(), start_code, start_code + sizeof(start_code));
   expected.insert(expected.end(), {H264::NaluType::kPps, 27});
   expected.insert(expected.end(), start_code, start_code + sizeof(start_code));
   expected.insert(expected.end(), {1, 2, 3, 2, 1});
 
   EXPECT_EQ(memcmp(packet.dataPtr, expected.data(), expected.size()), 0);
   delete[] packet.dataPtr;
 }
 
+TEST_F(TestH264SpsPpsTracker, SpsPpsOutOfBand) {
+  std::vector<uint8_t> data;

[sprang_webrtc, 2017/01/18 09:34:05]

Looks like you don't really need a vector here. If you use a plain array
instead, eg

const uint8_t[] kData = {1, 2, 3};

...then you can clean up some code below and get rid of the delete.

[johan, 2017/01/18 10:42:55]

Acknowledged.

+
+  // Generated by "ffmpeg -r 30 -f avfoundation -i "default" out.h264" on macos.
+  const std::vector<uint8_t> sps(
+      {0x67, 0x7a, 0x00, 0x0d, 0xbc, 0xd9, 0x41, 0x41, 0xfa, 0x10, 0x00, 0x00,
+       0x03, 0x00, 0x10, 0x00, 0x00, 0x03, 0x03, 0xc0, 0xf1, 0x42, 0x99, 0x60});
+  const std::vector<uint8_t> pps({0x68, 0xeb, 0xe3, 0xcb, 0x22, 0xc0});

[sprang_webrtc, 2017/01/18 09:34:05]

Is there any data baked in here that's worth checking if it was parsed correctly
by the SpsPpsTracker, or is that already covered by other unit tests?

[johan, 2017/01/18 10:42:55]

SpsPpsTracker itself checks only the nalu headers. For further parsing it relies
on SpsParser and PpsParser. For that reason, data needs to represent valid
sps/pps nalus. Unit test of actual parsing is covered by the respective parser
unit tests. 

But, maybe adding a test case with invalid nalus and invalid nalu header could
be a good idea.

+  tracker_.InsertSpsPpsNalus(sps, pps);
+
+  // Insert first packet of the IDR

[sprang_webrtc, 2017/01/18 09:34:05]

Nit: End with comments with period.

[johan, 2017/01/18 10:42:55]

Acknowledged.

+  VCMPacket idr_packet = GetDefaultPacket();
+  idr_packet.video_header.is_first_packet_in_frame = true;
+  AddIdr(&idr_packet, 0);
+  data.insert(data.end(), {1, 2, 3});
+  idr_packet.dataPtr = data.data();
+  idr_packet.sizeBytes = data.size();
+  ASSERT_EQ(H264SpsPpsTracker::kInsert,
+            tracker_.CopyAndFixBitstream(&idr_packet));
+  delete[] idr_packet.dataPtr;
+}
+
 }  // namespace video_coding
 }  // namespace webrtc