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}
This commit is contained in:
Erik Språng
2019-11-28 13:44:25 +01:00
committed by Commit Bot
parent 67d3bc2b3d
commit 014dd3c9f7
14 changed files with 44 additions and 13 deletions

View File

@ -388,6 +388,7 @@ if (rtc_include_tests) {
"../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",

View File

@ -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 @@ struct CallHelper {
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 @@ struct CallHelper {
private:
webrtc::RtcEventLogNull event_log_;
webrtc::FieldTrialBasedConfig field_trials_;
std::unique_ptr<webrtc::TaskQueueFactory> task_queue_factory_;
std::unique_ptr<webrtc::Call> call_;
};

View File

@ -56,7 +56,8 @@ TargetRateConstraints ConvertConstraints(const BitrateConstraints& contraints,
}
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 @@ RtpTransportControllerSend::RtpTransportControllerSend(
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 @@ RtpTransportControllerSend::RtpTransportControllerSend(
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 @@ RtpTransportControllerSend::RtpTransportControllerSend(
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),

View File

@ -139,9 +139,6 @@ class RtpTransportControllerSend final
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_;

View File

@ -537,6 +537,7 @@ if (rtc_include_tests) {
"../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",

View File

@ -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 @@ class WebRtcVideoEngineTest : public ::testing::Test {
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 @@ class WebRtcVideoEngineTest : public ::testing::Test {
// 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 @@ TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, Vp8) {
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 @@ TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, NullDecoder) {
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 @@ class WebRtcVideoChannelBaseTest : public ::testing::Test {
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 @@ class WebRtcVideoChannelBaseTest : public ::testing::Test {
}
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>

View File

@ -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 @@ TEST(WebRtcVoiceEngineTest, StartupShutdown) {
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 @@ TEST(WebRtcVoiceEngineTest, StartupShutdownWithExternalADM) {
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 @@ TEST(WebRtcVoiceEngineTest, Has32Channels) {
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 @@ TEST(WebRtcVoiceEngineTest, SetRecvCodecs) {
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(),

View File

@ -773,6 +773,7 @@ rtc_library("test_common") {
"../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",

View File

@ -222,12 +222,14 @@ void CallTest::CreateSenderCall(const Call::Config& config) {
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));
}

View File

@ -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 @@ class CallTest : public ::testing::Test {
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_;

View File

@ -65,6 +65,7 @@ Call* CreateCall(TimeController* time_controller,
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 @@ CallClient::CallClient(
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());

View File

@ -152,6 +152,8 @@ class CallClient : public EmulatedNetworkReceiverInterface {
std::map<uint32_t, MediaType> ssrc_media_types_;
// Defined last so it's destroyed first.
TaskQueueForTest task_queue_;
const FieldTrialBasedConfig field_trials_;
};
class CallClientPair {

View File

@ -55,6 +55,7 @@ struct TransportControllerConfig {
struct CallClientConfig {
TransportControllerConfig transport;
const WebRtcKeyValueConfig* field_trials = nullptr;
};
struct PacketStreamConfig {

View File

@ -49,6 +49,8 @@ void MultiStreamTester::RunTest() {
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;