From da4c102cbdfbcd1432116ef946b6a2bf0c973888 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Henrik=20Bostr=C3=B6m?= <hbos@webrtc.org>
Date: Tue, 15 Nov 2022 15:45:41 +0100
Subject: [PATCH] Refactor some config plumbing in call/.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Address perkj's comments left in
https://webrtc-review.googlesource.com/c/src/+/283420. I was a bit
trigger-happy with the submit button.
Bug: chromium:1354491
Change-Id: Ifd052f75af3763b0b52807c31ea790e3efee921d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/283521
Reviewed-by: Erik Språng <sprang@webrtc.org>
Auto-Submit: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38638}
---
call/rtp_transport_controller_send.cc | 56 ++++++++++----------
call/rtp_transport_controller_send.h | 11 +---
call/rtp_transport_controller_send_factory.h | 5 +-
call/rtp_video_sender_unittest.cc | 16 +++---
modules/pacing/task_queue_paced_sender.cc | 7 ++-
pc/peer_connection_factory.cc | 11 ++--
pc/peer_connection_factory.h | 2 +-
7 files changed, 48 insertions(+), 60 deletions(-)
diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc
index f1a7f305ce..18c75df4e5 100644
--- a/call/rtp_transport_controller_send.cc
+++ b/call/rtp_transport_controller_send.cc
@@ -84,55 +84,55 @@ RtpTransportControllerSend::PacerSettings::PacerSettings(
RtpTransportControllerSend::RtpTransportControllerSend(
Clock* clock,
- webrtc::RtcEventLog* event_log,
- NetworkStatePredictorFactoryInterface* predictor_factory,
- NetworkControllerFactoryInterface* controller_factory,
- const BitrateConstraints& bitrate_config,
- TaskQueueFactory* task_queue_factory,
- const FieldTrialsView& trials,
- absl::optional<TimeDelta> pacer_burst_interval)
+ const RtpTransportConfig& config)
: clock_(clock),
- event_log_(event_log),
- task_queue_factory_(task_queue_factory),
- bitrate_configurator_(bitrate_config),
+ event_log_(config.event_log),
+ task_queue_factory_(config.task_queue_factory),
+ bitrate_configurator_(config.bitrate_config),
pacer_started_(false),
- pacer_settings_(trials),
+ pacer_settings_(*config.trials),
pacer_(clock,
&packet_router_,
- trials,
- task_queue_factory,
+ *config.trials,
+ config.task_queue_factory,
pacer_settings_.holdback_window.Get(),
pacer_settings_.holdback_packets.Get(),
- pacer_burst_interval),
+ config.pacer_burst_interval),
observer_(nullptr),
- controller_factory_override_(controller_factory),
+ controller_factory_override_(config.network_controller_factory),
controller_factory_fallback_(
- std::make_unique<GoogCcNetworkControllerFactory>(predictor_factory)),
+ std::make_unique<GoogCcNetworkControllerFactory>(
+ config.network_state_predictor_factory)),
process_interval_(controller_factory_fallback_->GetProcessInterval()),
last_report_block_time_(Timestamp::Millis(clock_->TimeInMilliseconds())),
reset_feedback_on_route_change_(
- !IsEnabled(trials, "WebRTC-Bwe-NoFeedbackReset")),
+ !IsEnabled(*config.trials, "WebRTC-Bwe-NoFeedbackReset")),
send_side_bwe_with_overhead_(
- !IsDisabled(trials, "WebRTC-SendSideBwe-WithOverhead")),
+ !IsDisabled(*config.trials, "WebRTC-SendSideBwe-WithOverhead")),
add_pacing_to_cwin_(
- IsEnabled(trials, "WebRTC-AddPacingToCongestionWindowPushback")),
+ IsEnabled(*config.trials,
+ "WebRTC-AddPacingToCongestionWindowPushback")),
relay_bandwidth_cap_("relay_cap", DataRate::PlusInfinity()),
transport_overhead_bytes_per_packet_(0),
network_available_(false),
congestion_window_size_(DataSize::PlusInfinity()),
is_congested_(false),
retransmission_rate_limiter_(clock, kRetransmitWindowSizeMs),
- task_queue_(trials, "rtp_send_controller", task_queue_factory),
- field_trials_(trials) {
+ task_queue_(*config.trials,
+ "rtp_send_controller",
+ config.task_queue_factory),
+ field_trials_(*config.trials) {
ParseFieldTrial({&relay_bandwidth_cap_},
- trials.Lookup("WebRTC-Bwe-NetworkRouteConstraints"));
- initial_config_.constraints = ConvertConstraints(bitrate_config, clock_);
- initial_config_.event_log = event_log;
- initial_config_.key_value_config = &trials;
- RTC_DCHECK(bitrate_config.start_bitrate_bps > 0);
+ config.trials->Lookup("WebRTC-Bwe-NetworkRouteConstraints"));
+ initial_config_.constraints =
+ ConvertConstraints(config.bitrate_config, clock_);
+ initial_config_.event_log = config.event_log;
+ initial_config_.key_value_config = config.trials;
+ RTC_DCHECK(config.bitrate_config.start_bitrate_bps > 0);
- pacer_.SetPacingRates(DataRate::BitsPerSec(bitrate_config.start_bitrate_bps),
- DataRate::Zero());
+ pacer_.SetPacingRates(
+ DataRate::BitsPerSec(config.bitrate_config.start_bitrate_bps),
+ DataRate::Zero());
}
RtpTransportControllerSend::~RtpTransportControllerSend() {
diff --git a/call/rtp_transport_controller_send.h b/call/rtp_transport_controller_send.h
index d66f104e86..2ba0fda9ba 100644
--- a/call/rtp_transport_controller_send.h
+++ b/call/rtp_transport_controller_send.h
@@ -25,6 +25,7 @@
#include "api/transport/network_control.h"
#include "api/units/data_rate.h"
#include "call/rtp_bitrate_configurator.h"
+#include "call/rtp_transport_config.h"
#include "call/rtp_transport_controller_send_interface.h"
#include "call/rtp_video_sender.h"
#include "modules/congestion_controller/rtp/control_handler.h"
@@ -50,15 +51,7 @@ class RtpTransportControllerSend final
public TransportFeedbackObserver,
public NetworkStateEstimateObserver {
public:
- RtpTransportControllerSend(
- Clock* clock,
- RtcEventLog* event_log,
- NetworkStatePredictorFactoryInterface* predictor_factory,
- NetworkControllerFactoryInterface* controller_factory,
- const BitrateConstraints& bitrate_config,
- TaskQueueFactory* task_queue_factory,
- const FieldTrialsView& trials,
- absl::optional<TimeDelta> pacer_burst_interval);
+ RtpTransportControllerSend(Clock* clock, const RtpTransportConfig& config);
~RtpTransportControllerSend() override;
RtpTransportControllerSend(const RtpTransportControllerSend&) = delete;
diff --git a/call/rtp_transport_controller_send_factory.h b/call/rtp_transport_controller_send_factory.h
index 592ca91f6b..6349302e45 100644
--- a/call/rtp_transport_controller_send_factory.h
+++ b/call/rtp_transport_controller_send_factory.h
@@ -25,10 +25,7 @@ class RtpTransportControllerSendFactory
const RtpTransportConfig& config,
Clock* clock) override {
RTC_CHECK(config.trials);
- return std::make_unique<RtpTransportControllerSend>(
- clock, config.event_log, config.network_state_predictor_factory,
- config.network_controller_factory, config.bitrate_config,
- config.task_queue_factory, *config.trials, config.pacer_burst_interval);
+ return std::make_unique<RtpTransportControllerSend>(clock, config);
}
virtual ~RtpTransportControllerSendFactory() {}
diff --git a/call/rtp_video_sender_unittest.cc b/call/rtp_video_sender_unittest.cc
index ab3dd3b653..3ce9ca2ddc 100644
--- a/call/rtp_video_sender_unittest.cc
+++ b/call/rtp_video_sender_unittest.cc
@@ -129,14 +129,14 @@ class RtpVideoSenderTestFixture {
payload_type)),
send_delay_stats_(time_controller_.GetClock()),
bitrate_config_(GetBitrateConfig()),
- transport_controller_(time_controller_.GetClock(),
- &event_log_,
- nullptr,
- nullptr,
- bitrate_config_,
- time_controller_.GetTaskQueueFactory(),
- field_trials ? *field_trials : field_trials_,
- absl::nullopt),
+ transport_controller_(
+ time_controller_.GetClock(),
+ RtpTransportConfig{
+ .bitrate_config = bitrate_config_,
+ .event_log = &event_log_,
+ .task_queue_factory = time_controller_.GetTaskQueueFactory(),
+ .trials = field_trials ? field_trials : &field_trials_,
+ }),
stats_proxy_(time_controller_.GetClock(),
config_,
VideoEncoderConfig::ContentType::kRealtimeVideo,
diff --git a/modules/pacing/task_queue_paced_sender.cc b/modules/pacing/task_queue_paced_sender.cc
index 8879d19087..e8b695ca5f 100644
--- a/modules/pacing/task_queue_paced_sender.cc
+++ b/modules/pacing/task_queue_paced_sender.cc
@@ -86,12 +86,11 @@ TaskQueuePacedSender::TaskQueuePacedSender(
burst = slacked_burst;
}
}
- // Burst can also be controlled via the `burst_interval` argument.
- if (burst_interval.has_value() &&
- (!burst.has_value() || burst.value() < burst_interval.value())) {
+ // If not overriden by an experiment, the burst is specified by the
+ // `burst_interval` argument.
+ if (!burst.has_value()) {
burst = burst_interval;
}
-
if (burst.has_value()) {
pacing_controller_.SetSendBurstInterval(burst.value());
}
diff --git a/pc/peer_connection_factory.cc b/pc/peer_connection_factory.cc
index 27efe73e0b..afebdd79a5 100644
--- a/pc/peer_connection_factory.cc
+++ b/pc/peer_connection_factory.cc
@@ -242,10 +242,9 @@ PeerConnectionFactory::CreatePeerConnectionOrError(
const FieldTrialsView* trials =
dependencies.trials ? dependencies.trials.get() : &field_trials();
- std::unique_ptr<Call> call = worker_thread()->BlockingCall(
- [this, &event_log, trials,
- pacer_burst_interval = configuration.pacer_burst_interval] {
- return CreateCall_w(event_log.get(), *trials, pacer_burst_interval);
+ std::unique_ptr<Call> call =
+ worker_thread()->BlockingCall([this, &event_log, trials, &configuration] {
+ return CreateCall_w(event_log.get(), *trials, configuration);
});
auto result = PeerConnection::Create(context_, options_, std::move(event_log),
@@ -305,7 +304,7 @@ std::unique_ptr<RtcEventLog> PeerConnectionFactory::CreateRtcEventLog_w() {
std::unique_ptr<Call> PeerConnectionFactory::CreateCall_w(
RtcEventLog* event_log,
const FieldTrialsView& field_trials,
- absl::optional<TimeDelta> pacer_burst_interval) {
+ const PeerConnectionInterface::RTCConfiguration& configuration) {
RTC_DCHECK_RUN_ON(worker_thread());
webrtc::Call::Config call_config(event_log, network_thread());
@@ -348,7 +347,7 @@ std::unique_ptr<Call> PeerConnectionFactory::CreateCall_w(
call_config.rtp_transport_controller_send_factory =
transport_controller_send_factory_.get();
call_config.metronome = metronome_.get();
- call_config.pacer_burst_interval = pacer_burst_interval;
+ call_config.pacer_burst_interval = configuration.pacer_burst_interval;
return std::unique_ptr<Call>(
context_->call_factory()->CreateCall(call_config));
}
diff --git a/pc/peer_connection_factory.h b/pc/peer_connection_factory.h
index 9e96a2bb8c..dac3702e37 100644
--- a/pc/peer_connection_factory.h
+++ b/pc/peer_connection_factory.h
@@ -139,7 +139,7 @@ class PeerConnectionFactory : public PeerConnectionFactoryInterface {
std::unique_ptr<Call> CreateCall_w(
RtcEventLog* event_log,
const FieldTrialsView& field_trials,
- absl::optional<TimeDelta> pacer_burst_interval);
+ const PeerConnectionInterface::RTCConfiguration& configuration);
rtc::scoped_refptr<ConnectionContext> context_;
PeerConnectionFactoryInterface::Options options_