diff --git a/modules/pacing/task_queue_paced_sender.cc b/modules/pacing/task_queue_paced_sender.cc index 4ba249582c..bfe97922ac 100644 --- a/modules/pacing/task_queue_paced_sender.cc +++ b/modules/pacing/task_queue_paced_sender.cc @@ -13,13 +13,11 @@ #include #include -#include "absl/memory/memory.h" #include "api/task_queue/pending_task_safety_flag.h" #include "api/transport/network_types.h" #include "rtc_base/checks.h" #include "rtc_base/experiments/field_trial_parser.h" #include "rtc_base/experiments/field_trial_units.h" -#include "rtc_base/system/unused.h" #include "rtc_base/trace_event.h" namespace webrtc { @@ -28,9 +26,6 @@ namespace { constexpr const char* kBurstyPacerFieldTrial = "WebRTC-BurstyPacer"; -constexpr const char* kSlackedTaskQueuePacedSenderFieldTrial = - "WebRTC-SlackedTaskQueuePacedSender"; - } // namespace const int TaskQueuePacedSender::kNoPacketHoldback = -1; @@ -41,16 +36,6 @@ TaskQueuePacedSender::BurstyPacerFlags::BurstyPacerFlags( ParseFieldTrial({&burst}, field_trials.Lookup(kBurstyPacerFieldTrial)); } -TaskQueuePacedSender::SlackedPacerFlags::SlackedPacerFlags( - const FieldTrialsView& field_trials) - : allow_low_precision("Enabled"), - max_low_precision_expected_queue_time("max_queue_time"), - send_burst_interval("send_burst_interval") { - ParseFieldTrial({&allow_low_precision, &max_low_precision_expected_queue_time, - &send_burst_interval}, - field_trials.Lookup(kSlackedTaskQueuePacedSenderFieldTrial)); -} - TaskQueuePacedSender::TaskQueuePacedSender( Clock* clock, PacingController::PacketSender* packet_sender, @@ -61,13 +46,8 @@ TaskQueuePacedSender::TaskQueuePacedSender( absl::optional burst_interval) : clock_(clock), bursty_pacer_flags_(field_trials), - slacked_pacer_flags_(field_trials), - max_hold_back_window_(slacked_pacer_flags_.allow_low_precision - ? PacingController::kMinSleepTime - : max_hold_back_window), - max_hold_back_window_in_packets_(slacked_pacer_flags_.allow_low_precision - ? 0 - : max_hold_back_window_in_packets), + max_hold_back_window_(max_hold_back_window), + max_hold_back_window_in_packets_(max_hold_back_window_in_packets), pacing_controller_(clock, packet_sender, field_trials), next_process_time_(Timestamp::MinusInfinity()), is_started_(false), @@ -79,13 +59,6 @@ TaskQueuePacedSender::TaskQueuePacedSender( // There are multiple field trials that can affect burst. If multiple bursts // are specified we pick the largest of the values. absl::optional burst = bursty_pacer_flags_.burst.GetOptional(); - if (slacked_pacer_flags_.allow_low_precision && - slacked_pacer_flags_.send_burst_interval) { - TimeDelta slacked_burst = slacked_pacer_flags_.send_burst_interval.Value(); - if (!burst.has_value() || burst.value() < slacked_burst) { - burst = slacked_burst; - } - } // If not overriden by an experiment, the burst is specified by the // `burst_interval` argument. if (!burst.has_value()) { @@ -320,32 +293,7 @@ void TaskQueuePacedSender::MaybeProcessPackets( if (next_process_time_.IsMinusInfinity() || next_process_time_ > next_send_time) { // Prefer low precision if allowed and not probing. - TaskQueueBase::DelayPrecision precision = - slacked_pacer_flags_.allow_low_precision && - !pacing_controller_.IsProbing() - ? TaskQueueBase::DelayPrecision::kLow - : TaskQueueBase::DelayPrecision::kHigh; - // Check for cases where we need high precision. - if (precision == TaskQueueBase::DelayPrecision::kLow) { - auto& packets_per_type = - pacing_controller_.SizeInPacketsPerRtpPacketMediaType(); - bool audio_or_retransmission_packets_in_queue = - packets_per_type[static_cast(RtpPacketMediaType::kAudio)] > - 0 || - packets_per_type[static_cast( - RtpPacketMediaType::kRetransmission)] > 0; - bool queue_time_too_large = - slacked_pacer_flags_.max_low_precision_expected_queue_time && - pacing_controller_.ExpectedQueueTime() >= - slacked_pacer_flags_.max_low_precision_expected_queue_time - .Value(); - if (audio_or_retransmission_packets_in_queue || queue_time_too_large) { - precision = TaskQueueBase::DelayPrecision::kHigh; - } - } - - task_queue_.TaskQueueForDelayedTasks()->PostDelayedTaskWithPrecision( - precision, + task_queue_.TaskQueueForDelayedTasks()->PostDelayedHighPrecisionTask( task_queue_.MaybeSafeTask( safety_.flag(), [this, next_send_time]() { MaybeProcessPackets(next_send_time); }), diff --git a/modules/pacing/task_queue_paced_sender.h b/modules/pacing/task_queue_paced_sender.h index ea335fd8e3..4d5169561e 100644 --- a/modules/pacing/task_queue_paced_sender.h +++ b/modules/pacing/task_queue_paced_sender.h @@ -150,26 +150,7 @@ class TaskQueuePacedSender : public RtpPacketPacer, public RtpPacketSender { FieldTrialOptional burst; }; const BurstyPacerFlags bursty_pacer_flags_; - struct SlackedPacerFlags { - // Parses `kSlackedTaskQueuePacedSenderFieldTrial`. Example: - // --force-fieldtrials=WebRTC-SlackedTaskQueuePacedSender/Enabled,max_queue_time:75ms/ - explicit SlackedPacerFlags(const FieldTrialsView& field_trials); - // When "Enabled", delayed tasks invoking MaybeProcessPackets() are - // scheduled using low precision instead of high precision, resulting in - // less idle wake ups and packets being sent in bursts if the `task_queue_` - // implementation supports slack. When probing, high precision is used - // regardless to ensure good bandwidth estimation. - FieldTrialFlag allow_low_precision; - // Controlled via the "max_queue_time" experiment argument. If set, uses - // high precision scheduling of MaybeProcessPackets() whenever the expected - // queue time is greater than or equal to this value. - FieldTrialOptional max_low_precision_expected_queue_time; - // Controlled via "send_burst_interval" experiment argument. If set, the - // pacer is allowed to build up a packet "debt" that correspond to - // approximately the send rate during the specified interval. - FieldTrialOptional send_burst_interval; - }; - const SlackedPacerFlags slacked_pacer_flags_; + // The holdback window prevents too frequent delayed MaybeProcessPackets() // calls. These are only applicable if `allow_low_precision` is false. const TimeDelta max_hold_back_window_; diff --git a/modules/pacing/task_queue_paced_sender_unittest.cc b/modules/pacing/task_queue_paced_sender_unittest.cc index 69c7b9b7ef..23a72ce4ff 100644 --- a/modules/pacing/task_queue_paced_sender_unittest.cc +++ b/modules/pacing/task_queue_paced_sender_unittest.cc @@ -811,103 +811,5 @@ TEST_P(TaskQueuePacedSenderTest, Stats) { EXPECT_TRUE(pacer.ExpectedQueueTime().IsZero()); } -// TODO(webrtc:14502): Rewrite these tests if the functionality is needed if -// pacing is done on the worker thread. -TEST(TaskQueuePacedSenderTest, HighPrecisionPacingWhenSlackIsDisabled) { - ScopedKeyValueConfig trials("WebRTC-SlackedTaskQueuePacedSender/Disabled/"); - - GlobalSimulatedTimeController time_controller(Timestamp::Millis(1234)); - TaskQueueWithFakePrecisionFactory task_queue_factory( - time_controller.GetTaskQueueFactory()); - - MockPacketRouter packet_router; - TaskQueuePacedSender pacer( - time_controller.GetClock(), &packet_router, trials, &task_queue_factory, - PacingController::kMinSleepTime, TaskQueuePacedSender::kNoPacketHoldback); - - // Send enough packets (covering one second) that pacing is triggered, i.e. - // delayed tasks being scheduled. - static constexpr size_t kPacketsToSend = 42; - static constexpr DataRate kPacingRate = - DataRate::BitsPerSec(kDefaultPacketSize * 8 * kPacketsToSend); - pacer.SetPacingRates(kPacingRate, DataRate::Zero()); - pacer.EnsureStarted(); - pacer.EnqueuePackets( - GeneratePackets(RtpPacketMediaType::kVideo, kPacketsToSend)); - // Expect all of them to be sent. - size_t packets_sent = 0; - EXPECT_CALL(packet_router, SendPacket) - .WillRepeatedly( - [&](std::unique_ptr packet, - const PacedPacketInfo& cluster_info) { ++packets_sent; }); - time_controller.AdvanceTime(TimeDelta::Seconds(1)); - EXPECT_EQ(packets_sent, kPacketsToSend); - - // Expect pacing to make use of high precision. - EXPECT_EQ(task_queue_factory.delayed_low_precision_count(), 0); - EXPECT_GT(task_queue_factory.delayed_high_precision_count(), 0); - - // Create probe cluster which is also high precision. - pacer.CreateProbeClusters( - {{.at_time = time_controller.GetClock()->CurrentTime(), - .target_data_rate = kPacingRate, - .target_duration = TimeDelta::Millis(15), - .target_probe_count = 4, - .id = 123}}); - pacer.EnqueuePackets(GeneratePackets(RtpPacketMediaType::kVideo, 1)); - time_controller.AdvanceTime(TimeDelta::Seconds(1)); - EXPECT_EQ(task_queue_factory.delayed_low_precision_count(), 0); - EXPECT_GT(task_queue_factory.delayed_high_precision_count(), 0); -} - -// TODO(webrtc:14502): Rewrite these tests if the functionality is needed if -// pacing is done on the worker thread. -TEST(TaskQueuePacedSenderTest, LowPrecisionPacingWhenSlackIsEnabled) { - ScopedKeyValueConfig trials("WebRTC-SlackedTaskQueuePacedSender/Enabled/"); - - GlobalSimulatedTimeController time_controller(Timestamp::Millis(1234)); - TaskQueueWithFakePrecisionFactory task_queue_factory( - time_controller.GetTaskQueueFactory()); - - MockPacketRouter packet_router; - TaskQueuePacedSender pacer( - time_controller.GetClock(), &packet_router, trials, &task_queue_factory, - PacingController::kMinSleepTime, TaskQueuePacedSender::kNoPacketHoldback); - - // Send enough packets (covering one second) that pacing is triggered, i.e. - // delayed tasks being scheduled. - static constexpr size_t kPacketsToSend = 42; - static constexpr DataRate kPacingRate = - DataRate::BitsPerSec(kDefaultPacketSize * 8 * kPacketsToSend); - pacer.SetPacingRates(kPacingRate, DataRate::Zero()); - pacer.EnsureStarted(); - pacer.EnqueuePackets( - GeneratePackets(RtpPacketMediaType::kVideo, kPacketsToSend)); - // Expect all of them to be sent. - size_t packets_sent = 0; - EXPECT_CALL(packet_router, SendPacket) - .WillRepeatedly( - [&](std::unique_ptr packet, - const PacedPacketInfo& cluster_info) { ++packets_sent; }); - time_controller.AdvanceTime(TimeDelta::Seconds(1)); - EXPECT_EQ(packets_sent, kPacketsToSend); - - // Expect pacing to make use of low precision. - EXPECT_GT(task_queue_factory.delayed_low_precision_count(), 0); - EXPECT_EQ(task_queue_factory.delayed_high_precision_count(), 0); - - // Create probe cluster, which uses high precision despite regular pacing - // being low precision. - pacer.CreateProbeClusters( - {{.at_time = time_controller.GetClock()->CurrentTime(), - .target_data_rate = kPacingRate, - .target_duration = TimeDelta::Millis(15), - .target_probe_count = 4, - .id = 123}}); - pacer.EnqueuePackets(GeneratePackets(RtpPacketMediaType::kVideo, 1)); - time_controller.AdvanceTime(TimeDelta::Seconds(1)); - EXPECT_GT(task_queue_factory.delayed_high_precision_count(), 0); -} - } // namespace test } // namespace webrtc