Add check for send-side bwe before applying alr settings

Bug: webrtc:7694
Change-Id: I359b27b96239af4e067055fc77ea285824e69edf
Reviewed-on: https://webrtc-review.googlesource.com/14603
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20423}

modules/pacing/paced_sender.cc

@@ -365,6 +365,11 @@ void PacedSender::SetPacingFactor(float pacing_factor) {
   SetEstimatedBitrate(estimated_bitrate_bps_);
 }
 
+float PacedSender::GetPacingFactor() const {
+  rtc::CritScope cs(&critsect_);
+  return pacing_factor_;
+}
+
 void PacedSender::SetQueueTimeLimit(int limit_ms) {
   rtc::CritScope cs(&critsect_);
   queue_time_limit = limit_ms;

modules/pacing/paced_sender.h

@@ -144,6 +144,7 @@ class PacedSender : public Pacer {
   // Called when the prober is associated with a process thread.
   void ProcessThreadAttached(ProcessThread* process_thread) override;
   void SetPacingFactor(float pacing_factor);
+  float GetPacingFactor() const;
   void SetQueueTimeLimit(int limit_ms);
 
  private:

video/video_send_stream.cc

@@ -143,9 +143,13 @@ std::unique_ptr<FlexfecSender> MaybeCreateFlexfecSender(
       RTPSender::FecExtensionSizes(), rtp_state, Clock::GetRealTimeClock()));
 }
 
