Trials should always be populated in call config.
The trials are always set when a Call instead is created by a
CallFactory, but a lot of unit tests creates instances directly.
This CL updates those call site. There is still a fallback in place
in RtpTransportControllerSend, since there are down-stream usages that
need to be clean up. After that, we'll remove the fallback.
Bug: webrtc:10809
Change-Id: I0aacf0473317bcd64252dd43d93c42de730e2ffa
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/160408
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29978}
diff --git a/call/BUILD.gn b/call/BUILD.gn
index 81afe55..76e1b45 100644
--- a/call/BUILD.gn
+++ b/call/BUILD.gn
@@ -388,6 +388,7 @@
"../api/audio_codecs:builtin_audio_decoder_factory",
"../api/rtc_event_log",
"../api/task_queue:default_task_queue_factory",
+ "../api/transport:field_trial_based_config",
"../api/video:video_frame",
"../api/video:video_rtp_headers",
"../audio",
diff --git a/call/call_unittest.cc b/call/call_unittest.cc
index 754be81..a8cf534 100644
--- a/call/call_unittest.cc
+++ b/call/call_unittest.cc
@@ -20,6 +20,7 @@
#include "api/rtc_event_log/rtc_event_log.h"
#include "api/task_queue/default_task_queue_factory.h"
#include "api/test/mock_audio_mixer.h"
+#include "api/transport/field_trial_based_config.h"
#include "audio/audio_receive_stream.h"
#include "audio/audio_send_stream.h"
#include "call/audio_state.h"
@@ -46,6 +47,7 @@
webrtc::Call::Config config(&event_log_);
config.audio_state = webrtc::AudioState::Create(audio_state_config);
config.task_queue_factory = task_queue_factory_.get();
+ config.trials = &field_trials_;
call_.reset(webrtc::Call::Create(config));
}
@@ -53,6 +55,7 @@
private:
webrtc::RtcEventLogNull event_log_;
+ webrtc::FieldTrialBasedConfig field_trials_;
std::unique_ptr<webrtc::TaskQueueFactory> task_queue_factory_;
std::unique_ptr<webrtc::Call> call_;
};
diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc
index a44b534..bd8e2d0 100644
--- a/call/rtp_transport_controller_send.cc
+++ b/call/rtp_transport_controller_send.cc
@@ -56,7 +56,8 @@
}
bool IsEnabled(const WebRtcKeyValueConfig* trials, absl::string_view key) {
- return trials && trials->Lookup(key).find("Enabled") == 0;
+ RTC_DCHECK(trials != nullptr);
+ return trials->Lookup(key).find("Enabled") == 0;
}
} // namespace
@@ -72,22 +73,21 @@
const WebRtcKeyValueConfig* trials)
: clock_(clock),
event_log_(event_log),
- field_trials_(trials ? trials : &fallback_field_trials_),
bitrate_configurator_(bitrate_config),
process_thread_(std::move(process_thread)),
- use_task_queue_pacer_(IsEnabled(field_trials_, "WebRTC-TaskQueuePacer")),
+ use_task_queue_pacer_(IsEnabled(trials, "WebRTC-TaskQueuePacer")),
process_thread_pacer_(use_task_queue_pacer_
? nullptr
: new PacedSender(clock,
&packet_router_,
event_log,
- field_trials_,
+ trials,
process_thread_.get())),
task_queue_pacer_(use_task_queue_pacer_
? new TaskQueuePacedSender(clock,
&packet_router_,
event_log,
- field_trials_,
+ trials,
task_queue_factory)
: nullptr),
observer_(nullptr),
@@ -97,12 +97,11 @@
process_interval_(controller_factory_fallback_->GetProcessInterval()),
last_report_block_time_(Timestamp::ms(clock_->TimeInMilliseconds())),
reset_feedback_on_route_change_(
- !IsEnabled(field_trials_, "WebRTC-Bwe-NoFeedbackReset")),
+ !IsEnabled(trials, "WebRTC-Bwe-NoFeedbackReset")),
send_side_bwe_with_overhead_(
- IsEnabled(field_trials_, "WebRTC-SendSideBwe-WithOverhead")),
+ IsEnabled(trials, "WebRTC-SendSideBwe-WithOverhead")),
add_pacing_to_cwin_(
- IsEnabled(field_trials_,
- "WebRTC-AddPacingToCongestionWindowPushback")),
+ IsEnabled(trials, "WebRTC-AddPacingToCongestionWindowPushback")),
transport_overhead_bytes_per_packet_(0),
network_available_(false),
retransmission_rate_limiter_(clock, kRetransmitWindowSizeMs),
@@ -111,7 +110,7 @@
TaskQueueFactory::Priority::NORMAL)) {
initial_config_.constraints = ConvertConstraints(bitrate_config, clock_);
initial_config_.event_log = event_log;
- initial_config_.key_value_config = field_trials_;
+ initial_config_.key_value_config = trials;
RTC_DCHECK(bitrate_config.start_bitrate_bps > 0);
pacer()->SetPacingRates(DataRate::bps(bitrate_config.start_bitrate_bps),
diff --git a/call/rtp_transport_controller_send.h b/call/rtp_transport_controller_send.h
index 32c762b..b5a53d7 100644
--- a/call/rtp_transport_controller_send.h
+++ b/call/rtp_transport_controller_send.h
@@ -139,9 +139,6 @@
Clock* const clock_;
RtcEventLog* const event_log_;
- // TODO(sprang): Remove fallback field-trials.
- const FieldTrialBasedConfig fallback_field_trials_;
- const WebRtcKeyValueConfig* field_trials_;
PacketRouter packet_router_;
std::vector<std::unique_ptr<RtpVideoSenderInterface>> video_rtp_senders_;
RtpBitrateConfigurator bitrate_configurator_;
diff --git a/media/BUILD.gn b/media/BUILD.gn
index 9912d29..7d4056a 100644
--- a/media/BUILD.gn
+++ b/media/BUILD.gn
@@ -537,6 +537,7 @@
"../api/task_queue",
"../api/task_queue:default_task_queue_factory",
"../api/test/video:function_video_factory",
+ "../api/transport:field_trial_based_config",
"../api/transport/media:media_transport_interface",
"../api/units:time_delta",
"../api/video:builtin_video_bitrate_allocator_factory",
diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc
index 8870cd6..2c49c87 100644
--- a/media/engine/webrtc_video_engine_unittest.cc
+++ b/media/engine/webrtc_video_engine_unittest.cc
@@ -26,6 +26,7 @@
#include "api/test/mock_video_bitrate_allocator_factory.h"
#include "api/test/mock_video_decoder_factory.h"
#include "api/test/mock_video_encoder_factory.h"
+#include "api/transport/field_trial_based_config.h"
#include "api/transport/media/media_transport_config.h"
#include "api/units/time_delta.h"
#include "api/video/builtin_video_bitrate_allocator_factory.h"
@@ -237,6 +238,7 @@
call_(webrtc::Call::Create([&] {
webrtc::Call::Config call_config(&event_log_);
call_config.task_queue_factory = task_queue_factory_.get();
+ call_config.trials = &field_trials_;
return call_config;
}())),
encoder_factory_(new cricket::FakeWebRtcVideoEncoderFactory),
@@ -275,6 +277,7 @@
// race condition in the clock access.
rtc::ScopedFakeClock fake_clock_;
std::unique_ptr<webrtc::test::ScopedFieldTrials> override_field_trials_;
+ webrtc::FieldTrialBasedConfig field_trials_;
webrtc::RtcEventLogNull event_log_;
std::unique_ptr<webrtc::TaskQueueFactory> task_queue_factory_;
// Used in WebRtcVideoEngineVoiceTest, but defined here so it's properly
@@ -1152,6 +1155,8 @@
webrtc::RtcEventLogNull event_log;
auto task_queue_factory = webrtc::CreateDefaultTaskQueueFactory();
webrtc::Call::Config call_config(&event_log);
+ webrtc::FieldTrialBasedConfig field_trials;
+ call_config.trials = &field_trials;
call_config.task_queue_factory = task_queue_factory.get();
const auto call = absl::WrapUnique(webrtc::Call::Create(call_config));
@@ -1222,6 +1227,8 @@
webrtc::RtcEventLogNull event_log;
auto task_queue_factory = webrtc::CreateDefaultTaskQueueFactory();
webrtc::Call::Config call_config(&event_log);
+ webrtc::FieldTrialBasedConfig field_trials;
+ call_config.trials = &field_trials;
call_config.task_queue_factory = task_queue_factory.get();
const auto call = absl::WrapUnique(webrtc::Call::Create(call_config));
@@ -1314,6 +1321,7 @@
if (!call_) {
webrtc::Call::Config call_config(&event_log_);
call_config.task_queue_factory = task_queue_factory_.get();
+ call_config.trials = &field_trials_;
call_.reset(webrtc::Call::Create(call_config));
}
cricket::MediaConfig media_config;
@@ -1495,6 +1503,7 @@
}
webrtc::RtcEventLogNull event_log_;
+ webrtc::FieldTrialBasedConfig field_trials_;
std::unique_ptr<webrtc::TaskQueueFactory> task_queue_factory_;
std::unique_ptr<webrtc::Call> call_;
std::unique_ptr<webrtc::VideoBitrateAllocatorFactory>
diff --git a/media/engine/webrtc_voice_engine_unittest.cc b/media/engine/webrtc_voice_engine_unittest.cc
index f72fad7..775d586 100644
--- a/media/engine/webrtc_voice_engine_unittest.cc
+++ b/media/engine/webrtc_voice_engine_unittest.cc
@@ -21,6 +21,7 @@
#include "api/rtp_parameters.h"
#include "api/scoped_refptr.h"
#include "api/task_queue/default_task_queue_factory.h"
+#include "api/transport/field_trial_based_config.h"
#include "call/call.h"
#include "media/base/fake_media_engine.h"
#include "media/base/fake_network_interface.h"
@@ -3465,6 +3466,8 @@
engine.Init();
webrtc::RtcEventLogNull event_log;
webrtc::Call::Config call_config(&event_log);
+ webrtc::FieldTrialBasedConfig field_trials;
+ call_config.trials = &field_trials;
call_config.task_queue_factory = task_queue_factory.get();
auto call = absl::WrapUnique(webrtc::Call::Create(call_config));
cricket::VoiceMediaChannel* channel = engine.CreateMediaChannel(
@@ -3493,6 +3496,8 @@
engine.Init();
webrtc::RtcEventLogNull event_log;
webrtc::Call::Config call_config(&event_log);
+ webrtc::FieldTrialBasedConfig field_trials;
+ call_config.trials = &field_trials;
call_config.task_queue_factory = task_queue_factory.get();
auto call = absl::WrapUnique(webrtc::Call::Create(call_config));
cricket::VoiceMediaChannel* channel = engine.CreateMediaChannel(
@@ -3567,6 +3572,8 @@
engine.Init();
webrtc::RtcEventLogNull event_log;
webrtc::Call::Config call_config(&event_log);
+ webrtc::FieldTrialBasedConfig field_trials;
+ call_config.trials = &field_trials;
call_config.task_queue_factory = task_queue_factory.get();
auto call = absl::WrapUnique(webrtc::Call::Create(call_config));
@@ -3610,6 +3617,8 @@
engine.Init();
webrtc::RtcEventLogNull event_log;
webrtc::Call::Config call_config(&event_log);
+ webrtc::FieldTrialBasedConfig field_trials;
+ call_config.trials = &field_trials;
call_config.task_queue_factory = task_queue_factory.get();
auto call = absl::WrapUnique(webrtc::Call::Create(call_config));
cricket::WebRtcVoiceMediaChannel channel(&engine, cricket::MediaConfig(),
diff --git a/test/BUILD.gn b/test/BUILD.gn
index 1a1147f..8c22c92 100644
--- a/test/BUILD.gn
+++ b/test/BUILD.gn
@@ -773,6 +773,7 @@
"../api/task_queue",
"../api/task_queue:default_task_queue_factory",
"../api/test/video:function_video_factory",
+ "../api/transport:field_trial_based_config",
"../api/video:builtin_video_bitrate_allocator_factory",
"../api/video:video_bitrate_allocator_factory",
"../api/video:video_frame",
diff --git a/test/call_test.cc b/test/call_test.cc
index 10b631a..e8c067a 100644
--- a/test/call_test.cc
+++ b/test/call_test.cc
@@ -222,12 +222,14 @@
sender_config.network_state_predictor_factory =
network_state_predictor_factory_.get();
sender_config.network_controller_factory = network_controller_factory_.get();
+ sender_config.trials = &field_trials_;
sender_call_.reset(Call::Create(sender_config));
}
void CallTest::CreateReceiverCall(const Call::Config& config) {
auto receiver_config = config;
receiver_config.task_queue_factory = task_queue_factory_.get();
+ receiver_config.trials = &field_trials_;
receiver_call_.reset(Call::Create(receiver_config));
}
diff --git a/test/call_test.h b/test/call_test.h
index ba9740d..3f4aa07 100644
--- a/test/call_test.h
+++ b/test/call_test.h
@@ -21,6 +21,7 @@
#include "api/task_queue/task_queue_factory.h"
#include "api/test/video/function_video_decoder_factory.h"
#include "api/test/video/function_video_encoder_factory.h"
+#include "api/transport/field_trial_based_config.h"
#include "api/video/video_bitrate_allocator_factory.h"
#include "call/call.h"
#include "modules/audio_device/include/test_audio_device.h"
@@ -176,6 +177,7 @@
TaskQueueBase* task_queue() { return task_queue_.get(); }
Clock* const clock_;
+ const FieldTrialBasedConfig field_trials_;
std::unique_ptr<TaskQueueFactory> task_queue_factory_;
std::unique_ptr<webrtc::RtcEventLog> send_event_log_;
diff --git a/test/scenario/call_client.cc b/test/scenario/call_client.cc
index cb29ff6..f89b13f 100644
--- a/test/scenario/call_client.cc
+++ b/test/scenario/call_client.cc
@@ -65,6 +65,7 @@
call_config.task_queue_factory = time_controller->GetTaskQueueFactory();
call_config.network_controller_factory = network_controller_factory;
call_config.audio_state = audio_state;
+ call_config.trials = config.field_trials;
return Call::Create(call_config, time_controller->GetClock(),
time_controller->CreateProcessThread("CallModules"),
time_controller->CreateProcessThread("Pacer"));
@@ -207,6 +208,7 @@
task_queue_(time_controller->GetTaskQueueFactory()->CreateTaskQueue(
"CallClient",
TaskQueueFactory::Priority::NORMAL)) {
+ config.field_trials = &field_trials_;
SendTask([this, config] {
event_log_ = CreateEventLog(time_controller_->GetTaskQueueFactory(),
log_writer_factory_.get());
diff --git a/test/scenario/call_client.h b/test/scenario/call_client.h
index 77c5986..34a15c1 100644
--- a/test/scenario/call_client.h
+++ b/test/scenario/call_client.h
@@ -152,6 +152,8 @@
std::map<uint32_t, MediaType> ssrc_media_types_;
// Defined last so it's destroyed first.
TaskQueueForTest task_queue_;
+
+ const FieldTrialBasedConfig field_trials_;
};
class CallClientPair {
diff --git a/test/scenario/scenario_config.h b/test/scenario/scenario_config.h
index e769e80..301fc71 100644
--- a/test/scenario/scenario_config.h
+++ b/test/scenario/scenario_config.h
@@ -55,6 +55,7 @@
struct CallClientConfig {
TransportControllerConfig transport;
+ const WebRtcKeyValueConfig* field_trials = nullptr;
};
struct PacketStreamConfig {
diff --git a/video/end_to_end_tests/multi_stream_tester.cc b/video/end_to_end_tests/multi_stream_tester.cc
index 2299f11..c8e63e1 100644
--- a/video/end_to_end_tests/multi_stream_tester.cc
+++ b/video/end_to_end_tests/multi_stream_tester.cc
@@ -49,6 +49,8 @@
auto task_queue = task_queue_factory->CreateTaskQueue(
"TaskQueue", TaskQueueFactory::Priority::HIGH);
Call::Config config(&event_log);
+ FieldTrialBasedConfig field_trials;
+ config.trials = &field_trials;
config.task_queue_factory = task_queue_factory.get();
std::unique_ptr<Call> sender_call;
std::unique_ptr<Call> receiver_call;
