Let alr dectector use a budged to detect underuse.

webrtc/modules/pacing/interval_budget.cc

+/*
+ *  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/modules/pacing/interval_budget.h"
+
+#include <algorithm>
+
+namespace webrtc {
+namespace {
+constexpr int kWindowMs = 500;
+}
+
+IntervalBudget::IntervalBudget(int initial_target_rate_kbps)
+    : IntervalBudget(initial_target_rate_kbps, false) {}
+
+IntervalBudget::IntervalBudget(int initial_target_rate_kbps,
+                               bool can_build_up_underuse)
+    : bytes_remaining_(0), can_build_up_underuse_(can_build_up_underuse) {
+  set_target_rate_kbps(initial_target_rate_kbps);
+}
+
+void IntervalBudget::set_target_rate_kbps(int target_rate_kbps) {
+  target_rate_kbps_ = target_rate_kbps;
+  max_bytes_in_budget_ = (kWindowMs * target_rate_kbps_) / 8;
+  bytes_remaining_ = std::min(std::max(-max_bytes_in_budget_, bytes_remaining_),
+                              max_bytes_in_budget_);
+}
+
+void IntervalBudget::IncreaseBudget(int64_t delta_time_ms) {

[philipel, 2017/07/10 11:41:05]

I think it might be good to merge IncreaseBudget and UseBudget into a
UpdateBudget(size_t bytes_used, int64_t delta_time_ms) function. That way we can
calculate remaining bytes without a bias.

Maybe it doesn't matter that much, but I think it's good to avoid quirks so they
don't come back and bite you later on.

[tschumi, 2017/07/11 09:33:31]

Would be a bit of a refactoring in the pacer => maybe in another cl ?

[philipel, 2017/07/11 12:29:05]

Yeah, a separate CL sounds good. Please add a TODO.

[tschumi, 2017/07/11 12:43:35]

Done.

+  int bytes = target_rate_kbps_ * delta_time_ms / 8;
+  if (bytes_remaining_ < 0 || can_build_up_underuse_) {
+    // We overused last interval, compensate this interval.
+    bytes_remaining_ = std::min(bytes_remaining_ + bytes, max_bytes_in_budget_);
+  } else {
+    // If we underused last interval we can't use it this interval.
+    bytes_remaining_ = std::min(bytes, max_bytes_in_budget_);

[philipel, 2017/07/10 11:41:05]

It looks like |bytes_remaining_| can be positive even when
|can_build_up_underuse_| is true. If we stop sending for a short while then
|bytes| can be pretty large.

Shouldn't we always update |bytes_remaining_| like we do on line 40 and then
just:
if (!can_build_up_underuse_)
  bytes_remaining_ = std::min(bytes_remaining_, 1);

[tschumi, 2017/07/11 09:33:31]

delta_time_ms is clamped by the pacer to 5ms so bytes should not become very
large.
The actual logic kind of make sense to me. Me may could implement it in a nicer
way.
For example:
Change Interval Budget so that we can define a over- and under-use window in ms.
So we could use the a budget for the pacer like budget(5ms,-500ms) and the
remove the clamp in the pacer.

[philipel, 2017/07/11 12:29:05]

Sound like we rely on external behavior, don't think we should do that.
I think specifying min/max budget in terms of ms is a good idea, another TODO
you could add :)

[tschumi, 2017/07/11 12:43:35]

Done.

[tschumi, 2017/07/11 12:43:35]

Done.

+  }
+}
+
+void IntervalBudget::UseBudget(size_t bytes) {
+  bytes_remaining_ = std::max(bytes_remaining_ - static_cast<int>(bytes),
+                              -max_bytes_in_budget_);
+}
+
+size_t IntervalBudget::bytes_remaining() const {
+  return static_cast<size_t>(std::max(0, bytes_remaining_));
+}
+
+int IntervalBudget::budget_level_percent() const {
+  return bytes_remaining_ * 100 / max_bytes_in_budget_;
+}
+
+int IntervalBudget::target_rate_kbps() const {
+  return target_rate_kbps_;
+}
+
+}  // namespace webrtc