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..3190a8122fd6f25d2110db1380c805de6d6d3f4e
--- /dev/null
+++ b/webrtc/modules/rtp_rtcp/source/rtp_format_video_generic_unittest.cc
@@ -0,0 +1,194 @@
+/*
+ *  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
+  size_t difference = *minmax.second - *minmax.first;
+  // 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;

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

nit: for constants use kCamelCase style

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

Done.

[danilchap, 2017/05/18 11:14:24]

fyi: "This convention is optional for variables of other storage classes, e.g.
automatic variables"
https://google.github.io/styleguide/cppguide.html#Constant_Names

+  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);

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

nit: for simpler types, prefer explicit type rather than auto
auto is better for iterator types and lambdas etc that are very verbose or
difficult type explicitly

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

Done.

+  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;
+  // Computed by hand. 3 packets would have capacity 3*(6-1)-3=12 (max length -
+  // generic header lengh for each packet minus last packet reduction).
+  // 4 packets is enough for payload_size.
+  const size_t min_num_packets = 4;
+  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;
+  // Computed by hand. 4 packets would have capacity 4*(8-1)-5=23 (max length -
+  // generic header lengh for each packet minus last packet reduction).
+  // 5 packets is enough for payload_size.
+  const size_t min_num_packets = 5;
+  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