From 7ef34f850584f6fd2de7cebb1d8897ab890a1f5f Mon Sep 17 00:00:00 2001 From: Per Kjellander Date: Fri, 22 Feb 2019 13:09:32 +0100 Subject: [PATCH] Replace field trials with WebRtcKeyValueConfig in PacedSender Replaces use of field trials in PacedSender with injectable WebRtcKeyValueConfig. Implementation still defaults to field trials. BUG: webrtc:10335 Change-Id: Ie8870d93d51e996e762f2c2de7545bad261b6bb7 Reviewed-on: https://webrtc-review.googlesource.com/c/123521 Commit-Queue: Per Kjellander Reviewed-by: Danil Chapovalov Reviewed-by: Sebastian Jansson Cr-Commit-Position: refs/heads/master@{#26818} --- modules/pacing/BUILD.gn | 2 ++ modules/pacing/DEPS | 4 +++- modules/pacing/paced_sender.cc | 37 ++++++++++++++++++++++++++-------- modules/pacing/paced_sender.h | 10 ++++++++- 4 files changed, 43 insertions(+), 10 deletions(-) diff --git a/modules/pacing/BUILD.gn b/modules/pacing/BUILD.gn index 7ebd37be9e..00fde9ec2a 100644 --- a/modules/pacing/BUILD.gn +++ b/modules/pacing/BUILD.gn @@ -29,7 +29,9 @@ rtc_static_library("pacing") { deps = [ ":interval_budget", "..:module_api", + "../../api/transport:field_trial_based_config", "../../api/transport:network_control", + "../../api/transport:webrtc_key_value_config", "../../logging:rtc_event_bwe", "../../logging:rtc_event_log_api", "../../logging:rtc_event_pacing", diff --git a/modules/pacing/DEPS b/modules/pacing/DEPS index 3088a75e68..1b2e6dcf45 100644 --- a/modules/pacing/DEPS +++ b/modules/pacing/DEPS @@ -1,4 +1,6 @@ include_rules = [ - "+system_wrappers", + "+system_wrappers", + # Avoid directly using field_trial. Instead use WebRtcKeyValueConfig. + "-system_wrappers/include/field_trial.h", "+logging/rtc_event_log" ] diff --git a/modules/pacing/paced_sender.cc b/modules/pacing/paced_sender.cc index 44bb774002..70c8ad435f 100644 --- a/modules/pacing/paced_sender.cc +++ b/modules/pacing/paced_sender.cc @@ -22,8 +22,8 @@ #include "rtc_base/checks.h" #include "rtc_base/logging.h" #include "system_wrappers/include/clock.h" -#include "system_wrappers/include/field_trial.h" +namespace webrtc { namespace { // Time limit in milliseconds between packet bursts. const int64_t kDefaultMinPacketLimitMs = 5; @@ -35,23 +35,44 @@ const int64_t kMaxElapsedTimeMs = 2000; // time. const int64_t kMaxIntervalTimeMs = 30; -} // namespace +bool IsDisabled(const WebRtcKeyValueConfig& field_trials, + absl::string_view key) { + return field_trials.Lookup(key).find("Disabled") == 0; +} -namespace webrtc { +bool IsEnabled(const WebRtcKeyValueConfig& field_trials, + absl::string_view key) { + return field_trials.Lookup(key).find("Enabled") == 0; +} + +} // namespace const int64_t PacedSender::kMaxQueueLengthMs = 2000; const float PacedSender::kDefaultPaceMultiplier = 2.5f; PacedSender::PacedSender(Clock* clock, PacketSender* packet_sender, - RtcEventLog* event_log) + RtcEventLog* event_log, + const WebRtcKeyValueConfig* field_trials) + : PacedSender(clock, + packet_sender, + event_log, + field_trials + ? *field_trials + : static_cast( + FieldTrialBasedConfig())) {} + +PacedSender::PacedSender(Clock* clock, + PacketSender* packet_sender, + RtcEventLog* event_log, + const WebRtcKeyValueConfig& field_trials) : clock_(clock), packet_sender_(packet_sender), alr_detector_(), - drain_large_queues_(!field_trial::IsDisabled("WebRTC-Pacer-DrainQueue")), + drain_large_queues_(!IsDisabled(field_trials, "WebRTC-Pacer-DrainQueue")), send_padding_if_silent_( - field_trial::IsEnabled("WebRTC-Pacer-PadInSilence")), - pace_audio_(!field_trial::IsDisabled("WebRTC-Pacer-BlockAudio")), + IsEnabled(field_trials, "WebRTC-Pacer-PadInSilence")), + pace_audio_(!IsDisabled(field_trials, "WebRTC-Pacer-BlockAudio")), min_packet_limit_ms_("", kDefaultMinPacketLimitMs), last_timestamp_ms_(clock_->TimeInMilliseconds()), paused_(false), @@ -76,7 +97,7 @@ PacedSender::PacedSender(Clock* clock, "pushback experiment must be enabled."; } ParseFieldTrial({&min_packet_limit_ms_}, - field_trial::FindFullName("WebRTC-Pacer-MinPacketLimitMs")); + field_trials.Lookup("WebRTC-Pacer-MinPacketLimitMs")); UpdateBudgetWithElapsedTime(min_packet_limit_ms_); } diff --git a/modules/pacing/paced_sender.h b/modules/pacing/paced_sender.h index 438748f806..a6aad24c11 100644 --- a/modules/pacing/paced_sender.h +++ b/modules/pacing/paced_sender.h @@ -17,7 +17,9 @@ #include #include "absl/types/optional.h" +#include "api/transport/field_trial_based_config.h" #include "api/transport/network_types.h" +#include "api/transport/webrtc_key_value_config.h" #include "modules/pacing/bitrate_prober.h" #include "modules/pacing/interval_budget.h" #include "modules/pacing/pacer.h" @@ -71,7 +73,8 @@ class PacedSender : public Pacer { PacedSender(Clock* clock, PacketSender* packet_sender, - RtcEventLog* event_log); + RtcEventLog* event_log, + const WebRtcKeyValueConfig* field_trials = nullptr); ~PacedSender() override; @@ -147,6 +150,11 @@ class PacedSender : public Pacer { void SetQueueTimeLimit(int limit_ms); private: + PacedSender(Clock* clock, + PacketSender* packet_sender, + RtcEventLog* event_log, + const WebRtcKeyValueConfig& field_trials); + int64_t UpdateTimeAndGetElapsedMs(int64_t now_us) RTC_EXCLUSIVE_LOCKS_REQUIRED(critsect_); bool ShouldSendKeepalive(int64_t at_time_us) const