UMA histogram WebRTC.AecDelayBasedQuality updated

The UMA histogram is used to verify performance of the Finch experiment "UseDelayAgnosticAEC".

In webrtc:r8170 the Echo Delay Metrics queried through webrtc::EchoCancellation::GetEchoDelayMetrics() were changed from reset upon query to fixed aggregation window.
These metrics are used in GetStats() and further to calculate and log WebRTC.AecDelayBasedQuality.
Before the change we could not handle multiple clients, so we queried the metrics when a client called GetStats(). Now we can handle these two types of stats independently.

In webrtc:r8230 a new metric was added. This metric calculates the fraction of poor delays.
Before the change we calculated such a metric in chromium for logging WebRTC.AecDelayBasedQuality. Now we can query the metric directly and average several queries to match the desired aggregation length.

- Adds a fourth histogram bucket to track invalid data
- Adjusts the algorithm to log UMA histogram w.r.t. the new fraction_poor_delay metric
- Removes UMA histogram dependency on someone else calling GetStats()
- Changed GetAecStats() to take EchoCancellation instead of AudioProcessing.

BUG=450193

Review URL: https://codereview.chromium.org/904203003

Cr-Commit-Position: refs/heads/master@{#315331}

content/renderer/media/media_stream_audio_processor.cc

@@ -418,9 +418,7 @@ void MediaStreamAudioProcessor::OnPlayoutDataSourceChanged() {
 void MediaStreamAudioProcessor::GetStats(AudioProcessorStats* stats) {
   stats->typing_noise_detected =
       (base::subtle::Acquire_Load(&typing_detected_) != false);
-  GetAecStats(audio_processing_.get(), stats);
-  if (echo_information_)
-    echo_information_.get()->UpdateAecDelayStats(stats->echo_delay_median_ms);
+  GetAecStats(audio_processing_.get()->echo_cancellation(), stats);
 }
 
 void MediaStreamAudioProcessor::InitializeAudioProcessingModule(
@@ -679,6 +677,10 @@ int MediaStreamAudioProcessor::ProcessData(const float* const* process_ptrs,
     base::subtle::Release_Store(&typing_detected_, detected);
   }
 
+  if (echo_information_) {
+    echo_information_.get()->UpdateAecDelayStats(ap->echo_cancellation());
+  }
+
   // Return 0 if the volume hasn't been changed, and otherwise the new volume.
   return (agc->stream_analog_level() == volume) ?
       0 : agc->stream_analog_level();

content/renderer/media/media_stream_audio_processor_options.cc

@@ -11,7 +11,6 @@
 #include "base/metrics/histogram.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
-#include "base/time/time.h"
 #include "content/common/media/media_stream_options.h"
 #include "content/renderer/media/media_stream_constraints_util.h"
 #include "content/renderer/media/media_stream_source.h"
@@ -82,6 +81,7 @@ enum DelayBasedEchoQuality {
   DELAY_BASED_ECHO_QUALITY_GOOD = 0,
   DELAY_BASED_ECHO_QUALITY_SPURIOUS,
   DELAY_BASED_ECHO_QUALITY_BAD,
+  DELAY_BASED_ECHO_QUALITY_INVALID,
   DELAY_BASED_ECHO_QUALITY_MAX
 };
 
@@ -94,7 +94,11 @@ DelayBasedEchoQuality EchoDelayFrequencyToQuality(float delay_frequency) {
   //   delay is out of bounds 10-80 % of the time.
   // DELAY_BASED_ECHO_QUALITY_BAD
   //   delay is mostly out of bounds >= 80 % of the time.
-  if (delay_frequency <= kEchoDelayFrequencyLowerLimit)
+  // DELAY_BASED_ECHO_QUALITY_INVALID
+  //   delay_frequency is negative which happens if we have insufficient data.
+  if (delay_frequency < 0)
+    return DELAY_BASED_ECHO_QUALITY_INVALID;
+  else if (delay_frequency <= kEchoDelayFrequencyLowerLimit)
     return DELAY_BASED_ECHO_QUALITY_GOOD;
   else if (delay_frequency < kEchoDelayFrequencyUpperLimit)
     return DELAY_BASED_ECHO_QUALITY_SPURIOUS;
@@ -214,52 +218,60 @@ bool MediaAudioConstraints::GetDefaultValueForConstraint(
 }
 
 EchoInformation::EchoInformation()
-    : echo_poor_delay_counts_(0),
-      echo_total_delay_counts_(0),
-      last_log_time_(base::TimeTicks::Now()) {}
+    : num_chunks_(0),
+      num_queries_(0),
+      echo_fraction_poor_delays_(0.0f) {}
 
 EchoInformation::~EchoInformation() {}
 
-void EchoInformation::UpdateAecDelayStats(int delay) {
-  // One way to get an indication of how well the echo cancellation performs is
-  // to compare the, by AEC, estimated delay with the AEC filter length.
-  // |kMaxAecFilterLengthMs| is the maximum delay we can allow before we
-  // consider the AEC to fail. This value should not be larger than the filter
-  // length used inside AEC. This is for now set to match the extended filter
-  // mode which is turned on for all platforms.
-  const int kMaxAecFilterLengthMs = 128;
-  if ((delay < -2) || (delay > kMaxAecFilterLengthMs)) {
-    // The |delay| is out of bounds which indicates that the echo cancellation
-    // filter can not handle the echo. Hence, we have a potential full echo
-    // case. |delay| values {-1, -2} are reserved for errors.
-    ++echo_poor_delay_counts_;
+void EchoInformation::UpdateAecDelayStats(
+    webrtc::EchoCancellation* echo_cancellation) {
+  // In WebRTC, three echo delay metrics are calculated and updated every
+  // second. We use one of them, |fraction_poor_delays|, but aggregate over
+  // five seconds to log in a UMA histogram to monitor Echo Cancellation
+  // quality. Since the stat in WebRTC has a fixed aggregation window of one
+  // second we query the stat every second and average over five such queries.
+  // WebRTC process audio in 10 ms chunks.
+  const int kNumChunksInOneSecond = 100;
+  if (!echo_cancellation->is_delay_logging_enabled() ||
+      !echo_cancellation->is_enabled()) {
+    return;
+  }
+
+  num_chunks_++;
+  if (num_chunks_ < kNumChunksInOneSecond) {
+    return;
+  }
+
+  int dummy_median = 0, dummy_std = 0;
+  float fraction_poor_delays = 0;
+  if (echo_cancellation->GetDelayMetrics(
+          &dummy_median, &dummy_std, &fraction_poor_delays) ==
+      webrtc::AudioProcessing::kNoError) {
+    echo_fraction_poor_delays_ += fraction_poor_delays;
+    num_queries_++;
+    num_chunks_ = 0;
   }
-  ++echo_total_delay_counts_;
   LogAecDelayStats();
 }
 
 void EchoInformation::LogAecDelayStats() {
   // We update the UMA statistics every 5 seconds.
-  const int kTimeBetweenLogsInSeconds = 5;
-  const base::TimeDelta time_since_last_log =
-      base::TimeTicks::Now() - last_log_time_;
-  if (time_since_last_log.InSeconds() < kTimeBetweenLogsInSeconds)
+  const int kNumQueriesIn5Seconds = 5;
+  if (num_queries_ < kNumQueriesIn5Seconds) {
     return;
+  }
 
   // Calculate how frequent the AEC delay was out of bounds since last time we
-  // updated UMA histograms. Then store the result into one of three histogram
-  // buckets; see DelayBasedEchoQuality.
-  float poor_delay_frequency = 0.f;
-  if (echo_total_delay_counts_ > 0) {
-    poor_delay_frequency = static_cast<float>(echo_poor_delay_counts_) /
-        static_cast<float>(echo_total_delay_counts_);
+  // updated UMA histograms by averaging |echo_fraction_poor_delays_| over
+  // |num_queries_|. Then store the result into one of four histogram buckets;
+  // see DelayBasedEchoQuality.
+  float poor_delay_frequency = echo_fraction_poor_delays_ / num_queries_;
   UMA_HISTOGRAM_ENUMERATION("WebRTC.AecDelayBasedQuality",
                             EchoDelayFrequencyToQuality(poor_delay_frequency),
                             DELAY_BASED_ECHO_QUALITY_MAX);
-  }
-  echo_poor_delay_counts_ = 0;
-  echo_total_delay_counts_ = 0;
-  last_log_time_ = base::TimeTicks::Now();
+  num_queries_ = 0;
+  echo_fraction_poor_delays_ = 0.0f;
 }
 
 void EnableEchoCancellation(AudioProcessing* audio_processing) {
@@ -338,14 +350,14 @@ void EnableAutomaticGainControl(AudioProcessing* audio_processing) {
   CHECK_EQ(err, 0);
 }
 
-void GetAecStats(AudioProcessing* audio_processing,
+void GetAecStats(webrtc::EchoCancellation* echo_cancellation,
                  webrtc::AudioProcessorInterface::AudioProcessorStats* stats) {
   // These values can take on valid negative values, so use the lowest possible
   // level as default rather than -1.
   stats->echo_return_loss = -100;
   stats->echo_return_loss_enhancement = -100;
 
-  // These values can also be negative, but in practice -1 is only used to
+  // The median value can also be negative, but in practice -1 is only used to
   // signal insufficient data, since the resolution is limited to multiples
   // of 4ms.
   stats->echo_delay_median_ms = -1;
@@ -354,9 +366,9 @@ void GetAecStats(AudioProcessing* audio_processing,
   // TODO(ajm): Re-enable this metric once we have a reliable implementation.
   stats->aec_quality_min = -1.0f;
 
-  if (!audio_processing->echo_cancellation()->are_metrics_enabled() ||
-      !audio_processing->echo_cancellation()->is_delay_logging_enabled() ||
-      !audio_processing->echo_cancellation()->is_enabled()) {
+  if (!echo_cancellation->are_metrics_enabled() ||
+      !echo_cancellation->is_delay_logging_enabled() ||
+      !echo_cancellation->is_enabled()) {
     return;
   }
 
@@ -364,14 +376,16 @@ void GetAecStats(AudioProcessing* audio_processing,
   // here, but it appears to be unsuitable currently. Revisit after this is
   // investigated: http://b/issue?id=5666755
   webrtc::EchoCancellation::Metrics echo_metrics;
-  if (!audio_processing->echo_cancellation()->GetMetrics(&echo_metrics)) {
+  if (!echo_cancellation->GetMetrics(&echo_metrics)) {
     stats->echo_return_loss = echo_metrics.echo_return_loss.instant;
     stats->echo_return_loss_enhancement =
         echo_metrics.echo_return_loss_enhancement.instant;
   }
 
   int median = 0, std = 0;
-  if (!audio_processing->echo_cancellation()->GetDelayMetrics(&median, &std)) {
+  float dummy = 0;
+  if (echo_cancellation->GetDelayMetrics(&median, &std, &dummy) ==
+      webrtc::AudioProcessing::kNoError) {
     stats->echo_delay_median_ms = median;
     stats->echo_delay_std_ms = std;
   }

content/renderer/media/media_stream_audio_processor_options.h

@@ -8,7 +8,6 @@
 #include <string>
 
 #include "base/files/file.h"
-#include "base/time/time.h"
 #include "content/common/content_export.h"
 #include "third_party/WebKit/public/platform/WebMediaConstraints.h"
 #include "third_party/libjingle/source/talk/app/webrtc/mediastreaminterface.h"
@@ -17,6 +16,7 @@ namespace webrtc {
 
 class AudioFrame;
 class AudioProcessing;
+class EchoCancellation;
 class MediaConstraintsInterface;
 class TypingDetection;
 
@@ -93,18 +93,17 @@ class CONTENT_EXPORT EchoInformation {
   EchoInformation();
   virtual ~EchoInformation();
 
-  // Updates delay statistics with a new |delay|.
-  void UpdateAecDelayStats(int delay);
+  void UpdateAecDelayStats(webrtc::EchoCancellation* echo_cancellation);
 
  private:
-  // Updates UMA histograms with an interval of |kTimeBetweenLogsInSeconds|.
+  // Updates UMA histograms with an interval of 5 seconds.
   void LogAecDelayStats();
 
-  // Counters for determining how often the estimated delay in the AEC is out of
-  // bounds.
-  int echo_poor_delay_counts_;
-  int echo_total_delay_counts_;
-  base::TimeTicks last_log_time_;
+  // Counters to be able to aquire a 5 second aggregated metric out of 1 second
+  // aggregated webrtc::EchoCancellation::GetEchoDelayMetrics() queries.
+  int num_chunks_;
+  int num_queries_;
+  float echo_fraction_poor_delays_;
 
   DISALLOW_COPY_AND_ASSIGN(EchoInformation);
 };
@@ -133,7 +132,7 @@ void StopEchoCancellationDump(AudioProcessing* audio_processing);
 
 void EnableAutomaticGainControl(AudioProcessing* audio_processing);
 
-void GetAecStats(AudioProcessing* audio_processing,
+void GetAecStats(webrtc::EchoCancellation* echo_cancellation,
                  webrtc::AudioProcessorInterface::AudioProcessorStats* stats);
 
 }  // namespace content

tools/metrics/histograms/histograms.xml

@@ -41913,9 +41913,9 @@ Therefore, the affected-histogram name has to have at least one dot in it.
   <summary>
     Captures if the estimated delay between rendered and captured audio is out
     of bounds which can cause the Echo Cancellation to fail. This is logged
-    roughly ones every 5 seconds, but only if there is an application that calls
-    GetStats(). The values are logged in three buckets reflecting how well the
-    Echo Cancellation likely performs based on the estimated delay.
+    roughly once every 5 seconds. The values are logged in four buckets
+    reflecting how well the Echo Cancellation likely performs based on the
+    estimated delay.
   </summary>
 </histogram>
 
@@ -44874,12 +44874,14 @@ Therefore, the affected-histogram name has to have at least one dot in it.
 </enum>
 
 <enum name="DelayBasedEchoQuality" type="int">
-  <int value="0"
-      label="Delay in bounds: Echo Cancellation quality most likely good"/>
-  <int value="1"
-      label="Delay spuriously out of bounds: Echo Cancellation may suffer"/>
-  <int value="2"
-      label="Delay out of bounds: Echo Cancellation most likely fails"/>
+  <int value="0" label="Good delays">
+    Echo Cancellation quality most likely good.
+  </int>
+  <int value="1" label="Spurious delays">Echo Cancellation may suffer.</int>
+  <int value="2" label="Poor delays">
+    Echo Cancellation likely fails unless user is using headset.
+  </int>
+  <int value="3" label="Invalid delays">Insufficient amount of data.</int>
 </enum>
 
 <enum name="DesktopCaptureCounters" type="int">