Bitrate prober and paced sender improvements

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 ae9743434029335af98a76a0cc34875aa3ff1552..7e7631471de3d2230275632f5c822cb8dda5d42d 100644
--- a/webrtc/modules/pacing/paced_sender.cc
+++ b/webrtc/modules/pacing/paced_sender.cc
@@ -250,7 +250,6 @@ PacedSender::PacedSender(Clock* clock, PacketSender* packet_sender)
       packet_sender_(packet_sender),
       critsect_(CriticalSectionWrapper::CreateCriticalSection()),
       paused_(false),
-      probing_enabled_(true),
       media_budget_(new paced_sender::IntervalBudget(0)),
       padding_budget_(new paced_sender::IntervalBudget(0)),
       prober_(new BitrateProber()),
@@ -278,7 +277,8 @@ void PacedSender::Resume() {
 
 void PacedSender::SetProbingEnabled(bool enabled) {
   RTC_CHECK_EQ(0u, packet_counter_);
-  probing_enabled_ = enabled;
+  CriticalSectionScoped cs(critsect_.get());
+  prober_->SetEnabled(enabled);
 }
 
 void PacedSender::SetEstimatedBitrate(uint32_t bitrate_bps) {
@@ -313,8 +313,6 @@ void PacedSender::InsertPacket(RtpPacketSender::Priority priority,
   RTC_DCHECK(estimated_bitrate_bps_ > 0)
         << "SetEstimatedBitrate must be called before InsertPacket.";
 
-  if (probing_enabled_ && !prober_->IsProbing())
-    prober_->SetEnabled(true);
   int64_t now_ms = clock_->TimeInMilliseconds();
   prober_->OnIncomingPacket(estimated_bitrate_bps_, bytes, now_ms);
 
@@ -372,8 +370,9 @@ void PacedSender::Process() {
   int64_t elapsed_time_ms = (now_us - time_last_update_us_ + 500) / 1000;
   time_last_update_us_ = now_us;
   int target_bitrate_kbps = pacing_bitrate_kbps_;
-  // TODO(holmer): Remove the !paused_ check when issue 5307 has been fixed.
-  if (!paused_ && elapsed_time_ms > 0) {
+  if (paused_)
+    return;
+  if (elapsed_time_ms > 0) {
     size_t queue_size_bytes = packets_->SizeInBytes();
     if (queue_size_bytes > 0) {
       // Assuming equal size packets and input/output rate, the average packet
@@ -418,8 +417,7 @@ void PacedSender::Process() {
     }
   }
 
-  // TODO(holmer): Remove the paused_ check when issue 5307 has been fixed.
-  if (paused_ || !packets_->Empty())
+  if (!packets_->Empty())
     return;
 
   // We can not send padding unless a normal packet has first been sent. If we
@@ -435,10 +433,7 @@ void PacedSender::Process() {
 
 bool PacedSender::SendPacket(const paced_sender::Packet& packet,
                              int probe_cluster_id) {
-  // TODO(holmer): Because of this bug issue 5307 we have to send audio
-  // packets even when the pacer is paused. Here we assume audio packets are
-  // always high priority and that they are the only high priority packets.
-  if (paused_ && packet.priority != kHighPriority)

[stefan-webrtc, 2016/07/29 07:12:07]

If there're still bugs in the network layer code which causes the pacer to be
paused when it's not supposed to be, removing this can break audio. Therefore
I'd prefer if we at least did it in a separate CL from all the other
improvements you are making here.

Same for the other TODOs in this file that you're fixing.

[Irfan, 2016/08/01 18:29:47]

ok I will pull the paused changes into a seperate CL

+  if (paused_)
     return false;
   critsect_->Leave();
   const bool success = packet_sender_->TimeToSendPacket(