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}
This commit is contained in:
Danil Chapovalov 2023-12-05 17:11:15 +01:00 committed by WebRTC LUCI CQ
parent 539bca9ebb
commit abd7814e47
5 changed files with 43 additions and 59 deletions

View file

@ -1391,12 +1391,12 @@ if (rtc_include_tests) {
":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" ]
}

View file

@ -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 @@ void EnableMediaWithDefaultsAndTimeController(
: 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(

View file

@ -183,8 +183,7 @@ class Call final : public webrtc::Call,
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 @@ class Call final : public webrtc::Call,
// 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::string Call::Stats::ToString(int64_t time_ms) const {
}
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 @@ void Call::SendStats::SetMinAllocatableRate(BitrateAllocationLimits limits) {
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 @@ Call::Call(Clock* clock,
: 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 @@ Call::Call(Clock* clock,
/*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 @@ Call::Call(Clock* clock,
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 @@ Call::~Call() {
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 @@ webrtc::AudioSendStream* Call::CreateAudioSendStream(
}
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 @@ webrtc::AudioReceiveStreamInterface* Call::CreateAudioReceiveStream(
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 @@ webrtc::VideoSendStream* Call::CreateVideoSendStream(
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 @@ webrtc::VideoSendStream* Call::CreateVideoSendStream(
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 @@ webrtc::VideoReceiveStreamInterface* Call::CreateVideoReceiveStream(
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 @@ FlexfecReceiveStream* Call::CreateFlexfecReceiveStream(
// 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 @@ void Call::OnSentPacket(const rtc::SentPacket& sent_packet) {
// 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 @@ void Call::DeliverRtpPacket(
// 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));
}

View file

@ -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 @@ class Call {
};
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;

View file

@ -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 @@ std::unique_ptr<Call> CreateCall(const CallConfig& config) {
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()) {