Add option to provide Environment for CongestionConroller construction
This would allow network controllers, GoogCcNetworkController in particular to have access to Environment at construction and thus it can rely on propagated field trials and won't need to fallback to the global field trial string
Bug: webrtc:42220378
Change-Id: I36099522e3866a89a8c8d6303da03f7d5b1cad8e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/350201
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42260}
diff --git a/api/transport/BUILD.gn b/api/transport/BUILD.gn
index 9f89df5..756e1cc 100644
--- a/api/transport/BUILD.gn
+++ b/api/transport/BUILD.gn
@@ -39,6 +39,7 @@
deps = [
"../../api:field_trials_view",
+ "../environment",
"../rtc_event_log",
"../units:data_rate",
"../units:data_size",
diff --git a/api/transport/network_control.h b/api/transport/network_control.h
index 8623224..3b76dff 100644
--- a/api/transport/network_control.h
+++ b/api/transport/network_control.h
@@ -15,6 +15,7 @@
#include <memory>
#include "absl/base/attributes.h"
+#include "api/environment/environment.h"
#include "api/field_trials_view.h"
#include "api/rtc_event_log/rtc_event_log.h"
#include "api/transport/network_types.h"
@@ -35,6 +36,12 @@
// Configuration sent to factory create function. The parameters here are
// optional to use for a network controller implementation.
struct NetworkControllerConfig {
+ // TODO: bugs.webrtc.org/42220378 - Delete the default constructor and
+ // thus make Environment (including field trials) a required parameter.
+ [[deprecated]] NetworkControllerConfig() = default;
+ explicit NetworkControllerConfig(const Environment& env)
+ : key_value_config(&env.field_trials()), event_log(&env.event_log()) {}
+
// The initial constraints to start with, these can be changed at any later
// time by calls to OnTargetRateConstraints. Note that the starting rate
// has to be set initially to provide a starting state for the network
diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc
index 6a168de..0104d3d 100644
--- a/call/rtp_transport_controller_send.cc
+++ b/call/rtp_transport_controller_send.cc
@@ -90,6 +90,7 @@
process_interval_(controller_factory_fallback_->GetProcessInterval()),
last_report_block_time_(
Timestamp::Millis(env_.clock().TimeInMilliseconds())),
+ initial_config_(env_),
reset_feedback_on_route_change_(
!env_.field_trials().IsEnabled("WebRTC-Bwe-NoFeedbackReset")),
add_pacing_to_cwin_(env_.field_trials().IsEnabled(
@@ -105,8 +106,6 @@
env_.field_trials().Lookup("WebRTC-Bwe-NetworkRouteConstraints"));
initial_config_.constraints =
ConvertConstraints(config.bitrate_config, &env_.clock());
- initial_config_.event_log = &env_.event_log();
- initial_config_.key_value_config = &env_.field_trials();
RTC_DCHECK(config.bitrate_config.start_bitrate_bps > 0);
pacer_.SetPacingRates(
diff --git a/modules/congestion_controller/goog_cc/BUILD.gn b/modules/congestion_controller/goog_cc/BUILD.gn
index c017d39..c6660cb 100644
--- a/modules/congestion_controller/goog_cc/BUILD.gn
+++ b/modules/congestion_controller/goog_cc/BUILD.gn
@@ -339,6 +339,8 @@
":send_side_bwe",
"../../../api:field_trials_view",
"../../../api:network_state_predictor_api",
+ "../../../api/environment",
+ "../../../api/environment:environment_factory",
"../../../api/rtc_event_log",
"../../../api/test/network_emulation",
"../../../api/test/network_emulation:create_cross_traffic",
diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc b/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc
index c651f13..41aa3e5 100644
--- a/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc
+++ b/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc
@@ -19,6 +19,8 @@
#include "absl/strings/string_view.h"
#include "absl/types/optional.h"
+#include "api/environment/environment.h"
+#include "api/environment/environment_factory.h"
#include "api/test/network_emulation/create_cross_traffic.h"
#include "api/test/network_emulation/cross_traffic.h"
#include "api/transport/goog_cc_factory.h"
@@ -274,7 +276,7 @@
int starting_bandwidth_kbps = kInitialBitrateKbps,
int min_data_rate_kbps = 0,
int max_data_rate_kbps = 5 * kInitialBitrateKbps) {
- NetworkControllerConfig config;
+ NetworkControllerConfig config(env_);
config.constraints.at_time = Timestamp::Zero();
config.constraints.min_data_rate =
DataRate::KilobitsPerSec(min_data_rate_kbps);
@@ -282,11 +284,11 @@
DataRate::KilobitsPerSec(max_data_rate_kbps);
config.constraints.starting_rate =
DataRate::KilobitsPerSec(starting_bandwidth_kbps);
- config.event_log = &event_log_;
return config;
}
NiceMock<MockRtcEventLog> event_log_;
+ const Environment env_ = CreateEnvironment(&event_log_);
GoogCcNetworkControllerFactory factory_;
};
diff --git a/modules/congestion_controller/pcc/BUILD.gn b/modules/congestion_controller/pcc/BUILD.gn
index 85b12b3..51e3b4f 100644
--- a/modules/congestion_controller/pcc/BUILD.gn
+++ b/modules/congestion_controller/pcc/BUILD.gn
@@ -111,6 +111,7 @@
":pcc_controller",
":rtt_tracker",
":utility_function",
+ "../../../api/environment:environment_factory",
"../../../api/transport:network_control",
"../../../api/units:data_rate",
"../../../api/units:data_size",
diff --git a/modules/congestion_controller/pcc/pcc_network_controller_unittest.cc b/modules/congestion_controller/pcc/pcc_network_controller_unittest.cc
index f7cf2c4..cf4a884 100644
--- a/modules/congestion_controller/pcc/pcc_network_controller_unittest.cc
+++ b/modules/congestion_controller/pcc/pcc_network_controller_unittest.cc
@@ -12,6 +12,7 @@
#include <memory>
+#include "api/environment/environment_factory.h"
#include "modules/congestion_controller/pcc/pcc_factory.h"
#include "test/gmock.h"
#include "test/gtest.h"
@@ -41,35 +42,19 @@
AllOf(Ge(min_data_rate), Le(max_data_rate)));
}
-NetworkControllerConfig InitialConfig(
- int starting_bandwidth_kbps = kInitialBitrate.kbps(),
- int min_data_rate_kbps = 0,
- int max_data_rate_kbps = 5 * kInitialBitrate.kbps()) {
- NetworkControllerConfig config;
- config.constraints.at_time = kDefaultStartTime;
- config.constraints.min_data_rate =
- DataRate::KilobitsPerSec(min_data_rate_kbps);
- config.constraints.max_data_rate =
- DataRate::KilobitsPerSec(max_data_rate_kbps);
- config.constraints.starting_rate =
- DataRate::KilobitsPerSec(starting_bandwidth_kbps);
- return config;
-}
-
-ProcessInterval InitialProcessInterval() {
- ProcessInterval process_interval;
- process_interval.at_time = kDefaultStartTime;
- return process_interval;
-}
-
} // namespace
TEST(PccNetworkControllerTest, SendsConfigurationOnFirstProcess) {
- std::unique_ptr<NetworkControllerInterface> controller_;
- controller_.reset(new pcc::PccNetworkController(InitialConfig()));
+ Environment env = CreateEnvironment();
+ NetworkControllerConfig config(env);
+ config.constraints.at_time = kDefaultStartTime;
+ config.constraints.min_data_rate = DataRate::Zero();
+ config.constraints.max_data_rate = 5 * kInitialBitrate;
+ config.constraints.starting_rate = kInitialBitrate;
+ pcc::PccNetworkController controller(config);
NetworkControlUpdate update =
- controller_->OnProcessInterval(InitialProcessInterval());
+ controller.OnProcessInterval({.at_time = kDefaultStartTime});
EXPECT_THAT(*update.target_rate, TargetRateCloseTo(kInitialBitrate));
EXPECT_THAT(*update.pacer_config,
Property(&PacerConfig::data_rate, Ge(kInitialBitrate)));
diff --git a/rtc_tools/rtc_event_log_visualizer/analyzer.cc b/rtc_tools/rtc_event_log_visualizer/analyzer.cc
index 5ed4783..527b869 100644
--- a/rtc_tools/rtc_event_log_visualizer/analyzer.cc
+++ b/rtc_tools/rtc_event_log_visualizer/analyzer.cc
@@ -1651,11 +1651,10 @@
TimeDelta process_interval = factory.GetProcessInterval();
// TODO(holmer): Log the call config and use that here instead.
static const uint32_t kDefaultStartBitrateBps = 300000;
- NetworkControllerConfig cc_config;
+ NetworkControllerConfig cc_config(CreateEnvironment(&null_event_log));
cc_config.constraints.at_time = Timestamp::Micros(clock.TimeInMicroseconds());
cc_config.constraints.starting_rate =
DataRate::BitsPerSec(kDefaultStartBitrateBps);
- cc_config.event_log = &null_event_log;
auto goog_cc = factory.Create(cc_config);
TimeSeries time_series("Delay-based estimate", LineStyle::kStep,
diff --git a/rtc_tools/rtc_event_log_visualizer/log_simulation.cc b/rtc_tools/rtc_event_log_visualizer/log_simulation.cc
index a32c800..bfa0b4b 100644
--- a/rtc_tools/rtc_event_log_visualizer/log_simulation.cc
+++ b/rtc_tools/rtc_event_log_visualizer/log_simulation.cc
@@ -15,6 +15,7 @@
#include <memory>
#include <utility>
+#include "api/environment/environment_factory.h"
#include "api/transport/network_control.h"
#include "api/transport/network_types.h"
#include "api/units/data_rate.h"
@@ -46,11 +47,10 @@
void LogBasedNetworkControllerSimulation::ProcessUntil(Timestamp to_time) {
if (last_process_.IsInfinite()) {
- NetworkControllerConfig config;
+ NetworkControllerConfig config(CreateEnvironment(&null_event_log_));
config.constraints.at_time = to_time;
config.constraints.min_data_rate = DataRate::KilobitsPerSec(30);
config.constraints.starting_rate = DataRate::KilobitsPerSec(300);
- config.event_log = &null_event_log_;
controller_ = factory_->Create(config);
}
if (last_process_.IsInfinite() ||
