Disable pacer disabling.

Since the pacer is always enabled, removing enable/disable which makes
all packet queueing succeed. Also renaming one of the ::SendPackets
::InsertPacket to avoid confusion.

BUG=webrtc:1695, webrtc:2629
R=stefan@webrtc.org

Review URL: https://codereview.webrtc.org/1392513002 .

Cr-Commit-Position: refs/heads/master@{#10211}

3 files changed, 108 insertions, 153 deletions

diff --git a/webrtc/modules/pacing/include/paced_sender.h b/webrtc/modules/pacing/include/paced_sender.h
index afb196fe45..f142f55173 100644
--- a/webrtc/modules/pacing/include/paced_sender.h
+++ b/webrtc/modules/pacing/include/paced_sender.h
@@ -71,11 +71,6 @@ class PacedSender : public Module, public RtpPacketSender {
 
   virtual ~PacedSender();
 
-  // Enable/disable pacing.
-  void SetStatus(bool enable);
-
-  bool Enabled() const;
-
   // Temporarily pause all sending.
   void Pause();
 
@@ -98,12 +93,12 @@ class PacedSender : public Module, public RtpPacketSender {
 
   // Returns true if we send the packet now, else it will add the packet
   // information to the queue and call TimeToSendPacket when it's time to send.
-  bool SendPacket(RtpPacketSender::Priority priority,
-                  uint32_t ssrc,
-                  uint16_t sequence_number,
-                  int64_t capture_time_ms,
-                  size_t bytes,
-                  bool retransmission) override;
+  void InsertPacket(RtpPacketSender::Priority priority,
+                    uint32_t ssrc,
+                    uint16_t sequence_number,
+                    int64_t capture_time_ms,
+                    size_t bytes,
+                    bool retransmission) override;
 
   // Returns the time since the oldest queued packet was enqueued.
   virtual int64_t QueueInMs() const;
@@ -134,7 +129,6 @@ class PacedSender : public Module, public RtpPacketSender {
   Callback* const callback_;
 
   rtc::scoped_ptr<CriticalSectionWrapper> critsect_;
-  bool enabled_ GUARDED_BY(critsect_);
   bool paused_ GUARDED_BY(critsect_);
   bool probing_enabled_;
   // This is the media budget, keeping track of how many bits of media
diff --git a/webrtc/modules/pacing/paced_sender.cc b/webrtc/modules/pacing/paced_sender.cc
index 55c361b085..6a7d19a251 100644
--- a/webrtc/modules/pacing/paced_sender.cc
+++ b/webrtc/modules/pacing/paced_sender.cc
@@ -219,7 +219,6 @@ PacedSender::PacedSender(Clock* clock,
     : clock_(clock),
       callback_(callback),
       critsect_(CriticalSectionWrapper::CreateCriticalSection()),
-      enabled_(true),
       paused_(false),
       probing_enabled_(true),
       media_budget_(new paced_sender::IntervalBudget(max_bitrate_kbps)),
@@ -249,16 +248,6 @@ void PacedSender::SetProbingEnabled(bool enabled) {
   probing_enabled_ = enabled;
 }
 
-void PacedSender::SetStatus(bool enable) {
-  CriticalSectionScoped cs(critsect_.get());
-  enabled_ = enable;
-}
-
-bool PacedSender::Enabled() const {
-  CriticalSectionScoped cs(critsect_.get());
-  return enabled_;
-}
-
 void PacedSender::UpdateBitrate(int bitrate_kbps,
                                 int max_bitrate_kbps,
                                 int min_bitrate_kbps) {
@@ -268,17 +257,14 @@ void PacedSender::UpdateBitrate(int bitrate_kbps,
   bitrate_bps_ = 1000 * bitrate_kbps;
 }
 
-bool PacedSender::SendPacket(RtpPacketSender::Priority priority,
-                             uint32_t ssrc,
-                             uint16_t sequence_number,
-                             int64_t capture_time_ms,
-                             size_t bytes,
-                             bool retransmission) {
+void PacedSender::InsertPacket(RtpPacketSender::Priority priority,
+                               uint32_t ssrc,
+                               uint16_t sequence_number,
+                               int64_t capture_time_ms,
+                               size_t bytes,
+                               bool retransmission) {
   CriticalSectionScoped cs(critsect_.get());
 
-  if (!enabled_) {
-    return true;  // We can send now.
-  }
   if (probing_enabled_ && !prober_->IsProbing()) {
     prober_->SetEnabled(true);
   }
@@ -291,7 +277,6 @@ bool PacedSender::SendPacket(RtpPacketSender::Priority priority,
   packets_->Push(paced_sender::Packet(
       priority, ssrc, sequence_number, capture_time_ms,
       clock_->TimeInMilliseconds(), bytes, retransmission, packet_counter_++));
-  return false;
 }
 
 int64_t PacedSender::ExpectedQueueTimeMs() const {
@@ -334,48 +319,45 @@ int32_t PacedSender::Process() {
   CriticalSectionScoped cs(critsect_.get());
   int64_t elapsed_time_ms = (now_us - time_last_update_us_ + 500) / 1000;
   time_last_update_us_ = now_us;
-  if (!enabled_) {
+  if (paused_)
     return 0;
+  if (elapsed_time_ms > 0) {
+    int64_t delta_time_ms = std::min(kMaxIntervalTimeMs, elapsed_time_ms);
+    UpdateBytesPerInterval(delta_time_ms);
   }
-  if (!paused_) {
-    if (elapsed_time_ms > 0) {
-      int64_t delta_time_ms = std::min(kMaxIntervalTimeMs, elapsed_time_ms);
-      UpdateBytesPerInterval(delta_time_ms);
+  while (!packets_->Empty()) {
+    if (media_budget_->bytes_remaining() == 0 && !prober_->IsProbing()) {
+      return 0;
     }
-    while (!packets_->Empty()) {
-      if (media_budget_->bytes_remaining() == 0 && !prober_->IsProbing()) {
-        return 0;
-      }
 
-      // Since we need to release the lock in order to send, we first pop the
-      // element from the priority queue but keep it in storage, so that we can
-      // reinsert it if send fails.
-      const paced_sender::Packet& packet = packets_->BeginPop();
-      if (SendPacket(packet)) {
-        // Send succeeded, remove it from the queue.
-        packets_->FinalizePop(packet);
-        if (prober_->IsProbing()) {
-          return 0;
-        }
-      } else {
-        // Send failed, put it back into the queue.
-        packets_->CancelPop(packet);
+    // Since we need to release the lock in order to send, we first pop the
+    // element from the priority queue but keep it in storage, so that we can
+    // reinsert it if send fails.
+    const paced_sender::Packet& packet = packets_->BeginPop();
+    if (SendPacket(packet)) {
+      // Send succeeded, remove it from the queue.
+      packets_->FinalizePop(packet);
+      if (prober_->IsProbing()) {
         return 0;
       }
+    } else {
+      // Send failed, put it back into the queue.
+      packets_->CancelPop(packet);
+      return 0;
     }
+  }
 
-    if (!packets_->Empty())
-      return 0;
+  if (!packets_->Empty())
+    return 0;
 
-    size_t padding_needed;
-    if (prober_->IsProbing())
-      padding_needed = prober_->RecommendedPacketSize();
-    else
-      padding_needed = padding_budget_->bytes_remaining();
+  size_t padding_needed;
+  if (prober_->IsProbing())
+    padding_needed = prober_->RecommendedPacketSize();
+  else
+    padding_needed = padding_budget_->bytes_remaining();
 
-    if (padding_needed > 0)
-      SendPadding(static_cast<size_t>(padding_needed));
-  }
+  if (padding_needed > 0)
+    SendPadding(static_cast<size_t>(padding_needed));
   return 0;
 }
 
diff --git a/webrtc/modules/pacing/paced_sender_unittest.cc b/webrtc/modules/pacing/paced_sender_unittest.cc
index a00b5fa58d..1e701ff8ea 100644
--- a/webrtc/modules/pacing/paced_sender_unittest.cc
+++ b/webrtc/modules/pacing/paced_sender_unittest.cc
@@ -124,8 +124,8 @@ class PacedSenderTest : public ::testing::Test {
                            int64_t capture_time_ms,
                            size_t size,
                            bool retransmission) {
-    EXPECT_FALSE(send_bucket_->SendPacket(priority, ssrc,
-        sequence_number, capture_time_ms, size, retransmission));
+    send_bucket_->InsertPacket(priority, ssrc, sequence_number, capture_time_ms,
+                               size, retransmission);
     EXPECT_CALL(callback_,
                 TimeToSendPacket(ssrc, sequence_number, capture_time_ms, false))
         .Times(1)
@@ -160,8 +160,9 @@ TEST_F(PacedSenderTest, QueuePacket) {
                       250,
                       false);
   int64_t queued_packet_timestamp = clock_.TimeInMilliseconds();
-  EXPECT_FALSE(send_bucket_->SendPacket(PacedSender::kNormalPriority, ssrc,
-      sequence_number, queued_packet_timestamp, 250, false));
+  send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
+                             sequence_number, queued_packet_timestamp, 250,
+                             false);
   send_bucket_->Process();
   EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
   EXPECT_CALL(callback_, TimeToSendPadding(_)).Times(0);
@@ -188,8 +189,9 @@ TEST_F(PacedSenderTest, QueuePacket) {
                       clock_.TimeInMilliseconds(),
                       250,
                       false);
-  EXPECT_FALSE(send_bucket_->SendPacket(PacedSender::kNormalPriority, ssrc,
-      sequence_number++, clock_.TimeInMilliseconds(), 250, false));
+  send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
+                             sequence_number++, clock_.TimeInMilliseconds(),
+                             250, false);
   send_bucket_->Process();
 }
 
@@ -207,8 +209,9 @@ TEST_F(PacedSenderTest, PaceQueuedPackets) {
                         false);
   }
   for (int j = 0; j < 30; ++j) {
-    EXPECT_FALSE(send_bucket_->SendPacket(PacedSender::kNormalPriority, ssrc,
-        sequence_number++, clock_.TimeInMilliseconds(), 250, false));
+    send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
+                               sequence_number++, clock_.TimeInMilliseconds(),
+                               250, false);
   }
   send_bucket_->Process();
   EXPECT_CALL(callback_, TimeToSendPadding(_)).Times(0);
@@ -243,8 +246,9 @@ TEST_F(PacedSenderTest, PaceQueuedPackets) {
                       clock_.TimeInMilliseconds(),
                       250,
                       false);
-  EXPECT_FALSE(send_bucket_->SendPacket(PacedSender::kNormalPriority, ssrc,
-      sequence_number, clock_.TimeInMilliseconds(), 250, false));
+  send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
+                             sequence_number, clock_.TimeInMilliseconds(), 250,
+                             false);
   send_bucket_->Process();
 }
 
@@ -266,10 +270,12 @@ TEST_F(PacedSenderTest, PaceQueuedPacketsWithDuplicates) {
 
   for (int j = 0; j < 30; ++j) {
     // Send in duplicate packets.
-    EXPECT_FALSE(send_bucket_->SendPacket(PacedSender::kNormalPriority, ssrc,
-        sequence_number, clock_.TimeInMilliseconds(), 250, false));
-    EXPECT_FALSE(send_bucket_->SendPacket(PacedSender::kNormalPriority, ssrc,
-        sequence_number++, clock_.TimeInMilliseconds(), 250, false));
+    send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
+                               sequence_number, clock_.TimeInMilliseconds(),
+                               250, false);
+    send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
+                               sequence_number++, clock_.TimeInMilliseconds(),
+                               250, false);
   }
   EXPECT_CALL(callback_, TimeToSendPadding(_)).Times(0);
   send_bucket_->Process();
@@ -308,8 +314,9 @@ TEST_F(PacedSenderTest, PaceQueuedPacketsWithDuplicates) {
                       clock_.TimeInMilliseconds(),
                       250,
                       false);
-  EXPECT_FALSE(send_bucket_->SendPacket(PacedSender::kNormalPriority, ssrc,
-      sequence_number++, clock_.TimeInMilliseconds(), 250, false));
+  send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
+                             sequence_number++, clock_.TimeInMilliseconds(),
+                             250, false);
   send_bucket_->Process();
 }
 
@@ -377,23 +384,6 @@ TEST_F(PacedSenderTest, Padding) {
   EXPECT_EQ(0, send_bucket_->Process());
 }
 
-TEST_F(PacedSenderTest, NoPaddingWhenDisabled) {
-  send_bucket_->SetStatus(false);
-  send_bucket_->UpdateBitrate(
-      kTargetBitrate, kPaceMultiplier * kTargetBitrate, kTargetBitrate);
-  // No padding is expected since the pacer is disabled.
-  EXPECT_CALL(callback_, TimeToSendPadding(_)).Times(0);
-  EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
-  clock_.AdvanceTimeMilliseconds(5);
-  EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
-  EXPECT_EQ(0, send_bucket_->Process());
-  EXPECT_CALL(callback_, TimeToSendPadding(_)).Times(0);
-  EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
-  clock_.AdvanceTimeMilliseconds(5);
-  EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
-  EXPECT_EQ(0, send_bucket_->Process());
-}
-
 TEST_F(PacedSenderTest, VerifyPaddingUpToBitrate) {
   uint32_t ssrc = 12345;
   uint16_t sequence_number = 1234;
@@ -433,9 +423,9 @@ TEST_F(PacedSenderTest, VerifyAverageBitrateVaryingMediaPayload) {
   size_t media_bytes = 0;
   while (clock_.TimeInMilliseconds() - start_time < kBitrateWindow) {
     size_t media_payload = rand() % 100 + 200;  // [200, 300] bytes.
-    EXPECT_FALSE(send_bucket_->SendPacket(PacedSender::kNormalPriority, ssrc,
-                                          sequence_number++, capture_time_ms,
-                                          media_payload, false));
+    send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
+                               sequence_number++, capture_time_ms,
+                               media_payload, false);
     media_bytes += media_payload;
     clock_.AdvanceTimeMilliseconds(kTimeStep);
     send_bucket_->Process();
@@ -474,15 +464,15 @@ TEST_F(PacedSenderTest, Priority) {
   send_bucket_->Process();
 
   // Expect normal and low priority to be queued and high to pass through.
-  EXPECT_FALSE(send_bucket_->SendPacket(PacedSender::kLowPriority,
-      ssrc_low_priority, sequence_number++, capture_time_ms_low_priority, 250,
-      false));
-  EXPECT_FALSE(send_bucket_->SendPacket(PacedSender::kNormalPriority,
-      ssrc, sequence_number++, capture_time_ms, 250, false));
-  EXPECT_FALSE(send_bucket_->SendPacket(PacedSender::kNormalPriority,
-      ssrc, sequence_number++, capture_time_ms, 250, false));
-  EXPECT_FALSE(send_bucket_->SendPacket(PacedSender::kHighPriority,
-      ssrc, sequence_number++, capture_time_ms, 250, false));
+  send_bucket_->InsertPacket(PacedSender::kLowPriority, ssrc_low_priority,
+                             sequence_number++, capture_time_ms_low_priority,
+                             250, false);
+  send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
+                             sequence_number++, capture_time_ms, 250, false);
+  send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
+                             sequence_number++, capture_time_ms, 250, false);
+  send_bucket_->InsertPacket(PacedSender::kHighPriority, ssrc,
+                             sequence_number++, capture_time_ms, 250, false);
 
   // Expect all high and normal priority to be sent out first.
   EXPECT_CALL(callback_, TimeToSendPadding(_)).Times(0);
@@ -538,20 +528,20 @@ TEST_F(PacedSenderTest, Pause) {
 
   send_bucket_->Pause();
 
-  EXPECT_FALSE(send_bucket_->SendPacket(PacedSender::kNormalPriority,
-      ssrc, sequence_number++, capture_time_ms, 250, false));
-  EXPECT_FALSE(send_bucket_->SendPacket(PacedSender::kNormalPriority,
-      ssrc, sequence_number++, capture_time_ms, 250, false));
-  EXPECT_FALSE(send_bucket_->SendPacket(PacedSender::kHighPriority,
-      ssrc, sequence_number++, capture_time_ms, 250, false));
+  send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
+                             sequence_number++, capture_time_ms, 250, false);
+  send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
+                             sequence_number++, capture_time_ms, 250, false);
+  send_bucket_->InsertPacket(PacedSender::kHighPriority, ssrc,
+                             sequence_number++, capture_time_ms, 250, false);
 
   clock_.AdvanceTimeMilliseconds(10000);
   int64_t second_capture_time_ms = clock_.TimeInMilliseconds();
 
   // Expect everything to be queued.
-  EXPECT_FALSE(send_bucket_->SendPacket(PacedSender::kLowPriority,
-      ssrc_low_priority, sequence_number++, second_capture_time_ms, 250,
-      false));
+  send_bucket_->InsertPacket(PacedSender::kLowPriority, ssrc_low_priority,
+                             sequence_number++, second_capture_time_ms, 250,
+                             false);
 
   EXPECT_EQ(clock_.TimeInMilliseconds() - capture_time_ms,
             send_bucket_->QueueInMs());
@@ -593,19 +583,12 @@ TEST_F(PacedSenderTest, ResendPacket) {
   int64_t capture_time_ms = clock_.TimeInMilliseconds();
   EXPECT_EQ(0, send_bucket_->QueueInMs());
 
-  EXPECT_FALSE(send_bucket_->SendPacket(PacedSender::kNormalPriority,
-                                        ssrc,
-                                        sequence_number,
-                                        capture_time_ms,
-                                        250,
-                                        false));
+  send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
+                             sequence_number, capture_time_ms, 250, false);
   clock_.AdvanceTimeMilliseconds(1);
-  EXPECT_FALSE(send_bucket_->SendPacket(PacedSender::kNormalPriority,
-                                        ssrc,
-                                        sequence_number + 1,
-                                        capture_time_ms + 1,
-                                        250,
-                                        false));
+  send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
+                             sequence_number + 1, capture_time_ms + 1, 250,
+                             false);
   clock_.AdvanceTimeMilliseconds(9999);
   EXPECT_EQ(clock_.TimeInMilliseconds() - capture_time_ms,
             send_bucket_->QueueInMs());
@@ -726,12 +709,9 @@ TEST_F(PacedSenderTest, ProbingWithInitialFrame) {
                       0));
 
   for (int i = 0; i < kNumPackets; ++i) {
-    EXPECT_FALSE(send_bucket_->SendPacket(PacedSender::kNormalPriority,
-                                          ssrc,
-                                          sequence_number++,
-                                          clock_.TimeInMilliseconds(),
-                                          kPacketSize,
-                                          false));
+    send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
+                               sequence_number++, clock_.TimeInMilliseconds(),
+                               kPacketSize, false);
   }
   while (callback.packets_sent() < kNumPackets) {
     int time_until_process = send_bucket_->TimeUntilNextProcess();
@@ -758,9 +738,9 @@ TEST_F(PacedSenderTest, ProbingWithTooSmallInitialFrame) {
                                      kPaceMultiplier * kInitialBitrateKbps, 0));
 
   for (int i = 0; i < kNumPackets - 5; ++i) {
-    EXPECT_FALSE(send_bucket_->SendPacket(
-        PacedSender::kNormalPriority, ssrc, sequence_number++,
-        clock_.TimeInMilliseconds(), kPacketSize, false));
+    send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
+                               sequence_number++, clock_.TimeInMilliseconds(),
+                               kPacketSize, false);
   }
   while (callback.packets_sent() < kNumPackets) {
     int time_until_process = send_bucket_->TimeUntilNextProcess();
@@ -783,21 +763,20 @@ TEST_F(PacedSenderTest, PriorityInversion) {
   uint16_t sequence_number = 1234;
   const size_t kPacketSize = 1200;
 
-  EXPECT_FALSE(send_bucket_->SendPacket(
+  send_bucket_->InsertPacket(
       PacedSender::kHighPriority, ssrc, sequence_number + 3,
-      clock_.TimeInMilliseconds() + 33, kPacketSize, true));
+      clock_.TimeInMilliseconds() + 33, kPacketSize, true);
 
-  EXPECT_FALSE(send_bucket_->SendPacket(
+  send_bucket_->InsertPacket(
       PacedSender::kHighPriority, ssrc, sequence_number + 2,
-      clock_.TimeInMilliseconds() + 33, kPacketSize, true));
+      clock_.TimeInMilliseconds() + 33, kPacketSize, true);
 
-  EXPECT_FALSE(send_bucket_->SendPacket(
-      PacedSender::kHighPriority, ssrc, sequence_number,
-      clock_.TimeInMilliseconds(), kPacketSize, true));
+  send_bucket_->InsertPacket(PacedSender::kHighPriority, ssrc, sequence_number,
+                             clock_.TimeInMilliseconds(), kPacketSize, true);
 
-  EXPECT_FALSE(send_bucket_->SendPacket(
-      PacedSender::kHighPriority, ssrc, sequence_number + 1,
-      clock_.TimeInMilliseconds(), kPacketSize, true));
+  send_bucket_->InsertPacket(PacedSender::kHighPriority, ssrc,
+                             sequence_number + 1, clock_.TimeInMilliseconds(),
+                             kPacketSize, true);
 
   // Packets from earlier frames should be sent first.
   {
@@ -842,9 +821,9 @@ TEST_F(PacedSenderTest, PaddingOveruse) {
   clock_.AdvanceTimeMilliseconds(5);
   send_bucket_->UpdateBitrate(60, 90, 30);
 
-  EXPECT_FALSE(send_bucket_->SendPacket(
-      PacedSender::kHighPriority, ssrc, sequence_number++,
-      clock_.TimeInMilliseconds(), kPacketSize, false));
+  send_bucket_->InsertPacket(PacedSender::kHighPriority, ssrc,
+                             sequence_number++, clock_.TimeInMilliseconds(),
+                             kPacketSize, false);
 
   // Don't send padding if queue is non-empty, even if padding budget > 0.
   EXPECT_CALL(callback_, TimeToSendPadding(_)).Times(0);