New statistics interface for APM

webrtc/modules/audio_processing/include/audio_processing.h

 /*
  *  Copyright (c) 2012 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.
  */
 
@@ skipping 451 matching lines @@
   virtual int StartDebugRecordingForPlatformFile(rtc::PlatformFile handle) = 0;
 
   // Stops recording debugging information, and closes the file. Recording
   // cannot be resumed in the same file (without overwriting it).
   virtual int StopDebugRecording() = 0;
 
   // Use to send UMA histograms at end of a call. Note that all histogram
   // specific member variables are reset.
   virtual void UpdateHistogramsOnCallEnd() = 0;
 
+  struct Statistic {
+    int instant;  // Instantaneous value.

[the sun, 2016/10/24 08:26:58]

Since you're changing the API, I think it'd be better if these were floats.
Also, I think you should make them values private and have a setter, so that we
can enforce the invariant (instant, avg should lie within min/max).

Please also default init to 0.0f.

[ivoc, 2016/10/24 14:15:49]

I'm actually trying not to change the old statistics API, since it's used in a
lot of places. So instead of changing this struct I will add a new Statistic
struct to AudioProcessingStatics that behaves like you suggest (I named it
ValueStatistic, not sure if that makes sense), and leave this one in place for
the old API for now. Once the code that uses the statistics is updated we can
get rid of the old API and this struct as well.

I made one setter for all 4 values, otherwise the order of calling the setters
would determine if the invariants hold, which is not nice.

+    int average;  // Long-term average.
+    int maximum;  // Long-term maximum.
+    int minimum;  // Long-term minimum.
+  };
+  struct AudioProcessingStatistics {
+    // AEC Statistics.
+    // RERL = ERL + ERLE
+    Statistic residual_echo_return_loss = {0, 0, 0, 0};

[the sun, 2016/10/24 08:26:58]

With default init, no need for this (error-prone) manual initing.

[ivoc, 2016/10/24 14:15:49]

Good idea.

+    // ERL = 10log_10(P_far / P_echo)
+    Statistic echo_return_loss = {0, 0, 0, 0};
+    // ERLE = 10log_10(P_echo / P_out)
+    Statistic echo_return_loss_enhancement = {0, 0, 0, 0};
+    // (Pre non-linear processing suppression) A_NLP = 10log_10(P_echo / P_a)
+    Statistic a_nlp = {0, 0, 0, 0};
+    // Fraction of time that the AEC linear filter is divergent, in a 1-second
+    // non-overlapped aggregation window.
+    float divergent_filter_fraction = 0.0f;
+
+    // The delay metrics consists of the delay median and standard deviation. It
+    // also consists of the fraction of delay estimates that can make the echo
+    // cancellation perform poorly. The values are aggregated until the first
+    // call to |GetStatistics()| and afterwards aggregated and updated every
+    // second. Note that if there are several clients pulling metrics from
+    // |GetStatistics()| during a session the first call from any of them will
+    // change to one second aggregation window for all.
+    int delay_median = 0;
+    int delay_standard_deviation = 0;
+    float fraction_poor_delays = 0.0f;
+
+    // Residual echo detector likelihood. This value is not yet calculated and
+    // is currently always set to zero.
+    // TODO(ivoc): Implement this stat.
+    float residual_echo_likelihood = 0.0f;
+  };
+
+  virtual void SetStatisticsEnabled(bool enabled) = 0;

[peah-webrtc, 2016/10/21 07:02:45]

Do we need to have an enable statistics method? Can't we just always compute
statistics? They should anyway not be anything computationally complex.

[the sun, 2016/10/24 08:26:58]

Agree, but are you sure they're not expensive?

[peah-webrtc, 2016/10/24 09:44:38]

Almost, yes, in particular since 
-The AEC metrics are not run on mobile platforms and their complexity should be
negligible to the complexity of the AEC.
-The residual echo detector metrics are only run when the residual echo detector
is on, in which case they are obtained for free.

