Use Environment in RtpTransportyControllerSend

RtpTransportControllerSend uses all 4 utilities of the environment and
thus cleaner to propagate them as single parameter instead of 4 separate

Bug: None
Change-Id: I38932c21a73ea41d4bdf2fa04bf3961a2adb25a2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/331821
Reviewed-by: Jakob Ivarsson‎ <jakobi@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41422}

audio/BUILD.gn

@@ -175,7 +175,8 @@ if (rtc_include_tests) {
       "../api/audio_codecs/opus:audio_decoder_opus",
       "../api/audio_codecs/opus:audio_encoder_opus",
       "../api/crypto:frame_decryptor_interface",
-      "../api/rtc_event_log",
+      "../api/environment",
+      "../api/environment:environment_factory",
       "../api/task_queue:default_task_queue_factory",
       "../api/task_queue/test:mock_task_queue_base",
       "../api/units:time_delta",

audio/channel_send_unittest.cc

@@ -14,7 +14,8 @@
 
 #include "api/audio/audio_frame.h"
 #include "api/audio_codecs/builtin_audio_encoder_factory.h"
-#include "api/rtc_event_log/rtc_event_log.h"
+#include "api/environment/environment.h"
+#include "api/environment/environment_factory.h"
 #include "api/scoped_refptr.h"
 #include "api/units/time_delta.h"
 #include "api/units/timestamp.h"
@@ -53,18 +54,17 @@ class ChannelSendTest : public ::testing::Test {
  protected:
   ChannelSendTest()
       : time_controller_(Timestamp::Seconds(1)),
+        env_(CreateEnvironment(&field_trials_,
+                               time_controller_.GetClock(),
+                               time_controller_.CreateTaskQueueFactory())),
         transport_controller_(
-            time_controller_.GetClock(),
+            RtpTransportConfig{.env = env_,
-            RtpTransportConfig{
+                               .bitrate_config = GetBitrateConfig()}) {
-                .bitrate_config = GetBitrateConfig(),
-                .event_log = &event_log_,
-                .task_queue_factory = time_controller_.GetTaskQueueFactory(),
-                .trials = &field_trials_,
-            }) {
     channel_ = voe::CreateChannelSend(
         time_controller_.GetClock(), time_controller_.GetTaskQueueFactory(),
-        &transport_, nullptr, &event_log_, nullptr, crypto_options_, false,
+        &transport_, nullptr, &env_.event_log(), nullptr, crypto_options_,
-        kRtcpIntervalMs, kSsrc, nullptr, &transport_controller_, field_trials_);
+        false, kRtcpIntervalMs, kSsrc, nullptr, &transport_controller_,
+        env_.field_trials());
     encoder_factory_ = CreateBuiltinAudioEncoderFactory();
     SdpAudioFormat opus = SdpAudioFormat("opus", kRtpRateHz, 2);
     std::unique_ptr<AudioEncoder> encoder =
@@ -94,7 +94,7 @@ class ChannelSendTest : public ::testing::Test {
 
   GlobalSimulatedTimeController time_controller_;
   webrtc::test::ScopedKeyValueConfig field_trials_;
-  RtcEventLogNull event_log_;
+  Environment env_;
   NiceMock<MockTransport> transport_;
   CryptoOptions crypto_options_;
   RtpTransportControllerSend transport_controller_;

call/BUILD.gn

@@ -110,20 +110,20 @@ rtc_library("rtp_interfaces") {
   deps = [
     "../api:array_view",
     "../api:fec_controller_api",
-    "../api:field_trials_view",
     "../api:frame_transformer_interface",
     "../api:network_state_predictor_api",
     "../api:rtp_headers",
     "../api:rtp_parameters",
     "../api/crypto:options",
+    "../api/environment",
     "../api/rtc_event_log",
     "../api/transport:bitrate_settings",
     "../api/transport:network_control",
+    "../api/units:time_delta",
     "../api/units:timestamp",
     "../common_video:frame_counts",
     "../modules/rtp_rtcp:rtp_rtcp_format",
     "../rtc_base:checks",
-    "../rtc_base:rtc_task_queue",
     "../rtc_base:stringutils",
   ]
   absl_deps = [
@@ -185,6 +185,7 @@ rtc_library("rtp_sender") {
     "../api:rtp_parameters",
     "../api:sequence_checker",
     "../api:transport_api",
+    "../api/environment",
     "../api/rtc_event_log",
     "../api/task_queue:pending_task_safety_flag",
     "../api/task_queue:task_queue",

call/call.cc

@@ -470,10 +470,10 @@ std::unique_ptr<Call> Call::Create(const CallConfig& config) {
   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());
+        config.ExtractTransportConfig());
   } else {
     transport_send = RtpTransportControllerSendFactory().Create(
-        config.ExtractTransportConfig(), &config.env.clock());
+        config.ExtractTransportConfig());
   }
 
   return std::make_unique<internal::Call>(config, std::move(transport_send));

call/call_config.cc

@@ -23,17 +23,14 @@ CallConfig::CallConfig(const Environment& env,
 CallConfig::CallConfig(const CallConfig& config) = default;
 
 RtpTransportConfig CallConfig::ExtractTransportConfig() const {
-  RtpTransportConfig transportConfig;
+  RtpTransportConfig transport_config = {.env = env};
-  transportConfig.bitrate_config = bitrate_config;
+  transport_config.bitrate_config = bitrate_config;
-  transportConfig.event_log = &env.event_log();
+  transport_config.network_controller_factory = network_controller_factory;
-  transportConfig.network_controller_factory = network_controller_factory;
+  transport_config.network_state_predictor_factory =
-  transportConfig.network_state_predictor_factory =
       network_state_predictor_factory;
-  transportConfig.task_queue_factory = &env.task_queue_factory();
+  transport_config.pacer_burst_interval = pacer_burst_interval;
-  transportConfig.trials = &env.field_trials();
-  transportConfig.pacer_burst_interval = pacer_burst_interval;
 
-  return transportConfig;
+  return transport_config;
 }
 
 CallConfig::~CallConfig() = default;

call/rtp_transport_config.h

@@ -13,27 +13,22 @@
 
 #include <memory>
 
-#include "api/field_trials_view.h"
+#include "absl/types/optional.h"
+#include "api/environment/environment.h"
 #include "api/network_state_predictor.h"
-#include "api/rtc_event_log/rtc_event_log.h"
 #include "api/transport/bitrate_settings.h"
 #include "api/transport/network_control.h"
-#include "rtc_base/task_queue.h"
+#include "api/units/time_delta.h"
 
 namespace webrtc {
 
 struct RtpTransportConfig {
+  Environment env;
+
   // Bitrate config used until valid bitrate estimates are calculated. Also
   // used to cap total bitrate used. This comes from the remote connection.
   BitrateConstraints bitrate_config;
 
-  // RtcEventLog to use for this call. Required.
-  // Use webrtc::RtcEventLog::CreateNull() for a null implementation.
-  RtcEventLog* event_log = nullptr;
-
-  // Task Queue Factory to be used in this call. Required.
-  TaskQueueFactory* task_queue_factory = nullptr;
-
   // NetworkStatePredictor to use for this call.
   NetworkStatePredictorFactoryInterface* network_state_predictor_factory =
       nullptr;
@@ -41,10 +36,6 @@ struct RtpTransportConfig {
   // Network controller factory to use for this call.
   NetworkControllerFactoryInterface* network_controller_factory = nullptr;
 
-  // Key-value mapping of internal configurations to apply,
-  // e.g. field trials.
-  const FieldTrialsView* trials = nullptr;
-
   // The burst interval of the pacer, see TaskQueuePacedSender constructor.
   absl::optional<TimeDelta> pacer_burst_interval;
 };

call/rtp_transport_controller_send.cc

@@ -62,50 +62,47 @@ TargetRateConstraints ConvertConstraints(const BitrateConstraints& contraints,
                             contraints.start_bitrate_bps, clock);
 }
 
-bool IsEnabled(const FieldTrialsView& trials, absl::string_view key) {
-  return absl::StartsWith(trials.Lookup(key), "Enabled");
-}
-
 bool IsRelayed(const rtc::NetworkRoute& route) {
   return route.local.uses_turn() || route.remote.uses_turn();
 }
 }  // namespace
 
 RtpTransportControllerSend::RtpTransportControllerSend(
-    Clock* clock,
     const RtpTransportConfig& config)
-    : clock_(clock),
+    : env_(config.env),
-      event_log_(config.event_log),
-      task_queue_factory_(config.task_queue_factory),
       task_queue_(TaskQueueBase::Current()),
       bitrate_configurator_(config.bitrate_config),
       pacer_started_(false),
-      pacer_(clock, &packet_router_, *config.trials, TimeDelta::Millis(5), 3),
+      pacer_(&env_.clock(),
+             &packet_router_,
+             env_.field_trials(),
+             TimeDelta::Millis(5),
+             3),
       observer_(nullptr),
       controller_factory_override_(config.network_controller_factory),
       controller_factory_fallback_(
           std::make_unique<GoogCcNetworkControllerFactory>(
               config.network_state_predictor_factory)),
       process_interval_(controller_factory_fallback_->GetProcessInterval()),
-      last_report_block_time_(Timestamp::Millis(clock_->TimeInMilliseconds())),
+      last_report_block_time_(
+          Timestamp::Millis(env_.clock().TimeInMilliseconds())),
       reset_feedback_on_route_change_(
-          !IsEnabled(*config.trials, "WebRTC-Bwe-NoFeedbackReset")),
+          !env_.field_trials().IsEnabled("WebRTC-Bwe-NoFeedbackReset")),
-      add_pacing_to_cwin_(
+      add_pacing_to_cwin_(env_.field_trials().IsEnabled(
-          IsEnabled(*config.trials,
+          "WebRTC-AddPacingToCongestionWindowPushback")),
-                    "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),
+      retransmission_rate_limiter_(&env_.clock(), kRetransmitWindowSizeMs) {
-      field_trials_(*config.trials) {
+  ParseFieldTrial(
-  ParseFieldTrial({&relay_bandwidth_cap_},
+      {&relay_bandwidth_cap_},
-                  config.trials->Lookup("WebRTC-Bwe-NetworkRouteConstraints"));
+      env_.field_trials().Lookup("WebRTC-Bwe-NetworkRouteConstraints"));
   initial_config_.constraints =
-      ConvertConstraints(config.bitrate_config, clock_);
+      ConvertConstraints(config.bitrate_config, &env_.clock());
-  initial_config_.event_log = config.event_log;
+  initial_config_.event_log = &env_.event_log();
-  initial_config_.key_value_config = config.trials;
+  initial_config_.key_value_config = &env_.field_trials();
   RTC_DCHECK(config.bitrate_config.start_bitrate_bps > 0);
 
   pacer_.SetPacingRates(
@@ -137,14 +134,14 @@ RtpVideoSenderInterface* RtpTransportControllerSend::CreateRtpVideoSender(
     rtc::scoped_refptr<FrameTransformerInterface> frame_transformer) {
   RTC_DCHECK_RUN_ON(&sequence_checker_);
   video_rtp_senders_.push_back(std::make_unique<RtpVideoSender>(
-      clock_, suspended_ssrcs, states, rtp_config, rtcp_report_interval_ms,
+      &env_.clock(), suspended_ssrcs, states, rtp_config,
-      send_transport, observers,
+      rtcp_report_interval_ms, send_transport, observers,
       // TODO(holmer): Remove this circular dependency by injecting
       // the parts of RtpTransportControllerSendInterface that are really used.
       this, event_log, &retransmission_rate_limiter_, std::move(fec_controller),
       frame_encryption_config.frame_encryptor,
       frame_encryption_config.crypto_options, std::move(frame_transformer),
-      field_trials_, task_queue_factory_));
+      env_.field_trials(), &env_.task_queue_factory()));
   return video_rtp_senders_.back().get();
 }
 
@@ -306,13 +303,11 @@ void RtpTransportControllerSend::OnNetworkRouteChanged(
                      << " bps.";
     RTC_DCHECK_GT(bitrate_config.start_bitrate_bps, 0);
 
-    if (event_log_) {
+    env_.event_log().Log(std::make_unique<RtcEventRouteChange>(
-      event_log_->Log(std::make_unique<RtcEventRouteChange>(
+        network_route.connected, network_route.packet_overhead));
-          network_route.connected, network_route.packet_overhead));
-    }
     NetworkRouteChange msg;
-    msg.at_time = Timestamp::Millis(clock_->TimeInMilliseconds());
+    msg.at_time = Timestamp::Millis(env_.clock().TimeInMilliseconds());
-    msg.constraints = ConvertConstraints(bitrate_config, clock_);
+    msg.constraints = ConvertConstraints(bitrate_config, &env_.clock());
     transport_overhead_bytes_per_packet_ = network_route.packet_overhead;
     if (reset_feedback_on_route_change_) {
       transport_feedback_adapter_.SetNetworkRoute(network_route);
@@ -331,7 +326,7 @@ void RtpTransportControllerSend::OnNetworkAvailability(bool network_available) {
   RTC_LOG(LS_VERBOSE) << "SignalNetworkState "
                       << (network_available ? "Up" : "Down");
   NetworkAvailability msg;
-  msg.at_time = Timestamp::Millis(clock_->TimeInMilliseconds());
+  msg.at_time = Timestamp::Millis(env_.clock().TimeInMilliseconds());
   msg.network_available = network_available;
   network_available_ = network_available;
   if (network_available) {
@@ -429,7 +424,7 @@ void RtpTransportControllerSend::OnReceivedPacket(
 void RtpTransportControllerSend::UpdateBitrateConstraints(
     const BitrateConstraints& updated) {
   RTC_DCHECK_RUN_ON(&sequence_checker_);
-  TargetRateConstraints msg = ConvertConstraints(updated, clock_);
+  TargetRateConstraints msg = ConvertConstraints(updated, &env_.clock());
   if (controller_) {
     PostUpdates(controller_->OnTargetRateConstraints(msg));
   } else {
@@ -532,7 +527,8 @@ void RtpTransportControllerSend::OnRttUpdate(Timestamp receive_time,
 void RtpTransportControllerSend::OnAddPacket(
     const RtpPacketSendInfo& packet_info) {
   RTC_DCHECK_RUN_ON(&sequence_checker_);
-  Timestamp creation_time = Timestamp::Millis(clock_->TimeInMilliseconds());
+  Timestamp creation_time =
+      Timestamp::Millis(env_.clock().TimeInMilliseconds());
   feedback_demuxer_.AddPacket(packet_info);
   transport_feedback_adapter_.AddPacket(
       packet_info, transport_overhead_bytes_per_packet_, creation_time);
@@ -558,11 +554,9 @@ void RtpTransportControllerSend::OnTransportFeedback(
 void RtpTransportControllerSend::OnRemoteNetworkEstimate(
     NetworkStateEstimate estimate) {
   RTC_DCHECK_RUN_ON(&sequence_checker_);
-  if (event_log_) {
+  env_.event_log().Log(std::make_unique<RtcEventRemoteEstimate>(
-    event_log_->Log(std::make_unique<RtcEventRemoteEstimate>(
+      estimate.link_capacity_lower, estimate.link_capacity_upper));
-        estimate.link_capacity_lower, estimate.link_capacity_upper));
+  estimate.update_time = Timestamp::Millis(env_.clock().TimeInMilliseconds());
-  }
-  estimate.update_time = Timestamp::Millis(clock_->TimeInMilliseconds());
   if (controller_)
     PostUpdates(controller_->OnNetworkStateEstimate(estimate));
 }
@@ -576,7 +570,7 @@ void RtpTransportControllerSend::MaybeCreateControllers() {
   control_handler_ = std::make_unique<CongestionControlHandler>();
 
   initial_config_.constraints.at_time =
-      Timestamp::Millis(clock_->TimeInMilliseconds());
+      Timestamp::Millis(env_.clock().TimeInMilliseconds());
   initial_config_.stream_based_config = streams_config_;
 
   // TODO(srte): Use fallback controller if no feedback is available.
@@ -627,14 +621,15 @@ void RtpTransportControllerSend::StartProcessPeriodicTasks() {
 void RtpTransportControllerSend::UpdateControllerWithTimeInterval() {
   RTC_DCHECK(controller_);
   ProcessInterval msg;
-  msg.at_time = Timestamp::Millis(clock_->TimeInMilliseconds());
+  msg.at_time = Timestamp::Millis(env_.clock().TimeInMilliseconds());
   if (add_pacing_to_cwin_)
     msg.pacer_queue = pacer_.QueueSizeData();
   PostUpdates(controller_->OnProcessInterval(msg));
 }
 
 void RtpTransportControllerSend::UpdateStreamsConfig() {
-  streams_config_.at_time = Timestamp::Millis(clock_->TimeInMilliseconds());
+  streams_config_.at_time =
+      Timestamp::Millis(env_.clock().TimeInMilliseconds());
   if (controller_)
     PostUpdates(controller_->OnStreamsConfig(streams_config_));
 }

call/rtp_transport_controller_send.h

@@ -18,6 +18,7 @@
 #include <vector>
 
 #include "absl/strings/string_view.h"
+#include "api/environment/environment.h"
 #include "api/network_state_predictor.h"
 #include "api/sequence_checker.h"
 #include "api/task_queue/task_queue_base.h"
@@ -41,7 +42,6 @@
 #include "rtc_base/task_utils/repeating_task.h"
 
 namespace webrtc {
-class Clock;
 class FrameEncryptorInterface;
 class RtcEventLog;
 
@@ -51,7 +51,7 @@ class RtpTransportControllerSend final
       public TransportFeedbackObserver,
       public NetworkStateEstimateObserver {
  public:
-  RtpTransportControllerSend(Clock* clock, const RtpTransportConfig& config);
+  explicit RtpTransportControllerSend(const RtpTransportConfig& config);
   ~RtpTransportControllerSend() override;
 
   RtpTransportControllerSend(const RtpTransportControllerSend&) = delete;
@@ -146,9 +146,7 @@ class RtpTransportControllerSend final
   void ProcessSentPacketUpdates(NetworkControlUpdate updates)
       RTC_RUN_ON(sequence_checker_);
 
-  Clock* const clock_;
+  const Environment env_;
-  RtcEventLog* const event_log_;
-  TaskQueueFactory* const task_queue_factory_;
   SequenceChecker sequence_checker_;
   TaskQueueBase* task_queue_;
   PacketRouter packet_router_;
@@ -207,8 +205,6 @@ class RtpTransportControllerSend final
   RateLimiter retransmission_rate_limiter_;
 
   ScopedTaskSafety safety_;
-
-  const FieldTrialsView& field_trials_;
 };
 
 }  // namespace webrtc

call/rtp_transport_controller_send_factory.h

@@ -22,10 +22,8 @@ class RtpTransportControllerSendFactory
     : public RtpTransportControllerSendFactoryInterface {
  public:
   std::unique_ptr<RtpTransportControllerSendInterface> Create(
-      const RtpTransportConfig& config,
+      const RtpTransportConfig& config) override {
-      Clock* clock) override {
+    return std::make_unique<RtpTransportControllerSend>(config);
-    RTC_CHECK(config.trials);
-    return std::make_unique<RtpTransportControllerSend>(clock, config);
   }
 
   virtual ~RtpTransportControllerSendFactory() {}

call/rtp_transport_controller_send_factory_interface.h

@@ -20,11 +20,10 @@ namespace webrtc {
 // controller.
 class RtpTransportControllerSendFactoryInterface {
  public:
-  virtual std::unique_ptr<RtpTransportControllerSendInterface> Create(
+  virtual ~RtpTransportControllerSendFactoryInterface() = default;
-      const RtpTransportConfig& config,
-      Clock* clock) = 0;
 
-  virtual ~RtpTransportControllerSendFactoryInterface() {}
+  virtual std::unique_ptr<RtpTransportControllerSendInterface> Create(
+      const RtpTransportConfig& config) = 0;
 };
 }  // namespace webrtc
 #endif  // CALL_RTP_TRANSPORT_CONTROLLER_SEND_FACTORY_INTERFACE_H_

call/rtp_video_sender_unittest.cc

@@ -16,6 +16,8 @@
 #include <utility>
 
 #include "absl/functional/any_invocable.h"
+#include "api/environment/environment.h"
+#include "api/environment/environment_factory.h"
 #include "call/rtp_transport_controller_send.h"
 #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
 #include "modules/rtp_rtcp/source/byte_io.h"
@@ -118,23 +120,21 @@ class RtpVideoSenderTestFixture {
       rtc::scoped_refptr<FrameTransformerInterface> frame_transformer,
       const FieldTrialsView* field_trials = nullptr)
       : time_controller_(Timestamp::Millis(1000000)),
+        env_(CreateEnvironment(&field_trials_,
+                               field_trials,
+                               time_controller_.GetClock(),
+                               time_controller_.CreateTaskQueueFactory())),
         config_(CreateVideoSendStreamConfig(&transport_,
                                             ssrcs,
                                             rtx_ssrcs,
                                             payload_type)),
         bitrate_config_(GetBitrateConfig()),
         transport_controller_(
-            time_controller_.GetClock(),
+            RtpTransportConfig{.env = env_, .bitrate_config = bitrate_config_}),
-            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,
-                     field_trials ? *field_trials : field_trials_),
+                     env_.field_trials()),
         retransmission_rate_limiter_(time_controller_.GetClock(),
                                      kRetransmitWindowSizeMs) {
     transport_controller_.EnsureStarted();
@@ -144,10 +144,10 @@ class RtpVideoSenderTestFixture {
         config_.rtp, config_.rtcp_report_interval_ms, &transport_,
         CreateObservers(&encoder_feedback_, &stats_proxy_, &stats_proxy_,
                         &stats_proxy_, frame_count_observer, &stats_proxy_),
-        &transport_controller_, &event_log_, &retransmission_rate_limiter_,
+        &transport_controller_, &env_.event_log(),
+        &retransmission_rate_limiter_,
         std::make_unique<FecControllerDefault>(time_controller_.GetClock()),
-        nullptr, CryptoOptions{}, frame_transformer,
+        nullptr, CryptoOptions{}, frame_transformer, env_.field_trials(),
-        field_trials ? *field_trials : field_trials_,
         time_controller_.GetTaskQueueFactory());
   }
 
@@ -197,7 +197,7 @@ class RtpVideoSenderTestFixture {
   NiceMock<MockTransport> transport_;
   NiceMock<MockRtcpIntraFrameObserver> encoder_feedback_;
   GlobalSimulatedTimeController time_controller_;
-  RtcEventLogNull event_log_;
+  Environment env_;
   VideoSendStream::Config config_;
   BitrateConstraints bitrate_config_;
   RtpTransportControllerSend transport_controller_;