H.264 packetization mode 0 (try 2)

webrtc/modules/rtp_rtcp/source/rtp_format_h264_unittest.cc

Index: webrtc/modules/rtp_rtcp/source/rtp_format_h264_unittest.cc
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_format_h264_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_format_h264_unittest.cc
index 894415da8a7960d70bad206f115182e67e35f235..2e8be2e23b9b6f3c22e3d3b9b441d72015b16a93 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_format_h264_unittest.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_format_h264_unittest.cc
@@ -43,6 +43,17 @@ enum NalDefs { kFBit = 0x80, kNriMask = 0x60, kTypeMask = 0x1F };
 // Bit masks for FU (A and B) headers.
 enum FuDefs { kSBit = 0x80, kEBit = 0x40, kRBit = 0x20 };
 
+static const H264PacketizationMode packetization_modes[] = {
+    kH264PacketizationMode0, kH264PacketizationMode1};
+
+RtpPacketizer* CreateH264Packetizer(H264PacketizationMode mode,
+                                    size_t max_payload_size) {
+  RTPVideoTypeHeader type_header;
+  type_header.H264.packetization_mode = mode;
+  return RtpPacketizer::Create(kRtpVideoH264, max_payload_size, &type_header,
+                               kEmptyFrame);
+}
+
 void VerifyFua(size_t fua_index,
                const uint8_t* expected_payload,
                int offset,
@@ -84,8 +95,8 @@ void TestFua(size_t frame_size,
   fragmentation.VerifyAndAllocateFragmentationHeader(1);
   fragmentation.fragmentationOffset[0] = 0;
   fragmentation.fragmentationLength[0] = frame_size;
-  std::unique_ptr<RtpPacketizer> packetizer(RtpPacketizer::Create(
-      kRtpVideoH264, max_payload_size, NULL, kEmptyFrame));
+  std::unique_ptr<RtpPacketizer> packetizer(
+      CreateH264Packetizer(kH264PacketizationMode1, max_payload_size));
   packetizer->SetPayloadData(frame.get(), frame_size, &fragmentation);
 
   std::unique_ptr<uint8_t[]> packet(new uint8_t[max_payload_size]);
@@ -158,18 +169,21 @@ TEST(RtpPacketizerH264Test, TestSingleNalu) {
   fragmentation.VerifyAndAllocateFragmentationHeader(1);
   fragmentation.fragmentationOffset[0] = 0;
   fragmentation.fragmentationLength[0] = sizeof(frame);
-  std::unique_ptr<RtpPacketizer> packetizer(
-      RtpPacketizer::Create(kRtpVideoH264, kMaxPayloadSize, NULL, kEmptyFrame));
-  packetizer->SetPayloadData(frame, sizeof(frame), &fragmentation);
-  uint8_t packet[kMaxPayloadSize] = {0};
-  size_t length = 0;
-  bool last = false;
-  ASSERT_TRUE(packetizer->NextPacket(packet, &length, &last));
-  EXPECT_EQ(2u, length);
-  EXPECT_TRUE(last);
-  VerifySingleNaluPayload(
-      fragmentation, 0, frame, sizeof(frame), packet, length);
-  EXPECT_FALSE(packetizer->NextPacket(packet, &length, &last));
+  // This test should work in both packetization mode 0 and mode 1.
+  for (auto packetization_mode : packetization_modes) {

[magjed_webrtc, 2016/11/04 13:57:05]

I'm reposting one of my previous comments that I think was missed: 
Maybe it would be better to parameterize this test using TEST_P instead of
manually looping. You can see
https://codereview.webrtc.org/2337453002/diff/160001/webrtc/modules/rtp_rtcp/...
for an example how it's done.

The benefit of such a change is that if/when the test fails, you can immediately
see from the test failure what packetization_mode was used.

[hta-webrtc, 2016/11/09 09:27:10]

Done.
Not enjoying the printout format (it prints the index not the value), but it
makes the tests easier to read.

[magjed_webrtc, 2016/11/09 15:18:25]

When the tests run, it will just print the index, but if something fails, it
will print out the actual value of the parameters. So it will look something
like this:
[ RUN      ] PacketMode/RtpPacketizerH264ModeTest.TestSingleNalu/1
... Failure ...
[  FAILED  ] PacketMode/RtpPacketizerH264ModeTest.TestSingleNalu/1, where
GetParam() = 2
Since your parameter is an enum, it will just print out the integer value, which
is not super helpful, but still helpful.

Please move the tests back to this position to keep the diff small.

+    std::unique_ptr<RtpPacketizer> packetizer(
+        CreateH264Packetizer(packetization_mode, kMaxPayloadSize));
+    packetizer->SetPayloadData(frame, sizeof(frame), &fragmentation);
+    uint8_t packet[kMaxPayloadSize] = {0};
+    size_t length = 0;
+    bool last = false;
+    ASSERT_TRUE(packetizer->NextPacket(packet, &length, &last));
+    EXPECT_EQ(2u, length);
+    EXPECT_TRUE(last);
+    VerifySingleNaluPayload(fragmentation, 0, frame, sizeof(frame), packet,
+                            length);
+    EXPECT_FALSE(packetizer->NextPacket(packet, &length, &last));
+  }
 }
 
 TEST(RtpPacketizerH264Test, TestSingleNaluTwoPackets) {
@@ -187,23 +201,27 @@ TEST(RtpPacketizerH264Test, TestSingleNaluTwoPackets) {
   frame[fragmentation.fragmentationOffset[0]] = 0x01;
   frame[fragmentation.fragmentationOffset[1]] = 0x01;
 
-  std::unique_ptr<RtpPacketizer> packetizer(
-      RtpPacketizer::Create(kRtpVideoH264, kMaxPayloadSize, NULL, kEmptyFrame));
-  packetizer->SetPayloadData(frame, kFrameSize, &fragmentation);
+  for (auto packetization_mode : packetization_modes) {
+    std::unique_ptr<RtpPacketizer> packetizer(
+        CreateH264Packetizer(packetization_mode, kMaxPayloadSize));
+    packetizer->SetPayloadData(frame, kFrameSize, &fragmentation);
 
-  uint8_t packet[kMaxPayloadSize] = {0};
-  size_t length = 0;
-  bool last = false;
-  ASSERT_TRUE(packetizer->NextPacket(packet, &length, &last));
-  ASSERT_EQ(fragmentation.fragmentationOffset[1], length);
-  VerifySingleNaluPayload(fragmentation, 0, frame, kFrameSize, packet, length);
+    uint8_t packet[kMaxPayloadSize] = {0};
+    size_t length = 0;
+    bool last = false;
+    ASSERT_TRUE(packetizer->NextPacket(packet, &length, &last));
+    ASSERT_EQ(fragmentation.fragmentationOffset[1], length);
+    VerifySingleNaluPayload(fragmentation, 0, frame, kFrameSize, packet,
+                            length);
 
-  ASSERT_TRUE(packetizer->NextPacket(packet, &length, &last));
-  ASSERT_EQ(fragmentation.fragmentationLength[1], length);
-  VerifySingleNaluPayload(fragmentation, 1, frame, kFrameSize, packet, length);
-  EXPECT_TRUE(last);
+    ASSERT_TRUE(packetizer->NextPacket(packet, &length, &last));
+    ASSERT_EQ(fragmentation.fragmentationLength[1], length);
+    VerifySingleNaluPayload(fragmentation, 1, frame, kFrameSize, packet,
+                            length);
+    EXPECT_TRUE(last);
 
-  EXPECT_FALSE(packetizer->NextPacket(packet, &length, &last));
+    EXPECT_FALSE(packetizer->NextPacket(packet, &length, &last));
+  }
 }
 
 TEST(RtpPacketizerH264Test, TestStapA) {
@@ -225,7 +243,7 @@ TEST(RtpPacketizerH264Test, TestStapA) {
   fragmentation.fragmentationLength[2] =
       kNalHeaderSize + kFrameSize - kPayloadOffset;
   std::unique_ptr<RtpPacketizer> packetizer(
-      RtpPacketizer::Create(kRtpVideoH264, kMaxPayloadSize, NULL, kEmptyFrame));
+      CreateH264Packetizer(kH264PacketizationMode1, kMaxPayloadSize));
   packetizer->SetPayloadData(frame, kFrameSize, &fragmentation);
 
   uint8_t packet[kMaxPayloadSize] = {0};
@@ -242,6 +260,39 @@ TEST(RtpPacketizerH264Test, TestStapA) {
   EXPECT_FALSE(packetizer->NextPacket(packet, &length, &last));
 }
 
+TEST(RtpPacketizerH264Test, TestMode0HasNoStapA) {
+  // This is the same setup as for the TestStapA test.
+  const size_t kFrameSize =
+      kMaxPayloadSize - 3 * kLengthFieldLength - kNalHeaderSize;
+  uint8_t frame[kFrameSize] = {0x07, 0xFF,  // F=0, NRI=0, Type=7 (SPS).
+                               0x08, 0xFF,  // F=0, NRI=0, Type=8 (PPS).
+                               0x05};       // F=0, NRI=0, Type=5 (IDR).
+  const size_t kPayloadOffset = 5;
+  for (size_t i = 0; i < kFrameSize - kPayloadOffset; ++i)
+    frame[i + kPayloadOffset] = i;
+  RTPFragmentationHeader fragmentation;
+  fragmentation.VerifyAndAllocateFragmentationHeader(3);
+  fragmentation.fragmentationOffset[0] = 0;
+  fragmentation.fragmentationLength[0] = 2;
+  fragmentation.fragmentationOffset[1] = 2;
+  fragmentation.fragmentationLength[1] = 2;
+  fragmentation.fragmentationOffset[2] = 4;
+  fragmentation.fragmentationLength[2] =
+      kNalHeaderSize + kFrameSize - kPayloadOffset;
+  std::unique_ptr<RtpPacketizer> packetizer(
+      CreateH264Packetizer(kH264PacketizationMode0, kMaxPayloadSize));
+  packetizer->SetPayloadData(frame, kFrameSize, &fragmentation);
+
+  uint8_t packet[kMaxPayloadSize] = {0};
+  size_t length = 0;
+  bool last = false;
+  // The three fragments should be returned as three packets.
+  ASSERT_TRUE(packetizer->NextPacket(packet, &length, &last));
+  ASSERT_TRUE(packetizer->NextPacket(packet, &length, &last));
+  ASSERT_TRUE(packetizer->NextPacket(packet, &length, &last));
+  EXPECT_FALSE(packetizer->NextPacket(packet, &length, &last));
+}
+
 TEST(RtpPacketizerH264Test, TestTooSmallForStapAHeaders) {
   const size_t kFrameSize = kMaxPayloadSize - 1;
   uint8_t frame[kFrameSize] = {0x07, 0xFF,  // F=0, NRI=0, Type=7.
@@ -260,7 +311,7 @@ TEST(RtpPacketizerH264Test, TestTooSmallForStapAHeaders) {
   fragmentation.fragmentationLength[2] =
       kNalHeaderSize + kFrameSize - kPayloadOffset;
   std::unique_ptr<RtpPacketizer> packetizer(
-      RtpPacketizer::Create(kRtpVideoH264, kMaxPayloadSize, NULL, kEmptyFrame));
+      CreateH264Packetizer(kH264PacketizationMode1, kMaxPayloadSize));
   packetizer->SetPayloadData(frame, kFrameSize, &fragmentation);
 
   uint8_t packet[kMaxPayloadSize] = {0};
@@ -308,7 +359,7 @@ TEST(RtpPacketizerH264Test, TestMixedStapA_FUA) {
     }
   }
   std::unique_ptr<RtpPacketizer> packetizer(
-      RtpPacketizer::Create(kRtpVideoH264, kMaxPayloadSize, NULL, kEmptyFrame));
+      CreateH264Packetizer(kH264PacketizationMode1, kMaxPayloadSize));
   packetizer->SetPayloadData(frame, kFrameSize, &fragmentation);
 
   // First expecting two FU-A packets.
@@ -381,6 +432,28 @@ TEST(RtpPacketizerH264Test, TestFUABig) {
                               sizeof(kExpectedPayloadSizes) / sizeof(size_t)));
 }
 
+#if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
+
+TEST(RtpPacketizerH264DeathTest, SendOverlongDataInPacketizationMode0) {
+  const size_t kFrameSize = kMaxPayloadSize + 100;
+  uint8_t frame[kFrameSize] = {0};
+  for (size_t i = 0; i < kFrameSize; ++i)
+    frame[i] = i;
+  RTPFragmentationHeader fragmentation;
+  fragmentation.VerifyAndAllocateFragmentationHeader(1);
+  fragmentation.fragmentationOffset[0] = 0;
+  fragmentation.fragmentationLength[0] = kFrameSize;
+  // Set NAL headers.
+  frame[fragmentation.fragmentationOffset[0]] = 0x01;
+
+  std::unique_ptr<RtpPacketizer> packetizer(
+      CreateH264Packetizer(kH264PacketizationMode0, kMaxPayloadSize));
+  EXPECT_DEATH(packetizer->SetPayloadData(frame, kFrameSize, &fragmentation),
+               "payload_size");
+}
+
+#endif  // RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
+
 namespace {
 const uint8_t kStartSequence[] = {0x00, 0x00, 0x00, 0x01};
 const uint8_t kOriginalSps[] = {kSps, 0x00, 0x00, 0x03, 0x03,
@@ -421,9 +494,8 @@ TEST_F(RtpPacketizerH264TestSpsRewriting, FuASps) {
   const size_t kHeaderOverhead = kFuAHeaderSize + 1;
 
   // Set size to fragment SPS into two FU-A packets.
-  packetizer_.reset(RtpPacketizer::Create(
-      kRtpVideoH264, sizeof(kOriginalSps) - 2 + kHeaderOverhead, nullptr,
-      kEmptyFrame));
+  packetizer_.reset(CreateH264Packetizer(
+      kH264PacketizationMode1, sizeof(kOriginalSps) - 2 + kHeaderOverhead));
 
   packetizer_->SetPayloadData(in_buffer_.data(), in_buffer_.size(),
                               &fragmentation_header_);
@@ -459,9 +531,8 @@ TEST_F(RtpPacketizerH264TestSpsRewriting, StapASps) {
                                     sizeof(kIdrTwo) + (kLengthFieldLength * 3);
 
   // Set size to include SPS and the rest of the packets in a Stap-A package.
-  packetizer_.reset(RtpPacketizer::Create(kRtpVideoH264,
-                                          kExpectedTotalSize + kHeaderOverhead,
-                                          nullptr, kEmptyFrame));
+  packetizer_.reset(CreateH264Packetizer(kH264PacketizationMode1,
+                                         kExpectedTotalSize + kHeaderOverhead));
 
   packetizer_->SetPayloadData(in_buffer_.data(), in_buffer_.size(),
                               &fragmentation_header_);