From bace3a4bd60713206ed71dc1b51a05822cab4be0 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Wed, 5 Sep 2018 16:15:08 +0200 Subject: [PATCH] in RtpPacketizer add support for first packet reduction length Bug: webrtc:9680 Change-Id: I68580a8a44cbbf932ed0b2448b31169b26df5044 Reviewed-on: https://webrtc-review.googlesource.com/98005 Commit-Queue: Danil Chapovalov Reviewed-by: Ilya Nikolaevskiy Cr-Commit-Position: refs/heads/master@{#24584} --- modules/rtp_rtcp/source/rtp_format.cc | 19 ++++- modules/rtp_rtcp/source/rtp_format.h | 1 + .../rtp_rtcp/source/rtp_format_unittest.cc | 76 ++++++++++++++++++- 3 files changed, 88 insertions(+), 8 deletions(-) diff --git a/modules/rtp_rtcp/source/rtp_format.cc b/modules/rtp_rtcp/source/rtp_format.cc index bddec73687..030236c2af 100644 --- a/modules/rtp_rtcp/source/rtp_format.cc +++ b/modules/rtp_rtcp/source/rtp_format.cc @@ -65,11 +65,15 @@ std::unique_ptr RtpPacketizer::Create( std::vector RtpPacketizer::SplitAboutEqually( size_t payload_len, const PayloadSizeLimits& limits) { + RTC_CHECK_GT(limits.max_payload_len, limits.first_packet_reduction_len); RTC_CHECK_GT(limits.max_payload_len, limits.last_packet_reduction_len); - // Last packet can be smaller. Pretend that it's the same size, but we must - // write more payload to it. - size_t total_bytes = payload_len + limits.last_packet_reduction_len; + // First and last packet of the frame can be smaller. Pretend that it's + // the same size, but we must write more payload to it. + // Assume frame fits in single packet if packet has extra space for sum + // of first and last packets reductions. + size_t total_bytes = payload_len + limits.first_packet_reduction_len + + limits.last_packet_reduction_len; // Integer divisions with rounding up. size_t num_packets_left = (total_bytes + limits.max_payload_len - 1) / limits.max_payload_len; @@ -79,12 +83,19 @@ std::vector RtpPacketizer::SplitAboutEqually( std::vector result; result.reserve(num_packets_left); + bool first_packet = true; while (remaining_data > 0) { // Last num_larger_packets are 1 byte wider than the rest. Increase // per-packet payload size when needed. if (num_packets_left == num_larger_packets) ++bytes_per_packet; size_t current_packet_bytes = bytes_per_packet; + if (first_packet) { + if (current_packet_bytes > limits.first_packet_reduction_len + 1) + current_packet_bytes -= limits.first_packet_reduction_len; + else + current_packet_bytes = 1; + } if (current_packet_bytes > remaining_data) { current_packet_bytes = remaining_data; } @@ -93,11 +104,11 @@ std::vector RtpPacketizer::SplitAboutEqually( if (num_packets_left == 2 && current_packet_bytes == remaining_data) { --current_packet_bytes; } - result.push_back(current_packet_bytes); remaining_data -= current_packet_bytes; --num_packets_left; + first_packet = false; } return result; diff --git a/modules/rtp_rtcp/source/rtp_format.h b/modules/rtp_rtcp/source/rtp_format.h index f945d24b0a..7a1f494d7c 100644 --- a/modules/rtp_rtcp/source/rtp_format.h +++ b/modules/rtp_rtcp/source/rtp_format.h @@ -28,6 +28,7 @@ class RtpPacketizer { public: struct PayloadSizeLimits { size_t max_payload_len = 1200; + size_t first_packet_reduction_len = 0; size_t last_packet_reduction_len = 0; }; static std::unique_ptr Create( diff --git a/modules/rtp_rtcp/source/rtp_format_unittest.cc b/modules/rtp_rtcp/source/rtp_format_unittest.cc index 6b834325c8..8fb780ec2c 100644 --- a/modules/rtp_rtcp/source/rtp_format_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_format_unittest.cc @@ -21,6 +21,7 @@ namespace { using ::testing::ElementsAre; using ::testing::Le; +using ::testing::Gt; using ::testing::Each; using ::testing::IsEmpty; using ::testing::Not; @@ -66,6 +67,20 @@ TEST(RtpPacketizerSplitAboutEqually, AllPacketsAreEqualRespectsMaxPayloadSize) { EXPECT_THAT(payload_sizes, Each(Le(limits.max_payload_len))); } +TEST(RtpPacketizerSplitAboutEqually, + AllPacketsAreEqualRespectsFirstPacketReduction) { + RtpPacketizer::PayloadSizeLimits limits; + limits.max_payload_len = 5; + limits.first_packet_reduction_len = 2; + + std::vector payload_sizes = + RtpPacketizer::SplitAboutEqually(13, limits); + + ASSERT_THAT(payload_sizes, Not(IsEmpty())); + EXPECT_EQ(payload_sizes.front() + limits.first_packet_reduction_len, + limits.max_payload_len); +} + TEST(RtpPacketizerSplitAboutEqually, AllPacketsAreEqualRespectsLastPacketReductionLength) { RtpPacketizer::PayloadSizeLimits limits; @@ -115,7 +130,8 @@ TEST(RtpPacketizerSplitAboutEqually, SomePacketsAreSmallerSumToPayloadLen) { EXPECT_THAT(Sum(payload_sizes), 28); } -TEST(RtpPacketizerVideoGeneric, SomePacketsAreSmallerRespectsMaxPayloadSize) { +TEST(RtpPacketizerSplitAboutEqually, + SomePacketsAreSmallerRespectsMaxPayloadSize) { RtpPacketizer::PayloadSizeLimits limits; limits.max_payload_len = 7; limits.last_packet_reduction_len = 5; @@ -126,7 +142,20 @@ TEST(RtpPacketizerVideoGeneric, SomePacketsAreSmallerRespectsMaxPayloadSize) { EXPECT_THAT(payload_sizes, Each(Le(limits.max_payload_len))); } -TEST(RtpPacketizerVideoGeneric, +TEST(RtpPacketizerSplitAboutEqually, + SomePacketsAreSmallerRespectsFirstPacketReduction) { + RtpPacketizer::PayloadSizeLimits limits; + limits.max_payload_len = 7; + limits.first_packet_reduction_len = 5; + + std::vector payload_sizes = + RtpPacketizer::SplitAboutEqually(28, limits); + + EXPECT_LE(payload_sizes.front() + limits.first_packet_reduction_len, + limits.max_payload_len); +} + +TEST(RtpPacketizerSplitAboutEqually, SomePacketsAreSmallerRespectsLastPacketReductionLength) { RtpPacketizer::PayloadSizeLimits limits; limits.max_payload_len = 7; @@ -139,7 +168,8 @@ TEST(RtpPacketizerVideoGeneric, limits.max_payload_len - limits.last_packet_reduction_len); } -TEST(RtpPacketizerVideoGeneric, SomePacketsAreSmallerPacketsAlmostEqualInSize) { +TEST(RtpPacketizerSplitAboutEqually, + SomePacketsAreSmallerPacketsAlmostEqualInSize) { RtpPacketizer::PayloadSizeLimits limits; limits.max_payload_len = 7; limits.last_packet_reduction_len = 5; @@ -150,7 +180,7 @@ TEST(RtpPacketizerVideoGeneric, SomePacketsAreSmallerPacketsAlmostEqualInSize) { EXPECT_LE(EffectivePacketsSizeDifference(payload_sizes, limits), 1); } -TEST(RtpPacketizerVideoGeneric, +TEST(RtpPacketizerSplitAboutEqually, SomePacketsAreSmallerGeneratesMinimumNumberOfPackets) { RtpPacketizer::PayloadSizeLimits limits; limits.max_payload_len = 7; @@ -164,5 +194,43 @@ TEST(RtpPacketizerVideoGeneric, EXPECT_THAT(payload_sizes, SizeIs(5)); } +TEST(RtpPacketizerSplitAboutEqually, GivesNonZeroPayloadLengthEachPacket) { + RtpPacketizer::PayloadSizeLimits limits; + limits.max_payload_len = 600; + limits.first_packet_reduction_len = 500; + limits.last_packet_reduction_len = 550; + + // Naive implementation would split 1450 payload + 1050 reduction bytes into 5 + // packets 500 bytes each, thus leaving first packet zero bytes and even less + // to last packet. + std::vector payload_sizes = + RtpPacketizer::SplitAboutEqually(1450, limits); + + EXPECT_EQ(Sum(payload_sizes), 1450u); + EXPECT_THAT(payload_sizes, Each(Gt(0u))); +} + +TEST(RtpPacketizerSplitAboutEqually, + OnePacketWhenExtraSpaceIsEnoughForSumOfFirstAndLastPacketReductions) { + RtpPacketizer::PayloadSizeLimits limits; + limits.max_payload_len = 30; + limits.first_packet_reduction_len = 6; + limits.last_packet_reduction_len = 4; + + EXPECT_THAT(RtpPacketizer::SplitAboutEqually(20, limits), ElementsAre(20)); +} + +TEST(RtpPacketizerSplitAboutEqually, + TwoPacketsWhenExtraSpaceIsTooSmallForSumOfFirstAndLastPacketReductions) { + RtpPacketizer::PayloadSizeLimits limits; + limits.max_payload_len = 29; + limits.first_packet_reduction_len = 6; + limits.last_packet_reduction_len = 4; + + // First packet needs two more extra bytes compared to last one, + // so should have two less payload bytes. + EXPECT_THAT(RtpPacketizer::SplitAboutEqually(20, limits), ElementsAre(9, 11)); +} + } // namespace } // namespace webrtc