Fix normalization of noise estimate in NoiseSuppressor

webrtc/modules/audio_processing/noise_suppression_impl.cc

 /*
  *  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 159 matching lines @@
   // TODO(peah): Returning error code as a float! Remove this.
   // Currently not available for the fixed point implementation.
   return AudioProcessing::kUnsupportedFunctionError;
 #endif
 }
 
 std::vector<float> NoiseSuppressionImpl::NoiseEstimate() {
   rtc::CritScope cs(crit_);
   std::vector<float> noise_estimate;
 #if defined(WEBRTC_NS_FLOAT)
-  const float kNormalizationFactor = 1.f / (1 << 15);
   noise_estimate.assign(WebRtcNs_num_freq(), 0.f);
   for (auto& suppressor : suppressors_) {
     const float* noise = WebRtcNs_noise_estimate(suppressor->state());
     for (size_t i = 0; i < noise_estimate.size(); ++i) {
-      noise_estimate[i] +=
-          kNormalizationFactor * noise[i] / suppressors_.size();
+      noise_estimate[i] += noise[i] / suppressors_.size();

[peah-webrtc, 2016/03/21 12:25:48]

This change basically removes the normalization of the noise, right? Is that
because it is no longer needed and instead done elsewhere? I guess that for the
IE functionality to work properly, there should be a corresponding change in IE,
but I cannot see that in this CL.

[turaj, 2016/03/21 13:50:23]

My concern is that both forward and reverse streams to be in the same range, not
one in int16 and the other one in [-1, 1] range. But I don't know how to enforce
it.

[aluebs-webrtc, 2016/03/29 23:15:16]

Exactly, I was testing it in the intelligibility_proc, which had one
normalization to much (removed on this CL) compared to the APM, so the results
were different. So for the IE to work properly once the normalization in both
tests were the same, this had to be corrected here. The good thing is that the
APM always works with int16 range in both directions.

     }
   }
 #elif defined(WEBRTC_NS_FIXED)
-  const float kNormalizationFactor = 1.f / (1 << 23);
+  const float kNormalizationFactor = 1.f / (1 << 9);

[hlundin-webrtc, 2016/03/29 19:19:36]

Why change from 23 to 9 (a reduction by 14 shifts)? I would have guessed a
reduction by 15 to match the deleted normalization in the float path.

[aluebs-webrtc, 2016/03/29 23:15:16]

Good catch! I was just checking when the output was more similar in the float
and fixed cases. But digging some deeper, I realized that the domain is not
fixed between chunks (although it tends to stabilize towards a value). So now I
fixed the problem by normalizing it dynamically inside the noise_suppressor_x
and not having this module need to know about it. 

   noise_estimate.assign(WebRtcNsx_num_freq(), 0.f);
   for (auto& suppressor : suppressors_) {
     const uint32_t* noise = WebRtcNsx_noise_estimate(suppressor->state());
     for (size_t i = 0; i < noise_estimate.size(); ++i) {
       noise_estimate[i] += kNormalizationFactor *
           static_cast<float>(noise[i]) / suppressors_.size();
     }
   }
 #endif
   return noise_estimate;
 }
 
 size_t NoiseSuppressionImpl::num_noise_bins() {
 #if defined(WEBRTC_NS_FLOAT)
   return WebRtcNs_num_freq();
 #elif defined(WEBRTC_NS_FIXED)
   return WebRtcNsx_num_freq();
 #endif
 }
 
 }  // namespace webrtc