Added functionality for specifying the initial signal level to use for the gain estimation in the l…

webrtc/modules/audio_processing/level_controller/peak_level_estimator.cc

Index: webrtc/modules/audio_processing/level_controller/peak_level_estimator.cc
diff --git a/webrtc/modules/audio_processing/level_controller/peak_level_estimator.cc b/webrtc/modules/audio_processing/level_controller/peak_level_estimator.cc
index 2ba806c8ee0d15b58c366421aaa1e2b5c20d635b..9abc8c4c700a62c939c07469e0f43a494ff8d36b 100644
--- a/webrtc/modules/audio_processing/level_controller/peak_level_estimator.cc
+++ b/webrtc/modules/audio_processing/level_controller/peak_level_estimator.cc
@@ -13,10 +13,15 @@
 #include <algorithm>
 
 #include "webrtc/modules/audio_processing/audio_buffer.h"
-#include "webrtc/modules/audio_processing/level_controller/lc_constants.h"
 #include "webrtc/modules/audio_processing/logging/apm_data_dumper.h"
 
 namespace webrtc {
+namespace {
+
+const float kMinLevel = 30.f;
+const float kMaxLevel = 32768.f;
+
+}  // namespace
 
 PeakLevelEstimator::PeakLevelEstimator() {
   Initialize();
@@ -25,11 +30,20 @@ PeakLevelEstimator::PeakLevelEstimator() {
 PeakLevelEstimator::~PeakLevelEstimator() {}
 
 void PeakLevelEstimator::Initialize() {
-  peak_level_ = kTargetLcPeakLevel;
+  peak_level_ = initial_peak_level_;
   hold_counter_ = 0;
   initialization_phase_ = true;
 }
 
+void PeakLevelEstimator::SetInitialPeakLevel(float level) {
+  RTC_DCHECK_LE(0.f, level);
+  RTC_DCHECK_GE(32768.f, level);

[the sun, 2016/08/22 11:58:22]

Why not use the constants here?

[peah-webrtc, 2016/08/22 12:09:04]

That is a good question:

The thinking was like this:
-I don't want to complicate that APM API by imposing a complicated level
requirement, so therefore I basically chose the full range.
-If the internal range, e.g., currently 30.0f-32768.0f would change (for reasons
of adjusting the performance, it is nice not to have to change the API because
of that for that.
-If I DCHECK on the internal range, that would cause valid input data (according
to the API description) to cause DCHECKS, which does not make sense.

[the sun, 2016/08/22 14:18:39]

Ok that makes sense, but when I think about it, 0 dBFS in is usually 1.0 in the
floating point world, so from that perspective, the limits here appear a bit
random.
I think you need to explain that to me in person. :)
Agree.

[peah-webrtc, 2016/08/23 05:33:44]

You are right! Excellent point. 

I changed the API to instead receive the level in dBFS with a specified range of
[-100, 0]. What do you think of that solution?

[the sun, 2016/08/23 06:23:38]

Nice, I would have gone for range 0-1.0 myself, but this is even better. Unless,
of course, if this method is called often, which I guess it wouldn't be?

[peah-webrtc, 2016/08/26 10:47:13]

That is correct, this is not something that will happen often.

+
+  // Limit the supplied level to the level range used internally.
+  initial_peak_level_ = std::min(std::max(level, kMinLevel), kMaxLevel);

[the sun, 2016/08/22 11:58:22]

Do you really need to DCHECK as well as clamp?

[peah-webrtc, 2016/08/22 12:09:04]

That is a good question. Please see above response for the answer.

[the sun, 2016/08/22 14:18:39]

Right. I think generally APIs should aggressively assert the specified contract,
as you say. In this case you should only need the std::max() to limit the range
though.

[peah-webrtc, 2016/08/23 05:33:44]

True, I removed the std::min.

Done.

+  Initialize();
+}
+
 float PeakLevelEstimator::Analyze(SignalClassifier::SignalType signal_type,
                                   float frame_peak_level) {
   if (frame_peak_level == 0) {