Adding metrics to AEC3

webrtc/modules/audio_processing/aec3/block_processor_metrics.cc

+/*
+ *  Copyright (c) 2017 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/audio_processing/aec3/block_processor_metrics.h"
+
+#include "webrtc/modules/audio_processing/aec3/aec3_common.h"
+#include "webrtc/system_wrappers/include/metrics.h"
+
+namespace webrtc {
+
+BlockProcessorMetrics::BlockProcessorMetrics() {
+  ResetMetrics();

[hlundin-webrtc, 2017/02/28 10:04:22]

No need to call reset here, since nothing changes from the initialized values.
Thus, you can make this = default, or maybe even omit the ctor from h- and
cc-files.

[peah-webrtc, 2017/02/28 12:57:25]

Done.

+}
+
+void BlockProcessorMetrics::ResetMetrics() {

[hlundin-webrtc, 2017/02/28 10:04:22]

Wrong order of methods compared to h file.

[peah-webrtc, 2017/02/28 12:57:25]

Done.

+  render_buffer_underruns_ = 0;
+  render_buffer_overruns_ = 0;
+  buffer_render_calls_ = 0;
+}
+
+void BlockProcessorMetrics::UpdateCapture(bool underrun) {
+  ++capture_block_counter_;
+  if (underrun) {
+    ++render_buffer_underruns_;
+  }
+
+  if (capture_block_counter_ == kMetricsReportingInterval) {
+    metric_reported_ = true;
+
+    int value_to_report;
+    if (render_buffer_underruns_ == 0) {
+      value_to_report = 0;

[hlundin-webrtc, 2017/02/28 10:04:22]

Is this some kind of enum metric? If so, I suggest you define the enum properly.
And use RTC_HISTOGRAM_ENUMERATION.

[peah-webrtc, 2017/02/28 12:57:25]

Done.

+    } else if (render_buffer_underruns_ > (capture_block_counter_ >> 1)) {
+      value_to_report = 4;
+    } else if (render_buffer_underruns_ > 100) {
+      value_to_report = 3;
+    } else if (render_buffer_underruns_ > 10) {
+      value_to_report = 2;
+    } else {
+      value_to_report = 1;
+    }
+    RTC_HISTOGRAM_COUNTS_LINEAR("WebRTC.Audio.EchoCanceller.RenderUnderruns",
+                                value_to_report, 0, 4, 5);
+
+    if (render_buffer_overruns_ == 0) {
+      value_to_report = 0;
+    } else if (render_buffer_overruns_ > (buffer_render_calls_ >> 1)) {
+      value_to_report = 4;
+    } else if (render_buffer_overruns_ > 100) {
+      value_to_report = 3;
+    } else if (render_buffer_overruns_ > 10) {
+      value_to_report = 2;
+    } else {
+      value_to_report = 1;
+    }
+    RTC_HISTOGRAM_COUNTS_LINEAR("WebRTC.Audio.EchoCanceller.RenderOverruns",
+                                value_to_report, 0, 4, 5);
+
+    capture_block_counter_ = 0;

[hlundin-webrtc, 2017/02/28 10:04:22]

Shouldn't buffer_render_calls_ be reset too?

[peah-webrtc, 2017/02/28 12:57:25]

Good find! The call to ResetMetrics() was missing.

Done.

+  } else {
+    metric_reported_ = false;
+  }
+}
+
+void BlockProcessorMetrics::UpdateRender(bool overrun) {
+  ++buffer_render_calls_;
+  if (overrun) {
+    ++render_buffer_overruns_;
+  }
+}
+
+}  // namespace webrtc