Refactor NACK bitrate allocation

webrtc/base/rate_limiter.cc

Index: webrtc/base/rate_limiter.cc
diff --git a/webrtc/base/rate_limiter.cc b/webrtc/base/rate_limiter.cc
new file mode 100644
index 0000000000000000000000000000000000000000..a3011c2cec8f42434b2395d26b47746b37783845
--- /dev/null
+++ b/webrtc/base/rate_limiter.cc
@@ -0,0 +1,62 @@
+/*
+ *  Copyright (c) 2016 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.
+ */
+
+#include "webrtc/base/rate_limiter.h"
+#include "webrtc/system_wrappers/include/clock.h"
+
+namespace webrtc {
+
+RateLimiter::RateLimiter(Clock* clock, int64_t max_retransmission_window_ms)
+    : clock_(clock),
+      current_rate_(max_retransmission_window_ms, RateStatistics::kBpsScale),
+      window_size_ms_(max_retransmission_window_ms),
+      max_rate_bps_(std::numeric_limits<uint32_t>::max()) {}
+
+RateLimiter::~RateLimiter() {}
+
+// Try to use rate to send bytes. Returns true on success and if so updates
+// current rate.
+bool RateLimiter::TryUseRate(size_t packet_size_bytes) {
+  rtc::CritScope cs(&lock_);
+  rtc::Optional<uint32_t> current_rate =
+      current_rate_.Rate(clock_->TimeInMilliseconds());
+  if (current_rate) {
+    // If there is a current rate, check if adding bytes would cause maximum
+    // bitrate target to be exceeded. If there is NOT a valid current rate,
+    // allow allocating rate even if target is exceeded. This prevents
+    // problems
+    // at very low rates, where for instance retransmissions would never be
+    // allowed due to too high bitrate caused by a single packet.

[stefan-webrtc, 2016/07/06 12:43:21]

When is there not a current rate? Does this always happen on low packet rates?
Sounds dangerous to always allow retransmits in that case.

[sprang_webrtc, 2016/07/06 14:13:07]

Less than one packet per window (1pps). How would we otherwise deal with the
situation that (for instance) estimated rate is 40kbps, RTT is 250ms and a nack
arrives for packet of 1500 bytes. That would indicate a retransmission rate of
1500 * 8 / 0.250 = 48kbps and so would always be denied...

I think one packet per RTT is OK, even if it technically is too much.

[stefan-webrtc, 2016/07/08 11:55:58]

Agree... :)

+
+    size_t bitrate_addition_bps =
+        (packet_size_bytes * 8 * 1000) / window_size_ms_;
+    if (*current_rate + bitrate_addition_bps > max_rate_bps_)
+      return false;
+  }
+
+  current_rate_.Update(packet_size_bytes, clock_->TimeInMilliseconds());
+  return true;
+}
+
+// Set the maximum bitrate, in bps, that this limiter allows to send.
+void RateLimiter::SetMaxRate(uint32_t max_rate_bps) {
+  rtc::CritScope cs(&lock_);
+  max_rate_bps_ = max_rate_bps;
+}
+
+// Set the window size over which to measure the current bitrate.
+// For retransmissions, this is typically the RTT.
+void RateLimiter::SetWindowSize(int64_t window_size_ms) {
+  rtc::CritScope cs(&lock_);
+  window_size_ms_ = window_size_ms;
+  current_rate_.SetWindowSize(window_size_ms, clock_->TimeInMilliseconds());
+}
+
+}  // namespace webrtc