Refactor RMSLevel and give it new functionality

webrtc/modules/audio_processing/rms_level.h

Index: webrtc/modules/audio_processing/rms_level.h
diff --git a/webrtc/modules/audio_processing/rms_level.h b/webrtc/modules/audio_processing/rms_level.h
index 12fa2125f08acdd3820971dddbcb370f3d25cdfa..0286268185b539fe6cca7d3890840c6bea60b690 100644
--- a/webrtc/modules/audio_processing/rms_level.h
+++ b/webrtc/modules/audio_processing/rms_level.h
@@ -11,8 +11,7 @@
 #ifndef WEBRTC_MODULES_AUDIO_PROCESSING_RMS_LEVEL_H_
 #define WEBRTC_MODULES_AUDIO_PROCESSING_RMS_LEVEL_H_
 
-#include <cstddef>
-
+#include "webrtc/base/array_view.h"
 #include "webrtc/typedefs.h"
 
 namespace webrtc {
@@ -27,7 +26,10 @@ namespace webrtc {
 // RMS() to get the audio level indicator for the RTP header.
 class RMSLevel {
  public:
-  static const int kMinLevel = 127;
+  struct Levels {
+    int average;
+    int peak;
+  };
 
   RMSLevel();
   ~RMSLevel();
@@ -37,7 +39,7 @@ class RMSLevel {
   void Reset();
 
   // Pass each chunk of audio to Process() to accumulate the level.
-  void Process(const int16_t* data, size_t length);
+  void Process(rtc::ArrayView<const int16_t> data);

[peah-webrtc, 2016/11/29 08:57:13]

While you are anyway changing this: Would it make sense to change Process to
Analyze?

Elsewhere inside APM the convention seems to be that if a method changes the
signal, it is called Process, otherwise it is called Analyze.

[hlundin-webrtc, 2016/11/29 10:24:55]

Done.

 
   // If all samples with the given |length| have a magnitude of zero, this is
   // a shortcut to avoid some computation.
@@ -45,12 +47,18 @@ class RMSLevel {
 
   // Computes the RMS level over all data passed to Process() since the last
   // call to RMS(). The returned value is positive but should be interpreted as
-  // negative as per the RFC. It is constrained to [0, 127].
+  // negative as per the RFC. It is constrained to [0, 127]. Resets the internal
+  // state to start a new measurement period.
   int RMS();

[peah-webrtc, 2016/11/29 08:57:13]

Suggestion: I think it would make sense to rename this method while you are at
it.

The class is called RMSLevel and having a method called RMS is then kind of
double bookeeping.

For instance, I envision a typical usage being as
{
RMSLevel rms_level_;

...
rms_value = rms_level_.RMS();
}

To me it looks better if it instead would be
{
RMSLevel rms_level_;

...
rms_value = rms_level_.Average();
}

or 

{
RMSLevel rms_level_;

...
rms_value = rms_level_.Value();
}


This is even more the case when you add the AverageAndPeak() method.

[hlundin-webrtc, 2016/11/29 10:24:55]

Done.

 
+  // Like RMS() above, but also returns the RMS peak value. Resets the internal
+  // state to start a new measurement period.
+  Levels AverageAndPeak();
+
  private:
   float sum_square_;
   size_t sample_count_;
+  float max_mean_square_;
 };
 
 }  // namespace webrtc