Use padding to achieve bitrate probing if the initial key frame has too few packets.

BUG=4350
R=mflodman@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/44879004

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

webrtc/modules/pacing/bitrate_prober.cc

@@ -11,6 +11,7 @@
 #include "webrtc/modules/pacing/bitrate_prober.h"
 
 #include <assert.h>
+#include <algorithm>
 #include <limits>
 #include <sstream>
 
@@ -107,7 +108,11 @@ int BitrateProber::TimeUntilNextProbe(int64_t now_ms) {
       time_until_probe_ms = 0;
     }
   }
-  return time_until_probe_ms;
+  return std::max(time_until_probe_ms, 0);
+}
+
+size_t BitrateProber::RecommendedPacketSize() const {
+  return packet_size_last_send_;
 }
 
 void BitrateProber::PacketSent(int64_t now_ms, size_t packet_size) {

webrtc/modules/pacing/bitrate_prober.h

@@ -38,6 +38,10 @@ class BitrateProber {
   // get accurate probing.
   int TimeUntilNextProbe(int64_t now_ms);
 
+  // Returns the number of bytes that the prober recommends for the next probe
+  // packet.
+  size_t RecommendedPacketSize() const;
+
   // Called to report to the prober that a packet has been sent, which helps the
   // prober know when to move to the next packet in a probe.
   void PacketSent(int64_t now_ms, size_t packet_size);

webrtc/modules/pacing/include/paced_sender.h

@@ -126,6 +126,9 @@ class PacedSender : public Module {
   // Process any pending packets in the queue(s).
   int32_t Process() override;
 
+ protected:
+  virtual bool ProbingExperimentIsEnabled() const;
+
  private:
   // Updates the number of bytes that can be sent for the next time interval.
   void UpdateBytesPerInterval(int64_t delta_time_in_ms)

webrtc/modules/pacing/paced_sender.cc

@@ -193,7 +193,9 @@ class IntervalBudget {
                                 -500 * target_rate_kbps_ / 8);
   }
 
-  int bytes_remaining() const { return bytes_remaining_; }
+  size_t bytes_remaining() const {
+    return static_cast<size_t>(std::max(0, bytes_remaining_));
+  }
 
   int target_rate_kbps() const { return target_rate_kbps_; }
 
@@ -334,7 +336,7 @@ int32_t PacedSender::Process() {
       UpdateBytesPerInterval(delta_time_ms);
     }
     while (!packets_->Empty()) {
-      if (media_budget_->bytes_remaining() <= 0 && !prober_->IsProbing()) {
+      if (media_budget_->bytes_remaining() == 0 && !prober_->IsProbing()) {
         return 0;
       }
 
@@ -355,10 +357,14 @@ int32_t PacedSender::Process() {
       }
     }
 
-    int padding_needed = padding_budget_->bytes_remaining();
-    if (padding_needed > 0) {
+    size_t padding_needed;
+    if (prober_->IsProbing() && ProbingExperimentIsEnabled())
+      padding_needed = prober_->RecommendedPacketSize();
+    else
+      padding_needed = padding_budget_->bytes_remaining();
+
+    if (padding_needed > 0)
       SendPadding(static_cast<size_t>(padding_needed));
-    }
   }
   return 0;
 }
@@ -386,13 +392,20 @@ void PacedSender::SendPadding(size_t padding_needed) {
   size_t bytes_sent = callback_->TimeToSendPadding(padding_needed);
   critsect_->Enter();
 
-  // Update padding bytes sent.
-  media_budget_->UseBudget(bytes_sent);
-  padding_budget_->UseBudget(bytes_sent);
+  if (bytes_sent > 0) {
+    prober_->PacketSent(clock_->TimeInMilliseconds(), bytes_sent);
+    media_budget_->UseBudget(bytes_sent);
+    padding_budget_->UseBudget(bytes_sent);
+  }
 }
 
 void PacedSender::UpdateBytesPerInterval(int64_t delta_time_ms) {
   media_budget_->IncreaseBudget(delta_time_ms);
   padding_budget_->IncreaseBudget(delta_time_ms);
 }
+
+bool PacedSender::ProbingExperimentIsEnabled() const {
+  return webrtc::field_trial::FindFullName("WebRTC-BitrateProbing") ==
+         "Enabled";
+}
 }  // namespace webrtc

webrtc/modules/pacing/paced_sender_unittest.cc

@@ -71,22 +71,26 @@ class PacedSenderProbing : public PacedSender::Callback {
                         uint16_t sequence_number,
                         int64_t capture_time_ms,
                         bool retransmission) {
+    ExpectAndCountPacket();
+    return true;
+  }
+
+  size_t TimeToSendPadding(size_t bytes) {
+    ExpectAndCountPacket();
+    return bytes;
+  }
+
+  void ExpectAndCountPacket() {
     ++packets_sent_;
     EXPECT_FALSE(expected_deltas_.empty());
     if (expected_deltas_.empty())
-      return false;
+      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;
-    return true;
-  }
-
-  size_t TimeToSendPadding(size_t bytes) {
-    EXPECT_TRUE(false);
-    return bytes;
   }
 
   int packets_sent() const { return packets_sent_; }
