Makes dynamic pacer select paddig target based on rate.
Today when the pacing debt is cleared, we blindly ask for 50 bytes of
padding, which is above a static magic number for RTX payload padding.
Instead, we should adjust the target size based on the current padding
rate. The old pacer sort-of does this, it allows the budget to grow up
to one process interval (usually 5ms).
This CL makes the dynamic pacer also use a duration as target, by
default 5ms to match old pacer but with a trial to allow tweaking it.
This will be important for good behavior due to
https://bugs.chromium.org/p/webrtc/issues/detail?id=11508
Bug: webrtc:10809
Change-Id: I9c14acc5730c6e2e0d7821adf5fb058b8d5487c6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/173687
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31091}
diff --git a/modules/pacing/pacing_controller.cc b/modules/pacing/pacing_controller.cc
index 14feacf..f21e637 100644
--- a/modules/pacing/pacing_controller.cc
+++ b/modules/pacing/pacing_controller.cc
@@ -20,6 +20,7 @@
#include "modules/pacing/interval_budget.h"
#include "modules/utility/include/process_thread.h"
#include "rtc_base/checks.h"
+#include "rtc_base/experiments/field_trial_parser.h"
#include "rtc_base/logging.h"
#include "rtc_base/time_utils.h"
#include "system_wrappers/include/clock.h"
@@ -33,10 +34,9 @@
// The maximum debt level, in terms of time, capped when sending packets.
constexpr TimeDelta kMaxDebtInTime = TimeDelta::Millis(500);
constexpr TimeDelta kMaxElapsedTime = TimeDelta::Seconds(2);
-constexpr DataSize kDefaultPaddingTarget = DataSize::Bytes(50);
// Upper cap on process interval, in case process has not been called in a long
-// time.
+// time. Applies only to periodic mode.
constexpr TimeDelta kMaxProcessingInterval = TimeDelta::Millis(30);
constexpr int kFirstPriority = 0;
@@ -51,6 +51,14 @@
return absl::StartsWith(field_trials.Lookup(key), "Enabled");
}
+TimeDelta GetDynamicPaddingTarget(const WebRtcKeyValueConfig& field_trials) {
+ FieldTrialParameter<TimeDelta> padding_target("timedelta",
+ TimeDelta::Millis(5));
+ ParseFieldTrial({&padding_target},
+ field_trials.Lookup("WebRTC-Pacer-DynamicPaddingTarget"));
+ return padding_target.Get();
+}
+
int GetPriorityForType(RtpPacketMediaType type) {
// Lower number takes priority over higher.
switch (type) {
@@ -102,6 +110,7 @@
IsEnabled(*field_trials_, "WebRTC-Pacer-SmallFirstProbePacket")),
ignore_transport_overhead_(
IsEnabled(*field_trials_, "WebRTC-Pacer-IgnoreTransportOverhead")),
+ padding_target_duration_(GetDynamicPaddingTarget(*field_trials_)),
min_packet_limit_(kDefaultMinPacketLimit),
transport_overhead_per_packet_(DataSize::Zero()),
last_timestamp_(clock_->CurrentTime()),
@@ -605,7 +614,7 @@
return DataSize::Bytes(padding_budget_.bytes_remaining());
} else if (padding_rate_ > DataRate::Zero() &&
padding_debt_ == DataSize::Zero()) {
- return kDefaultPaddingTarget;
+ return padding_target_duration_ * padding_rate_;
}
return DataSize::Zero();
}
diff --git a/modules/pacing/pacing_controller.h b/modules/pacing/pacing_controller.h
index 4ffcbd3..27f1614 100644
--- a/modules/pacing/pacing_controller.h
+++ b/modules/pacing/pacing_controller.h
@@ -181,6 +181,9 @@
const bool pace_audio_;
const bool small_first_probe_packet_;
const bool ignore_transport_overhead_;
+ // In dynamic mode, indicates the target size when requesting padding,
+ // expressed as a duration in order to adjust for varying padding rate.
+ const TimeDelta padding_target_duration_;
TimeDelta min_packet_limit_;
diff --git a/modules/pacing/pacing_controller_unittest.cc b/modules/pacing/pacing_controller_unittest.cc
index 3226c02..fa23da7 100644
--- a/modules/pacing/pacing_controller_unittest.cc
+++ b/modules/pacing/pacing_controller_unittest.cc
@@ -184,7 +184,9 @@
class PacingControllerTest
: public ::testing::TestWithParam<PacingController::ProcessMode> {
protected:
- PacingControllerTest() : clock_(123456) {
+ PacingControllerTest() : clock_(123456) {}
+
+ void SetUp() override {
srand(0);
// Need to initialize PacingController after we initialize clock.
pacer_ = std::make_unique<PacingController>(&clock_, &callback_, nullptr,
@@ -1983,6 +1985,46 @@
EXPECT_EQ(pacer_->NextSendTime() - clock_.CurrentTime(), kPacketPacingTime);
}
+TEST_P(PacingControllerTest, PaddingTargetAccountsForPaddingRate) {
+ if (PeriodicProcess()) {
+ // This test applies only when NOT using interval budget.
+ return;
+ }
+
+ // Re-init pacer with an explicitly set padding target of 10ms;
+ const TimeDelta kPaddingTarget = TimeDelta::Millis(10);
+ ScopedFieldTrials field_trials(
+ "WebRTC-Pacer-DynamicPaddingTarget/timedelta:10ms/");
+ SetUp();
+
+ const uint32_t kSsrc = 12345;
+ const DataRate kPacingDataRate = DataRate::KilobitsPerSec(125);
+ const DataSize kPacketSize = DataSize::Bytes(130);
+
+ uint32_t sequnce_number = 1;
+
+ // Start with pacing and padding rate equal.
+ pacer_->SetPacingRates(kPacingDataRate, kPacingDataRate);
+
+ // Send a single packet.
+ SendAndExpectPacket(RtpPacketMediaType::kVideo, kSsrc, sequnce_number++,
+ clock_.TimeInMilliseconds(), kPacketSize.bytes());
+ AdvanceTimeAndProcess();
+ ::testing::Mock::VerifyAndClearExpectations(&callback_);
+
+ size_t expected_padding_target_bytes =
+ (kPaddingTarget * kPacingDataRate).bytes();
+ EXPECT_CALL(callback_, SendPadding(expected_padding_target_bytes))
+ .WillOnce(Return(expected_padding_target_bytes));
+ AdvanceTimeAndProcess();
+
+ // Half the padding rate - expect half the padding target.
+ pacer_->SetPacingRates(kPacingDataRate, kPacingDataRate / 2);
+ EXPECT_CALL(callback_, SendPadding(expected_padding_target_bytes / 2))
+ .WillOnce(Return(expected_padding_target_bytes / 2));
+ AdvanceTimeAndProcess();
+}
+
INSTANTIATE_TEST_SUITE_P(
WithAndWithoutIntervalBudget,
PacingControllerTest,
