diff --git a/modules/rtp_rtcp/source/rtp_format.cc b/modules/rtp_rtcp/source/rtp_format.cc index 8a6e8f1578..e9668336a1 100644 --- a/modules/rtp_rtcp/source/rtp_format.cc +++ b/modules/rtp_rtcp/source/rtp_format.cc @@ -59,26 +59,42 @@ std::unique_ptr RtpPacketizer::Create( } } -std::vector RtpPacketizer::SplitAboutEqually( - size_t payload_len, +std::vector RtpPacketizer::SplitAboutEqually( + int 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); + RTC_DCHECK_GT(payload_len, 0); + // First or last packet larger than normal are unsupported. + RTC_DCHECK_GE(limits.first_packet_reduction_len, 0); + RTC_DCHECK_GE(limits.last_packet_reduction_len, 0); + std::vector result; + if (limits.max_payload_len - limits.first_packet_reduction_len < 1 || + limits.max_payload_len - limits.last_packet_reduction_len < 1) { + // Capacity is not enough to put a single byte into one of the packets. + return result; + } // 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; + int total_bytes = payload_len + limits.first_packet_reduction_len + + limits.last_packet_reduction_len; // Integer divisions with rounding up. - size_t num_packets_left = + int num_packets_left = (total_bytes + limits.max_payload_len - 1) / limits.max_payload_len; - size_t bytes_per_packet = total_bytes / num_packets_left; - size_t num_larger_packets = total_bytes % num_packets_left; - size_t remaining_data = payload_len; - std::vector result; + if (payload_len < num_packets_left) { + // Edge case where limits force to have more packets than there are payload + // bytes. This may happen when there is single byte of payload that can't be + // put into single packet if + // first_packet_reduction + last_packet_reduction >= max_payload_len. + return result; + } + + int bytes_per_packet = total_bytes / num_packets_left; + int num_larger_packets = total_bytes % num_packets_left; + int remaining_data = payload_len; + result.reserve(num_packets_left); bool first_packet = true; while (remaining_data > 0) { @@ -86,7 +102,7 @@ std::vector RtpPacketizer::SplitAboutEqually( // per-packet payload size when needed. if (num_packets_left == num_larger_packets) ++bytes_per_packet; - size_t current_packet_bytes = bytes_per_packet; + int 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; diff --git a/modules/rtp_rtcp/source/rtp_format.h b/modules/rtp_rtcp/source/rtp_format.h index 7a1f494d7c..9fad4cf8f9 100644 --- a/modules/rtp_rtcp/source/rtp_format.h +++ b/modules/rtp_rtcp/source/rtp_format.h @@ -27,9 +27,9 @@ class RtpPacketToSend; 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; + int max_payload_len = 1200; + int first_packet_reduction_len = 0; + int last_packet_reduction_len = 0; }; static std::unique_ptr Create( VideoCodecType type, @@ -51,8 +51,9 @@ class RtpPacketizer { virtual bool NextPacket(RtpPacketToSend* packet) = 0; // Split payload_len into sum of integers with respect to |limits|. - static std::vector SplitAboutEqually(size_t payload_len, - const PayloadSizeLimits& limits); + // Returns empty vector on failure. + static std::vector SplitAboutEqually(int payload_len, + const PayloadSizeLimits& limits); }; // TODO(sprang): Update the depacketizer to return a std::unqie_ptr with a copy diff --git a/modules/rtp_rtcp/source/rtp_format_unittest.cc b/modules/rtp_rtcp/source/rtp_format_unittest.cc index 8fb780ec2c..a79c434b62 100644 --- a/modules/rtp_rtcp/source/rtp_format_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_format_unittest.cc @@ -31,7 +31,7 @@ using ::testing::SizeIs; // adjustement provided by limits, // i.e. last packet expected to be smaller than 'average' by reduction_len. int EffectivePacketsSizeDifference( - std::vector sizes, + std::vector sizes, const RtpPacketizer::PayloadSizeLimits& limits) { // Account for larger last packet header. sizes.back() += limits.last_packet_reduction_len; @@ -41,7 +41,7 @@ int EffectivePacketsSizeDifference( return *minmax.second - *minmax.first; } -size_t Sum(const std::vector& sizes) { +int Sum(const std::vector& sizes) { return std::accumulate(sizes.begin(), sizes.end(), 0); } @@ -50,8 +50,7 @@ TEST(RtpPacketizerSplitAboutEqually, AllPacketsAreEqualSumToPayloadLen) { limits.max_payload_len = 5; limits.last_packet_reduction_len = 2; - std::vector payload_sizes = - RtpPacketizer::SplitAboutEqually(13, limits); + std::vector payload_sizes = RtpPacketizer::SplitAboutEqually(13, limits); EXPECT_THAT(Sum(payload_sizes), 13); } @@ -61,8 +60,7 @@ TEST(RtpPacketizerSplitAboutEqually, AllPacketsAreEqualRespectsMaxPayloadSize) { limits.max_payload_len = 5; limits.last_packet_reduction_len = 2; - std::vector payload_sizes = - RtpPacketizer::SplitAboutEqually(13, limits); + std::vector payload_sizes = RtpPacketizer::SplitAboutEqually(13, limits); EXPECT_THAT(payload_sizes, Each(Le(limits.max_payload_len))); } @@ -73,8 +71,7 @@ TEST(RtpPacketizerSplitAboutEqually, limits.max_payload_len = 5; limits.first_packet_reduction_len = 2; - std::vector payload_sizes = - RtpPacketizer::SplitAboutEqually(13, limits); + std::vector payload_sizes = RtpPacketizer::SplitAboutEqually(13, limits); ASSERT_THAT(payload_sizes, Not(IsEmpty())); EXPECT_EQ(payload_sizes.front() + limits.first_packet_reduction_len, @@ -87,8 +84,7 @@ TEST(RtpPacketizerSplitAboutEqually, limits.max_payload_len = 5; limits.last_packet_reduction_len = 2; - std::vector payload_sizes = - RtpPacketizer::SplitAboutEqually(13, limits); + std::vector payload_sizes = RtpPacketizer::SplitAboutEqually(13, limits); ASSERT_THAT(payload_sizes, Not(IsEmpty())); EXPECT_LE(payload_sizes.back() + limits.last_packet_reduction_len, @@ -100,8 +96,7 @@ TEST(RtpPacketizerSplitAboutEqually, AllPacketsAreEqualInSize) { limits.max_payload_len = 5; limits.last_packet_reduction_len = 2; - std::vector payload_sizes = - RtpPacketizer::SplitAboutEqually(13, limits); + std::vector payload_sizes = RtpPacketizer::SplitAboutEqually(13, limits); EXPECT_EQ(EffectivePacketsSizeDifference(payload_sizes, limits), 0); } @@ -112,8 +107,7 @@ TEST(RtpPacketizerSplitAboutEqually, limits.max_payload_len = 5; limits.last_packet_reduction_len = 2; - std::vector payload_sizes = - RtpPacketizer::SplitAboutEqually(13, limits); + std::vector payload_sizes = RtpPacketizer::SplitAboutEqually(13, limits); // Computed by hand. 3 packets would have exactly capacity 3*5-2=13 // (max length - for each packet minus last packet reduction). EXPECT_THAT(payload_sizes, SizeIs(3)); @@ -124,8 +118,7 @@ TEST(RtpPacketizerSplitAboutEqually, SomePacketsAreSmallerSumToPayloadLen) { limits.max_payload_len = 7; limits.last_packet_reduction_len = 5; - std::vector payload_sizes = - RtpPacketizer::SplitAboutEqually(28, limits); + std::vector payload_sizes = RtpPacketizer::SplitAboutEqually(28, limits); EXPECT_THAT(Sum(payload_sizes), 28); } @@ -136,8 +129,7 @@ TEST(RtpPacketizerSplitAboutEqually, limits.max_payload_len = 7; limits.last_packet_reduction_len = 5; - std::vector payload_sizes = - RtpPacketizer::SplitAboutEqually(28, limits); + std::vector payload_sizes = RtpPacketizer::SplitAboutEqually(28, limits); EXPECT_THAT(payload_sizes, Each(Le(limits.max_payload_len))); } @@ -148,8 +140,7 @@ TEST(RtpPacketizerSplitAboutEqually, limits.max_payload_len = 7; limits.first_packet_reduction_len = 5; - std::vector payload_sizes = - RtpPacketizer::SplitAboutEqually(28, limits); + std::vector payload_sizes = RtpPacketizer::SplitAboutEqually(28, limits); EXPECT_LE(payload_sizes.front() + limits.first_packet_reduction_len, limits.max_payload_len); @@ -161,8 +152,7 @@ TEST(RtpPacketizerSplitAboutEqually, limits.max_payload_len = 7; limits.last_packet_reduction_len = 5; - std::vector payload_sizes = - RtpPacketizer::SplitAboutEqually(28, limits); + std::vector payload_sizes = RtpPacketizer::SplitAboutEqually(28, limits); EXPECT_LE(payload_sizes.back(), limits.max_payload_len - limits.last_packet_reduction_len); @@ -174,8 +164,7 @@ TEST(RtpPacketizerSplitAboutEqually, limits.max_payload_len = 7; limits.last_packet_reduction_len = 5; - std::vector payload_sizes = - RtpPacketizer::SplitAboutEqually(28, limits); + std::vector payload_sizes = RtpPacketizer::SplitAboutEqually(28, limits); EXPECT_LE(EffectivePacketsSizeDifference(payload_sizes, limits), 1); } @@ -186,8 +175,7 @@ TEST(RtpPacketizerSplitAboutEqually, limits.max_payload_len = 7; limits.last_packet_reduction_len = 5; - std::vector payload_sizes = - RtpPacketizer::SplitAboutEqually(24, limits); + std::vector payload_sizes = RtpPacketizer::SplitAboutEqually(24, limits); // Computed by hand. 4 packets would have capacity 4*7-5=23 (max length - // for each packet minus last packet reduction). // 5 packets is enough for kPayloadSize. @@ -203,11 +191,11 @@ TEST(RtpPacketizerSplitAboutEqually, GivesNonZeroPayloadLengthEachPacket) { // 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 = + std::vector payload_sizes = RtpPacketizer::SplitAboutEqually(1450, limits); - EXPECT_EQ(Sum(payload_sizes), 1450u); - EXPECT_THAT(payload_sizes, Each(Gt(0u))); + EXPECT_EQ(Sum(payload_sizes), 1450); + EXPECT_THAT(payload_sizes, Each(Gt(0))); } TEST(RtpPacketizerSplitAboutEqually, @@ -232,5 +220,55 @@ TEST(RtpPacketizerSplitAboutEqually, EXPECT_THAT(RtpPacketizer::SplitAboutEqually(20, limits), ElementsAre(9, 11)); } +TEST(RtpPacketizerSplitAboutEqually, RejectsZeroMaxPayloadLen) { + RtpPacketizer::PayloadSizeLimits limits; + limits.max_payload_len = 0; + + EXPECT_THAT(RtpPacketizer::SplitAboutEqually(20, limits), IsEmpty()); +} + +TEST(RtpPacketizerSplitAboutEqually, RejectsZeroFirstPacketLen) { + RtpPacketizer::PayloadSizeLimits limits; + limits.max_payload_len = 5; + limits.first_packet_reduction_len = 5; + + EXPECT_THAT(RtpPacketizer::SplitAboutEqually(20, limits), IsEmpty()); +} + +TEST(RtpPacketizerSplitAboutEqually, RejectsZeroLastPacketLen) { + RtpPacketizer::PayloadSizeLimits limits; + limits.max_payload_len = 5; + limits.last_packet_reduction_len = 5; + + EXPECT_THAT(RtpPacketizer::SplitAboutEqually(20, limits), IsEmpty()); +} + +TEST(RtpPacketizerSplitAboutEqually, CantPutSinglePayloadByteInTwoPackets) { + RtpPacketizer::PayloadSizeLimits limits; + limits.max_payload_len = 10; + limits.first_packet_reduction_len = 6; + limits.last_packet_reduction_len = 4; + + EXPECT_THAT(RtpPacketizer::SplitAboutEqually(1, limits), IsEmpty()); +} + +TEST(RtpPacketizerSplitAboutEqually, CanPutTwoPayloadBytesInTwoPackets) { + RtpPacketizer::PayloadSizeLimits limits; + limits.max_payload_len = 10; + limits.first_packet_reduction_len = 6; + limits.last_packet_reduction_len = 4; + + EXPECT_THAT(RtpPacketizer::SplitAboutEqually(2, limits), ElementsAre(1, 1)); +} + +TEST(RtpPacketizerSplitAboutEqually, CanPutSinglePayloadByteInOnePacket) { + RtpPacketizer::PayloadSizeLimits limits; + limits.max_payload_len = 11; + limits.first_packet_reduction_len = 6; + limits.last_packet_reduction_len = 4; + + EXPECT_THAT(RtpPacketizer::SplitAboutEqually(1, limits), ElementsAre(1)); +} + } // namespace } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtp_format_video_generic.h b/modules/rtp_rtcp/source/rtp_format_video_generic.h index 7d14886cac..03509f5041 100644 --- a/modules/rtp_rtcp/source/rtp_format_video_generic.h +++ b/modules/rtp_rtcp/source/rtp_format_video_generic.h @@ -52,8 +52,8 @@ class RtpPacketizerGeneric : public RtpPacketizer { uint8_t header_[3]; size_t header_size_; rtc::ArrayView remaining_payload_; - std::vector payload_sizes_; - std::vector::const_iterator current_packet_; + std::vector payload_sizes_; + std::vector::const_iterator current_packet_; RTC_DISALLOW_COPY_AND_ASSIGN(RtpPacketizerGeneric); }; diff --git a/modules/rtp_rtcp/source/rtp_format_video_generic_unittest.cc b/modules/rtp_rtcp/source/rtp_format_video_generic_unittest.cc index 8cf1ffacba..aa2829b3bc 100644 --- a/modules/rtp_rtcp/source/rtp_format_video_generic_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_format_video_generic_unittest.cc @@ -33,10 +33,9 @@ using ::testing::Contains; constexpr RtpPacketizer::PayloadSizeLimits kNoSizeLimits; -std::vector NextPacketFillPayloadSizes( - RtpPacketizerGeneric* packetizer) { +std::vector NextPacketFillPayloadSizes(RtpPacketizerGeneric* packetizer) { RtpPacketToSend packet(nullptr); - std::vector result; + std::vector result; while (packetizer->NextPacket(&packet)) { result.push_back(packet.payload_size()); } @@ -52,7 +51,7 @@ TEST(RtpPacketizerVideoGeneric, RespectsMaxPayloadSize) { RtpPacketizerGeneric packetizer(kPayload, limits, RTPVideoHeader(), kVideoFrameKey); - std::vector payload_sizes = NextPacketFillPayloadSizes(&packetizer); + std::vector payload_sizes = NextPacketFillPayloadSizes(&packetizer); EXPECT_THAT(payload_sizes, Each(Le(limits.max_payload_len))); } @@ -66,7 +65,7 @@ TEST(RtpPacketizerVideoGeneric, UsesMaxPayloadSize) { RtpPacketizerGeneric packetizer(kPayload, limits, RTPVideoHeader(), kVideoFrameKey); - std::vector payload_sizes = NextPacketFillPayloadSizes(&packetizer); + std::vector payload_sizes = NextPacketFillPayloadSizes(&packetizer); // With kPayloadSize > max_payload_len^2, there should be packets that use // all the payload, otherwise it is possible to use less packets. @@ -94,7 +93,7 @@ TEST(RtpPacketizerVideoGeneric, WritesExtendedHeaderWhenPictureIdIsSet) { } TEST(RtpPacketizerVideoGeneric, RespectsMaxPayloadSizeWithExtendedHeader) { - const size_t kPayloadSize = 50; + const int kPayloadSize = 50; const uint8_t kPayload[kPayloadSize] = {}; RtpPacketizer::PayloadSizeLimits limits; @@ -104,13 +103,13 @@ TEST(RtpPacketizerVideoGeneric, RespectsMaxPayloadSizeWithExtendedHeader) { RtpPacketizerGeneric packetizer(kPayload, limits, rtp_video_header, kVideoFrameKey); - std::vector payload_sizes = NextPacketFillPayloadSizes(&packetizer); + std::vector payload_sizes = NextPacketFillPayloadSizes(&packetizer); EXPECT_THAT(payload_sizes, Each(Le(limits.max_payload_len))); } TEST(RtpPacketizerVideoGeneric, UsesMaxPayloadSizeWithExtendedHeader) { - const size_t kPayloadSize = 50; + const int kPayloadSize = 50; const uint8_t kPayload[kPayloadSize] = {}; RtpPacketizer::PayloadSizeLimits limits; @@ -119,7 +118,7 @@ TEST(RtpPacketizerVideoGeneric, UsesMaxPayloadSizeWithExtendedHeader) { rtp_video_header.generic.emplace().frame_id = 37; RtpPacketizerGeneric packetizer(kPayload, limits, rtp_video_header, kVideoFrameKey); - std::vector payload_sizes = NextPacketFillPayloadSizes(&packetizer); + std::vector payload_sizes = NextPacketFillPayloadSizes(&packetizer); // With kPayloadSize > max_payload_len^2, there should be packets that use // all the payload, otherwise it is possible to use less packets. @@ -127,7 +126,7 @@ TEST(RtpPacketizerVideoGeneric, UsesMaxPayloadSizeWithExtendedHeader) { } TEST(RtpPacketizerVideoGeneric, FrameIdOver15bitsWrapsAround) { - const size_t kPayloadSize = 13; + const int kPayloadSize = 13; const uint8_t kPayload[kPayloadSize] = {}; RTPVideoHeader rtp_video_header; @@ -146,7 +145,7 @@ TEST(RtpPacketizerVideoGeneric, FrameIdOver15bitsWrapsAround) { } TEST(RtpPacketizerVideoGeneric, NoFrameIdDoesNotWriteExtendedHeader) { - const size_t kPayloadSize = 13; + const int kPayloadSize = 13; const uint8_t kPayload[kPayloadSize] = {}; RtpPacketizerGeneric packetizer(kPayload, kNoSizeLimits, RTPVideoHeader(), diff --git a/modules/rtp_rtcp/source/rtp_format_vp8.cc b/modules/rtp_rtcp/source/rtp_format_vp8.cc index b219dace94..40bc291c7c 100644 --- a/modules/rtp_rtcp/source/rtp_format_vp8.cc +++ b/modules/rtp_rtcp/source/rtp_format_vp8.cc @@ -173,13 +173,6 @@ RtpPacketizerVp8::RtpPacketizerVp8(rtc::ArrayView payload, PayloadSizeLimits limits, const RTPVideoHeaderVP8& hdr_info) : hdr_(BuildHeader(hdr_info)), remaining_payload_(payload) { - if (limits.max_payload_len - limits.last_packet_reduction_len < - hdr_.size() + 1) { - // The provided payload length is not long enough for the payload - // descriptor and one payload byte in the last packet. - current_packet_ = payload_sizes_.begin(); - return; - } limits.max_payload_len -= hdr_.size(); payload_sizes_ = SplitAboutEqually(payload.size(), limits); current_packet_ = payload_sizes_.begin(); diff --git a/modules/rtp_rtcp/source/rtp_format_vp8.h b/modules/rtp_rtcp/source/rtp_format_vp8.h index 2fdd6c5137..e4bc36e3f3 100644 --- a/modules/rtp_rtcp/source/rtp_format_vp8.h +++ b/modules/rtp_rtcp/source/rtp_format_vp8.h @@ -61,8 +61,8 @@ class RtpPacketizerVp8 : public RtpPacketizer { RawHeader hdr_; rtc::ArrayView remaining_payload_; - std::vector payload_sizes_; - std::vector::const_iterator current_packet_; + std::vector payload_sizes_; + std::vector::const_iterator current_packet_; RTC_DISALLOW_COPY_AND_ASSIGN(RtpPacketizerVp8); }; diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc index bf8150d469..f72740e9a6 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -335,20 +335,18 @@ bool RTPSenderVideo::SendVideo(enum VideoCodecType video_type, retransmission_settings = retransmission_settings_; } - size_t packet_capacity = rtp_sender_->MaxRtpPacketSize() - - fec_packet_overhead - - (rtp_sender_->RtxStatus() ? kRtxHeaderSize : 0); + int packet_capacity = rtp_sender_->MaxRtpPacketSize() - fec_packet_overhead - + (rtp_sender_->RtxStatus() ? kRtxHeaderSize : 0); RTC_DCHECK_LE(packet_capacity, rtp_header->capacity()); RTC_DCHECK_GT(packet_capacity, rtp_header->headers_size()); RTC_DCHECK_GT(packet_capacity, last_packet->headers_size()); - size_t max_data_payload_length = packet_capacity - rtp_header->headers_size(); + RtpPacketizer::PayloadSizeLimits limits; + limits.max_payload_len = packet_capacity - rtp_header->headers_size(); + RTC_DCHECK_GE(last_packet->headers_size(), rtp_header->headers_size()); - size_t last_packet_reduction_len = + limits.last_packet_reduction_len = last_packet->headers_size() - rtp_header->headers_size(); - RtpPacketizer::PayloadSizeLimits limits; - limits.max_payload_len = max_data_payload_length; - limits.last_packet_reduction_len = last_packet_reduction_len; std::unique_ptr packetizer = RtpPacketizer::Create( video_type, rtc::MakeArrayView(payload_data, payload_size), limits, *video_header, frame_type, fragmentation); @@ -368,9 +366,10 @@ bool RTPSenderVideo::SendVideo(enum VideoCodecType video_type, : absl::make_unique(*rtp_header); if (!packetizer->NextPacket(packet.get())) return false; - RTC_DCHECK_LE(packet->payload_size(), - last ? max_data_payload_length - last_packet_reduction_len - : max_data_payload_length); + RTC_DCHECK_LE( + packet->payload_size(), + last ? limits.max_payload_len - limits.last_packet_reduction_len + : limits.max_payload_len); if (!rtp_sender_->AssignSequenceNumber(packet.get())) return false;