-}  // namespace
-
-namespace {
+bool TransportSeqNumExtensionConfigured(const VideoSendStream::Config& config) {
+  const std::vector<RtpExtension>& extensions = config.rtp.extensions;
+  return std::find_if(
+             extensions.begin(), extensions.end(), [](const RtpExtension& ext) {
+               return ext.uri == RtpExtension::kTransportSequenceNumberUri;
+             }) != extensions.end();
+}
 
 bool PayloadTypeSupportsSkippingFecPackets(const std::string& payload_name) {
   const VideoCodecType codecType = PayloadStringToCodecType(payload_name);
@@ -719,18 +723,22 @@ VideoSendStreamImpl::VideoSendStreamImpl(
             field_trial::FindFullName(
                 AlrDetector::kScreenshareProbingBweExperimentName)
                 .empty());
-  rtc::Optional<AlrDetector::AlrExperimentSettings> alr_settings;
-  if (content_type == VideoEncoderConfig::ContentType::kScreen) {
-    alr_settings = AlrDetector::ParseAlrSettingsFromFieldTrial(
-        AlrDetector::kScreenshareProbingBweExperimentName);
-  } else {
-    alr_settings = AlrDetector::ParseAlrSettingsFromFieldTrial(
-        AlrDetector::kStrictPacingAndProbingExperimentName);
-  }
-  if (alr_settings) {
-    transport->send_side_cc()->EnablePeriodicAlrProbing(true);
-    transport->pacer()->SetPacingFactor(alr_settings->pacing_factor);
-    transport->pacer()->SetQueueTimeLimit(alr_settings->max_paced_queue_time);
+  // If send-side BWE is enabled, check if we should apply updated probing and
+  // pacing settings.
+  if (TransportSeqNumExtensionConfigured(*config_)) {
+    rtc::Optional<AlrDetector::AlrExperimentSettings> alr_settings;
+    if (content_type == VideoEncoderConfig::ContentType::kScreen) {
+      alr_settings = AlrDetector::ParseAlrSettingsFromFieldTrial(
+          AlrDetector::kScreenshareProbingBweExperimentName);
+    } else {
+      alr_settings = AlrDetector::ParseAlrSettingsFromFieldTrial(
+          AlrDetector::kStrictPacingAndProbingExperimentName);
+    }
+    if (alr_settings) {
+      transport->send_side_cc()->EnablePeriodicAlrProbing(true);
+      transport->pacer()->SetPacingFactor(alr_settings->pacing_factor);
+      transport->pacer()->SetQueueTimeLimit(alr_settings->max_paced_queue_time);
+    }
   }
 
   if (config_->periodic_alr_bandwidth_probing) {

video/video_send_stream_tests.cc

@@ -15,6 +15,7 @@
 #include "call/rtp_transport_controller_send.h"
 #include "common_video/include/frame_callback.h"
 #include "common_video/include/video_frame.h"
+#include "modules/pacing/alr_detector.h"
 #include "modules/rtp_rtcp/include/rtp_header_parser.h"
 #include "modules/rtp_rtcp/include/rtp_rtcp.h"
 #include "modules/rtp_rtcp/source/rtcp_sender.h"
@@ -3511,4 +3512,81 @@ TEST_F(VideoSendStreamTest, SendsKeepAlive) {
   RunBaseTest(&test);
 }
 
+TEST_F(VideoSendStreamTest, ConfiguresAlrWhenSendSideOn) {
+  const std::string kAlrProbingExperiment =
+      std::string(AlrDetector::kScreenshareProbingBweExperimentName) +
+      "/1.1,2875,85,20,-20,0/";
+  test::ScopedFieldTrials alr_experiment(kAlrProbingExperiment);
+  class PacingFactorObserver : public test::SendTest {
+   public:
+    PacingFactorObserver(bool configure_send_side, float expected_pacing_factor)
+        : test::SendTest(kDefaultTimeoutMs),
+          configure_send_side_(configure_send_side),
+          expected_pacing_factor_(expected_pacing_factor),
+          paced_sender_(nullptr) {}
+
+    void ModifyVideoConfigs(
+        VideoSendStream::Config* send_config,
+        std::vector<VideoReceiveStream::Config>* receive_configs,
+        VideoEncoderConfig* encoder_config) override {
+      // Check if send-side bwe extension is already present, and remove it if
+      // it is not desired.
+      bool has_send_side = false;
+      for (auto it = send_config->rtp.extensions.begin();
+           it != send_config->rtp.extensions.end(); ++it) {
+        if (it->uri == RtpExtension::kTransportSequenceNumberUri) {
+          if (configure_send_side_) {
+            has_send_side = true;
+          } else {
+            send_config->rtp.extensions.erase(it);
+          }
+          break;
+        }
+      }
+
+      if (configure_send_side_ && !has_send_side) {
+        // Want send side, not present by default, so add it.
+        send_config->rtp.extensions.emplace_back(
+            RtpExtension::kTransportSequenceNumberUri,
+            RtpExtension::kTransportSequenceNumberDefaultId);
+      }
+
+      // ALR only enabled for screenshare.
+      encoder_config->content_type = VideoEncoderConfig::ContentType::kScreen;
+    }
+
+    void OnRtpTransportControllerSendCreated(
+        RtpTransportControllerSend* controller) override {
+      // Grab a reference to the pacer.
+      paced_sender_ = controller->pacer();
+    }
+
+    void OnVideoStreamsCreated(
+        VideoSendStream* send_stream,
+        const std::vector<VideoReceiveStream*>& receive_streams) override {
+      // Video streams created, check that pacer is correctly configured.
+      EXPECT_EQ(expected_pacing_factor_, paced_sender_->GetPacingFactor());
+      observation_complete_.Set();
+    }
+
+    void PerformTest() override {
+      EXPECT_TRUE(Wait()) << "Timed out while waiting for pacer config.";
+    }
+
+   private:
+    const bool configure_send_side_;
+    const float expected_pacing_factor_;
+    const PacedSender* paced_sender_;
+  };
+
+  // Send-side bwe on, use pacing factor from |kAlrProbingExperiment| above.
+  PacingFactorObserver test_with_send_side(true, 1.1f);
+  RunBaseTest(&test_with_send_side);
+
+  // Send-side bwe off, use default pacing factor.
+  PacingFactorObserver test_without_send_side(
+      false, PacedSender::kDefaultPaceMultiplier);
+  RunBaseTest(&test_without_send_side);
+}
+
 }  // namespace webrtc