Refactor how padding is calculated.

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 227 matching lines @@
   static const int kWindowMs = 500;
 
   int target_rate_kbps_;
   int bytes_remaining_;
 };
 }  // namespace paced_sender
 
 const int64_t PacedSender::kMaxQueueLengthMs = 2000;
 const float PacedSender::kDefaultPaceMultiplier = 2.5f;
 
-PacedSender::PacedSender(Clock* clock,
-                         PacketSender* packet_sender)
+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() {
@@ skipping 19 matching lines @@
       kDefaultPaceMultiplier;
 }
 
 void PacedSender::SetAllocatedSendBitrate(int allocated_bitrate,
                                           int padding_bitrate) {
   CriticalSectionScoped cs(critsect_.get());
   min_send_bitrate_kbps_ = allocated_bitrate / 1000;
   pacing_bitrate_kbps_ =
       std::max(min_send_bitrate_kbps_, estimated_bitrate_bps_ / 1000) *
       kDefaultPaceMultiplier;
-  padding_budget_->set_target_rate_kbps(padding_bitrate / 1000);
+  max_padding_bitrate_kbps_ = padding_bitrate / 1000;
 }
 
 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)
@@ skipping 51 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_;
+  padding_budget_->set_target_rate_kbps(
+      std::min(estimated_bitrate_bps_ / 1000, max_padding_bitrate_kbps_));

[stefan-webrtc, 2016/06/08 09:25:13]

Why was this change needed? Seems like we're setting it more often than needed
now?

[perkj_webrtc, 2016/06/08 15:35:27]

Done.

   // TODO(holmer): Remove the !paused_ check when issue 5307 has been fixed.
   if (!paused_ && 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());
@@ skipping 30 matching lines @@
       // 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())
     return;
 
-  size_t padding_needed;
-  if (is_probing) {
-    padding_needed = prober_->RecommendedPacketSize();
-  } else {
-    padding_needed = padding_budget_->bytes_remaining();
+  // We can not send padding unless a normal packet has first been sent. If we
+  // do, time stamps gets messed up.

[stefan-webrtc, 2016/06/08 09:25:13]

"timestamps get messsed up"

[perkj_webrtc, 2016/06/08 15:35:27]

Done.

[stefan-webrtc, 2016/06/10 13:54:13]

I was referring to that "gets" should be "get" :)

[perkj_webrtc, 2016/06/14 10:58:02]

oops

+  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);
   }
-
-  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)
     return false;
   critsect_->Leave();
@@ skipping 27 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