More reliable ALR detection

webrtc/modules/pacing/alr_detector.cc

Index: webrtc/modules/pacing/alr_detector.cc
diff --git a/webrtc/modules/pacing/alr_detector.cc b/webrtc/modules/pacing/alr_detector.cc
index 677c7e018e59823faddcaafabe2c6ca804f383b4..d0c5618b9c0f73217386f14058001bde9cf31842 100644
--- a/webrtc/modules/pacing/alr_detector.cc
+++ b/webrtc/modules/pacing/alr_detector.cc
@@ -15,56 +15,39 @@
 
 namespace {
 
-// Time period over which outgoing traffic is measured and considered a single
-// data point.
-constexpr int kMeasurementPeriodMs = 100;
+// Time period over which outgoing traffic is measured.
+constexpr int kMeasurementPeriodMs = 500;
 
-// Minimum number of consecutive measurements over |kMeasurementPeriodMs| time
-// that indicate sending rate is below |kUsagePercent| to consider being
-// application limited.
-constexpr int kApplicationLimitedThreshold = 5;
-
-// Sent traffic percentage as a function of network capaicty to consider traffic
-// as application limited.
+// Sent traffic percentage as a function of network capacity used to determine
+// application-limited region. ALR region start when bandwidth usage drops below
+// kAlrStartUsagePercent and ends when it raises above kAlrEndUsagePercent.
 // NOTE: This is intentionally conservative at the moment until BW adjustments
 // of application limited region is fine tuned.
-constexpr int kUsagePercent = 30;
+constexpr int kAlrStartUsagePercent = 30;
+constexpr int kAlrEndUsagePercent = 50;
 
 }  // namespace
 
 namespace webrtc {
 
 AlrDetector::AlrDetector()
-    : measurement_interval_bytes_sent_(0),
-      measurement_interval_elapsed_time_ms_(0),
-      estimated_bitrate_bps_(0),
-      application_limited_count_(0) {}
+    : rate_(kMeasurementPeriodMs, RateStatistics::kBpsScale) {}
 
 AlrDetector::~AlrDetector() {}
 
-void AlrDetector::OnBytesSent(size_t bytes_sent, int64_t elapsed_time_ms) {
-  if (measurement_interval_elapsed_time_ms_ > kMeasurementPeriodMs) {
-    RTC_DCHECK(estimated_bitrate_bps_);
-    int max_bytes =
-        (measurement_interval_elapsed_time_ms_ * estimated_bitrate_bps_) /
-        (8 * 1000);
-    RTC_DCHECK_GT(max_bytes, 0);
-    int utilization =
-        static_cast<int>((measurement_interval_bytes_sent_ * 100) / max_bytes);
-    if (utilization < kUsagePercent) {
-      application_limited_count_++;
-      if (application_limited_count_ == kApplicationLimitedThreshold)
-        LOG(LS_INFO) << "ALR start";
-    } else {
-      if (application_limited_count_ >= kApplicationLimitedThreshold)
-        LOG(LS_INFO) << "ALR stop";
-      application_limited_count_ = 0;
-    }
-    measurement_interval_elapsed_time_ms_ = elapsed_time_ms;
-    measurement_interval_bytes_sent_ = bytes_sent;
-  } else {
-    measurement_interval_elapsed_time_ms_ += elapsed_time_ms;
-    measurement_interval_bytes_sent_ += bytes_sent;
+void AlrDetector::OnBytesSent(size_t bytes_sent, int64_t now_ms) {
+  RTC_DCHECK(estimated_bitrate_bps_);
+
+  rate_.Update(bytes_sent, now_ms);
+  rtc::Optional<uint32_t> rate = rate_.Rate(now_ms);
+  if (!rate)
+    return;
+
+  int percentage = static_cast<int>(*rate) * 100 / estimated_bitrate_bps_;
+  if (percentage < kAlrStartUsagePercent && !application_limited_) {

[philipel, 2016/11/15 18:20:54]

I think we should add a unittest to test this new behavior (also, I see that we
don't have a single unittest where we test if we are not in ALR, that would also
be good :))

[philipel, 2016/11/15 18:23:00]

Oops, we do have a unittest where we are not in ALR. Still, we should add a
unittest for this new behavior.

+    application_limited_ = true;
+  } else if (percentage > kAlrEndUsagePercent && application_limited_) {
+    application_limited_ = false;
   }
 }
 
@@ -74,7 +57,7 @@ void AlrDetector::SetEstimatedBitrate(int bitrate_bps) {
 }
 
 bool AlrDetector::InApplicationLimitedRegion() {
-  return application_limited_count_ >= kApplicationLimitedThreshold;
+  return application_limited_;
 }
 
 }  // namespace webrtc