diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc index f1a6201ce7..569ba9d730 100644 --- a/call/rtp_transport_controller_send.cc +++ b/call/rtp_transport_controller_send.cc @@ -59,14 +59,12 @@ TargetRateConstraints ConvertConstraints(const BitrateConstraints& contraints, contraints.start_bitrate_bps, clock); } -bool IsEnabled(const WebRtcKeyValueConfig* trials, absl::string_view key) { - RTC_DCHECK(trials != nullptr); - return absl::StartsWith(trials->Lookup(key), "Enabled"); +bool IsEnabled(const WebRtcKeyValueConfig& trials, absl::string_view key) { + return absl::StartsWith(trials.Lookup(key), "Enabled"); } -bool IsDisabled(const WebRtcKeyValueConfig* trials, absl::string_view key) { - RTC_DCHECK(trials != nullptr); - return absl::StartsWith(trials->Lookup(key), "Disabled"); +bool IsDisabled(const WebRtcKeyValueConfig& trials, absl::string_view key) { + return absl::StartsWith(trials.Lookup(key), "Disabled"); } bool IsRelayed(const rtc::NetworkRoute& route) { @@ -76,12 +74,12 @@ bool IsRelayed(const rtc::NetworkRoute& route) { } // namespace RtpTransportControllerSend::PacerSettings::PacerSettings( - const WebRtcKeyValueConfig* trials) + const WebRtcKeyValueConfig& trials) : tq_disabled("Disabled"), holdback_window("holdback_window", PacingController::kMinSleepTime), holdback_packets("holdback_packets", -1) { ParseFieldTrial({&tq_disabled, &holdback_window, &holdback_packets}, - trials->Lookup("WebRTC-TaskQueuePacer")); + trials.Lookup("WebRTC-TaskQueuePacer")); } RtpTransportControllerSend::RtpTransportControllerSend( @@ -92,7 +90,7 @@ RtpTransportControllerSend::RtpTransportControllerSend( const BitrateConstraints& bitrate_config, std::unique_ptr process_thread, TaskQueueFactory* task_queue_factory, - const WebRtcKeyValueConfig* trials) + const WebRtcKeyValueConfig& trials) : clock_(clock), event_log_(event_log), bitrate_configurator_(bitrate_config), @@ -135,18 +133,18 @@ RtpTransportControllerSend::RtpTransportControllerSend( task_queue_(task_queue_factory->CreateTaskQueue( "rtp_send_controller", TaskQueueFactory::Priority::NORMAL)), - field_trials_(*trials) { + field_trials_(trials) { ParseFieldTrial({&relay_bandwidth_cap_}, - trials->Lookup("WebRTC-Bwe-NetworkRouteConstraints")); + trials.Lookup("WebRTC-Bwe-NetworkRouteConstraints")); initial_config_.constraints = ConvertConstraints(bitrate_config, clock_); initial_config_.event_log = event_log; - initial_config_.key_value_config = trials; + initial_config_.key_value_config = &trials; RTC_DCHECK(bitrate_config.start_bitrate_bps > 0); pacer()->SetPacingRates( DataRate::BitsPerSec(bitrate_config.start_bitrate_bps), DataRate::Zero()); - if (absl::StartsWith(trials->Lookup("WebRTC-LazyPacerStart"), "Disabled")) { + if (absl::StartsWith(trials.Lookup("WebRTC-LazyPacerStart"), "Disabled")) { EnsureStarted(); } } diff --git a/call/rtp_transport_controller_send.h b/call/rtp_transport_controller_send.h index e45ab21cda..471ee7f5f4 100644 --- a/call/rtp_transport_controller_send.h +++ b/call/rtp_transport_controller_send.h @@ -59,7 +59,7 @@ class RtpTransportControllerSend final const BitrateConstraints& bitrate_config, std::unique_ptr process_thread, TaskQueueFactory* task_queue_factory, - const WebRtcKeyValueConfig* trials); + const WebRtcKeyValueConfig& trials); ~RtpTransportControllerSend() override; RtpTransportControllerSend(const RtpTransportControllerSend&) = delete; @@ -132,7 +132,7 @@ class RtpTransportControllerSend final private: struct PacerSettings { - explicit PacerSettings(const WebRtcKeyValueConfig* trials); + explicit PacerSettings(const WebRtcKeyValueConfig& trials); bool use_task_queue_pacer() const { return !tq_disabled.Get(); } diff --git a/call/rtp_transport_controller_send_factory.h b/call/rtp_transport_controller_send_factory.h index a857ca7e6f..bda0be04af 100644 --- a/call/rtp_transport_controller_send_factory.h +++ b/call/rtp_transport_controller_send_factory.h @@ -25,10 +25,11 @@ class RtpTransportControllerSendFactory const RtpTransportConfig& config, Clock* clock, std::unique_ptr process_thread) override { + RTC_CHECK(config.trials); return std::make_unique( clock, config.event_log, config.network_state_predictor_factory, config.network_controller_factory, config.bitrate_config, - std::move(process_thread), config.task_queue_factory, config.trials); + std::move(process_thread), config.task_queue_factory, *config.trials); } virtual ~RtpTransportControllerSendFactory() {} diff --git a/call/rtp_video_sender_unittest.cc b/call/rtp_video_sender_unittest.cc index 7c7d005fa6..7962ce29c9 100644 --- a/call/rtp_video_sender_unittest.cc +++ b/call/rtp_video_sender_unittest.cc @@ -135,7 +135,7 @@ class RtpVideoSenderTestFixture { bitrate_config_, time_controller_.CreateProcessThread("PacerThread"), time_controller_.GetTaskQueueFactory(), - field_trials ? field_trials : &field_trials_), + field_trials ? *field_trials : field_trials_), stats_proxy_(time_controller_.GetClock(), config_, VideoEncoderConfig::ContentType::kRealtimeVideo, diff --git a/modules/pacing/BUILD.gn b/modules/pacing/BUILD.gn index 013c2de599..4064c12151 100644 --- a/modules/pacing/BUILD.gn +++ b/modules/pacing/BUILD.gn @@ -102,9 +102,8 @@ if (rtc_include_tests) { "../../rtc_base:rtc_base_tests_utils", "../../rtc_base/experiments:alr_experiment", "../../system_wrappers", - "../../system_wrappers:field_trial", "../../test:explicit_key_value_config", - "../../test:field_trial", + "../../test:scoped_key_value_config", "../../test:test_support", "../../test/time_controller:time_controller", "../rtp_rtcp", diff --git a/modules/pacing/paced_sender.cc b/modules/pacing/paced_sender.cc index 2b2d064ef4..5eed6c2607 100644 --- a/modules/pacing/paced_sender.cc +++ b/modules/pacing/paced_sender.cc @@ -32,12 +32,11 @@ const float PacedSender::kDefaultPaceMultiplier = 2.5f; PacedSender::PacedSender(Clock* clock, PacketRouter* packet_router, RtcEventLog* event_log, - const WebRtcKeyValueConfig* field_trials, + const WebRtcKeyValueConfig& field_trials, ProcessThread* process_thread) : process_mode_( - (field_trials != nullptr && - absl::StartsWith(field_trials->Lookup("WebRTC-Pacer-DynamicProcess"), - "Enabled")) + absl::StartsWith(field_trials.Lookup("WebRTC-Pacer-DynamicProcess"), + "Enabled") ? PacingController::ProcessMode::kDynamic : PacingController::ProcessMode::kPeriodic), pacing_controller_(clock, diff --git a/modules/pacing/paced_sender.h b/modules/pacing/paced_sender.h index 4a53e0f9ba..88fd79697d 100644 --- a/modules/pacing/paced_sender.h +++ b/modules/pacing/paced_sender.h @@ -58,7 +58,7 @@ class PacedSender : public RtpPacketPacer, public RtpPacketSender { PacedSender(Clock* clock, PacketRouter* packet_router, RtcEventLog* event_log, - const WebRtcKeyValueConfig* field_trials = nullptr, + const WebRtcKeyValueConfig& field_trials, ProcessThread* process_thread = nullptr); ~PacedSender() override; diff --git a/modules/pacing/paced_sender_unittest.cc b/modules/pacing/paced_sender_unittest.cc index 7abd532417..27f373746f 100644 --- a/modules/pacing/paced_sender_unittest.cc +++ b/modules/pacing/paced_sender_unittest.cc @@ -19,7 +19,6 @@ #include "modules/pacing/packet_router.h" #include "modules/utility/include/mock/mock_process_thread.h" #include "system_wrappers/include/clock.h" -#include "test/field_trial.h" #include "test/gmock.h" #include "test/gtest.h" @@ -80,7 +79,7 @@ class PacedSenderTest .WillOnce(SaveArg<0>(&paced_module_)); pacer_ = std::make_unique(&clock_, &callback_, nullptr, - &trials_, &process_thread_); + trials_, &process_thread_); EXPECT_CALL(process_thread_, WakeUp).WillRepeatedly([&](Module* module) { clock_.AdvanceTimeMilliseconds(module->TimeUntilNextProcess()); }); diff --git a/modules/pacing/pacing_controller.cc b/modules/pacing/pacing_controller.cc index 9215462239..9bfe85c0a8 100644 --- a/modules/pacing/pacing_controller.cc +++ b/modules/pacing/pacing_controller.cc @@ -98,22 +98,20 @@ const TimeDelta PacingController::kMinSleepTime = TimeDelta::Millis(1); PacingController::PacingController(Clock* clock, PacketSender* packet_sender, RtcEventLog* event_log, - const WebRtcKeyValueConfig* field_trials, + const WebRtcKeyValueConfig& field_trials, ProcessMode mode) : mode_(mode), clock_(clock), packet_sender_(packet_sender), - fallback_field_trials_( - !field_trials ? std::make_unique() : nullptr), - field_trials_(field_trials ? field_trials : fallback_field_trials_.get()), + field_trials_(field_trials), drain_large_queues_( - !IsDisabled(*field_trials_, "WebRTC-Pacer-DrainQueue")), + !IsDisabled(field_trials_, "WebRTC-Pacer-DrainQueue")), send_padding_if_silent_( - IsEnabled(*field_trials_, "WebRTC-Pacer-PadInSilence")), - pace_audio_(IsEnabled(*field_trials_, "WebRTC-Pacer-BlockAudio")), + IsEnabled(field_trials_, "WebRTC-Pacer-PadInSilence")), + pace_audio_(IsEnabled(field_trials_, "WebRTC-Pacer-BlockAudio")), ignore_transport_overhead_( - IsEnabled(*field_trials_, "WebRTC-Pacer-IgnoreTransportOverhead")), - padding_target_duration_(GetDynamicPaddingTarget(*field_trials_)), + 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()), @@ -124,12 +122,12 @@ PacingController::PacingController(Clock* clock, padding_debt_(DataSize::Zero()), media_rate_(DataRate::Zero()), padding_rate_(DataRate::Zero()), - prober_(*field_trials_), + prober_(field_trials_), probing_send_failure_(false), pacing_bitrate_(DataRate::Zero()), last_process_time_(clock->CurrentTime()), last_send_time_(last_process_time_), - packet_queue_(last_process_time_, field_trials_), + packet_queue_(last_process_time_), packet_counter_(0), congestion_window_size_(DataSize::PlusInfinity()), outstanding_data_(DataSize::Zero()), @@ -142,7 +140,7 @@ PacingController::PacingController(Clock* clock, } FieldTrialParameter min_packet_limit_ms("", min_packet_limit_.ms()); ParseFieldTrial({&min_packet_limit_ms}, - field_trials_->Lookup("WebRTC-Pacer-MinPacketLimitMs")); + field_trials_.Lookup("WebRTC-Pacer-MinPacketLimitMs")); min_packet_limit_ = TimeDelta::Millis(min_packet_limit_ms.Get()); UpdateBudgetWithElapsedTime(min_packet_limit_); } diff --git a/modules/pacing/pacing_controller.h b/modules/pacing/pacing_controller.h index 5d6d26b917..f7c5601c91 100644 --- a/modules/pacing/pacing_controller.h +++ b/modules/pacing/pacing_controller.h @@ -82,7 +82,7 @@ class PacingController { PacingController(Clock* clock, PacketSender* packet_sender, RtcEventLog* event_log, - const WebRtcKeyValueConfig* field_trials, + const WebRtcKeyValueConfig& field_trials, ProcessMode mode); ~PacingController(); @@ -176,8 +176,7 @@ class PacingController { const ProcessMode mode_; Clock* const clock_; PacketSender* const packet_sender_; - const std::unique_ptr fallback_field_trials_; - const WebRtcKeyValueConfig* field_trials_; + const WebRtcKeyValueConfig& field_trials_; const bool drain_large_queues_; const bool send_padding_if_silent_; diff --git a/modules/pacing/pacing_controller_unittest.cc b/modules/pacing/pacing_controller_unittest.cc index a24960d896..af2ce548e0 100644 --- a/modules/pacing/pacing_controller_unittest.cc +++ b/modules/pacing/pacing_controller_unittest.cc @@ -21,7 +21,6 @@ #include "modules/pacing/packet_router.h" #include "system_wrappers/include/clock.h" #include "test/explicit_key_value_config.h" -#include "test/field_trial.h" #include "test/gmock.h" #include "test/gtest.h" @@ -209,13 +208,13 @@ class PacingControllerProbing : public PacingController::PacketSender { class PacingControllerTest : public ::testing::TestWithParam { protected: - PacingControllerTest() : clock_(123456) {} + PacingControllerTest() : clock_(123456), trials_("") {} void SetUp() override { srand(0); // Need to initialize PacingController after we initialize clock. pacer_ = std::make_unique(&clock_, &callback_, nullptr, - nullptr, GetParam()); + trials_, GetParam()); Init(); } @@ -320,6 +319,7 @@ class PacingControllerTest SimulatedClock clock_; ::testing::NiceMock callback_; + ExplicitKeyValueConfig trials_; std::unique_ptr pacer_; }; @@ -364,7 +364,8 @@ class PacingControllerFieldTrialTest }; TEST_P(PacingControllerFieldTrialTest, DefaultNoPaddingInSilence) { - PacingController pacer(&clock_, &callback_, nullptr, nullptr, GetParam()); + const test::ExplicitKeyValueConfig trials(""); + PacingController pacer(&clock_, &callback_, nullptr, trials, GetParam()); pacer.SetPacingRates(kTargetRate, DataRate::Zero()); // Video packet to reset last send time and provide padding data. InsertPacket(&pacer, &video); @@ -378,8 +379,9 @@ TEST_P(PacingControllerFieldTrialTest, DefaultNoPaddingInSilence) { } TEST_P(PacingControllerFieldTrialTest, PaddingInSilenceWithTrial) { - ScopedFieldTrials trial("WebRTC-Pacer-PadInSilence/Enabled/"); - PacingController pacer(&clock_, &callback_, nullptr, nullptr, GetParam()); + const test::ExplicitKeyValueConfig trials( + "WebRTC-Pacer-PadInSilence/Enabled/"); + PacingController pacer(&clock_, &callback_, nullptr, trials, GetParam()); pacer.SetPacingRates(kTargetRate, DataRate::Zero()); // Video packet to reset last send time and provide padding data. InsertPacket(&pacer, &video); @@ -393,9 +395,9 @@ TEST_P(PacingControllerFieldTrialTest, PaddingInSilenceWithTrial) { } TEST_P(PacingControllerFieldTrialTest, CongestionWindowAffectsAudioInTrial) { - ScopedFieldTrials trial("WebRTC-Pacer-BlockAudio/Enabled/"); + const test::ExplicitKeyValueConfig trials("WebRTC-Pacer-BlockAudio/Enabled/"); EXPECT_CALL(callback_, SendPadding).Times(0); - PacingController pacer(&clock_, &callback_, nullptr, nullptr, GetParam()); + PacingController pacer(&clock_, &callback_, nullptr, trials, GetParam()); pacer.SetPacingRates(DataRate::KilobitsPerSec(10000), DataRate::Zero()); pacer.SetCongestionWindow(DataSize::Bytes(video.packet_size - 100)); pacer.UpdateOutstandingData(DataSize::Zero()); @@ -422,7 +424,8 @@ TEST_P(PacingControllerFieldTrialTest, CongestionWindowAffectsAudioInTrial) { TEST_P(PacingControllerFieldTrialTest, DefaultCongestionWindowDoesNotAffectAudio) { EXPECT_CALL(callback_, SendPadding).Times(0); - PacingController pacer(&clock_, &callback_, nullptr, nullptr, GetParam()); + const test::ExplicitKeyValueConfig trials(""); + PacingController pacer(&clock_, &callback_, nullptr, trials, GetParam()); pacer.SetPacingRates(DataRate::BitsPerSec(10000000), DataRate::Zero()); pacer.SetCongestionWindow(DataSize::Bytes(800)); pacer.UpdateOutstandingData(DataSize::Zero()); @@ -437,8 +440,8 @@ TEST_P(PacingControllerFieldTrialTest, } TEST_P(PacingControllerFieldTrialTest, BudgetAffectsAudioInTrial) { - ScopedFieldTrials trial("WebRTC-Pacer-BlockAudio/Enabled/"); - PacingController pacer(&clock_, &callback_, nullptr, nullptr, GetParam()); + ExplicitKeyValueConfig trials("WebRTC-Pacer-BlockAudio/Enabled/"); + PacingController pacer(&clock_, &callback_, nullptr, trials, GetParam()); DataRate pacing_rate = DataRate::BitsPerSec(video.packet_size / 3 * 8 * kProcessIntervalsPerSecond); pacer.SetPacingRates(pacing_rate, DataRate::Zero()); @@ -468,7 +471,8 @@ TEST_P(PacingControllerFieldTrialTest, BudgetAffectsAudioInTrial) { TEST_P(PacingControllerFieldTrialTest, DefaultBudgetDoesNotAffectAudio) { EXPECT_CALL(callback_, SendPadding).Times(0); - PacingController pacer(&clock_, &callback_, nullptr, nullptr, GetParam()); + const test::ExplicitKeyValueConfig trials(""); + PacingController pacer(&clock_, &callback_, nullptr, trials, GetParam()); pacer.SetPacingRates(DataRate::BitsPerSec(video.packet_size / 3 * 8 * kProcessIntervalsPerSecond), DataRate::Zero()); @@ -867,7 +871,7 @@ TEST_P(PacingControllerTest, VerifyAverageBitrateVaryingMediaPayload) { const TimeDelta kAveragingWindowLength = TimeDelta::Seconds(10); PacingControllerPadding callback; pacer_ = std::make_unique(&clock_, &callback, nullptr, - nullptr, GetParam()); + trials_, GetParam()); pacer_->SetProbingEnabled(false); pacer_->SetPacingRates(kTargetRate * kPaceMultiplier, kTargetRate); @@ -1304,7 +1308,7 @@ TEST_P(PacingControllerTest, Pause) { TEST_P(PacingControllerTest, InactiveFromStart) { // Recreate the pacer without the inital time forwarding. pacer_ = std::make_unique(&clock_, &callback_, nullptr, - nullptr, GetParam()); + trials_, GetParam()); pacer_->SetProbingEnabled(false); pacer_->SetPacingRates(kTargetRate * kPaceMultiplier, kTargetRate); @@ -1403,7 +1407,7 @@ TEST_P(PacingControllerTest, ProbingWithInsertedPackets) { PacingControllerProbing packet_sender; pacer_ = std::make_unique(&clock_, &packet_sender, nullptr, - nullptr, GetParam()); + trials_, GetParam()); pacer_->CreateProbeCluster(kFirstClusterRate, /*cluster_id=*/0); pacer_->CreateProbeCluster(kSecondClusterRate, @@ -1463,7 +1467,7 @@ TEST_P(PacingControllerTest, SkipsProbesWhenProcessIntervalTooLarge) { : "WebRTC-Bwe-ProbingBehavior/" "abort_delayed_probes:0,max_probe_delay:2ms/"); pacer_ = std::make_unique(&clock_, &packet_sender, - nullptr, &trials, GetParam()); + nullptr, trials, GetParam()); pacer_->SetPacingRates( DataRate::BitsPerSec(kInitialBitrateBps * kPaceMultiplier), DataRate::BitsPerSec(kInitialBitrateBps)); @@ -1570,7 +1574,7 @@ TEST_P(PacingControllerTest, ProbingWithPaddingSupport) { PacingControllerProbing packet_sender; pacer_ = std::make_unique(&clock_, &packet_sender, nullptr, - nullptr, GetParam()); + trials_, GetParam()); pacer_->CreateProbeCluster(kFirstClusterRate, /*cluster_id=*/0); pacer_->SetPacingRates( @@ -1636,7 +1640,7 @@ TEST_P(PacingControllerTest, ProbeClusterId) { MockPacketSender callback; pacer_ = std::make_unique(&clock_, &callback, nullptr, - nullptr, GetParam()); + trials_, GetParam()); Init(); uint32_t ssrc = 12346; @@ -1693,7 +1697,7 @@ TEST_P(PacingControllerTest, ProbeClusterId) { TEST_P(PacingControllerTest, OwnedPacketPrioritizedOnType) { MockPacketSender callback; pacer_ = std::make_unique(&clock_, &callback, nullptr, - nullptr, GetParam()); + trials_, GetParam()); Init(); // Insert a packet of each type, from low to high priority. Since priority @@ -1740,7 +1744,7 @@ TEST_P(PacingControllerTest, OwnedPacketPrioritizedOnType) { TEST_P(PacingControllerTest, SmallFirstProbePacket) { MockPacketSender callback; pacer_ = std::make_unique(&clock_, &callback, nullptr, - nullptr, GetParam()); + trials_, GetParam()); pacer_->CreateProbeCluster(kFirstClusterRate, /*cluster_id=*/0); pacer_->SetPacingRates(kTargetRate * kPaceMultiplier, DataRate::Zero()); @@ -1899,7 +1903,7 @@ TEST_P(PacingControllerTest, MockPacketSender callback; EXPECT_CALL(callback, SendPacket).Times(::testing::AnyNumber()); pacer_ = std::make_unique(&clock_, &callback, nullptr, - nullptr, GetParam()); + trials_, GetParam()); pacer_->SetAccountForAudioPackets(account_for_audio); // First, saturate the padding budget. @@ -2073,9 +2077,13 @@ TEST_P(PacingControllerTest, PaddingTargetAccountsForPaddingRate) { // Re-init pacer with an explicitly set padding target of 10ms; const TimeDelta kPaddingTarget = TimeDelta::Millis(10); - ScopedFieldTrials field_trials( + ExplicitKeyValueConfig field_trials( "WebRTC-Pacer-DynamicPaddingTarget/timedelta:10ms/"); - SetUp(); + srand(0); + // Need to initialize PacingController after we initialize clock. + pacer_ = std::make_unique(&clock_, &callback_, nullptr, + field_trials, GetParam()); + Init(); const uint32_t kSsrc = 12345; const DataRate kPacingDataRate = DataRate::KilobitsPerSec(125); diff --git a/modules/pacing/round_robin_packet_queue.cc b/modules/pacing/round_robin_packet_queue.cc index ef37e5256b..9014c30cb6 100644 --- a/modules/pacing/round_robin_packet_queue.cc +++ b/modules/pacing/round_robin_packet_queue.cc @@ -107,16 +107,7 @@ RoundRobinPacketQueue::Stream::Stream() : size(DataSize::Zero()), ssrc(0) {} RoundRobinPacketQueue::Stream::Stream(const Stream& stream) = default; RoundRobinPacketQueue::Stream::~Stream() = default; -bool IsEnabled(const WebRtcKeyValueConfig* field_trials, const char* name) { - if (!field_trials) { - return false; - } - return absl::StartsWith(field_trials->Lookup(name), "Enabled"); -} - -RoundRobinPacketQueue::RoundRobinPacketQueue( - Timestamp start_time, - const WebRtcKeyValueConfig* field_trials) +RoundRobinPacketQueue::RoundRobinPacketQueue(Timestamp start_time) : transport_overhead_per_packet_(DataSize::Zero()), time_last_updated_(start_time), paused_(false), diff --git a/modules/pacing/round_robin_packet_queue.h b/modules/pacing/round_robin_packet_queue.h index dd35b90d93..f8255834b3 100644 --- a/modules/pacing/round_robin_packet_queue.h +++ b/modules/pacing/round_robin_packet_queue.h @@ -22,7 +22,6 @@ #include #include "absl/types/optional.h" -#include "api/transport/webrtc_key_value_config.h" #include "api/units/data_size.h" #include "api/units/time_delta.h" #include "api/units/timestamp.h" @@ -34,8 +33,7 @@ namespace webrtc { class RoundRobinPacketQueue { public: - RoundRobinPacketQueue(Timestamp start_time, - const WebRtcKeyValueConfig* field_trials); + explicit RoundRobinPacketQueue(Timestamp start_time); ~RoundRobinPacketQueue(); void Push(int priority, diff --git a/modules/pacing/task_queue_paced_sender.cc b/modules/pacing/task_queue_paced_sender.cc index c392a88720..620a54135e 100644 --- a/modules/pacing/task_queue_paced_sender.cc +++ b/modules/pacing/task_queue_paced_sender.cc @@ -29,6 +29,22 @@ TaskQueuePacedSender::TaskQueuePacedSender( TaskQueueFactory* task_queue_factory, TimeDelta max_hold_back_window, int max_hold_back_window_in_packets) + : TaskQueuePacedSender(clock, + packet_sender, + event_log, + *field_trials, + task_queue_factory, + max_hold_back_window, + max_hold_back_window_in_packets) {} + +TaskQueuePacedSender::TaskQueuePacedSender( + Clock* clock, + PacingController::PacketSender* packet_sender, + RtcEventLog* event_log, + const WebRtcKeyValueConfig& field_trials, + TaskQueueFactory* task_queue_factory, + TimeDelta max_hold_back_window, + int max_hold_back_window_in_packets) : clock_(clock), max_hold_back_window_(max_hold_back_window), max_hold_back_window_in_packets_(max_hold_back_window_in_packets), diff --git a/modules/pacing/task_queue_paced_sender.h b/modules/pacing/task_queue_paced_sender.h index 353f137963..61a625521d 100644 --- a/modules/pacing/task_queue_paced_sender.h +++ b/modules/pacing/task_queue_paced_sender.h @@ -19,6 +19,7 @@ #include #include +#include "absl/base/attributes.h" #include "absl/types/optional.h" #include "api/sequence_checker.h" #include "api/task_queue/task_queue_factory.h" @@ -39,6 +40,16 @@ class RtcEventLog; class TaskQueuePacedSender : public RtpPacketPacer, public RtpPacketSender { public: + ABSL_DEPRECATED("Use the version with field_trials reference instead.") + TaskQueuePacedSender( + Clock* clock, + PacingController::PacketSender* packet_sender, + RtcEventLog* event_log, + const WebRtcKeyValueConfig* field_trials, + TaskQueueFactory* task_queue_factory, + TimeDelta max_hold_back_window = PacingController::kMinSleepTime, + int max_hold_back_window_in_packets = -1); + // The `hold_back_window` parameter sets a lower bound on time to sleep if // there is currently a pacer queue and packets can't immediately be // processed. Increasing this reduces thread wakeups at the expense of higher @@ -48,7 +59,7 @@ class TaskQueuePacedSender : public RtpPacketPacer, public RtpPacketSender { Clock* clock, PacingController::PacketSender* packet_sender, RtcEventLog* event_log, - const WebRtcKeyValueConfig* field_trials, + const WebRtcKeyValueConfig& field_trials, TaskQueueFactory* task_queue_factory, TimeDelta max_hold_back_window = PacingController::kMinSleepTime, int max_hold_back_window_in_packets = -1); diff --git a/modules/pacing/task_queue_paced_sender_unittest.cc b/modules/pacing/task_queue_paced_sender_unittest.cc index d78365d499..a22ff6d164 100644 --- a/modules/pacing/task_queue_paced_sender_unittest.cc +++ b/modules/pacing/task_queue_paced_sender_unittest.cc @@ -20,9 +20,9 @@ #include "api/transport/network_types.h" #include "modules/pacing/packet_router.h" #include "modules/utility/include/mock/mock_process_thread.h" -#include "test/field_trial.h" #include "test/gmock.h" #include "test/gtest.h" +#include "test/scoped_key_value_config.h" #include "test/time_controller/simulated_time_controller.h" using ::testing::_; @@ -116,10 +116,10 @@ std::vector> GeneratePackets( TEST(TaskQueuePacedSenderTest, PacesPackets) { GlobalSimulatedTimeController time_controller(Timestamp::Millis(1234)); MockPacketRouter packet_router; + ScopedKeyValueConfig trials; TaskQueuePacedSender pacer( time_controller.GetClock(), &packet_router, - /*event_log=*/nullptr, - /*field_trials=*/nullptr, time_controller.GetTaskQueueFactory(), + /*event_log=*/nullptr, trials, time_controller.GetTaskQueueFactory(), PacingController::kMinSleepTime, kNoPacketHoldback); // Insert a number of packets, covering one second. @@ -156,10 +156,10 @@ TEST(TaskQueuePacedSenderTest, PacesPackets) { TEST(TaskQueuePacedSenderTest, ReschedulesProcessOnRateChange) { GlobalSimulatedTimeController time_controller(Timestamp::Millis(1234)); MockPacketRouter packet_router; + ScopedKeyValueConfig trials; TaskQueuePacedSender pacer( time_controller.GetClock(), &packet_router, - /*event_log=*/nullptr, - /*field_trials=*/nullptr, time_controller.GetTaskQueueFactory(), + /*event_log=*/nullptr, trials, time_controller.GetTaskQueueFactory(), PacingController::kMinSleepTime, kNoPacketHoldback); // Insert a number of packets to be sent 200ms apart. @@ -208,10 +208,10 @@ TEST(TaskQueuePacedSenderTest, ReschedulesProcessOnRateChange) { TEST(TaskQueuePacedSenderTest, SendsAudioImmediately) { GlobalSimulatedTimeController time_controller(Timestamp::Millis(1234)); MockPacketRouter packet_router; + ScopedKeyValueConfig trials; TaskQueuePacedSender pacer( time_controller.GetClock(), &packet_router, - /*event_log=*/nullptr, - /*field_trials=*/nullptr, time_controller.GetTaskQueueFactory(), + /*event_log=*/nullptr, trials, time_controller.GetTaskQueueFactory(), PacingController::kMinSleepTime, kNoPacketHoldback); const DataRate kPacingDataRate = DataRate::KilobitsPerSec(125); @@ -241,9 +241,9 @@ TEST(TaskQueuePacedSenderTest, SleepsDuringCoalscingWindow) { const TimeDelta kCoalescingWindow = TimeDelta::Millis(5); GlobalSimulatedTimeController time_controller(Timestamp::Millis(1234)); MockPacketRouter packet_router; + ScopedKeyValueConfig trials; TaskQueuePacedSender pacer(time_controller.GetClock(), &packet_router, - /*event_log=*/nullptr, - /*field_trials=*/nullptr, + /*event_log=*/nullptr, trials, time_controller.GetTaskQueueFactory(), kCoalescingWindow, kNoPacketHoldback); @@ -278,9 +278,9 @@ TEST(TaskQueuePacedSenderTest, ProbingOverridesCoalescingWindow) { const TimeDelta kCoalescingWindow = TimeDelta::Millis(5); GlobalSimulatedTimeController time_controller(Timestamp::Millis(1234)); MockPacketRouter packet_router; + ScopedKeyValueConfig trials; TaskQueuePacedSender pacer(time_controller.GetClock(), &packet_router, - /*event_log=*/nullptr, - /*field_trials=*/nullptr, + /*event_log=*/nullptr, trials, time_controller.GetTaskQueueFactory(), kCoalescingWindow, kNoPacketHoldback); @@ -307,13 +307,13 @@ TEST(TaskQueuePacedSenderTest, ProbingOverridesCoalescingWindow) { } TEST(TaskQueuePacedSenderTest, SchedulesProbeAtSetTime) { - ScopedFieldTrials trials("WebRTC-Bwe-ProbingBehavior/min_probe_delta:1ms/"); + ScopedKeyValueConfig trials( + "WebRTC-Bwe-ProbingBehavior/min_probe_delta:1ms/"); GlobalSimulatedTimeController time_controller(Timestamp::Millis(1234)); MockPacketRouter packet_router; TaskQueuePacedSender pacer( time_controller.GetClock(), &packet_router, - /*event_log=*/nullptr, - /*field_trials=*/nullptr, time_controller.GetTaskQueueFactory(), + /*event_log=*/nullptr, trials, time_controller.GetTaskQueueFactory(), PacingController::kMinSleepTime, kNoPacketHoldback); // Set rates so one packet adds 4ms of buffer level. @@ -374,13 +374,13 @@ TEST(TaskQueuePacedSenderTest, SchedulesProbeAtSetTime) { TEST(TaskQueuePacedSenderTest, NoMinSleepTimeWhenProbing) { // Set min_probe_delta to be less than kMinSleepTime (1ms). const TimeDelta kMinProbeDelta = TimeDelta::Micros(100); - ScopedFieldTrials trials("WebRTC-Bwe-ProbingBehavior/min_probe_delta:100us/"); + ScopedKeyValueConfig trials( + "WebRTC-Bwe-ProbingBehavior/min_probe_delta:100us/"); GlobalSimulatedTimeController time_controller(Timestamp::Millis(1234)); MockPacketRouter packet_router; TaskQueuePacedSender pacer( time_controller.GetClock(), &packet_router, - /*event_log=*/nullptr, - /*field_trials=*/nullptr, time_controller.GetTaskQueueFactory(), + /*event_log=*/nullptr, trials, time_controller.GetTaskQueueFactory(), PacingController::kMinSleepTime, kNoPacketHoldback); // Set rates so one packet adds 4ms of buffer level. @@ -433,9 +433,9 @@ TEST(TaskQueuePacedSenderTest, PacketBasedCoalescing) { GlobalSimulatedTimeController time_controller(Timestamp::Millis(1234)); MockPacketRouter packet_router; + ScopedKeyValueConfig trials; TaskQueuePacedSender pacer(time_controller.GetClock(), &packet_router, - /*event_log=*/nullptr, - /*field_trials=*/nullptr, + /*event_log=*/nullptr, trials, time_controller.GetTaskQueueFactory(), kFixedCoalescingWindow, kPacketBasedHoldback); @@ -484,9 +484,9 @@ TEST(TaskQueuePacedSenderTest, FixedHoldBackHasPriorityOverPackets) { GlobalSimulatedTimeController time_controller(Timestamp::Millis(1234)); MockPacketRouter packet_router; + ScopedKeyValueConfig trials; TaskQueuePacedSender pacer(time_controller.GetClock(), &packet_router, - /*event_log=*/nullptr, - /*field_trials=*/nullptr, + /*event_log=*/nullptr, trials, time_controller.GetTaskQueueFactory(), kFixedCoalescingWindow, kPacketBasedHoldback); @@ -530,10 +530,10 @@ TEST(TaskQueuePacedSenderTest, Stats) { static constexpr Timestamp kStartTime = Timestamp::Millis(1234); GlobalSimulatedTimeController time_controller(kStartTime); MockPacketRouter packet_router; + ScopedKeyValueConfig trials; TaskQueuePacedSender pacer( time_controller.GetClock(), &packet_router, - /*event_log=*/nullptr, - /*field_trials=*/nullptr, time_controller.GetTaskQueueFactory(), + /*event_log=*/nullptr, trials, time_controller.GetTaskQueueFactory(), PacingController::kMinSleepTime, kNoPacketHoldback); // Simulate ~2mbps video stream, covering one second. diff --git a/rtc_tools/rtc_event_log_visualizer/analyzer.cc b/rtc_tools/rtc_event_log_visualizer/analyzer.cc index 4c718185df..0185c7d89d 100644 --- a/rtc_tools/rtc_event_log_visualizer/analyzer.cc +++ b/rtc_tools/rtc_event_log_visualizer/analyzer.cc @@ -38,8 +38,6 @@ #include "modules/congestion_controller/goog_cc/delay_based_bwe.h" #include "modules/congestion_controller/include/receive_side_congestion_controller.h" #include "modules/congestion_controller/rtp/transport_feedback_adapter.h" -#include "modules/pacing/paced_sender.h" -#include "modules/pacing/packet_router.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "modules/rtp_rtcp/source/rtcp_packet.h" #include "modules/rtp_rtcp/source/rtcp_packet/common_header.h" @@ -1199,8 +1197,6 @@ void EventLogAnalyzer::CreateSendSideBweSimulationGraph(Plot* plot) { SimulatedClock clock(0); BitrateObserver observer; RtcEventLogNull null_event_log; - PacketRouter packet_router; - PacedSender pacer(&clock, &packet_router, &null_event_log); TransportFeedbackAdapter transport_feedback; auto factory = GoogCcNetworkControllerFactory(); TimeDelta process_interval = factory.GetProcessInterval();