@@ -714,8 +718,6 @@ TEST_F(PacedSenderTest, ProbingWithInitialFrame) {
   std::list<int> expected_deltas_list(expected_deltas,
                                       expected_deltas + kNumPackets - 1);
   PacedSenderProbing callback(expected_deltas_list, &clock_);
-  // Probing implicitly enabled by creating a new PacedSender which defaults to
-  // probing on.
   send_bucket_.reset(
       new PacedSender(&clock_,
                       &callback,
@@ -741,6 +743,58 @@ TEST_F(PacedSenderTest, ProbingWithInitialFrame) {
   }
 }
 
+class ProbingPacedSender : public PacedSender {
+ public:
+  ProbingPacedSender(Clock* clock,
+                     Callback* callback,
+                     int bitrate_kbps,
+                     int max_bitrate_kbps,
+                     int min_bitrate_kbps)
+      : PacedSender(clock,
+                    callback,
+                    bitrate_kbps,
+                    max_bitrate_kbps,
+                    min_bitrate_kbps) {}
+
+  bool ProbingExperimentIsEnabled() const override { return true; }
+};
+
+TEST_F(PacedSenderTest, ProbingWithTooSmallInitialFrame) {
+  const int kNumPackets = 11;
+  const int kNumDeltas = kNumPackets - 1;
+  const size_t kPacketSize = 1200;
+  const int kInitialBitrateKbps = 300;
+  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 + kNumPackets - 1);
+  PacedSenderProbing callback(expected_deltas_list, &clock_);
+  send_bucket_.reset(
+      new ProbingPacedSender(&clock_, &callback, kInitialBitrateKbps,
+                             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));
+  }
+  while (callback.packets_sent() < kNumPackets) {
+    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());
+}
+
 TEST_F(PacedSenderTest, PriorityInversion) {
   uint32_t ssrc = 12346;
   uint16_t sequence_number = 1234;

webrtc/video/end_to_end_tests.cc

@@ -403,7 +403,8 @@ TEST_F(EndToEndTest, ReceivesAndRetransmitsNack) {
       if (sent_rtp_packets_ % kPacketsBetweenLossBursts == 0)
         packets_left_to_drop_ = kLossBurstSize;
 
-      if (packets_left_to_drop_ > 0) {
+      // Never drop padding packets as those won't be retransmitted.
+      if (packets_left_to_drop_ > 0 && header.paddingLength == 0) {
         --packets_left_to_drop_;
         dropped_packets_.insert(header.sequenceNumber);
         return DROP_PACKET;
@@ -463,11 +464,15 @@ TEST_F(EndToEndTest, CanReceiveFec) {
       RTPHeader header;
       EXPECT_TRUE(parser_->Parse(packet, length, &header));
 
-      EXPECT_EQ(kRedPayloadType, header.payloadType);
-      int encapsulated_payload_type =
-          static_cast<int>(packet[header.headerLength]);
-      if (encapsulated_payload_type != kFakeSendPayloadType)
-        EXPECT_EQ(kUlpfecPayloadType, encapsulated_payload_type);
+      int encapsulated_payload_type = -1;
+      if (header.payloadType == kRedPayloadType) {
+        encapsulated_payload_type =
+            static_cast<int>(packet[header.headerLength]);
+        if (encapsulated_payload_type != kFakeSendPayloadType)
+          EXPECT_EQ(kUlpfecPayloadType, encapsulated_payload_type);
+      } else {
+        EXPECT_EQ(kFakeSendPayloadType, header.payloadType);
+      }
 
       if (protected_sequence_numbers_.count(header.sequenceNumber) != 0) {
         // Retransmitted packet, should not count.
@@ -571,12 +576,16 @@ void EndToEndTest::TestReceivedFecPacketsNotNacked(
     Action OnSendRtp(const uint8_t* packet, size_t length) override {
       RTPHeader header;
       EXPECT_TRUE(parser_->Parse(packet, length, &header));
-      EXPECT_EQ(kRedPayloadType, header.payloadType);
 
-      int encapsulated_payload_type =
-          static_cast<int>(packet[header.headerLength]);
-      if (encapsulated_payload_type != kFakeSendPayloadType)
-        EXPECT_EQ(kUlpfecPayloadType, encapsulated_payload_type);
+      int encapsulated_payload_type = -1;
+      if (header.payloadType == kRedPayloadType) {
+        encapsulated_payload_type =
+            static_cast<int>(packet[header.headerLength]);
+        if (encapsulated_payload_type != kFakeSendPayloadType)
+          EXPECT_EQ(kUlpfecPayloadType, encapsulated_payload_type);
+      } else {
+        EXPECT_EQ(kFakeSendPayloadType, header.payloadType);
+      }
 
       if (has_last_sequence_number_ &&
           !IsNewerSequenceNumber(header.sequenceNumber,

webrtc/video/video_send_stream_tests.cc

@@ -306,14 +306,22 @@ TEST_F(VideoSendStreamTest, SupportsFec) {
         EXPECT_EQ(0, rtcp_sender.SendRTCP(feedback_state, kRtcpRr));
       }
 
-      EXPECT_EQ(kRedPayloadType, header.payloadType);
-
-      uint8_t encapsulated_payload_type = packet[header.headerLength];
-
-      if (encapsulated_payload_type == kUlpfecPayloadType) {
-        received_fec_ = true;
+      int encapsulated_payload_type = -1;
+      if (header.payloadType == kRedPayloadType) {
+        encapsulated_payload_type =
+            static_cast<int>(packet[header.headerLength]);
+        if (encapsulated_payload_type != kFakeSendPayloadType)
+          EXPECT_EQ(kUlpfecPayloadType, encapsulated_payload_type);
       } else {
-        received_media_ = true;
+        EXPECT_EQ(kFakeSendPayloadType, header.payloadType);
+      }
+
+      if (encapsulated_payload_type != -1) {
+        if (encapsulated_payload_type == kUlpfecPayloadType) {
+          received_fec_ = true;
+        } else {
+          received_media_ = true;
+        }
       }
 
       if (received_media_ && received_fec_)