diff --git a/modules/rtp_rtcp/source/rtp_format.cc b/modules/rtp_rtcp/source/rtp_format.cc index 13ec0afe9f..69a91a2228 100644 --- a/modules/rtp_rtcp/source/rtp_format.cc +++ b/modules/rtp_rtcp/source/rtp_format.cc @@ -63,6 +63,11 @@ std::vector RtpPacketizer::SplitAboutEqually( RTC_DCHECK_GE(limits.last_packet_reduction_len, 0); std::vector result; + if (limits.max_payload_len >= + limits.single_packet_reduction_len + payload_len) { + result.push_back(payload_len); + return 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. @@ -77,6 +82,10 @@ std::vector RtpPacketizer::SplitAboutEqually( // Integer divisions with rounding up. int num_packets_left = (total_bytes + limits.max_payload_len - 1) / limits.max_payload_len; + if (num_packets_left == 1) { + // Single packet is a special case handled above. + num_packets_left = 2; + } if (payload_len < num_packets_left) { // Edge case where limits force to have more packets than there are payload diff --git a/modules/rtp_rtcp/source/rtp_format.h b/modules/rtp_rtcp/source/rtp_format.h index 9fad4cf8f9..44e21e0738 100644 --- a/modules/rtp_rtcp/source/rtp_format.h +++ b/modules/rtp_rtcp/source/rtp_format.h @@ -30,6 +30,8 @@ class RtpPacketizer { int max_payload_len = 1200; int first_packet_reduction_len = 0; int last_packet_reduction_len = 0; + // Reduction len for packet that is first & last at the same time. + int single_packet_reduction_len = 0; }; static std::unique_ptr Create( VideoCodecType type, diff --git a/modules/rtp_rtcp/source/rtp_format_h264.cc b/modules/rtp_rtcp/source/rtp_format_h264.cc index 9793ebae25..a60457d186 100644 --- a/modules/rtp_rtcp/source/rtp_format_h264.cc +++ b/modules/rtp_rtcp/source/rtp_format_h264.cc @@ -186,9 +186,11 @@ bool RtpPacketizerH264::GeneratePackets( case H264PacketizationMode::NonInterleaved: int fragment_len = input_fragments_[i].length; int single_packet_capacity = limits_.max_payload_len; - if (i == 0) + if (input_fragments_.size() == 1) + single_packet_capacity -= limits_.single_packet_reduction_len; + else if (i == 0) single_packet_capacity -= limits_.first_packet_reduction_len; - if (i + 1 == input_fragments_.size()) + else if (i + 1 == input_fragments_.size()) single_packet_capacity -= limits_.last_packet_reduction_len; if (fragment_len > single_packet_capacity) { @@ -211,7 +213,10 @@ bool RtpPacketizerH264::PacketizeFuA(size_t fragment_index) { PayloadSizeLimits limits = limits_; // Leave room for the FU-A header. limits.max_payload_len -= kFuAHeaderSize; - // Ignore first/last packet reductions unless it is first/last fragment. + // Ignore single/first/last packet reductions unless it is single/first/last + // fragment. + if (input_fragments_.size() != 1) + limits.single_packet_reduction_len = 0; if (fragment_index != 0) limits.first_packet_reduction_len = 0; if (fragment_index != input_fragments_.size() - 1) @@ -243,17 +248,31 @@ bool RtpPacketizerH264::PacketizeFuA(size_t fragment_index) { size_t RtpPacketizerH264::PacketizeStapA(size_t fragment_index) { // Aggregate fragments into one packet (STAP-A). size_t payload_size_left = limits_.max_payload_len; - if (fragment_index == 0) + if (input_fragments_.size() == 1) + payload_size_left -= limits_.single_packet_reduction_len; + else if (fragment_index == 0) payload_size_left -= limits_.first_packet_reduction_len; int aggregated_fragments = 0; size_t fragment_headers_length = 0; const Fragment* fragment = &input_fragments_[fragment_index]; RTC_CHECK_GE(payload_size_left, fragment->length); ++num_packets_left_; - while (payload_size_left >= fragment->length + fragment_headers_length && - (fragment_index + 1 < input_fragments_.size() || - payload_size_left >= fragment->length + fragment_headers_length + - limits_.last_packet_reduction_len)) { + + auto payload_size_needed = [&] { + size_t fragment_size = fragment->length + fragment_headers_length; + if (input_fragments_.size() == 1) { + // Single fragment, single packet, payload_size_left already adjusted + // with limits_.single_packet_reduction_len. + return fragment_size; + } + if (fragment_index == input_fragments_.size() - 1) { + // Last fragment, so StrapA might be the last packet. + return fragment_size + limits_.last_packet_reduction_len; + } + return fragment_size; + }; + + while (payload_size_left >= payload_size_needed()) { RTC_CHECK_GT(fragment->length, 0); packets_.push(PacketUnit(*fragment, aggregated_fragments == 0, false, true, fragment->buffer[0])); @@ -282,9 +301,11 @@ size_t RtpPacketizerH264::PacketizeStapA(size_t fragment_index) { bool RtpPacketizerH264::PacketizeSingleNalu(size_t fragment_index) { // Add a single NALU to the queue, no aggregation. size_t payload_size_left = limits_.max_payload_len; - if (fragment_index == 0) + if (input_fragments_.size() == 1) + payload_size_left -= limits_.single_packet_reduction_len; + else if (fragment_index == 0) payload_size_left -= limits_.first_packet_reduction_len; - if (fragment_index + 1 == input_fragments_.size()) + else if (fragment_index + 1 == input_fragments_.size()) payload_size_left -= limits_.last_packet_reduction_len; const Fragment* fragment = &input_fragments_[fragment_index]; if (payload_size_left < fragment->length) { diff --git a/modules/rtp_rtcp/source/rtp_format_h264_unittest.cc b/modules/rtp_rtcp/source/rtp_format_h264_unittest.cc index edf907aac8..aeab813b53 100644 --- a/modules/rtp_rtcp/source/rtp_format_h264_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_format_h264_unittest.cc @@ -390,7 +390,7 @@ std::vector TestFua(size_t frame_payload_size, NoFragmentation(frame)); std::vector packets = FetchAllPackets(&packetizer); - RTC_CHECK_GE(packets.size(), 2); // Single packet indicates it is not FuA. + EXPECT_GE(packets.size(), 2u); // Single packet indicates it is not FuA. std::vector fua_header; std::vector payload_sizes; @@ -422,6 +422,7 @@ TEST(RtpPacketizerH264Test, FUAWithFirstPacketReduction) { RtpPacketizer::PayloadSizeLimits limits; limits.max_payload_len = 1200; limits.first_packet_reduction_len = 4; + limits.single_packet_reduction_len = 4; EXPECT_THAT(TestFua(1198, limits), ElementsAre(597, 601)); } @@ -429,14 +430,14 @@ TEST(RtpPacketizerH264Test, FUAWithLastPacketReduction) { RtpPacketizer::PayloadSizeLimits limits; limits.max_payload_len = 1200; limits.last_packet_reduction_len = 4; + limits.single_packet_reduction_len = 4; EXPECT_THAT(TestFua(1198, limits), ElementsAre(601, 597)); } -TEST(RtpPacketizerH264Test, FUAWithFirstAndLastPacketReduction) { +TEST(RtpPacketizerH264Test, FUAWithSinglePacketReduction) { RtpPacketizer::PayloadSizeLimits limits; limits.max_payload_len = 1199; - limits.first_packet_reduction_len = 100; - limits.last_packet_reduction_len = 100; + limits.single_packet_reduction_len = 200; EXPECT_THAT(TestFua(1000, limits), ElementsAre(500, 500)); } diff --git a/modules/rtp_rtcp/source/rtp_format_unittest.cc b/modules/rtp_rtcp/source/rtp_format_unittest.cc index a79c434b62..ae1b5b054a 100644 --- a/modules/rtp_rtcp/source/rtp_format_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_format_unittest.cc @@ -199,21 +199,32 @@ TEST(RtpPacketizerSplitAboutEqually, GivesNonZeroPayloadLengthEachPacket) { } TEST(RtpPacketizerSplitAboutEqually, - OnePacketWhenExtraSpaceIsEnoughForSumOfFirstAndLastPacketReductions) { + IgnoresFirstAndLastPacketReductionWhenPayloadFitsIntoSinglePacket) { RtpPacketizer::PayloadSizeLimits limits; limits.max_payload_len = 30; - limits.first_packet_reduction_len = 6; - limits.last_packet_reduction_len = 4; + limits.first_packet_reduction_len = 29; + limits.last_packet_reduction_len = 29; + limits.single_packet_reduction_len = 10; EXPECT_THAT(RtpPacketizer::SplitAboutEqually(20, limits), ElementsAre(20)); } TEST(RtpPacketizerSplitAboutEqually, - TwoPacketsWhenExtraSpaceIsTooSmallForSumOfFirstAndLastPacketReductions) { + OnePacketWhenExtraSpaceIsEnoughForSinglePacketReduction) { + RtpPacketizer::PayloadSizeLimits limits; + limits.max_payload_len = 30; + limits.single_packet_reduction_len = 10; + + EXPECT_THAT(RtpPacketizer::SplitAboutEqually(20, limits), ElementsAre(20)); +} + +TEST(RtpPacketizerSplitAboutEqually, + TwoPacketsWhenExtraSpaceIsTooSmallForSinglePacketReduction) { RtpPacketizer::PayloadSizeLimits limits; limits.max_payload_len = 29; - limits.first_packet_reduction_len = 6; - limits.last_packet_reduction_len = 4; + limits.first_packet_reduction_len = 3; + limits.last_packet_reduction_len = 1; + limits.single_packet_reduction_len = 10; // First packet needs two more extra bytes compared to last one, // so should have two less payload bytes. @@ -246,8 +257,7 @@ TEST(RtpPacketizerSplitAboutEqually, RejectsZeroLastPacketLen) { TEST(RtpPacketizerSplitAboutEqually, CantPutSinglePayloadByteInTwoPackets) { RtpPacketizer::PayloadSizeLimits limits; limits.max_payload_len = 10; - limits.first_packet_reduction_len = 6; - limits.last_packet_reduction_len = 4; + limits.single_packet_reduction_len = 10; EXPECT_THAT(RtpPacketizer::SplitAboutEqually(1, limits), IsEmpty()); } @@ -255,8 +265,7 @@ TEST(RtpPacketizerSplitAboutEqually, CantPutSinglePayloadByteInTwoPackets) { TEST(RtpPacketizerSplitAboutEqually, CanPutTwoPayloadBytesInTwoPackets) { RtpPacketizer::PayloadSizeLimits limits; limits.max_payload_len = 10; - limits.first_packet_reduction_len = 6; - limits.last_packet_reduction_len = 4; + limits.single_packet_reduction_len = 10; EXPECT_THAT(RtpPacketizer::SplitAboutEqually(2, limits), ElementsAre(1, 1)); } @@ -264,8 +273,7 @@ TEST(RtpPacketizerSplitAboutEqually, CanPutTwoPayloadBytesInTwoPackets) { TEST(RtpPacketizerSplitAboutEqually, CanPutSinglePayloadByteInOnePacket) { RtpPacketizer::PayloadSizeLimits limits; limits.max_payload_len = 11; - limits.first_packet_reduction_len = 6; - limits.last_packet_reduction_len = 4; + limits.single_packet_reduction_len = 10; EXPECT_THAT(RtpPacketizer::SplitAboutEqually(1, limits), ElementsAre(1)); } diff --git a/modules/rtp_rtcp/source/rtp_format_vp9.cc b/modules/rtp_rtcp/source/rtp_format_vp9.cc index 4419d1a30c..dae3151e3b 100644 --- a/modules/rtp_rtcp/source/rtp_format_vp9.cc +++ b/modules/rtp_rtcp/source/rtp_format_vp9.cc @@ -451,6 +451,7 @@ RtpPacketizerVp9::RtpPacketizerVp9(rtc::ArrayView payload, remaining_payload_(payload) { limits.max_payload_len -= header_size_; limits.first_packet_reduction_len += first_packet_extra_header_size_; + limits.single_packet_reduction_len += first_packet_extra_header_size_; payload_sizes_ = SplitAboutEqually(payload.size(), limits); current_packet_ = payload_sizes_.begin(); diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc index e8f0ea5c8d..2bcaea78d4 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -422,6 +422,16 @@ bool RTPSenderVideo::SendVideo(enum VideoCodecType video_type, limits.last_packet_reduction_len = last_packet->headers_size() - middle_packet->headers_size(); + // TODO(bugs.webrtc.org/9868, bugs.webrtc.org/7990): Calculate correctly: + // Single packet might need more space for extensions than sum of first and + // last packet reductions when two byte header extension is used. It also may + // need more even for one-byte header extension because of padding. + // e.g. if middle_packet uses 5 bytes for extensions, it is padded to 8. + // if first_packet and last_packet use 2 bytes for extensions each, reduction + // would be 0 for both of them, but single packet would need 4 extra bytes. + limits.single_packet_reduction_len = + limits.first_packet_reduction_len + limits.last_packet_reduction_len; + RTPVideoHeader minimized_video_header; const RTPVideoHeader* packetize_video_header = video_header; if (first_packet->HasExtension() && @@ -451,12 +461,8 @@ bool RTPSenderVideo::SendVideo(enum VideoCodecType video_type, packet = create_packet(); AddRtpHeaderExtensions(*video_header, frame_type, set_video_rotation, /*first=*/true, /*last=*/true, packet.get()); - // TODO(bugs.webrtc.org/7990): Revisit this case when two byte header - // extension are implemented because then single packet might need more - // space for extensions than sum of first and last packet reductions. - expected_payload_capacity = limits.max_payload_len - - limits.first_packet_reduction_len - - limits.last_packet_reduction_len; + expected_payload_capacity = + limits.max_payload_len - limits.single_packet_reduction_len; } else if (i == 0) { packet = std::move(first_packet); expected_payload_capacity =