Pass Clock through Environment when constructing Call
while cleaning up Call factory function,
- pick rtp_transport_controller_send_factory based on presence in the config instead of based on the call site thus removing one extra factory function.
- when Call is created through test helper TimeControllerBasedFactory use original media factory instead of direct factory, thus allow to configure degraded call through field trials in tests, and ensure difference with production code path stay minimal in the future.
Bug: webrtc:15656
Change-Id: If9c2a9fc871e139502db2bec0a241d8d64c53720
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/330061
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41329}
diff --git a/api/BUILD.gn b/api/BUILD.gn
index ac97da5..3413b93 100644
--- a/api/BUILD.gn
+++ b/api/BUILD.gn
@@ -1391,12 +1391,12 @@
":time_controller",
"../call",
"../call:call_interfaces",
- "../call:rtp_interfaces",
"../pc:media_factory",
"../rtc_base:checks",
"../system_wrappers",
"../test/time_controller",
"environment",
+ "environment:environment_factory",
]
absl_deps = [ "//third_party/abseil-cpp/absl/base:nullability" ]
}
diff --git a/api/test/create_time_controller.cc b/api/test/create_time_controller.cc
index 7523e05..cbf1f09 100644
--- a/api/test/create_time_controller.cc
+++ b/api/test/create_time_controller.cc
@@ -16,10 +16,10 @@
#include "absl/base/nullability.h"
#include "api/enable_media_with_defaults.h"
#include "api/environment/environment.h"
+#include "api/environment/environment_factory.h"
#include "api/peer_connection_interface.h"
#include "call/call.h"
-#include "call/rtp_transport_config.h"
-#include "call/rtp_transport_controller_send_factory_interface.h"
+#include "call/call_config.h"
#include "pc/media_factory.h"
#include "rtc_base/checks.h"
#include "system_wrappers/include/clock.h"
@@ -49,9 +49,12 @@
: clock_(clock), media_factory_(std::move(media_factory)) {}
std::unique_ptr<Call> CreateCall(const CallConfig& config) override {
- return Call::Create(config, clock_,
- config.rtp_transport_controller_send_factory->Create(
- config.ExtractTransportConfig(), clock_));
+ EnvironmentFactory env_factory(config.env);
+ env_factory.Set(clock_);
+
+ CallConfig config_with_custom_clock = config;
+ config_with_custom_clock.env = env_factory.Create();
+ return media_factory_->CreateCall(config_with_custom_clock);
}
std::unique_ptr<cricket::MediaEngineInterface> CreateMediaEngine(
diff --git a/call/call.cc b/call/call.cc
index d9ea94a..7e6d57a 100644
--- a/call/call.cc
+++ b/call/call.cc
@@ -183,8 +183,7 @@
public TargetTransferRateObserver,
public BitrateAllocator::LimitObserver {
public:
- Call(Clock* clock,
- const CallConfig& config,
+ Call(const CallConfig& config,
std::unique_ptr<RtpTransportControllerSendInterface> transport_send);
~Call() override;
@@ -344,9 +343,6 @@
// callbacks have been registered.
void EnsureStarted() RTC_RUN_ON(worker_thread_);
- // TODO: bugs.webrtc.org/15656 - Delete `clock_` when it would always be the
- // same as &env_.clock()
- Clock* const clock_;
const Environment env_;
TaskQueueBase* const worker_thread_;
TaskQueueBase* const network_thread_;
@@ -471,19 +467,16 @@
}
std::unique_ptr<Call> Call::Create(const CallConfig& config) {
- Clock* clock = &config.env.clock();
- return Create(config, clock,
- RtpTransportControllerSendFactory().Create(
- config.ExtractTransportConfig(), clock));
-}
+ std::unique_ptr<RtpTransportControllerSendInterface> transport_send;
+ if (config.rtp_transport_controller_send_factory != nullptr) {
+ transport_send = config.rtp_transport_controller_send_factory->Create(
+ config.ExtractTransportConfig(), &config.env.clock());
+ } else {
+ transport_send = RtpTransportControllerSendFactory().Create(
+ config.ExtractTransportConfig(), &config.env.clock());
+ }
-std::unique_ptr<Call> Call::Create(
- const CallConfig& config,
- Clock* clock,
- std::unique_ptr<RtpTransportControllerSendInterface>
- transportControllerSend) {
- return std::make_unique<internal::Call>(clock, config,
- std::move(transportControllerSend));
+ return std::make_unique<internal::Call>(config, std::move(transport_send));
}
// This method here to avoid subclasses has to implement this method.
@@ -649,11 +642,9 @@
min_allocated_send_bitrate_bps_ = limits.min_allocatable_rate.bps();
}
-Call::Call(Clock* clock,
- const CallConfig& config,
+Call::Call(const CallConfig& config,
std::unique_ptr<RtpTransportControllerSendInterface> transport_send)
- : clock_(clock),
- env_(config.env),
+ : env_(config.env),
worker_thread_(GetCurrentTaskQueueOrThread()),
// If `network_task_queue_` was set to nullptr, network related calls
// must be made on `worker_thread_` (i.e. they're one and the same).
@@ -661,20 +652,20 @@
: worker_thread_),
decode_sync_(
config.decode_metronome
- ? std::make_unique<DecodeSynchronizer>(clock_,
+ ? std::make_unique<DecodeSynchronizer>(&env_.clock(),
config.decode_metronome,
worker_thread_)
: nullptr),
num_cpu_cores_(CpuInfo::DetectNumberOfCores()),
- call_stats_(new CallStats(clock_, worker_thread_)),
+ call_stats_(new CallStats(&env_.clock(), worker_thread_)),
bitrate_allocator_(new BitrateAllocator(this)),
config_(config),
audio_network_state_(kNetworkDown),
video_network_state_(kNetworkDown),
aggregate_network_up_(false),
- receive_stats_(clock_),
- send_stats_(clock_),
- receive_side_cc_(clock,
+ receive_stats_(&env_.clock()),
+ send_stats_(&env_.clock()),
+ receive_side_cc_(&env_.clock(),
absl::bind_front(&PacketRouter::SendCombinedRtcpPacket,
transport_send->packet_router()),
absl::bind_front(&PacketRouter::SendRemb,
@@ -682,8 +673,8 @@
/*network_state_estimator=*/nullptr),
receive_time_calculator_(
ReceiveTimeCalculator::CreateFromFieldTrial(env_.field_trials())),
- video_send_delay_stats_(new SendDelayStats(clock_)),
- start_of_call_(clock_->CurrentTime()),
+ video_send_delay_stats_(new SendDelayStats(&env_.clock())),
+ start_of_call_(env_.clock().CurrentTime()),
transport_send_ptr_(transport_send.get()),
transport_send_(std::move(transport_send)) {
RTC_DCHECK(network_thread_);
@@ -703,7 +694,7 @@
receive_side_cc_periodic_task_ = RepeatingTaskHandle::Start(
worker_thread_,
[receive_side_cc] { return receive_side_cc->MaybeProcess(); },
- TaskQueueBase::DelayPrecision::kLow, clock_);
+ TaskQueueBase::DelayPrecision::kLow, &env_.clock());
}
Call::~Call() {
@@ -721,7 +712,7 @@
RTC_HISTOGRAM_COUNTS_100000(
"WebRTC.Call.LifetimeInSeconds",
- (clock_->CurrentTime() - start_of_call_).seconds());
+ (env_.clock().CurrentTime() - start_of_call_).seconds());
}
void Call::EnsureStarted() {
@@ -766,7 +757,7 @@
}
AudioSendStream* send_stream = new AudioSendStream(
- clock_, config, config_.audio_state, &env_.task_queue_factory(),
+ &env_.clock(), config, config_.audio_state, &env_.task_queue_factory(),
transport_send_.get(), bitrate_allocator_.get(), &env_.event_log(),
call_stats_->AsRtcpRttStats(), suspended_rtp_state, trials());
RTC_DCHECK(audio_send_ssrcs_.find(config.rtp.ssrc) ==
@@ -823,8 +814,8 @@
CreateRtcLogStreamConfig(config)));
AudioReceiveStreamImpl* receive_stream = new AudioReceiveStreamImpl(
- clock_, transport_send_->packet_router(), config_.neteq_factory, config,
- config_.audio_state, &env_.event_log());
+ &env_.clock(), transport_send_->packet_router(), config_.neteq_factory,
+ config, config_.audio_state, &env_.event_log());
audio_receive_streams_.insert(receive_stream);
// TODO(bugs.webrtc.org/11993): Make the registration on the network thread
@@ -896,8 +887,8 @@
std::vector<uint32_t> ssrcs = config.rtp.ssrcs;
VideoSendStream* send_stream = new VideoSendStream(
- clock_, num_cpu_cores_, &env_.task_queue_factory(), network_thread_,
- call_stats_->AsRtcpRttStats(), transport_send_.get(),
+ &env_.clock(), num_cpu_cores_, &env_.task_queue_factory(),
+ network_thread_, call_stats_->AsRtcpRttStats(), transport_send_.get(),
bitrate_allocator_.get(), video_send_delay_stats_.get(),
&env_.event_log(), std::move(config), std::move(encoder_config),
suspended_video_send_ssrcs_, suspended_video_payload_states_,
@@ -930,7 +921,7 @@
std::unique_ptr<FecController> fec_controller =
config_.fec_controller_factory
? config_.fec_controller_factory->CreateFecController()
- : std::make_unique<FecControllerDefault>(clock_);
+ : std::make_unique<FecControllerDefault>(&env_.clock());
return CreateVideoSendStream(std::move(config), std::move(encoder_config),
std::move(fec_controller));
}
@@ -997,7 +988,8 @@
VideoReceiveStream2* receive_stream = new VideoReceiveStream2(
&env_.task_queue_factory(), this, num_cpu_cores_,
transport_send_->packet_router(), std::move(configuration),
- call_stats_.get(), clock_, std::make_unique<VCMTiming>(clock_, trials()),
+ call_stats_.get(), &env_.clock(),
+ std::make_unique<VCMTiming>(&env_.clock(), trials()),
&nack_periodic_processor_, decode_sync_.get(), &env_.event_log());
// TODO(bugs.webrtc.org/11993): Set this up asynchronously on the network
// thread.
@@ -1041,7 +1033,7 @@
// OnRtpPacket until the constructor is finished and the object is
// in a valid state, since OnRtpPacket runs on the same thread.
FlexfecReceiveStreamImpl* receive_stream = new FlexfecReceiveStreamImpl(
- clock_, std::move(config), &video_receiver_controller_,
+ &env_.clock(), std::move(config), &video_receiver_controller_,
call_stats_->AsRtcpRttStats());
// TODO(bugs.webrtc.org/11993): Set this up asynchronously on the network
@@ -1245,7 +1237,7 @@
// on a ProcessThread. This is alright as is since we forward the call to
// implementations that either just do a PostTask or use locking.
video_send_delay_stats_->OnSentPacket(sent_packet.packet_id,
- clock_->CurrentTime());
+ env_.clock().CurrentTime());
transport_send_->OnSentPacket(sent_packet);
}
@@ -1369,7 +1361,8 @@
// Repair packet_time_us for clock resets by comparing a new read of
// the same clock (TimeUTCMicros) to a monotonic clock reading.
packet_time_us = receive_time_calculator_->ReconcileReceiveTimes(
- packet_time_us, rtc::TimeUTCMicros(), clock_->TimeInMicroseconds());
+ packet_time_us, rtc::TimeUTCMicros(),
+ env_.clock().TimeInMicroseconds());
packet.set_arrival_time(Timestamp::Micros(packet_time_us));
}
diff --git a/call/call.h b/call/call.h
index 6f8e4cd..a680335 100644
--- a/call/call.h
+++ b/call/call.h
@@ -24,7 +24,6 @@
#include "call/call_config.h"
#include "call/flexfec_receive_stream.h"
#include "call/packet_receiver.h"
-#include "call/rtp_transport_controller_send_interface.h"
#include "call/video_receive_stream.h"
#include "call/video_send_stream.h"
#include "rtc_base/copy_on_write_buffer.h"
@@ -57,11 +56,6 @@
};
static std::unique_ptr<Call> Create(const CallConfig& config);
- static std::unique_ptr<Call> Create(
- const CallConfig& config,
- Clock* clock,
- std::unique_ptr<RtpTransportControllerSendInterface>
- transportControllerSend);
virtual AudioSendStream* CreateAudioSendStream(
const AudioSendStream::Config& config) = 0;
diff --git a/call/create_call.cc b/call/create_call.cc
index 44b204d..8b56574 100644
--- a/call/create_call.cc
+++ b/call/create_call.cc
@@ -22,7 +22,6 @@
#include "api/units/time_delta.h"
#include "call/call.h"
#include "call/degraded_call.h"
-#include "call/rtp_transport_config.h"
#include "rtc_base/checks.h"
#include "rtc_base/experiments/field_trial_list.h"
#include "rtc_base/experiments/field_trial_parser.h"
@@ -85,12 +84,7 @@
receive_degradation_configs =
GetNetworkConfigs(config.env.field_trials(), /*send=*/false);
- RtpTransportConfig transportConfig = config.ExtractTransportConfig();
-
- std::unique_ptr<Call> call =
- Call::Create(config, Clock::GetRealTimeClock(),
- config.rtp_transport_controller_send_factory->Create(
- transportConfig, Clock::GetRealTimeClock()));
+ std::unique_ptr<Call> call = Call::Create(config);
if (!send_degradation_configs.empty() ||
!receive_degradation_configs.empty()) {
