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

webrtc/modules/rtp_rtcp/source/rtp_format_video_generic_unittest.cc

Index: webrtc/modules/rtp_rtcp/source/rtp_format_video_generic_unittest.cc
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_format_video_generic_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_format_video_generic_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..57e5f0eb0767c70727d6585b597caf73e5b89acd
--- /dev/null
+++ b/webrtc/modules/rtp_rtcp/source/rtp_format_video_generic_unittest.cc
@@ -0,0 +1,188 @@
+/*
+ *  Copyright (c) 2017 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.
+ */
+
+#include <algorithm>
+#include <limits>
+#include <memory>
+#include <vector>
+
+#include "webrtc/base/array_view.h"
+#include "webrtc/modules/include/module_common_types.h"
+#include "webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h"
+#include "webrtc/modules/rtp_rtcp/source/byte_io.h"
+#include "webrtc/modules/rtp_rtcp/source/rtp_format_video_generic.h"
+#include "webrtc/modules/rtp_rtcp/source/rtp_packet_to_send.h"
+#include "webrtc/test/gmock.h"
+#include "webrtc/test/gtest.h"
+
+namespace webrtc {
+namespace {
+
+using ::testing::ElementsAreArray;
+using ::testing::Le;
+using ::testing::SizeIs;
+using ::testing::Each;
+
+const size_t kMaxPayloadSize = 1200;
+
+uint8_t kTestPayload[kMaxPayloadSize];
+
+std::vector<size_t> NextPacketFillPayloadSizes(
+    RtpPacketizerGeneric* packetizer) {
+  RtpPacketToSend packet(nullptr);
+  std::vector<size_t> result;
+  while (packetizer->NextPacket(&packet)) {
+    result.push_back(packet.payload_size());
+  }
+  return result;
+}
+
+size_t GetEffectivePacketsSizeDifference(std::vector<size_t>* payload_sizes,
+                                         size_t last_packet_reduction_len) {
+  // Account for larger last packet header.
+  payload_sizes->back() += last_packet_reduction_len;
+  auto minmax =
+      std::minmax_element(payload_sizes->begin(), payload_sizes->end());
+  // MAX-MIN
+  auto difference = *minmax.second - *minmax.first;

[danilchap, 2017/05/16 10:25:11]

using size_t here instead of auto likely better.
https://google.github.io/styleguide/cppguide.html#auto

[ilnik, 2017/05/16 12:24:07]

Done.

+  // Revert temporary changes.
+  payload_sizes->back() -= last_packet_reduction_len;
+  return difference;
+}
+
+}  // namespace
+
+TEST(RtpPacketizerVideoGeneric, AllPacketsMayBeEqual_RespectsMaxPayloadSize) {
+  const size_t max_payload_length = 6;
+  const size_t last_packet_reduction_len = 2;
+  const size_t payload_size = 13;
+  RtpPacketizerGeneric packetizer(kVideoFrameKey, max_payload_length,
+                                  last_packet_reduction_len);
+  size_t num_packets =
+      packetizer.SetPayloadData(kTestPayload, payload_size, nullptr);
+  auto payload_sizes = NextPacketFillPayloadSizes(&packetizer);
+  EXPECT_THAT(payload_sizes, SizeIs(num_packets));
+
+  EXPECT_THAT(payload_sizes, Each(Le(max_payload_length)));
+}
+
+TEST(RtpPacketizerVideoGeneric,
+     AllPacketsMayBeEqual_RespectsLastPacketReductionLength) {
+  const size_t max_payload_length = 6;
+  const size_t last_packet_reduction_len = 2;
+  const size_t payload_size = 13;
+  RtpPacketizerGeneric packetizer(kVideoFrameKey, max_payload_length,
+                                  last_packet_reduction_len);
+  size_t num_packets =
+      packetizer.SetPayloadData(kTestPayload, payload_size, nullptr);
+  auto payload_sizes = NextPacketFillPayloadSizes(&packetizer);
+  EXPECT_THAT(payload_sizes, SizeIs(num_packets));
+
+  EXPECT_LE(payload_sizes.back(),
+            max_payload_length - last_packet_reduction_len);
+}
+
+TEST(RtpPacketizerVideoGeneric,
+     AllPacketsMayBeEqual_MakesPacketsAlmostEqualInSize) {
+  const size_t max_payload_length = 6;
+  const size_t last_packet_reduction_len = 2;
+  const size_t payload_size = 13;
+  RtpPacketizerGeneric packetizer(kVideoFrameKey, max_payload_length,
+                                  last_packet_reduction_len);
+  size_t num_packets =
+      packetizer.SetPayloadData(kTestPayload, payload_size, nullptr);
+  auto payload_sizes = NextPacketFillPayloadSizes(&packetizer);
+  EXPECT_THAT(payload_sizes, SizeIs(num_packets));
+
+  size_t sizes_difference = GetEffectivePacketsSizeDifference(
+      &payload_sizes, last_packet_reduction_len);
+  EXPECT_LE(sizes_difference, 1u);
+}
+
+TEST(RtpPacketizerVideoGeneric,
+     AllPacketsMayBeEqual_GeneratesMinimumNumberOfPackets) {
+  const size_t max_payload_length = 6;
+  const size_t last_packet_reduction_len = 3;
+  const size_t payload_size = 13;
+  const size_t min_num_packets = 4;  // Computed by hand.

[danilchap, 2017/05/16 10:25:11]

may be extend the comment how it was computed:
min_num_packets = roundup((payload_size + reduction_len) / (payload_per_packet -
generic_header_size = 1))

[ilnik, 2017/05/16 12:24:07]

Now there's a comment about how exactly it was calculated.

+  RtpPacketizerGeneric packetizer(kVideoFrameKey, max_payload_length,
+                                  last_packet_reduction_len);
+  size_t num_packets =
+      packetizer.SetPayloadData(kTestPayload, payload_size, nullptr);
+  auto payload_sizes = NextPacketFillPayloadSizes(&packetizer);
+  EXPECT_THAT(payload_sizes, SizeIs(num_packets));
+
+  EXPECT_EQ(num_packets, min_num_packets);
+}
+
+TEST(RtpPacketizerVideoGeneric, SomePacketsAreSmaller_RespectsMaxPayloadSize) {
+  const size_t max_payload_length = 8;
+  const size_t last_packet_reduction_len = 5;
+  const size_t payload_size = 28;
+  RtpPacketizerGeneric packetizer(kVideoFrameKey, max_payload_length,
+                                  last_packet_reduction_len);
+  size_t num_packets =
+      packetizer.SetPayloadData(kTestPayload, payload_size, nullptr);
+  auto payload_sizes = NextPacketFillPayloadSizes(&packetizer);
+  EXPECT_THAT(payload_sizes, SizeIs(num_packets));
+
+  EXPECT_THAT(payload_sizes, Each(Le(max_payload_length)));
+}
+
+TEST(RtpPacketizerVideoGeneric,
+     SomePacketsAreSmaller_RespectsLastPacketReductionLength) {
+  const size_t max_payload_length = 8;
+  const size_t last_packet_reduction_len = 5;
+  const size_t payload_size = 28;
+  RtpPacketizerGeneric packetizer(kVideoFrameKey, max_payload_length,
+                                  last_packet_reduction_len);
+  size_t num_packets =
+      packetizer.SetPayloadData(kTestPayload, payload_size, nullptr);
+  auto payload_sizes = NextPacketFillPayloadSizes(&packetizer);
+  EXPECT_THAT(payload_sizes, SizeIs(num_packets));
+
+  EXPECT_LE(payload_sizes.back(),
+            max_payload_length - last_packet_reduction_len);
+}
+
+TEST(RtpPacketizerVideoGeneric,
+     SomePacketsAreSmaller_MakesPacketsAlmostEqualInSize) {
+  const size_t max_payload_length = 8;
+  const size_t last_packet_reduction_len = 5;
+  const size_t payload_size = 28;
+  RtpPacketizerGeneric packetizer(kVideoFrameKey, max_payload_length,
+                                  last_packet_reduction_len);
+  size_t num_packets =
+      packetizer.SetPayloadData(kTestPayload, payload_size, nullptr);
+  auto payload_sizes = NextPacketFillPayloadSizes(&packetizer);
+  EXPECT_THAT(payload_sizes, SizeIs(num_packets));
+
+  size_t sizes_difference = GetEffectivePacketsSizeDifference(
+      &payload_sizes, last_packet_reduction_len);
+  EXPECT_LE(sizes_difference, 1u);
+}
+
+TEST(RtpPacketizerVideoGeneric,
+     SomePacketsAreSmaller_GeneratesMinimumNumberOfPackets) {
+  const size_t max_payload_length = 8;
+  const size_t last_packet_reduction_len = 5;
+  const size_t payload_size = 28;
+  const size_t min_num_packets = 5;  // Computed by hand.
+  RtpPacketizerGeneric packetizer(kVideoFrameKey, max_payload_length,
+                                  last_packet_reduction_len);
+  size_t num_packets =
+      packetizer.SetPayloadData(kTestPayload, payload_size, nullptr);
+  auto payload_sizes = NextPacketFillPayloadSizes(&packetizer);
+  EXPECT_THAT(payload_sizes, SizeIs(num_packets));
+
+  EXPECT_EQ(num_packets, min_num_packets);
+}
+
+}  // namespace webrtc