Add field trial for fast retransmissions.
This adds a (default off) flag which makes retransmissions be processed
immediately, just like audio packets normally are.
This might increase send rates and thus losses in some cases, but will
also reduce retranmission delays especially when timer slack or bursting
is used. Usefuleness TBD via experiment.
Bug: chromium:1354491
Change-Id: Icaa83125bfb30826ce72e6e786963d411e05ea57
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/272483
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37926}
diff --git a/modules/pacing/pacing_controller.cc b/modules/pacing/pacing_controller.cc
index 942d4ab..125db84 100644
--- a/modules/pacing/pacing_controller.cc
+++ b/modules/pacing/pacing_controller.cc
@@ -67,6 +67,8 @@
pace_audio_(IsEnabled(field_trials_, "WebRTC-Pacer-BlockAudio")),
ignore_transport_overhead_(
IsEnabled(field_trials_, "WebRTC-Pacer-IgnoreTransportOverhead")),
+ fast_retransmissions_(
+ IsEnabled(field_trials_, "WebRTC-Pacer-FastRetransmissions")),
min_packet_limit_(kDefaultMinPacketLimit),
transport_overhead_per_packet_(DataSize::Zero()),
send_burst_interval_(TimeDelta::Zero()),
@@ -313,13 +315,11 @@
}
}
- // Not pacing audio, if leading packet is audio its target send
- // time is the time at which it was enqueued.
- Timestamp unpaced_audio_time =
- pace_audio_ ? Timestamp::PlusInfinity()
- : packet_queue_.LeadingAudioPacketEnqueueTime();
- if (unpaced_audio_time.IsFinite()) {
- return unpaced_audio_time;
+ // If queue contains a packet which should not be paced, its target send time
+ // is the time at which it was enqueued.
+ Timestamp unpaced_send_time = NextUnpacedSendTime();
+ if (unpaced_send_time.IsFinite()) {
+ return unpaced_send_time;
}
if (congested_ || !seen_first_packet_) {
@@ -582,11 +582,8 @@
}
// First, check if there is any reason _not_ to send the next queued packet.
-
- // Unpaced audio packets and probes are exempted from send checks.
- bool unpaced_audio_packet =
- !pace_audio_ && packet_queue_.LeadingAudioPacketEnqueueTime().IsFinite();
- if (!unpaced_audio_packet && !is_probe) {
+ // Unpaced packets and probes are exempted from send checks.
+ if (NextUnpacedSendTime().IsInfinite() && !is_probe) {
if (congested_) {
// Don't send anything if congested.
return nullptr;
@@ -667,4 +664,23 @@
}
}
+Timestamp PacingController::NextUnpacedSendTime() const {
+ if (!pace_audio_) {
+ Timestamp leading_audio_send_time =
+ packet_queue_.LeadingPacketEnqueueTime(RtpPacketMediaType::kAudio);
+ if (leading_audio_send_time.IsFinite()) {
+ return leading_audio_send_time;
+ }
+ }
+ if (fast_retransmissions_) {
+ Timestamp leading_retransmission_send_time =
+ packet_queue_.LeadingPacketEnqueueTime(
+ RtpPacketMediaType::kRetransmission);
+ if (leading_retransmission_send_time.IsFinite()) {
+ return leading_retransmission_send_time;
+ }
+ }
+ return Timestamp::MinusInfinity();
+}
+
} // namespace webrtc
diff --git a/modules/pacing/pacing_controller.h b/modules/pacing/pacing_controller.h
index 5689780..9899fcd 100644
--- a/modules/pacing/pacing_controller.h
+++ b/modules/pacing/pacing_controller.h
@@ -176,6 +176,12 @@
Timestamp CurrentTime() const;
+ // Helper methods for packet that may not be paced. Returns a finite Timestamp
+ // if a packet type is configured to not be paced and the packet queue has at
+ // least one packet of that type. Otherwise returns
+ // Timestamp::MinusInfinity().
+ Timestamp NextUnpacedSendTime() const;
+
Clock* const clock_;
PacketSender* const packet_sender_;
const FieldTrialsView& field_trials_;
@@ -184,6 +190,7 @@
const bool send_padding_if_silent_;
const bool pace_audio_;
const bool ignore_transport_overhead_;
+ const bool fast_retransmissions_;
TimeDelta min_packet_limit_;
DataSize transport_overhead_per_packet_;
diff --git a/modules/pacing/pacing_controller_unittest.cc b/modules/pacing/pacing_controller_unittest.cc
index 0248f93..db6a22b 100644
--- a/modules/pacing/pacing_controller_unittest.cc
+++ b/modules/pacing/pacing_controller_unittest.cc
@@ -2061,5 +2061,31 @@
EXPECT_EQ(pacer.pacing_rate(), kNominalPacingRate);
}
+TEST_F(PacingControllerTest, BudgetDoesNotAffectRetransmissionInsTrial) {
+ const DataSize kPacketSize = DataSize::Bytes(1000);
+
+ EXPECT_CALL(callback_, SendPadding).Times(0);
+ const test::ExplicitKeyValueConfig trials(
+ "WebRTC-Pacer-FastRetransmissions/Enabled/");
+ PacingController pacer(&clock_, &callback_, trials);
+ pacer.SetPacingRates(kTargetRate, /*padding_rate=*/DataRate::Zero());
+
+ // Send a video packet so that we have a bit debt.
+ pacer.EnqueuePacket(BuildPacket(RtpPacketMediaType::kVideo, kVideoSsrc,
+ /*sequence_number=*/1,
+ /*capture_time=*/1, kPacketSize.bytes()));
+ EXPECT_CALL(callback_, SendPacket);
+ pacer.ProcessPackets();
+ EXPECT_GT(pacer.NextSendTime(), clock_.CurrentTime());
+
+ // A retransmission packet should still be immediately processed.
+ EXPECT_CALL(callback_, SendPacket);
+ pacer.EnqueuePacket(BuildPacket(RtpPacketMediaType::kRetransmission,
+ kVideoSsrc,
+ /*sequence_number=*/1,
+ /*capture_time=*/1, kPacketSize.bytes()));
+ pacer.ProcessPackets();
+}
+
} // namespace
} // namespace webrtc
diff --git a/modules/pacing/prioritized_packet_queue.cc b/modules/pacing/prioritized_packet_queue.cc
index 261d313..8b2c7c1 100644
--- a/modules/pacing/prioritized_packet_queue.cc
+++ b/modules/pacing/prioritized_packet_queue.cc
@@ -81,10 +81,10 @@
return true;
}
-Timestamp PrioritizedPacketQueue::StreamQueue::LeadingAudioPacketEnqueueTime()
- const {
- RTC_DCHECK(!packets_[kAudioPrioLevel].empty());
- return packets_[kAudioPrioLevel].begin()->enqueue_time;
+Timestamp PrioritizedPacketQueue::StreamQueue::LeadingPacketEnqueueTime(
+ int priority_level) const {
+ RTC_DCHECK(!packets_[priority_level].empty());
+ return packets_[priority_level].begin()->enqueue_time;
}
Timestamp PrioritizedPacketQueue::StreamQueue::LastEnqueueTime() const {
@@ -225,13 +225,14 @@
return size_packets_per_media_type_;
}
-Timestamp PrioritizedPacketQueue::LeadingAudioPacketEnqueueTime() const {
- if (streams_by_prio_[kAudioPrioLevel].empty()) {
+Timestamp PrioritizedPacketQueue::LeadingPacketEnqueueTime(
+ RtpPacketMediaType type) const {
+ const int priority_level = GetPriorityForType(type);
+ if (streams_by_prio_[priority_level].empty()) {
return Timestamp::MinusInfinity();
}
- return streams_by_prio_[kAudioPrioLevel]
- .front()
- ->LeadingAudioPacketEnqueueTime();
+ return streams_by_prio_[priority_level].front()->LeadingPacketEnqueueTime(
+ priority_level);
}
Timestamp PrioritizedPacketQueue::OldestEnqueueTime() const {
diff --git a/modules/pacing/prioritized_packet_queue.h b/modules/pacing/prioritized_packet_queue.h
index f730a37d..3b5748f 100644
--- a/modules/pacing/prioritized_packet_queue.h
+++ b/modules/pacing/prioritized_packet_queue.h
@@ -57,9 +57,10 @@
const std::array<int, kNumMediaTypes>& SizeInPacketsPerRtpPacketMediaType()
const;
- // The enqueue time of the next audio packet this queue will return via the
- // Pop() method. If queue has no audio packets, returns MinusInfinity().
- Timestamp LeadingAudioPacketEnqueueTime() const;
+ // The enqueue time of the next packet this queue will return via the Pop()
+ // method, for the given packet type. If queue has no packets, of that type,
+ // returns Timestamp::MinusInfinity().
+ Timestamp LeadingPacketEnqueueTime(RtpPacketMediaType type) const;
// Enqueue time of the oldest packet in the queue,
// Timestamp::MinusInfinity() if queue is empty.
@@ -110,7 +111,7 @@
bool HasPacketsAtPrio(int priority_level) const;
bool IsEmpty() const;
- Timestamp LeadingAudioPacketEnqueueTime() const;
+ Timestamp LeadingPacketEnqueueTime(int priority_level) const;
Timestamp LastEnqueueTime() const;
private:
diff --git a/modules/pacing/prioritized_packet_queue_unittest.cc b/modules/pacing/prioritized_packet_queue_unittest.cc
index 6e27ff0..5e79e7b 100644
--- a/modules/pacing/prioritized_packet_queue_unittest.cc
+++ b/modules/pacing/prioritized_packet_queue_unittest.cc
@@ -214,21 +214,39 @@
EXPECT_EQ(queue.AverageQueueTime(), TimeDelta::Millis(750));
}
-TEST(PrioritizedPacketQueue, ReportsLeadingAudioEnqueueTime) {
+TEST(PrioritizedPacketQueue, ReportsLeadingPacketEnqueueTime) {
PrioritizedPacketQueue queue(/*creation_time=*/Timestamp::Zero());
- EXPECT_EQ(queue.LeadingAudioPacketEnqueueTime(), Timestamp::MinusInfinity());
+ EXPECT_EQ(queue.LeadingPacketEnqueueTime(RtpPacketMediaType::kAudio),
+ Timestamp::MinusInfinity());
+ EXPECT_EQ(queue.LeadingPacketEnqueueTime(RtpPacketMediaType::kVideo),
+ Timestamp::MinusInfinity());
queue.Push(Timestamp::Millis(10),
CreatePacket(RtpPacketMediaType::kVideo, /*seq=*/1));
- EXPECT_EQ(queue.LeadingAudioPacketEnqueueTime(), Timestamp::MinusInfinity());
+ EXPECT_EQ(queue.LeadingPacketEnqueueTime(RtpPacketMediaType::kAudio),
+ Timestamp::MinusInfinity());
+ EXPECT_EQ(queue.LeadingPacketEnqueueTime(RtpPacketMediaType::kVideo),
+ Timestamp::Millis(10));
queue.Push(Timestamp::Millis(20),
CreatePacket(RtpPacketMediaType::kAudio, /*seq=*/2));
- EXPECT_EQ(queue.LeadingAudioPacketEnqueueTime(), Timestamp::Millis(20));
+ EXPECT_EQ(queue.LeadingPacketEnqueueTime(RtpPacketMediaType::kAudio),
+ Timestamp::Millis(20));
+ EXPECT_EQ(queue.LeadingPacketEnqueueTime(RtpPacketMediaType::kVideo),
+ Timestamp::Millis(10));
queue.Pop(); // Pop audio packet.
- EXPECT_EQ(queue.LeadingAudioPacketEnqueueTime(), Timestamp::MinusInfinity());
+ EXPECT_EQ(queue.LeadingPacketEnqueueTime(RtpPacketMediaType::kAudio),
+ Timestamp::MinusInfinity());
+ EXPECT_EQ(queue.LeadingPacketEnqueueTime(RtpPacketMediaType::kVideo),
+ Timestamp::Millis(10));
+
+ queue.Pop(); // Pop video packet.
+ EXPECT_EQ(queue.LeadingPacketEnqueueTime(RtpPacketMediaType::kAudio),
+ Timestamp::MinusInfinity());
+ EXPECT_EQ(queue.LeadingPacketEnqueueTime(RtpPacketMediaType::kVideo),
+ Timestamp::MinusInfinity());
}
TEST(PrioritizedPacketQueue,
