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

webrtc/modules/rtp_rtcp/source/rtp_format_video_generic_unittest.cc

+/*
+ *  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;

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

may be sort usings alphabetically

[ilnik, 2017/05/18 12:22:52]

Done.

+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>* kPayloadSizes,

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

"The names of variables (including function parameters) are all lowercase, with
underscores between words."
https://google.github.io/styleguide/cppguide.html#Variable_Names

[ilnik, 2017/05/18 12:42:19]

Done.

+                                         size_t kLastPacketReductionLen) {
+  // Account for larger last packet header.
+  kPayloadSizes->back() += kLastPacketReductionLen;
+  auto minmax =
+      std::minmax_element(kPayloadSizes->begin(), kPayloadSizes->end());
+  // MAX-MIN
+  size_t difference = *minmax.second - *minmax.first;
+  // Revert temporary changes.
+  kPayloadSizes->back() -= kLastPacketReductionLen;
+  return difference;
+}
+
+}  // namespace
+
+TEST(RtpPacketizerVideoGeneric, AllPacketsMayBeEqual_RespectsMaxPayloadSize) {
+  const size_t kMaxPayloadLen = 6;
+  const size_t kLastPacketReductionLen = 2;
+  const size_t kPayloadSize = 13;
+  RtpPacketizerGeneric packetizer(kVideoFrameKey, kMaxPayloadLen,
+                                  kLastPacketReductionLen);
+  size_t num_packets =
+      packetizer.SetPayloadData(kTestPayload, kPayloadSize, nullptr);
+  std::vector<size_t> kPayloadSizes = NextPacketFillPayloadSizes(&packetizer);

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

do not name variable payload_sizes as a constant.

[ilnik, 2017/05/18 12:22:52]

Done.

+  EXPECT_THAT(kPayloadSizes, SizeIs(num_packets));
+
+  EXPECT_THAT(kPayloadSizes, Each(Le(kMaxPayloadLen)));
+}
+
+TEST(RtpPacketizerVideoGeneric,
+     AllPacketsMayBeEqual_RespectsLastPacketReductionLength) {
+  const size_t kMaxPayloadLen = 6;
+  const size_t kLastPacketReductionLen = 2;
+  const size_t kPayloadSize = 13;
+  RtpPacketizerGeneric packetizer(kVideoFrameKey, kMaxPayloadLen,
+                                  kLastPacketReductionLen);
+  size_t num_packets =
+      packetizer.SetPayloadData(kTestPayload, kPayloadSize, nullptr);
+  auto kPayloadSizes = NextPacketFillPayloadSizes(&packetizer);
+  EXPECT_THAT(kPayloadSizes, SizeIs(num_packets));
+
+  EXPECT_LE(kPayloadSizes.back(), kMaxPayloadLen - kLastPacketReductionLen);
+}
+
+TEST(RtpPacketizerVideoGeneric,
+     AllPacketsMayBeEqual_MakesPacketsAlmostEqualInSize) {
+  const size_t kMaxPayloadLen = 6;
+  const size_t kLastPacketReductionLen = 2;
+  const size_t kPayloadSize = 13;
+  RtpPacketizerGeneric packetizer(kVideoFrameKey, kMaxPayloadLen,
+                                  kLastPacketReductionLen);
+  size_t num_packets =
+      packetizer.SetPayloadData(kTestPayload, kPayloadSize, nullptr);
+  auto kPayloadSizes = NextPacketFillPayloadSizes(&packetizer);
+  EXPECT_THAT(kPayloadSizes, SizeIs(num_packets));
+
+  size_t sizes_difference = GetEffectivePacketsSizeDifference(
+      &kPayloadSizes, kLastPacketReductionLen);
+  EXPECT_LE(sizes_difference, 1u);
+}
+
+TEST(RtpPacketizerVideoGeneric,
+     AllPacketsMayBeEqual_GeneratesMinimumNumberOfPackets) {
+  const size_t kMaxPayloadLen = 6;
+  const size_t kLastPacketReductionLen = 3;
+  const size_t kPayloadSize = 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 kPayloadSize.
+  const size_t kMinNumPackets = 4;
+  RtpPacketizerGeneric packetizer(kVideoFrameKey, kMaxPayloadLen,
+                                  kLastPacketReductionLen);
+  size_t num_packets =
+      packetizer.SetPayloadData(kTestPayload, kPayloadSize, nullptr);
+  auto kPayloadSizes = NextPacketFillPayloadSizes(&packetizer);
+  EXPECT_THAT(kPayloadSizes, SizeIs(num_packets));
+
+  EXPECT_EQ(num_packets, kMinNumPackets);
+}
+
+TEST(RtpPacketizerVideoGeneric, SomePacketsAreSmaller_RespectsMaxPayloadSize) {
+  const size_t kMaxPayloadLen = 8;
+  const size_t kLastPacketReductionLen = 5;
+  const size_t kPayloadSize = 28;
+  RtpPacketizerGeneric packetizer(kVideoFrameKey, kMaxPayloadLen,
+                                  kLastPacketReductionLen);
+  size_t num_packets =
+      packetizer.SetPayloadData(kTestPayload, kPayloadSize, nullptr);
+  auto kPayloadSizes = NextPacketFillPayloadSizes(&packetizer);
+  EXPECT_THAT(kPayloadSizes, SizeIs(num_packets));
+
+  EXPECT_THAT(kPayloadSizes, Each(Le(kMaxPayloadLen)));
+}
+
+TEST(RtpPacketizerVideoGeneric,
+     SomePacketsAreSmaller_RespectsLastPacketReductionLength) {
+  const size_t kMaxPayloadLen = 8;
+  const size_t kLastPacketReductionLen = 5;
+  const size_t kPayloadSize = 28;
+  RtpPacketizerGeneric packetizer(kVideoFrameKey, kMaxPayloadLen,
+                                  kLastPacketReductionLen);
+  size_t num_packets =
+      packetizer.SetPayloadData(kTestPayload, kPayloadSize, nullptr);
+  auto kPayloadSizes = NextPacketFillPayloadSizes(&packetizer);
+  EXPECT_THAT(kPayloadSizes, SizeIs(num_packets));
+
+  EXPECT_LE(kPayloadSizes.back(), kMaxPayloadLen - kLastPacketReductionLen);
+}
+
+TEST(RtpPacketizerVideoGeneric,
+     SomePacketsAreSmaller_MakesPacketsAlmostEqualInSize) {
+  const size_t kMaxPayloadLen = 8;
+  const size_t kLastPacketReductionLen = 5;
+  const size_t kPayloadSize = 28;
+  RtpPacketizerGeneric packetizer(kVideoFrameKey, kMaxPayloadLen,
+                                  kLastPacketReductionLen);
+  size_t num_packets =
+      packetizer.SetPayloadData(kTestPayload, kPayloadSize, nullptr);
+  auto kPayloadSizes = NextPacketFillPayloadSizes(&packetizer);
+  EXPECT_THAT(kPayloadSizes, SizeIs(num_packets));
+
+  size_t sizes_difference = GetEffectivePacketsSizeDifference(
+      &kPayloadSizes, kLastPacketReductionLen);
+  EXPECT_LE(sizes_difference, 1u);
+}
+
+TEST(RtpPacketizerVideoGeneric,
+     SomePacketsAreSmaller_GeneratesMinimumNumberOfPackets) {
+  const size_t kMaxPayloadLen = 8;
+  const size_t kLastPacketReductionLen = 5;
+  const size_t kPayloadSize = 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 kPayloadSize.
+  const size_t kMinNumPackets = 5;
+  RtpPacketizerGeneric packetizer(kVideoFrameKey, kMaxPayloadLen,
+                                  kLastPacketReductionLen);
+  size_t num_packets =
+      packetizer.SetPayloadData(kTestPayload, kPayloadSize, nullptr);
+  auto kPayloadSizes = NextPacketFillPayloadSizes(&packetizer);
+  EXPECT_THAT(kPayloadSizes, SizeIs(num_packets));
+
+  EXPECT_EQ(num_packets, kMinNumPackets);
+}
+
+}  // namespace webrtc