Refactor NACK bitrate allocation

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 c1741d690245610b6d8c936876d850fe075fd0b3..6f6996294f8edcf37899161e603ae127827ed6e4 100644
--- a/webrtc/modules/pacing/paced_sender.cc
+++ b/webrtc/modules/pacing/paced_sender.cc
@@ -32,6 +32,7 @@ const int64_t kMinPacketLimitMs = 5;
 // time.
 const int64_t kMaxIntervalTimeMs = 30;
 
+const int64_t kMaxRtt = 1000;

[danilchap, 2016/06/23 12:46:12]

kMaxRttMs

 }  // namespace
 
 // TODO(sprang): Move at least PacketQueue and MediaBudget out to separate
@@ -260,7 +261,8 @@ PacedSender::PacedSender(Clock* clock, PacketSender* packet_sender)
       pacing_bitrate_kbps_(0),
       time_last_update_us_(clock->TimeInMicroseconds()),
       packets_(new paced_sender::PacketQueue(clock)),
-      packet_counter_(0) {
+      packet_counter_(0),
+      retransmission_rate_(kMaxRtt, 8) {  // Scale 8 for bits per second.

[danilchap, 2016/06/23 12:46:12]

shouldn't this be 8000 instead of 8? (scale is for 8bps not 8bpms)

[sprang_webrtc, 2016/06/28 09:12:32]

Yes.

   UpdateBytesPerInterval(kMinPacketLimitMs);
 }
 
@@ -326,6 +328,42 @@ void PacedSender::InsertPacket(RtpPacketSender::Priority priority,
                                       retransmission, packet_counter_++));
 }
 
+bool PacedSender::AllocateRetransmissionBitrate(size_t bytes) {
+  int64_t now_ms = clock_->TimeInMilliseconds();
+  CriticalSectionScoped cs(critsect_.get());
+
+  rtc::Optional<uint32_t> retransmission_rate_bps =
+      retransmission_rate_.Rate(now_ms);
+  if (!retransmission_rate_bps) {
+    // Rate not high enough to measure, let it slide.
+    return true;
+  }
+
+  int64_t max_retransmission_rate_bps =

[danilchap, 2016/06/23 12:46:12]

rate (bits/s) * rtt (ms) should result in bits, not bits/s
Can you add a comment how you get bits/s or why is it valid to compare bits vs
bits/s next line

[sprang_webrtc, 2016/06/28 09:12:32]

Not sure what happened here, thought I had removed this code. I changed it to
update the window size of the rate limiter instead and just use rate limit. Not
sure how this (incorrect) code ended up back here...

+      (2 * rtt_ms_ *
+       std::max(min_send_bitrate_kbps_ * 1000, estimated_bitrate_bps_)) /
+      1000;
+  if (*retransmission_rate_bps < max_retransmission_rate_bps) {
+    retransmission_rate_.Update(bytes, now_ms);
+    return true;
+  }
+
+  return false;
+}
+
+int PacedSender::CurrentRetransmissionBitrate() {
+  int64_t now_ms = clock_->TimeInMilliseconds();
+  CriticalSectionScoped cs(critsect_.get());
+  return retransmission_rate_.Rate(now_ms).value_or(0);
+}
+
+void PacedSender::OnRttUpdate(int64_t rtt) {
+  RTC_DCHECK_GT(rtt, 0);
+  CriticalSectionScoped cs(critsect_.get());
+  rtt_ms_ = std::min(rtt, kMaxRtt);
+  retransmission_rate_.SetWindowSize(rtt_ms_, clock_->TimeInMilliseconds());
+}
+
 int64_t PacedSender::ExpectedQueueTimeMs() const {
   CriticalSectionScoped cs(critsect_.get());
   RTC_DCHECK_GT(pacing_bitrate_kbps_, 0u);