Logging basic bad call detection

webrtc/video/bad_call_threshold.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/video/bad_call_threshold.h"
+
+#include "webrtc/base/checks.h"
+#include "webrtc/base/logging.h"
+
+namespace webrtc {
+
+BadCallThreshold::BadCallThreshold(int threshold,
+                                   int default_measurement,
+                                   float fraction,
+                                   int max_measurements)
+    : buffer_(new int[max_measurements]),
+      max_measurements_(max_measurements),
+      fraction_(fraction),
+      threshold_(threshold),
+      next_index_(0),
+      is_high_(default_measurement >= threshold) {
+  RTC_CHECK_GE(fraction, 0.5f);
+  std::fill_n(buffer_.get(), max_measurements, default_measurement);
+}
+
+BadCallThreshold::BadCallState BadCallThreshold::AddMeasurement(
+    int measurement) {
+  buffer_[next_index_] = measurement;
+  next_index_ = (next_index_ + 1) % max_measurements_;
+
+  float frac_high = 0;
+  for (int i = 0; i < max_measurements_; ++i) {
+    if (buffer_[i] >= threshold_)
+      frac_high += 1.0f / max_measurements_;
+  }
+
+  if (!is_high_ && frac_high >= fraction_) {
+    is_high_ = true;
+    return NEWLY_HIGH;
+  }

[sprang_webrtc, 2016/11/14 13:14:15]

What tradeoffs are there with doing state change detection outside this class?

[palmkvist, 2016/11/15 15:01:01]

You're suggesting just calculating which state we're in here and letting a
caller figure out if that state is different from the previous?

It would essentially necessitate code like this:
bool high = thresh.getState();
thresh.AddMeasurement(thing);
if (high != thresh.getState())
  ...

Which is slightly more cumbersome than the current approach, but it would
simplify the code in AddMeasurement.

On the other hand the current scheme does not give a way to retrieve the state
without adding a measurement, which is weird, and any such method would either
only return STILL_LOW or STILL_HIGH, or need to do some extra caching.

Overally probably worth it to move the change detection out.

[sprang_webrtc, 2016/11/16 13:20:09]

Maybe also consider returning an rtc::Optional<State>, and don't report a value
if there are foo few data points.

+
+  if (is_high_ && frac_high <= 1 - fraction_) {
+    is_high_ = false;
+    return NEWLY_LOW;
+  }
+
+  return is_high_ ? STILL_HIGH : STILL_LOW;
+}
+
+}  // namespace webrtc