in RtpPacketizers separate case 'frame fits into single packet'.
Assumption extra needed bytes for single packet needs is sum
of extra bytes for first and last packet
moved up to RTPSenderVideo from individual packetizers.
There it can be fixed.
Bug: webrtc:9868
Change-Id: I24c80ffa5c174afd3fe3e92fa86ef75560bb961e
Reviewed-on: https://webrtc-review.googlesource.com/c/105662
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25160}diff --git a/modules/rtp_rtcp/source/rtp_format.cc b/modules/rtp_rtcp/source/rtp_format.cc
index 13ec0af..69a91a2 100644
--- a/modules/rtp_rtcp/source/rtp_format.cc
+++ b/modules/rtp_rtcp/source/rtp_format.cc
@@ -63,6 +63,11 @@
RTC_DCHECK_GE(limits.last_packet_reduction_len, 0);
std::vector<int> 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 @@
// 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 9fad4cf..44e21e0 100644
--- a/modules/rtp_rtcp/source/rtp_format.h
+++ b/modules/rtp_rtcp/source/rtp_format.h
@@ -30,6 +30,8 @@
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<RtpPacketizer> Create(
VideoCodecType type,
diff --git a/modules/rtp_rtcp/source/rtp_format_h264.cc b/modules/rtp_rtcp/source/rtp_format_h264.cc
index 9793eba..a60457d 100644
--- a/modules/rtp_rtcp/source/rtp_format_h264.cc
+++ b/modules/rtp_rtcp/source/rtp_format_h264.cc
@@ -186,9 +186,11 @@
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 @@
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 @@
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 @@
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 edf907a..aeab813 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 @@
NoFragmentation(frame));
std::vector<RtpPacketToSend> 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<uint16_t> fua_header;
std::vector<int> payload_sizes;
@@ -422,6 +422,7 @@
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 @@
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 a79c434..ae1b5b0 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,
- 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, 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, 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, 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 4419d1a..dae3151 100644
--- a/modules/rtp_rtcp/source/rtp_format_vp9.cc
+++ b/modules/rtp_rtcp/source/rtp_format_vp9.cc
@@ -451,6 +451,7 @@
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 e8f0ea5..2bcaea7 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_video.cc
@@ -422,6 +422,16 @@
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<RtpGenericFrameDescriptorExtension>() &&
@@ -451,12 +461,8 @@
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 =
