BitrateProber: Support higher probing bitrates

webrtc/modules/pacing/paced_sender.cc

Index: webrtc/modules/pacing/paced_sender.cc
diff --git a/webrtc/modules/pacing/paced_sender.cc b/webrtc/modules/pacing/paced_sender.cc
index b0b86ee40e73cce3af346a12876613484059cce4..f8ca07720ab46ec5b9a402270651f6844bbcb7d4 100644
--- a/webrtc/modules/pacing/paced_sender.cc
+++ b/webrtc/modules/pacing/paced_sender.cc
@@ -402,8 +402,13 @@ void PacedSender::Process() {
   }
 
   bool is_probing = prober_->IsProbing();
-  int probe_cluster_id = is_probing ? prober_->CurrentClusterId()
-                                    : PacketInfo::kNotAProbe;
+  int probe_cluster_id = PacketInfo::kNotAProbe;
+  size_t bytes_sent = 0;
+  size_t recommended_probe_size = 0;
+  if (is_probing) {
+    probe_cluster_id = prober_->CurrentClusterId();
+    recommended_probe_size = prober_->RecommendedProbeSize();
+  }
   while (!packets_->Empty()) {
     // 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
@@ -412,9 +417,12 @@ void PacedSender::Process() {
 
     if (SendPacket(packet, probe_cluster_id)) {
       // Send succeeded, remove it from the queue.
+      bytes_sent += packet.bytes;
       packets_->FinalizePop(packet);
-      if (is_probing)
+      if (is_probing && bytes_sent > recommended_probe_size) {

[stefan-webrtc, 2016/09/16 07:42:53]

This may look like a bug to the reader as it seems like we are sending packets
in the middle of a probe which are not part of the probe. It's by design because
we may have small audio packets that must be sent, but maybe we should say that
in a comment?

[Irfan, 2016/09/19 06:05:11]

Not sure I follow. The is_probing flag indicates whether we are in a probing
state or not. The check here is for ensuring we send enough probe packets before
we return.

Can you elaborate what comment what you would like to see here ?

[stefan-webrtc, 2016/09/19 07:24:56]

I'm thinking of the case where we have paced out, e.g., an audio packet which is
smaller than recommended_probe_size. In that case we're not reporting it as a
ProbeSent, which looks odd if we don't explain with a comment.

[Irfan, 2016/09/19 18:23:30]

We report once the aggregate bytes is above the recommended size. In the latest
PS, this should be more clear that we always report the byte size in the end
before returning from Process(). 

If there is an audio packet here, it will actually add up in this aggregate of
bytes_sent and be reported to be prober.

+        prober_->ProbeSent(clock_->TimeInMilliseconds(), bytes_sent);
         return;
+      }
     } else {
       // Send failed, put it back into the queue.
       packets_->CancelPop(packet);
@@ -430,12 +438,14 @@ void PacedSender::Process() {
   // We can not send padding unless a normal packet has first been sent. If we
   // do, timestamps get messed up.
   if (packet_counter_ > 0) {
-    size_t padding_needed = is_probing ? prober_->RecommendedPacketSize()
+    size_t padding_needed = is_probing ? (recommended_probe_size - bytes_sent)
                                        : padding_budget_->bytes_remaining();
 
     if (padding_needed > 0)
-      SendPadding(padding_needed, probe_cluster_id);
+      bytes_sent += SendPadding(padding_needed, probe_cluster_id);
   }
+  if (is_probing)
+    prober_->ProbeSent(clock_->TimeInMilliseconds(), bytes_sent);
 }
 
 bool PacedSender::SendPacket(const paced_sender::Packet& packet,
@@ -458,7 +468,6 @@ bool PacedSender::SendPacket(const paced_sender::Packet& packet,
   critsect_->Enter();
 
   if (success) {
-    prober_->PacketSent(clock_->TimeInMilliseconds(), packet.bytes);
     // TODO(holmer): High priority packets should only be accounted for if we
     // are allocating bandwidth for audio.
     if (packet.priority != kHighPriority) {
@@ -471,17 +480,17 @@ bool PacedSender::SendPacket(const paced_sender::Packet& packet,
   return success;
 }
 
-void PacedSender::SendPadding(size_t padding_needed, int probe_cluster_id) {
+size_t PacedSender::SendPadding(size_t padding_needed, int probe_cluster_id) {
   critsect_->Leave();
   size_t bytes_sent =
       packet_sender_->TimeToSendPadding(padding_needed, probe_cluster_id);
   critsect_->Enter();
 
   if (bytes_sent > 0) {
-    prober_->PacketSent(clock_->TimeInMilliseconds(), bytes_sent);
     media_budget_->UseBudget(bytes_sent);
     padding_budget_->UseBudget(bytes_sent);
   }
+  return bytes_sent;
 }
 
 void PacedSender::UpdateBytesPerInterval(int64_t delta_time_ms) {