Bitrate prober and paced sender improvements

webrtc/modules/pacing/paced_sender.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.
  */
 
@@ skipping 232 matching lines @@
 }  // namespace paced_sender
 
 const int64_t PacedSender::kMaxQueueLengthMs = 2000;
 const float PacedSender::kDefaultPaceMultiplier = 2.5f;
 
 PacedSender::PacedSender(Clock* clock, PacketSender* packet_sender)
     : clock_(clock),
       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()),
       estimated_bitrate_bps_(0),
       min_send_bitrate_kbps_(0u),
       max_padding_bitrate_kbps_(0u),
       pacing_bitrate_kbps_(0),
       time_last_update_us_(clock->TimeInMicroseconds()),
       packets_(new paced_sender::PacketQueue(clock)),
       packet_counter_(0) {
   UpdateBytesPerInterval(kMinPacketLimitMs);
 }
 
 PacedSender::~PacedSender() {}
 
 void PacedSender::Pause() {
   CriticalSectionScoped cs(critsect_.get());
   paused_ = true;
 }
 
 void PacedSender::Resume() {
   CriticalSectionScoped cs(critsect_.get());
   paused_ = false;
 }
 
 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) {
   CriticalSectionScoped cs(critsect_.get());
   estimated_bitrate_bps_ = bitrate_bps;
   padding_budget_->set_target_rate_kbps(
       std::min(estimated_bitrate_bps_ / 1000, max_padding_bitrate_kbps_));
   pacing_bitrate_kbps_ =
       std::max(min_send_bitrate_kbps_, estimated_bitrate_bps_ / 1000) *
       kDefaultPaceMultiplier;
@@ skipping 14 matching lines @@
 void PacedSender::InsertPacket(RtpPacketSender::Priority priority,
                                uint32_t ssrc,
                                uint16_t sequence_number,
                                int64_t capture_time_ms,
                                size_t bytes,
                                bool retransmission) {
   CriticalSectionScoped cs(critsect_.get());
   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);
 
   if (capture_time_ms < 0)
     capture_time_ms = now_ms;
 
   packets_->Push(paced_sender::Packet(priority, ssrc, sequence_number,
                                       capture_time_ms, now_ms, bytes,
                                       retransmission, packet_counter_++));
 }
@@ skipping 37 matching lines @@
   int64_t elapsed_time_ms = (elapsed_time_us + 500) / 1000;
   return std::max<int64_t>(kMinPacketLimitMs - elapsed_time_ms, 0);
 }
 
 void PacedSender::Process() {
   int64_t now_us = clock_->TimeInMicroseconds();
   CriticalSectionScoped cs(critsect_.get());
   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
       // has avg_time_left_ms left to get queue_size_bytes out of the queue, if
       // time constraint shall be met. Determine bitrate needed for that.
       packets_->UpdateQueueTime(clock_->TimeInMilliseconds());
       int64_t avg_time_left_ms = std::max<int64_t>(
           1, kMaxQueueLengthMs - packets_->AverageQueueTimeMs());
       int min_bitrate_needed_kbps =
           static_cast<int>(queue_size_bytes * 8 / avg_time_left_ms);
@@ skipping 24 matching lines @@
       packets_->FinalizePop(packet);
       if (is_probing)
         return;
     } else {
       // Send failed, put it back into the queue.
       packets_->CancelPop(packet);
       return;
     }
   }
 
-  // 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
   // do, timestamps get messed up.
   if (packet_counter_ > 0) {
     size_t padding_needed = is_probing ? prober_->RecommendedPacketSize()
                                        : padding_budget_->bytes_remaining();
 
     if (padding_needed > 0)
       SendPadding(padding_needed, probe_cluster_id);
   }
 }
 
 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(
       packet.ssrc, packet.sequence_number, packet.capture_time_ms,
       packet.retransmission, probe_cluster_id);
   critsect_->Enter();
 
   if (success) {
     prober_->PacketSent(clock_->TimeInMilliseconds(), packet.bytes);
     // TODO(holmer): High priority packets should only be accounted for if we
@@ skipping 19 matching lines @@
     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);
 }
 }  // namespace webrtc