BitrateProber: Support higher probing bitrates

webrtc/modules/pacing/paced_sender_unittest.cc

 /*
  *  Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #include <list>
 #include <memory>
 
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "webrtc/modules/pacing/paced_sender.h"
 #include "webrtc/system_wrappers/include/clock.h"
 
 using testing::_;
 using testing::Return;
 
+namespace {
+constexpr unsigned kFirstClusterBps = 900000;
+constexpr int kFirstClusterCount = 6;
+constexpr unsigned kSecondClusterBps = 1800000;
+constexpr int kSecondClusterCount = 5;
+}  // namespace
+
 namespace webrtc {
 namespace test {
 
 static const int kTargetBitrateBps = 800000;
 
 class MockPacedSenderCallback : public PacedSender::PacketSender {
  public:
   MOCK_METHOD5(TimeToSendPacket,
                bool(uint32_t ssrc,
                     uint16_t sequence_number,
@@ skipping 23 matching lines @@
   }
 
   size_t padding_sent() { return padding_sent_; }
 
  private:
   size_t padding_sent_;
 };
 
 class PacedSenderProbing : public PacedSender::PacketSender {
  public:
-  PacedSenderProbing(const std::list<int>& expected_deltas, Clock* clock)
-      : prev_packet_time_ms_(-1),
-        expected_deltas_(expected_deltas),
-        packets_sent_(0),
-        clock_(clock) {}
+  PacedSenderProbing() : packets_sent_(0), padding_sent_(0) {}
 
   bool TimeToSendPacket(uint32_t ssrc,
                         uint16_t sequence_number,
                         int64_t capture_time_ms,
                         bool retransmission,
                         int probe_cluster_id) override {
-    ExpectAndCountPacket();
+    packets_sent_++;
     return true;
   }
 
   size_t TimeToSendPadding(size_t bytes, int probe_cluster_id) override {
-    ExpectAndCountPacket();
-    return bytes;
-  }
-
-  void ExpectAndCountPacket() {
-    ++packets_sent_;
-    EXPECT_FALSE(expected_deltas_.empty());
-    if (expected_deltas_.empty())
-      return;
-    int64_t now_ms = clock_->TimeInMilliseconds();
-    if (prev_packet_time_ms_ >= 0) {
-      EXPECT_EQ(expected_deltas_.front(), now_ms - prev_packet_time_ms_);
-      expected_deltas_.pop_front();
-    }
-    prev_packet_time_ms_ = now_ms;
+    padding_sent_ += bytes;
+    return padding_sent_;
   }
 
   int packets_sent() const { return packets_sent_; }
 
+  int padding_sent() const { return padding_sent_; }
+
  private:
-  int64_t prev_packet_time_ms_;
-  std::list<int> expected_deltas_;
   int packets_sent_;
-  Clock* clock_;
+  int padding_sent_;
 };
 
 class PacedSenderTest : public ::testing::Test {
  protected:
   PacedSenderTest() : clock_(123456) {
     srand(0);
     // Need to initialize PacedSender after we initialize clock.
     send_bucket_.reset(new PacedSender(&clock_, &callback_));
-    send_bucket_->CreateProbeCluster(900000, 6);
-    send_bucket_->CreateProbeCluster(1800000, 5);
+    send_bucket_->CreateProbeCluster(kFirstClusterBps, kFirstClusterCount);
+    send_bucket_->CreateProbeCluster(kSecondClusterBps, kSecondClusterCount);
     // Default to bitrate probing disabled for testing purposes. Probing tests
     // have to enable probing, either by creating a new PacedSender instance or
     // by calling SetProbingEnabled(true).
     send_bucket_->SetProbingEnabled(false);
     send_bucket_->SetEstimatedBitrate(kTargetBitrateBps);
 
     clock_.AdvanceTimeMilliseconds(send_bucket_->TimeUntilNextProcess());
   }
 
   void SendAndExpectPacket(PacedSender::Priority priority,
@@ skipping 671 matching lines @@
                       clock_.TimeInMilliseconds(),
                       1200,
                       false);
 
   clock_.AdvanceTimeMilliseconds(500);
   EXPECT_EQ(500, send_bucket_->QueueInMs());
   send_bucket_->Process();
   EXPECT_EQ(0, send_bucket_->QueueInMs());
 }
 
-TEST_F(PacedSenderTest, ProbingWithInitialFrame) {
-  const int kNumPackets = 11;
-  const int kNumDeltas = kNumPackets - 1;
+TEST_F(PacedSenderTest, ProbingWithInsertedPackets) {
   const size_t kPacketSize = 1200;
   const int kInitialBitrateBps = 300000;
   uint32_t ssrc = 12346;
   uint16_t sequence_number = 1234;
 
-  const int expected_deltas[kNumDeltas] = {10, 10, 10, 10, 10, 5, 5, 5, 5, 5};
-  std::list<int> expected_deltas_list(expected_deltas,
-                                      expected_deltas + kNumDeltas);
-  PacedSenderProbing callback(expected_deltas_list, &clock_);
-  send_bucket_.reset(new PacedSender(&clock_, &callback));
-  send_bucket_->CreateProbeCluster(900000, 6);
-  send_bucket_->CreateProbeCluster(1800000, 5);
+  PacedSenderProbing packet_sender;
+  send_bucket_.reset(new PacedSender(&clock_, &packet_sender));
+  send_bucket_->CreateProbeCluster(kFirstClusterBps, kFirstClusterCount);
+  send_bucket_->CreateProbeCluster(kSecondClusterBps, kSecondClusterCount);
   send_bucket_->SetEstimatedBitrate(kInitialBitrateBps);
 
-  for (int i = 0; i < kNumPackets; ++i) {
+  for (int i = 0; i < kFirstClusterCount + kSecondClusterCount; ++i) {
     send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
                                sequence_number++, clock_.TimeInMilliseconds(),
                                kPacketSize, false);
   }
 
-  while (callback.packets_sent() < kNumPackets) {
+  int64_t start = clock_.TimeInMilliseconds();
+  while (packet_sender.packets_sent() < kFirstClusterCount) {

[philipel, 2016/09/21 09:26:40]

Simplify loop:
int time_until_process = send_bucket_->TimeUntilNextProcess();
clock_.AdvanceTimeMilliseconds(time_until_process);
send_bucket_->Process();

[Irfan, 2016/09/21 17:00:37]

Done.

     int time_until_process = send_bucket_->TimeUntilNextProcess();
     if (time_until_process <= 0) {
       send_bucket_->Process();
+    } else {
+      clock_.AdvanceTimeMilliseconds(time_until_process);
+    }
+  }
+  int packets_sent = packet_sender.packets_sent();
+  // Validate first cluster bitrate.
+  EXPECT_GT(

[philipel, 2016/09/21 09:26:40]

Why do we EXPECT_GT? Shouldn't we EXPECT_NEAR instead?

[Irfan, 2016/09/21 17:00:37]

I have clarified this in the comments. Have switched to EXPECT_NEAR since there
is value in validating the range.

+      packets_sent * kPacketSize * 8000 / (clock_.TimeInMilliseconds() - start),
+      kFirstClusterBps);
+
+  start = clock_.TimeInMilliseconds();
+  while (packet_sender.packets_sent() <

[philipel, 2016/09/21 09:26:40]

simplfy loop

[Irfan, 2016/09/21 17:00:37]

Done.

+         kFirstClusterCount + kSecondClusterCount) {
+    int time_until_process = send_bucket_->TimeUntilNextProcess();
+    if (time_until_process <= 0) {
+      send_bucket_->Process();
     } else {
       clock_.AdvanceTimeMilliseconds(time_until_process);
     }
   }
+  packets_sent = packet_sender.packets_sent() - packets_sent;
+  // Validate second cluster bitrate.
+  EXPECT_GT(

[philipel, 2016/09/21 09:26:40]

EXPECT_NEAR?

[Irfan, 2016/09/21 17:00:37]

Done.

+      packets_sent * kPacketSize * 8000 / (clock_.TimeInMilliseconds() - start),
+      kSecondClusterBps);
 }
 
-TEST_F(PacedSenderTest, ProbingWithTooSmallInitialFrame) {
-  const int kNumPackets = 11;
-  const int kNumDeltas = kNumPackets - 1;
+TEST_F(PacedSenderTest, ProbingWithPaddingSupport) {
   const size_t kPacketSize = 1200;
   const int kInitialBitrateBps = 300000;
   uint32_t ssrc = 12346;
   uint16_t sequence_number = 1234;
-  const int expected_deltas[kNumDeltas] = {10, 10, 10, 10, 10, 5, 5, 5, 5, 5};
-  std::list<int> expected_deltas_list(expected_deltas,
-                                      expected_deltas + kNumDeltas);
-  PacedSenderProbing callback(expected_deltas_list, &clock_);
-  send_bucket_.reset(new PacedSender(&clock_, &callback));
-  send_bucket_->CreateProbeCluster(900000, 6);
-  send_bucket_->CreateProbeCluster(1800000, 5);
+
+  PacedSenderProbing packet_sender;
+  send_bucket_.reset(new PacedSender(&clock_, &packet_sender));
+  send_bucket_->CreateProbeCluster(kFirstClusterBps, kFirstClusterCount);
   send_bucket_->SetEstimatedBitrate(kInitialBitrateBps);
 
-  for (int i = 0; i < kNumPackets - 5; ++i) {
+  for (int i = 0; i < kFirstClusterCount - 2; ++i) {
     send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
                                sequence_number++, clock_.TimeInMilliseconds(),
                                kPacketSize, false);
   }
-  while (callback.packets_sent() < kNumPackets) {
+
+  int64_t start = clock_.TimeInMilliseconds();
+  int process_count = 0;
+  while (process_count < kFirstClusterCount) {

[philipel, 2016/09/21 09:26:40]

Simplify loop

[Irfan, 2016/09/21 17:00:37]

Done.

+    int time_until_process = send_bucket_->TimeUntilNextProcess();
+    if (time_until_process <= 0) {
+      send_bucket_->Process();
+      process_count++;
+    } else {
+      clock_.AdvanceTimeMilliseconds(time_until_process);
+    }
+  }
+  int packets_sent = packet_sender.packets_sent();
+  int padding_sent = packet_sender.padding_sent();
+  // Validate first cluster bitrate
+  EXPECT_GT((packets_sent * kPacketSize * 8000 + padding_sent) /

[philipel, 2016/09/21 09:26:40]

EXPECT_NEAR

[Irfan, 2016/09/21 17:00:37]

Done.

+                (clock_.TimeInMilliseconds() - start),
+            kFirstClusterBps);
+}
+
+TEST_F(PacedSenderTest, ProbingUsesRetransmissions) {
+  const size_t kPacketSize = 1200;
+  const int kInitialBitrateBps = 300000;
+  uint32_t ssrc = 12346;
+  uint16_t sequence_number = 1234;
+
+  PacedSenderProbing packet_sender;
+  send_bucket_.reset(new PacedSender(&clock_, &packet_sender));
+  send_bucket_->CreateProbeCluster(kFirstClusterBps, kFirstClusterCount);
+  send_bucket_->SetEstimatedBitrate(kInitialBitrateBps);
+
+  // All packets inserted are retransmissions.
+  for (int i = 0; i < kFirstClusterCount; ++i) {
+    send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
+                               sequence_number++, clock_.TimeInMilliseconds(),
+                               kPacketSize, true);
+  }
+
+  int64_t start = clock_.TimeInMilliseconds();
+  while (packet_sender.packets_sent() < kFirstClusterCount) {

[philipel, 2016/09/21 09:26:39]

Simplify loop

[Irfan, 2016/09/21 17:00:37]

Done.

     int time_until_process = send_bucket_->TimeUntilNextProcess();
     if (time_until_process <= 0) {
       send_bucket_->Process();
     } else {
       clock_.AdvanceTimeMilliseconds(time_until_process);
     }
   }
-
-  // Process one more time and make sure we don't send any more probes.
-  int time_until_process = send_bucket_->TimeUntilNextProcess();
-  clock_.AdvanceTimeMilliseconds(time_until_process);
-  send_bucket_->Process();
-  EXPECT_EQ(kNumPackets, callback.packets_sent());
+  int packets_sent = packet_sender.packets_sent();
+  int padding_sent = packet_sender.padding_sent();
+  EXPECT_EQ(padding_sent, 0);
+  // Validate first cluster bitrate.
+  EXPECT_GT((packets_sent * kPacketSize * 8000) /

[philipel, 2016/09/21 09:26:40]

EXPECT_NEAR

[Irfan, 2016/09/21 17:00:37]

Done.

+                (clock_.TimeInMilliseconds() - start),
+            kFirstClusterBps);
 }
 
 TEST_F(PacedSenderTest, PriorityInversion) {
   uint32_t ssrc = 12346;
   uint16_t sequence_number = 1234;
   const size_t kPacketSize = 1200;
 
   send_bucket_->InsertPacket(
       PacedSender::kHighPriority, ssrc, sequence_number + 3,
       clock_.TimeInMilliseconds() + 33, kPacketSize, true);
@@ skipping 144 matching lines @@
 
   // No more probing packets.
   EXPECT_CALL(callback_, TimeToSendPadding(_, PacketInfo::kNotAProbe))
         .Times(1)
         .WillRepeatedly(Return(500));
   send_bucket_->Process();
 }
 
 }  // namespace test
 }  // namespace webrtc