Fix packetization logic to leave space for extensions in the last packet

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 1f926d9eeb010246d7fa06190798f60eddd8b2d4..bae950e917ec9ad80b89bbfecd089f57e2fd3a6f 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_format_h264_unittest.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_format_h264_unittest.cc
@@ -66,7 +66,7 @@ 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,
+  return RtpPacketizer::Create(kRtpVideoH264, max_payload_size, 0, &type_header,
                                kEmptyFrame);
 }
 
@@ -111,20 +111,19 @@ void TestFua(size_t frame_size,
   fragmentation.fragmentationLength[0] = frame_size;
   std::unique_ptr<RtpPacketizer> packetizer(CreateH264Packetizer(
       H264PacketizationMode::NonInterleaved, max_payload_size));
-  packetizer->SetPayloadData(frame.get(), frame_size, &fragmentation);
+  EXPECT_EQ(packetizer->SetPayloadData(frame.get(), frame_size, &fragmentation),
+            expected_sizes.size());

[sprang_webrtc, 2017/05/17 13:10:07]

nit: For EXPECT_EQ and ASSERT_EQ below, prefer having the expected value as the
first argument, as that's what the error message is gonna say if the test fails.

[danilchap, 2017/05/17 13:21:54]

Actually error message treat both parameters equally these days.
So recommendation shifting other way around for consistency with regular ==

[ilnik, 2017/05/17 15:06:33]

Actually, it's true that expected value is the first in the error message. At
least I've seen it. And, once I paid attention to that detail, in all places
around, expected value is the first. Done it in the same way here for
consistency.

[danilchap, 2017/05/17 15:28:39]

Let's make some test fail with EXPECT_EQ failure...
EXPECT_EQ(1, 2); Result messages:
../../webrtc/modules/rtp_rtcp/source/rtp_packet_unittest.cc:95: Failure
      Expected: 1
To be equal to: 2

I would say this message works equally fine both when value under test 1st
parameter and 2nd parameter.

https://github.com/google/googletest/blob/master/googletest/docs/Primer.md#bi...
"Historical note: Before February 2016 *_EQ had a convention of calling it as
ASSERT_EQ(expected, actual), so lots of existing code uses this order. Now *_EQ
treats both parameters in the same way."

[ilnik, 2017/05/18 07:33:01]

Yes, you are right. It just that I saw that the first number is labeled
"Expected:". If one carefully reads the second line too, and thinks about it a
bit, two numbers are treated equally. Still, I'll leave this part of code in the
same style as everything around it: expected value first. I'll remember this for
the future.

[sprang_webrtc, 2017/05/18 09:48:19]

Right, thanks. Old habits...

 
   RtpPacketToSend packet(kNoExtensions);
   ASSERT_LE(max_payload_size, packet.FreeCapacity());
-  bool last = false;
   size_t offset = kNalHeaderSize;
   for (size_t i = 0; i < expected_sizes.size(); ++i) {
-    ASSERT_TRUE(packetizer->NextPacket(&packet, &last));
+    ASSERT_TRUE(packetizer->NextPacket(&packet));
     VerifyFua(i, frame.get(), offset, packet.payload(), expected_sizes);
-    EXPECT_EQ(i == expected_sizes.size() - 1, last) << "FUA index: " << i;
     offset += expected_sizes[i];
   }
 
-  EXPECT_FALSE(packetizer->NextPacket(&packet, &last));
+  EXPECT_FALSE(packetizer->NextPacket(&packet));
 }
 
 size_t GetExpectedNaluOffset(const RTPFragmentationHeader& fragmentation,
@@ -183,15 +182,14 @@ TEST_P(RtpPacketizerH264ModeTest, TestSingleNalu) {
   fragmentation.fragmentationLength[0] = sizeof(frame);
   std::unique_ptr<RtpPacketizer> packetizer(
       CreateH264Packetizer(GetParam(), kMaxPayloadSize));
-  packetizer->SetPayloadData(frame, sizeof(frame), &fragmentation);
+  ASSERT_EQ(packetizer->SetPayloadData(frame, sizeof(frame), &fragmentation),
+            1u);
   RtpPacketToSend packet(kNoExtensions);
   ASSERT_LE(kMaxPayloadSize, packet.FreeCapacity());
-  bool last = false;
-  ASSERT_TRUE(packetizer->NextPacket(&packet, &last));
+  ASSERT_TRUE(packetizer->NextPacket(&packet));
   EXPECT_EQ(2u, packet.payload_size());
-  EXPECT_TRUE(last);
   VerifySingleNaluPayload(fragmentation, 0, frame, packet.payload());
-  EXPECT_FALSE(packetizer->NextPacket(&packet, &last));
+  EXPECT_FALSE(packetizer->NextPacket(&packet));
 }
 
 TEST_P(RtpPacketizerH264ModeTest, TestSingleNaluTwoPackets) {
@@ -211,20 +209,18 @@ TEST_P(RtpPacketizerH264ModeTest, TestSingleNaluTwoPackets) {
 
   std::unique_ptr<RtpPacketizer> packetizer(
       CreateH264Packetizer(GetParam(), kMaxPayloadSize));
-  packetizer->SetPayloadData(frame, kFrameSize, &fragmentation);
+  ASSERT_EQ(packetizer->SetPayloadData(frame, kFrameSize, &fragmentation), 2u);
 
   RtpPacketToSend packet(kNoExtensions);
-  bool last = false;
-  ASSERT_TRUE(packetizer->NextPacket(&packet, &last));
+  ASSERT_TRUE(packetizer->NextPacket(&packet));
   ASSERT_EQ(fragmentation.fragmentationOffset[1], packet.payload_size());
   VerifySingleNaluPayload(fragmentation, 0, frame, packet.payload());
 
-  ASSERT_TRUE(packetizer->NextPacket(&packet, &last));
+  ASSERT_TRUE(packetizer->NextPacket(&packet));
   ASSERT_EQ(fragmentation.fragmentationLength[1], packet.payload_size());
   VerifySingleNaluPayload(fragmentation, 1, frame, packet.payload());
-  EXPECT_TRUE(last);
 
-  EXPECT_FALSE(packetizer->NextPacket(&packet, &last));
+  EXPECT_FALSE(packetizer->NextPacket(&packet));
 }
 
 INSTANTIATE_TEST_CASE_P(
@@ -246,20 +242,19 @@ TEST(RtpPacketizerH264Test, TestStapA) {
   CreateThreeFragments(&fragmentation, kFrameSize, kPayloadOffset);
   std::unique_ptr<RtpPacketizer> packetizer(CreateH264Packetizer(
       H264PacketizationMode::NonInterleaved, kMaxPayloadSize));
-  packetizer->SetPayloadData(frame, kFrameSize, &fragmentation);
+  ASSERT_EQ(packetizer->SetPayloadData(frame, kFrameSize, &fragmentation), 1u);
 
   RtpPacketToSend packet(kNoExtensions);
   ASSERT_LE(kMaxPayloadSize, packet.FreeCapacity());
-  bool last = false;
-  ASSERT_TRUE(packetizer->NextPacket(&packet, &last));
+  ASSERT_TRUE(packetizer->NextPacket(&packet));
   size_t expected_packet_size =
       kNalHeaderSize + 3 * kLengthFieldLength + kFrameSize;
   ASSERT_EQ(expected_packet_size, packet.payload_size());
-  EXPECT_TRUE(last);
+
   for (size_t i = 0; i < fragmentation.fragmentationVectorSize; ++i)
     VerifyStapAPayload(fragmentation, 0, i, frame, packet.payload());
 
-  EXPECT_FALSE(packetizer->NextPacket(&packet, &last));
+  EXPECT_FALSE(packetizer->NextPacket(&packet));
 }
 
 TEST(RtpPacketizerH264Test, TestSingleNalUnitModeHasNoStapA) {
@@ -279,12 +274,11 @@ TEST(RtpPacketizerH264Test, TestSingleNalUnitModeHasNoStapA) {
   packetizer->SetPayloadData(frame, kFrameSize, &fragmentation);
 
   RtpPacketToSend packet(kNoExtensions);
-  bool last = false;
   // The three fragments should be returned as three packets.
-  ASSERT_TRUE(packetizer->NextPacket(&packet, &last));
-  ASSERT_TRUE(packetizer->NextPacket(&packet, &last));
-  ASSERT_TRUE(packetizer->NextPacket(&packet, &last));
-  EXPECT_FALSE(packetizer->NextPacket(&packet, &last));
+  ASSERT_TRUE(packetizer->NextPacket(&packet));
+  ASSERT_TRUE(packetizer->NextPacket(&packet));
+  ASSERT_TRUE(packetizer->NextPacket(&packet));
+  EXPECT_FALSE(packetizer->NextPacket(&packet));
 }
 
 TEST(RtpPacketizerH264Test, TestTooSmallForStapAHeaders) {
@@ -306,29 +300,26 @@ TEST(RtpPacketizerH264Test, TestTooSmallForStapAHeaders) {
       kNalHeaderSize + kFrameSize - kPayloadOffset;
   std::unique_ptr<RtpPacketizer> packetizer(CreateH264Packetizer(
       H264PacketizationMode::NonInterleaved, kMaxPayloadSize));
-  packetizer->SetPayloadData(frame, kFrameSize, &fragmentation);
+  ASSERT_EQ(packetizer->SetPayloadData(frame, kFrameSize, &fragmentation), 2u);
 
   RtpPacketToSend packet(kNoExtensions);
   ASSERT_LE(kMaxPayloadSize, packet.FreeCapacity());
-  bool last = false;
-  ASSERT_TRUE(packetizer->NextPacket(&packet, &last));
+  ASSERT_TRUE(packetizer->NextPacket(&packet));
   size_t expected_packet_size = kNalHeaderSize;
   for (size_t i = 0; i < 2; ++i) {
     expected_packet_size +=
         kLengthFieldLength + fragmentation.fragmentationLength[i];
   }
   ASSERT_EQ(expected_packet_size, packet.payload_size());
-  EXPECT_FALSE(last);
   for (size_t i = 0; i < 2; ++i)
     VerifyStapAPayload(fragmentation, 0, i, frame, packet.payload());
 
-  ASSERT_TRUE(packetizer->NextPacket(&packet, &last));
+  ASSERT_TRUE(packetizer->NextPacket(&packet));
   expected_packet_size = fragmentation.fragmentationLength[2];
   ASSERT_EQ(expected_packet_size, packet.payload_size());
-  EXPECT_TRUE(last);
   VerifySingleNaluPayload(fragmentation, 2, frame, packet.payload());
 
-  EXPECT_FALSE(packetizer->NextPacket(&packet, &last));
+  EXPECT_FALSE(packetizer->NextPacket(&packet));
 }
 
 TEST(RtpPacketizerH264Test, TestMixedStapA_FUA) {
@@ -354,7 +345,7 @@ TEST(RtpPacketizerH264Test, TestMixedStapA_FUA) {
   }
   std::unique_ptr<RtpPacketizer> packetizer(CreateH264Packetizer(
       H264PacketizationMode::NonInterleaved, kMaxPayloadSize));
-  packetizer->SetPayloadData(frame, kFrameSize, &fragmentation);
+  ASSERT_EQ(packetizer->SetPayloadData(frame, kFrameSize, &fragmentation), 3u);
 
   // First expecting two FU-A packets.
   std::vector<size_t> fua_sizes;
@@ -362,24 +353,21 @@ TEST(RtpPacketizerH264Test, TestMixedStapA_FUA) {
   fua_sizes.push_back(1099);
   RtpPacketToSend packet(kNoExtensions);
   ASSERT_LE(kMaxPayloadSize, packet.FreeCapacity());
-  bool last = false;
   int fua_offset = kNalHeaderSize;
   for (size_t i = 0; i < 2; ++i) {
-    ASSERT_TRUE(packetizer->NextPacket(&packet, &last));
+    ASSERT_TRUE(packetizer->NextPacket(&packet));
     VerifyFua(i, frame, fua_offset, packet.payload(), fua_sizes);
-    EXPECT_FALSE(last);
     fua_offset += fua_sizes[i];
   }
   // Then expecting one STAP-A packet with two nal units.
-  ASSERT_TRUE(packetizer->NextPacket(&packet, &last));
+  ASSERT_TRUE(packetizer->NextPacket(&packet));
   size_t expected_packet_size =
       kNalHeaderSize + 2 * kLengthFieldLength + 2 * kStapANaluSize;
   ASSERT_EQ(expected_packet_size, packet.payload_size());
-  EXPECT_TRUE(last);
   for (size_t i = 1; i < fragmentation.fragmentationVectorSize; ++i)
     VerifyStapAPayload(fragmentation, 1, i, frame, packet.payload());
 
-  EXPECT_FALSE(packetizer->NextPacket(&packet, &last));
+  EXPECT_FALSE(packetizer->NextPacket(&packet));
 }
 
 TEST(RtpPacketizerH264Test, TestFUAOddSize) {
@@ -403,8 +391,8 @@ TEST(RtpPacketizerH264Test, TestFUAEvenSize) {
 }
 
 TEST(RtpPacketizerH264Test, TestFUARounding) {
-  const size_t kExpectedPayloadSizes[8] = {1266, 1266, 1266, 1266,
-                                           1266, 1266, 1266, 1261};
+  const size_t kExpectedPayloadSizes[8] = {1266, 1266, 1266, 1265,
+                                           1265, 1265, 1265, 1265};
   TestFua(
       10124,
       1448,
@@ -495,18 +483,17 @@ TEST_F(RtpPacketizerH264TestSpsRewriting, FuASps) {
   packetizer_->SetPayloadData(in_buffer_.data(), in_buffer_.size(),
                               &fragmentation_header_);
 
-  bool last_packet = true;
   RtpPacketToSend packet(kNoExtensions);
   ASSERT_LE(sizeof(kOriginalSps) + kHeaderOverhead, packet.FreeCapacity());
 
-  EXPECT_TRUE(packetizer_->NextPacket(&packet, &last_packet));
+  EXPECT_TRUE(packetizer_->NextPacket(&packet));
   size_t offset = H264::kNaluTypeSize;
   size_t length = packet.payload_size() - kFuAHeaderSize;
   EXPECT_THAT(packet.payload().subview(kFuAHeaderSize),
               ElementsAreArray(&kRewrittenSps[offset], length));
   offset += length;
 
-  EXPECT_TRUE(packetizer_->NextPacket(&packet, &last_packet));
+  EXPECT_TRUE(packetizer_->NextPacket(&packet));
   length = packet.payload_size() - kFuAHeaderSize;
   EXPECT_THAT(packet.payload().subview(kFuAHeaderSize),
               ElementsAreArray(&kRewrittenSps[offset], length));
@@ -528,11 +515,10 @@ TEST_F(RtpPacketizerH264TestSpsRewriting, StapASps) {
   packetizer_->SetPayloadData(in_buffer_.data(), in_buffer_.size(),
                               &fragmentation_header_);
 
-  bool last_packet = true;
   RtpPacketToSend packet(kNoExtensions);
   ASSERT_LE(kExpectedTotalSize + kHeaderOverhead, packet.FreeCapacity());
 
-  EXPECT_TRUE(packetizer_->NextPacket(&packet, &last_packet));
+  EXPECT_TRUE(packetizer_->NextPacket(&packet));
   EXPECT_EQ(kExpectedTotalSize, packet.payload_size());
   EXPECT_THAT(packet.payload().subview(H264::kNaluTypeSize + kLengthFieldLength,
                                        sizeof(kRewrittenSps)),