Use the new interface to get APM stats

content/renderer/media/media_stream_audio_processor_options.cc

Index: content/renderer/media/media_stream_audio_processor_options.cc
diff --git a/content/renderer/media/media_stream_audio_processor_options.cc b/content/renderer/media/media_stream_audio_processor_options.cc
index 421643a94febaf95c4e6dfcbc66504a77636d69c..ab7ad8cb9c0099ab82d00b20417274570fa7f0b3 100644
--- a/content/renderer/media/media_stream_audio_processor_options.cc
+++ b/content/renderer/media/media_stream_audio_processor_options.cc
@@ -480,47 +480,19 @@ void EnableAutomaticGainControl(AudioProcessing* audio_processing) {
   CHECK_EQ(err, 0);
 }
 
-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;
-
-  // 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;
-  stats->echo_delay_std_ms = -1;
-
-  // TODO(ajm): Re-enable this metric once we have a reliable implementation.
-  stats->aec_quality_min = -1.0f;

[hbos_chromium, 2016/11/25 10:04:05]

I think that defaulting values to negative, whether here or in [1], is
problematic. If webrtc::AudioProcessorInterface::AudioProcessorStats's stats can
be optional, we should change them to rtc::Optional.

The old stats collector (statscollector.h) just copies values blindly. The new
stats collector (rtcstatscollector.h) supports leaving stats values undefined.
But to do so it needs to know if a value is undefined, and Optional is better
than some arbitrary/undocumented negative value.

Could you do a follow-up WebRTC CL that indicate that optional statistics are
optional? (Might be messy rolling into chromium)

Alternatively, we could make "void MediaStreamAudioProcessor::GetStats"
(overriding webrtc::AudioProcessor::GetStats) return false if stats are not
available, if it's an all-or-nothing OP. Is it?

Or, if undefined values are unexpected, DCHECK.

[1]
https://cs.chromium.org/chromium/src/third_party/webrtc/modules/audio_process...

[Ivo-OOO until feb 6, 2016/11/25 10:40:44]

I agree that this way of using default values is not very nice, and using
rtc::Optional sounds much better. I can do that in a follow-up CL (not sure if I
have time for it right away). 

The reason the stats can be undefined is that the AEC can be disabled, so in
that case an Optional is indeed a much nicer way to signal that.

[hbos_chromium, 2016/11/25 12:51:39]

That's fine, add a TODO comment? :)

[Ivo-OOO until feb 6, 2016/11/25 13:22:19]

Done.

-
-  if (!echo_cancellation->are_metrics_enabled() ||
-      !echo_cancellation->is_delay_logging_enabled() ||
-      !echo_cancellation->is_enabled()) {
-    return;
-  }
-
-  // TODO(ajm): we may want to use VoECallReport::GetEchoMetricsSummary
-  // here, but it appears to be unsuitable currently. Revisit after this is
-  // investigated: http://b/issue?id=5666755
-  webrtc::EchoCancellation::Metrics 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;
-    stats->aec_divergent_filter_fraction =
-        echo_metrics.divergent_filter_fraction;
-  }
-
-  int median = 0, std = 0;
-  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;
-  }
+void GetAudioProcessingStats(
+    AudioProcessing* audio_processing,
+    webrtc::AudioProcessorInterface::AudioProcessorStats* stats) {
+  auto apm_stats = audio_processing->GetStatistics();
+  stats->echo_return_loss = apm_stats.echo_return_loss.instant();
+  stats->echo_return_loss_enhancement =
+      apm_stats.echo_return_loss_enhancement.instant();

[hbos_chromium, 2016/11/25 10:04:05]

The new stats collector is only concerned with echoReturnLoss and
echoReturnLossEnhancement out of all audio statistics:
https://w3c.github.io/webrtc-stats/#dom-rtcmediastreamtrackstats-echoreturnloss

These are calculated in decibels. Are these values using the same units?

[Ivo-OOO until feb 6, 2016/11/25 10:40:44]

These are in exactly the same units, in fact the
audio_processing->GetStatistics() function calls the old functions internally. 

The reason for this change is that we want to get rid of direct pointers to the
AEC outside of APM, since it makes the APM/AEC locking scheme more complicated
than it needs to be.

[hbos_chromium, 2016/11/25 12:51:39]

Acknowledged.

+  stats->aec_divergent_filter_fraction = apm_stats.divergent_filter_fraction;
+
+  stats->echo_delay_median_ms = apm_stats.delay_median;
+  stats->echo_delay_std_ms = apm_stats.delay_standard_deviation;
+
+  stats->residual_echo_likelihood = apm_stats.residual_echo_likelihood;
 }
 
 std::vector<webrtc::Point> GetArrayGeometryPreferringConstraints(