H264SpsPpsTracker.InsertSpsPpsNalus() should accept Nalus with header.

webrtc/modules/video_coding/h264_sps_pps_tracker_unittest.cc

Index: webrtc/modules/video_coding/h264_sps_pps_tracker_unittest.cc
diff --git a/webrtc/modules/video_coding/h264_sps_pps_tracker_unittest.cc b/webrtc/modules/video_coding/h264_sps_pps_tracker_unittest.cc
index 83330b934ed9b9ebf1195f8c067197e672be25b5..1d79add42de4449ba7e73b77bca4e3f3e2ab801a 100644
--- a/webrtc/modules/video_coding/h264_sps_pps_tracker_unittest.cc
+++ b/webrtc/modules/video_coding/h264_sps_pps_tracker_unittest.cc
@@ -261,5 +261,27 @@ TEST_F(TestH264SpsPpsTracker, SpsPpsIdrInStapA) {
   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