The question is, however a bit bigger than that I think since it also applies on
future metrics, will they be too complex? For a metric, I think we should be
able to rely on it being extremely lightweight.

Does that make sense? 

[ivoc, 2016/10/24 14:15:49]

Ok, I removed this function.

+  virtual void GetStatistics(AudioProcessingStatistics* stats) const = 0;
+
   // These provide access to the component interfaces and should never return
   // NULL. The pointers will be valid for the lifetime of the APM instance.
   // The memory for these objects is entirely managed internally.
   virtual EchoCancellation* echo_cancellation() const = 0;
   virtual EchoControlMobile* echo_control_mobile() const = 0;
   virtual GainControl* gain_control() const = 0;
   virtual HighPassFilter* high_pass_filter() const = 0;
   virtual LevelEstimator* level_estimator() const = 0;
   virtual NoiseSuppression* noise_suppression() const = 0;
   virtual VoiceDetection* voice_detection() const = 0;
 
-  struct Statistic {
-    int instant;  // Instantaneous value.
-    int average;  // Long-term average.
-    int maximum;  // Long-term maximum.
-    int minimum;  // Long-term minimum.
-  };
-
   enum Error {
     // Fatal errors.
     kNoError = 0,
     kUnspecifiedError = -1,
     kCreationFailedError = -2,
     kUnsupportedComponentError = -3,
     kUnsupportedFunctionError = -4,
     kNullPointerError = -5,
     kBadParameterError = -6,
     kBadSampleRateError = -7,
@@ skipping 201 matching lines @@
     AudioProcessing::Statistic echo_return_loss_enhancement;
 
     // (Pre non-linear processing suppression) A_NLP = 10log_10(P_echo / P_a)
     AudioProcessing::Statistic a_nlp;
 
     // Fraction of time that the AEC linear filter is divergent, in a 1-second
     // non-overlapped aggregation window.
     float divergent_filter_fraction;
   };
 
+  // Deprecated. Use GetStatistics on the AudioProcessing interface instead.
   // TODO(ajm): discuss the metrics update period.
   virtual int GetMetrics(Metrics* metrics) = 0;
 
   // Enables computation and logging of delay values. Statistics are obtained
   // through |GetDelayMetrics()|.
   virtual int enable_delay_logging(bool enable) = 0;
   virtual bool is_delay_logging_enabled() const = 0;
 
   // The delay metrics consists of the delay |median| and the delay standard
   // deviation |std|. It also consists of the fraction of delay estimates
   // |fraction_poor_delays| that can make the echo cancellation perform poorly.
   // The values are aggregated until the first call to |GetDelayMetrics()| and
   // afterwards aggregated and updated every second.
   // Note that if there are several clients pulling metrics from
   // |GetDelayMetrics()| during a session the first call from any of them will
   // change to one second aggregation window for all.
-  // TODO(bjornv): Deprecated, remove.
+  // Deprecated. Use GetStatistics on the AudioProcessing interface instead.
   virtual int GetDelayMetrics(int* median, int* std) = 0;
+  // Deprecated. Use GetStatistics on the AudioProcessing interface instead.
   virtual int GetDelayMetrics(int* median, int* std,
                               float* fraction_poor_delays) = 0;
 
   // Returns a pointer to the low level AEC component.  In case of multiple
   // channels, the pointer to the first one is returned.  A NULL pointer is
   // returned when the AEC component is disabled or has not been initialized
   // successfully.
   virtual struct AecCore* aec_core() const = 0;
 
  protected:
@@ skipping 259 matching lines @@
   // This does not impact the size of frames passed to |ProcessStream()|.
   virtual int set_frame_size_ms(int size) = 0;
   virtual int frame_size_ms() const = 0;
 
  protected:
   virtual ~VoiceDetection() {}
 };
 }  // namespace webrtc
 
 #endif  // WEBRTC_MODULES_AUDIO_PROCESSING_INCLUDE_AUDIO_PROCESSING_H_