BitrateProber: Support higher probing bitrates

webrtc/modules/pacing/paced_sender_unittest.cc

Index: webrtc/modules/pacing/paced_sender_unittest.cc
diff --git a/webrtc/modules/pacing/paced_sender_unittest.cc b/webrtc/modules/pacing/paced_sender_unittest.cc
index 861452567f8d018a070ee8987a290bffdac3d54b..acaa4a9a0e5a486a2a40235cb31158412a4aff31 100644
--- a/webrtc/modules/pacing/paced_sender_unittest.cc
+++ b/webrtc/modules/pacing/paced_sender_unittest.cc
@@ -19,6 +19,13 @@
 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 {
 
@@ -62,46 +69,29 @@ class PacedSenderPadding : public PacedSender::PacketSender {
 
 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 {
@@ -110,8 +100,8 @@ class PacedSenderTest : public ::testing::Test {
     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).
@@ -803,30 +793,42 @@ TEST_F(PacedSenderTest, QueueTimeGrowsOverTime) {
   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();
@@ -834,43 +836,83 @@ TEST_F(PacedSenderTest, ProbingWithInitialFrame) {
       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);
+}
 
-  // 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());
+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);
+    }
+  }
+  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) {