From e902f28d2a5a1e032024fdcf17407dda5f2dbe89 Mon Sep 17 00:00:00 2001
From: Tommi <tommi@webrtc.org>
Date: Thu, 3 Jun 2021 12:02:02 +0200
Subject: [PATCH] Make VideoSendStreamImpl::configured_pacing_factor_ const

Bug: webrtc:12840
Change-Id: Ie479aa39437e373f3dc84de663dc5641d847ded9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/221110
Commit-Queue: Tommi <tommi@webrtc.org>
Reviewed-by: Markus Handell <handellm@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34215}
---
 video/video_send_stream.cc      |  2 +-
 video/video_send_stream_impl.cc | 39 +++++++++++++++++++++++----------
 video/video_send_stream_impl.h  |  6 +++--
 3 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/video/video_send_stream.cc b/video/video_send_stream.cc
index 9764542ce7..b245e8f4bd 100644
--- a/video/video_send_stream.cc
+++ b/video/video_send_stream.cc
@@ -227,7 +227,7 @@ VideoSendStream::Stats VideoSendStream::GetStats() {
 }
 
 absl::optional<float> VideoSendStream::GetPacingFactorOverride() const {
-  return send_stream_->configured_pacing_factor_;
+  return send_stream_->configured_pacing_factor();
 }
 
 void VideoSendStream::StopPermanentlyAndGetRtpStates(
diff --git a/video/video_send_stream_impl.cc b/video/video_send_stream_impl.cc
index 8f369d103d..11494dadd7 100644
--- a/video/video_send_stream_impl.cc
+++ b/video/video_send_stream_impl.cc
@@ -177,6 +177,27 @@ bool SameStreamsEnabled(const VideoBitrateAllocation& lhs,
   }
   return true;
 }
+
+// Returns an optional that has value iff TransportSeqNumExtensionConfigured
+// is `true` for the given video send stream config.
+absl::optional<float> GetConfiguredPacingFactor(
+    const VideoSendStream::Config& config,
+    VideoEncoderConfig::ContentType content_type,
+    const PacingConfig& default_pacing_config) {
+  if (!TransportSeqNumExtensionConfigured(config))
+    return absl::nullopt;
+
+  absl::optional<AlrExperimentSettings> alr_settings =
+      GetAlrSettings(content_type);
+  if (alr_settings)
+    return alr_settings->pacing_factor;
+
+  RateControlSettings rate_control_settings =
+      RateControlSettings::ParseFromFieldTrials();
+  return rate_control_settings.GetPacingFactor().value_or(
+      default_pacing_config.pacing_factor);
+}
+
 }  // namespace
 
 PacingConfig::PacingConfig()
@@ -221,7 +242,6 @@ VideoSendStreamImpl::VideoSendStreamImpl(
       encoder_min_bitrate_bps_(0),
       encoder_target_rate_bps_(0),
       encoder_bitrate_priority_(initial_encoder_bitrate_priority),
-      has_packet_feedback_(false),
       video_stream_encoder_(video_stream_encoder),
       encoder_feedback_(
           clock,
@@ -245,7 +265,9 @@ VideoSendStreamImpl::VideoSendStreamImpl(
                                            std::move(fec_controller),
                                            CreateFrameEncryptionConfig(config_),
                                            config->frame_transformer)),
-      weak_ptr_factory_(this) {
+      weak_ptr_factory_(this),
+      configured_pacing_factor_(
+          GetConfiguredPacingFactor(*config_, content_type, pacing_config_)) {
   video_stream_encoder->SetFecControllerOverride(rtp_video_sender_);
   RTC_DCHECK_RUN_ON(worker_queue_);
   RTC_LOG(LS_INFO) << "VideoSendStreamInternal: " << config_->ToString();
@@ -275,15 +297,11 @@ VideoSendStreamImpl::VideoSendStreamImpl(
   RTC_CHECK(AlrExperimentSettings::MaxOneFieldTrialEnabled());
   // If send-side BWE is enabled, check if we should apply updated probing and
   // pacing settings.
-  if (TransportSeqNumExtensionConfigured(*config_)) {
-    has_packet_feedback_ = true;
-
+  if (configured_pacing_factor_.has_value()) {
     absl::optional<AlrExperimentSettings> alr_settings =
         GetAlrSettings(content_type);
     if (alr_settings) {
       transport->EnablePeriodicAlrProbing(true);
-      transport->SetPacingFactor(alr_settings->pacing_factor);
-      configured_pacing_factor_ = alr_settings->pacing_factor;
       transport->SetQueueTimeLimit(alr_settings->max_paced_queue_time);
     } else {
       RateControlSettings rate_control_settings =
@@ -291,13 +309,10 @@ VideoSendStreamImpl::VideoSendStreamImpl(
 
       transport->EnablePeriodicAlrProbing(
           rate_control_settings.UseAlrProbing());
-      const double pacing_factor =
-          rate_control_settings.GetPacingFactor().value_or(
-              pacing_config_.pacing_factor);
-      transport->SetPacingFactor(pacing_factor);
-      configured_pacing_factor_ = pacing_factor;
       transport->SetQueueTimeLimit(pacing_config_.max_pacing_delay.Get().ms());
     }
+
+    transport->SetPacingFactor(*configured_pacing_factor_);
   }
 
   if (config_->periodic_alr_bandwidth_probing) {
diff --git a/video/video_send_stream_impl.h b/video/video_send_stream_impl.h
index 41a7859a77..6053507931 100644
--- a/video/video_send_stream_impl.h
+++ b/video/video_send_stream_impl.h
@@ -106,7 +106,9 @@ class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver,
 
   std::map<uint32_t, RtpPayloadState> GetRtpPayloadStates() const;
 
-  absl::optional<float> configured_pacing_factor_;
+  const absl::optional<float>& configured_pacing_factor() const {
+    return configured_pacing_factor_;
+  }
 
  private:
   // Implements BitrateAllocatorObserver.
@@ -172,7 +174,6 @@ class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver,
   uint32_t encoder_max_bitrate_bps_;
   uint32_t encoder_target_rate_bps_;
   double encoder_bitrate_priority_;
-  bool has_packet_feedback_;
 
   VideoStreamEncoderInterface* const video_stream_encoder_;
   EncoderRtcpFeedback encoder_feedback_;
@@ -197,6 +198,7 @@ class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver,
   };
   absl::optional<VbaSendContext> video_bitrate_allocation_context_
       RTC_GUARDED_BY(worker_queue_);
+  const absl::optional<float> configured_pacing_factor_;
 };
 }  // namespace internal
 }  // namespace webrtc