Added a bitexactness test for the intelligibility enhancer in the audio processing module

webrtc/modules/audio_processing/intelligibility_enhancer_unittest.cc

+/*
+ *  Copyright (c) 2016 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.
+ */
+#include <vector>
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "webrtc/base/array_view.h"
+#include "webrtc/modules/audio_processing/audio_buffer.h"
+#include "webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.h"
+#include "webrtc/modules/audio_processing/noise_suppression_impl.h"
+#include "webrtc/modules/audio_processing/test/audio_buffer_tools.h"
+#include "webrtc/modules/audio_processing/test/bitexactness_tools.h"
+
+namespace webrtc {
+namespace {
+
+const int kNumFramesToProcess = 1000;

[aluebs-webrtc, 2016/03/18 23:26:40]

Should probably be a size_t.

[peah-webrtc, 2016/03/21 11:54:50]

Done.

+
+// Process one frame of data and produce the output.
+void ProcessOneFrame(int sample_rate_hz,

[aluebs-webrtc, 2016/03/18 23:26:41]

You don't need to pass in the sample_rate_hz separately, you can get it from the
AudioBuffer.

[peah-webrtc, 2016/03/21 11:54:49]

I cannot see that the AudioBuffer returns the sample rate in any way. Could you
please help me find the function where that is done?

[aluebs-webrtc, 2016/03/22 11:53:29]

You are right, my bad. 

[peah-webrtc, 2016/03/23 22:09:34]

Acknowledged.

+                     AudioBuffer* render_audio_buffer,
+                     AudioBuffer* capture_audio_buffer,
+                     NoiseSuppressionImpl* noise_suppressor,
+                     IntelligibilityEnhancer* intelligibility_enhancer) {
+  if (sample_rate_hz > AudioProcessing::kSampleRate16kHz) {
+    render_audio_buffer->SplitIntoFrequencyBands();
+    capture_audio_buffer->SplitIntoFrequencyBands();
+  }
+
+  noise_suppressor->AnalyzeCaptureAudio(capture_audio_buffer);
+  noise_suppressor->ProcessCaptureAudio(capture_audio_buffer);
+
+  intelligibility_enhancer->ProcessRenderAudio(
+      render_audio_buffer->split_channels_f(kBand0To8kHz),
+      sample_rate_hz > 16000 ? 16000 : sample_rate_hz,

[aluebs-webrtc, 2016/03/18 23:26:41]

Use AudioProcessing::kSampleRate16kHz instead of 16000, or define your own
constant. Also, I would probably make the split_rate calculation in a separate
function with a clear name, specially since it is used more than once.

[peah-webrtc, 2016/03/21 11:54:50]

Done.

+      render_audio_buffer->num_channels());
+
+  intelligibility_enhancer->SetCaptureNoiseEstimate(

[aluebs-webrtc, 2016/03/18 23:26:40]

Just to mimic the APM, you should probably put this together with the
noise_suppressor calls in the "capture side".

[peah-webrtc, 2016/03/21 11:54:50]

Good point!
Done.

+      noise_suppressor->NoiseEstimate());
+
+  if (sample_rate_hz > AudioProcessing::kSampleRate16kHz) {
+    render_audio_buffer->MergeFrequencyBands();
+  }
+}
+
+// Processes a specified amount of frames, verifies the results and reports
+// any errors.
+void RunBitexactnessTest(int sample_rate_hz,
+                         size_t num_channels,
+                         const rtc::ArrayView<const float>& output_reference) {

[aluebs-webrtc, 2016/03/18 23:26:40]

In the ArrayView documentation it says: "ArrayView is tiny (just a pointer and a
count) and trivially copyable, so it's probably cheaper to pass it by value than
by const reference".

[peah-webrtc, 2016/03/21 11:54:51]

Done.

+  int samples_per_channel = rtc::CheckedDivExact(sample_rate_hz, 100);

[hlundin-webrtc, 2016/03/18 08:47:43]

const

[aluebs-webrtc, 2016/03/18 23:26:41]

You don't need to calculate this explicitly, you can use
render_config.num_frames() or even the render_buffer.num_frames().

[peah-webrtc, 2016/03/21 11:54:50]

Done.

[peah-webrtc, 2016/03/21 11:54:50]

Done.

+  const StreamConfig render_config(sample_rate_hz, num_channels, false);
+  AudioBuffer render_buffer(
+      render_config.num_frames(), render_config.num_channels(),
+      render_config.num_frames(), render_config.num_channels(),
+      render_config.num_frames());
+  test::InputAudioFile render_file(
+      test::GetApmRenderTestVectorFileName(sample_rate_hz));
+  std::vector<float> render_input(samples_per_channel * num_channels);

[aluebs-webrtc, 2016/03/18 23:26:40]

Use render_config.num_frames() or even the render_buffer.num_frames() instead of
samples_per_channel.

[peah-webrtc, 2016/03/21 11:54:49]

Done.

+
+  const StreamConfig capture_config(sample_rate_hz, num_channels, false);
+  AudioBuffer capture_buffer(
+      capture_config.num_frames(), capture_config.num_channels(),
+      capture_config.num_frames(), 1, capture_config.num_frames());

[aluebs-webrtc, 2016/03/18 23:26:41]

Why do you need to hard-code to mono? Also, it would be nice to have a constant
for it.

[peah-webrtc, 2016/03/21 11:54:51]

That is a mistake done caused by mistakenly only creating 1 noise suppressor.

Done.

+  test::InputAudioFile capture_file(
+      test::GetApmCaptureTestVectorFileName(sample_rate_hz));
+  std::vector<float> capture_input(samples_per_channel * num_channels);

[aluebs-webrtc, 2016/03/18 23:26:40]

Use render_config.num_frames() or even the render_buffer.num_frames() instead of
samples_per_channel.

[peah-webrtc, 2016/03/21 11:54:51]

Done.

+
+  rtc::CriticalSection crit_render;

[aluebs-webrtc, 2016/03/18 23:26:41]

Not used.

[peah-webrtc, 2016/03/21 11:54:49]

Done.

+  rtc::CriticalSection crit_capture;
+  NoiseSuppressionImpl noise_suppressor(&crit_capture);
+  noise_suppressor.Initialize(1, sample_rate_hz);

[aluebs-webrtc, 2016/03/18 23:26:41]

Why do you need to hard-code to mono? Also, it would be nice to have a constant
for it.

[peah-webrtc, 2016/03/21 11:54:51]

Done.

+  noise_suppressor.Enable(true);
+
+  IntelligibilityEnhancer intelligibility_enhancer(
+      sample_rate_hz > 16000 ? 16000 : sample_rate_hz,

[aluebs-webrtc, 2016/03/18 23:26:40]

Use AudioProcessing::kSampleRate16kHz instead of 16000, or define your own
constant. Also, I would probably make the split_rate calculation in a separate
function with a clear name, specially since it is used more than once.

[peah-webrtc, 2016/03/21 11:54:50]

Done.

+      render_config.num_channels(), NoiseSuppressionImpl::num_noise_bins());
+
+  for (int frame_no = 0; frame_no < kNumFramesToProcess; ++frame_no) {

[aluebs-webrtc, 2016/03/18 23:26:41]

This should probably be size_t.

[peah-webrtc, 2016/03/21 11:54:49]

Done.

+    ReadFloatSamplesFromStereoFile(samples_per_channel, num_channels,

[aluebs-webrtc, 2016/03/18 23:26:40]

Where does this function come from? Also, use render_config.num_frames() or even
the render_buffer.num_frames() instead of samples_per_channel.

[peah-webrtc, 2016/03/21 11:54:50]

The function is declared in
webrtc/modules/audio_processing/test/bitexactness_tools.h.

[aluebs-webrtc, 2016/03/22 11:53:28]

Thanks for clarifying. I don't know why my grepping didn't find it originally.

[peah-webrtc, 2016/03/23 22:13:53]

Done.

+                                   &render_file, render_input);
+    ReadFloatSamplesFromStereoFile(samples_per_channel, num_channels,

[aluebs-webrtc, 2016/03/18 23:26:40]

Where does this function come from? Also, use capture_config.num_frames() or
even the capture_buffer.num_frames() instead of samples_per_channel.

[peah-webrtc, 2016/03/21 11:54:50]

The function is declared in
webrtc/modules/audio_processing/test/bitexactness_tools.h.

Done.

[aluebs-webrtc, 2016/03/22 11:53:29]

Thanks for clarifying. I don't know why my grepping didn't find it originally.

[peah-webrtc, 2016/03/23 22:09:34]

Acknowledged.

+                                   &capture_file, capture_input);
+
+    test::CopyVectorToAudioBuffer(render_config, render_input, &render_buffer);
+    test::CopyVectorToAudioBuffer(capture_config, capture_input,
+                                  &capture_buffer);
+
+    ProcessOneFrame(sample_rate_hz, &render_buffer, &capture_buffer,
+                    &noise_suppressor, &intelligibility_enhancer);
+  }
+
+  // Extract and verify the test results.
+  std::vector<float> render_output;
+  test::ExtractVectorFromAudioBuffer(render_config, &render_buffer,
+                                     &render_output);
+
+  const float kTolerance = 1.0f / 32768.0f;

[aluebs-webrtc, 2016/03/18 23:26:40]

The 0s after the dot are not necessary. There is probably something in
numerical_limits you can use instead of this. Else you can at least use some
shifting to make clear it is the 1 << 15.

[peah-webrtc, 2016/03/21 11:54:50]

Great point!
Done.

+
+  // Compare the output with the reference. Only the first values of the output
+  // from last frame processed are compared in order not having to specify all
+  // preceeding frames as testvectors. As the algorithm being tested has a
+  // memory, testing only the last frame implicitly also tests the preceeding
+  // frames.
+  EXPECT_TRUE(test::BitExactFrame(render_config.num_frames(),

[aluebs-webrtc, 2016/03/18 23:26:40]

If it receives a tolerance it is not a bit-exact test. I think this should be
renamed to something that reflects the real test that is done.

[peah-webrtc, 2016/03/21 11:54:49]

Good point! 
What about VectorDifferenceBounded and that kTolerance is renamed to
kVectorItemBound?
I'll wait with that change until we have agreed on a name as that will affect
other files as well. It should for sure be part of this CL but I don't want to
create a patch for it until we have a good name.

[aluebs-webrtc, 2016/03/22 11:53:29]

I think your suggestion sounds much more intuitive of what it actually does. But
if it touches many other files, it might be a better idea to do it in another
CL.

[peah-webrtc, 2016/03/23 22:09:34]

Great! Then I'll do that in another CL.
Done.

[aluebs-webrtc, 2016/03/24 11:18:40]

Acknowledged.

+                                  render_config.num_channels(),
+                                  output_reference, render_output, kTolerance));
+}
+
+}  // namespace
+
+TEST(IntelligibilityEnhancerBitExactnessTest, Mono8kHz) {

[aluebs-webrtc, 2016/03/18 23:26:41]

This test would be a lot simpler if we use TEST_P and INSTANTIATE_TEST_CASE_P.

[peah-webrtc, 2016/03/21 11:54:49]

Very likely, but I could not come up with a good way to in an explicit manner
combine the testvectors with the input parameters in INSTANTIATE_TEST_CASE_P. Do
you have any suggestion on how to achieve that?

[aluebs-webrtc, 2016/03/22 11:53:29]

How about having a method that returns a reference vector from a sample rate and
a number of channels? In that way you can only instantiate an the tests using
these 2 parameters and then get the reference from them.

[peah-webrtc, 2016/03/23 22:09:34]

As I wrote in the CL for the bitexactness test for the beamformer I think that
is a good suggestion and most likely it would produce more
compact code. One drawback is, however, that it is not possible to disable
tests. The way the tests are set up in this test is how the reviewers of the
first of the submodule bitexactness tests preferred it to be. And even though it
is a bit verbose, I like that it is very explicit. 

[aluebs-webrtc, 2016/03/24 11:18:40]

As I wrote in the other CL, TEST_P would be exactly as verbose, since you need
to instantiate each test case manually, but it avoids repeated code. And to
disable tests, you can always comment out that specific instance. Did the
original reviewers evaluate TEST_P, because they might prefer this approach and
then it would be great to port all tests to this one.

+  const float kOutputReference[] = {-0.001892f, -0.003296f, -0.001953f};
+
+  RunBitexactnessTest(8000, 1, kOutputReference);

[aluebs-webrtc, 2016/03/18 23:26:41]

Use AudioProcessing::kSampleRate8kHz and define your own constant for mono.

[peah-webrtc, 2016/03/21 11:54:50]

Done.

+}
+
+TEST(IntelligibilityEnhancerBitExactnessTest, Mono16kHz) {
+  const float kOutputReference[] = {-0.000977f, -0.003296f, -0.002441f};
+
+  RunBitexactnessTest(16000, 1, kOutputReference);

[aluebs-webrtc, 2016/03/18 23:26:41]

Use AudioProcessing::kSampleRate16kHz and define your own constant for mono.

[peah-webrtc, 2016/03/21 11:54:49]

Done.

+}
+
+TEST(IntelligibilityEnhancerBitExactnessTest, Mono32kHz) {
+  const float kOutputReference[] = {0.003021f, -0.011780f, -0.008209f};
+
+  RunBitexactnessTest(32000, 1, kOutputReference);

[aluebs-webrtc, 2016/03/18 23:26:41]

Use AudioProcessing::kSampleRate32kHz and define your own constant for mono.

[peah-webrtc, 2016/03/21 11:54:50]

Done.

+}
+
+TEST(IntelligibilityEnhancerBitExactnessTest, Mono48kHz) {
+  const float kOutputReference[] = {-0.027696f, -0.026253f, -0.018001f};
+
+  RunBitexactnessTest(48000, 1, kOutputReference);

[aluebs-webrtc, 2016/03/18 23:26:41]

Use AudioProcessing::kSampleRate48kHz and define your own constant for mono.

[peah-webrtc, 2016/03/21 11:54:49]

Done.

+}
+
+TEST(IntelligibilityEnhancerBitExactnessTest, Stereo8kHz) {
+  const float kOutputReference[] = {0.021454f,  0.035919f, 0.026428f,
+                                    -0.000641f, 0.000366f, 0.000641f};
+
+  RunBitexactnessTest(8000, 2, kOutputReference);

[aluebs-webrtc, 2016/03/18 23:26:40]

Use AudioProcessing::kSampleRate8kHz and define your own constant for stereo.

[peah-webrtc, 2016/03/21 11:54:50]

Done.

+}
+
+TEST(IntelligibilityEnhancerBitExactnessTest, Stereo16kHz) {
+  const float kOutputReference[] = {0.021362f,  0.035736f,  0.023895f,
+                                    -0.001404f, -0.001465f, 0.000549f};
+
+  RunBitexactnessTest(16000, 2, kOutputReference);

[aluebs-webrtc, 2016/03/18 23:26:41]

Use AudioProcessing::kSampleRate16kHz and define your own constant for stereo.

[peah-webrtc, 2016/03/21 11:54:49]

Done.

+}
+
+TEST(IntelligibilityEnhancerBitExactnessTest, Stereo32kHz) {
+  const float kOutputReference[] = {0.030641f,  0.027406f,  0.028321f,
+                                    -0.001343f, -0.004578f, 0.000977f};
+
+  RunBitexactnessTest(32000, 2, kOutputReference);

[aluebs-webrtc, 2016/03/18 23:26:41]

Use AudioProcessing::kSampleRate32kHz and define your own constant for stereo.

[peah-webrtc, 2016/03/21 11:54:50]

Done.

+}
+
+TEST(IntelligibilityEnhancerBitExactnessTest, Stereo48kHz) {
+  const float kOutputReference[] = {-0.009276f, -0.001601f, -0.008255f,
+                                    -0.012975f, -0.015940f, -0.017820f};
+
+  RunBitexactnessTest(48000, 2, kOutputReference);

[aluebs-webrtc, 2016/03/18 23:26:41]

Use AudioProcessing::kSampleRate32kHz and define your own constant for stereo.

[peah-webrtc, 2016/03/21 11:54:50]

Done.

+}
+
+}  // namespace webrtc