From ae10029bff6e5b53a7791a60e4163cc4201d61d2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= <sprang@webrtc.org>
Date: Tue, 17 Dec 2019 17:49:49 +0100
Subject: [PATCH] Prevents probing while paused.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The pacing controller allowed sending bitrate probes, despite it being
paused. This CL adresses that, and makes sure the task-queue based mode
also properly repsects pausing.

Bug: webrtc:10809
Change-Id: I79643c9a24666110d7583fce3ed1bfd6865e9e10
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/162520
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30109}
---
 modules/pacing/pacing_controller.cc          |  9 +++---
 modules/pacing/pacing_controller_unittest.cc | 29 ++++++++++++++++++++
 2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/modules/pacing/pacing_controller.cc b/modules/pacing/pacing_controller.cc
index 0d0d1ae5dd..e346a838f5 100644
--- a/modules/pacing/pacing_controller.cc
+++ b/modules/pacing/pacing_controller.cc
@@ -308,6 +308,10 @@ bool PacingController::ShouldSendKeepalive(Timestamp now) const {
 Timestamp PacingController::NextSendTime() const {
   Timestamp now = CurrentTime();
 
+  if (paused_) {
+    return last_send_time_ + kPausedProcessInterval;
+  }
+
   // If probing is active, that always takes priority.
   if (prober_.IsProbing()) {
     Timestamp probe_time = prober_.NextProbeTime(now);
@@ -318,10 +322,7 @@ Timestamp PacingController::NextSendTime() const {
   }
 
   if (mode_ == ProcessMode::kPeriodic) {
-    // In periodc non-probing mode, we just have a fixed interval.
-    if (paused_) {
-      return last_send_time_ + kPausedProcessInterval;
-    }
+    // In periodic non-probing mode, we just have a fixed interval.
     return last_process_time_ + min_packet_limit_;
   }
 
diff --git a/modules/pacing/pacing_controller_unittest.cc b/modules/pacing/pacing_controller_unittest.cc
index 9337ad2f8a..e581e30492 100644
--- a/modules/pacing/pacing_controller_unittest.cc
+++ b/modules/pacing/pacing_controller_unittest.cc
@@ -1737,6 +1737,35 @@ TEST_P(PacingControllerTest, TaskLate) {
   pacer_->ProcessPackets();
 }
 
+TEST_P(PacingControllerTest, NoProbingWhilePaused) {
+  uint32_t ssrc = 12345;
+  uint16_t sequence_number = 1234;
+
+  pacer_->SetProbingEnabled(true);
+
+  // Send at least one packet so probing can initate.
+  SendAndExpectPacket(RtpPacketToSend::Type::kVideo, ssrc, sequence_number,
+                      clock_.TimeInMilliseconds(), 250);
+  while (pacer_->QueueSizePackets() > 0) {
+    AdvanceTimeAndProcess();
+  }
+
+  // Trigger probing.
+  pacer_->CreateProbeCluster(DataRate::kbps(10000),  // 10 Mbps.
+                             /*cluster_id=*/3);
+
+  // Time to next send time should be small.
+  EXPECT_LT(pacer_->NextSendTime() - clock_.CurrentTime(),
+            PacingController::kPausedProcessInterval);
+
+  // Pause pacer, time to next send time should now be the pause process
+  // interval.
+  pacer_->Pause();
+
+  EXPECT_EQ(pacer_->NextSendTime() - clock_.CurrentTime(),
+            PacingController::kPausedProcessInterval);
+}
+
 INSTANTIATE_TEST_SUITE_P(
     WithAndWithoutIntervalBudget,
     PacingControllerTest,