Added a bitexactness test for the beamformer in the audio processing module

webrtc/modules/audio_processing/beamformer_unittest.cc

Index: webrtc/modules/audio_processing/beamformer_unittest.cc
diff --git a/webrtc/modules/audio_processing/beamformer_unittest.cc b/webrtc/modules/audio_processing/beamformer_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..fa525918af0e7f402ef543670c87da1ba2000443
--- /dev/null
+++ b/webrtc/modules/audio_processing/beamformer_unittest.cc
@@ -0,0 +1,249 @@
+/*
+ *  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/beamformer/nonlinear_beamformer.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 22:51:51]

Should probably be a size_t.

[peah-webrtc, 2016/03/21 08:02:07]

Done.

+
+void ProcessOneFrame(int sample_rate_hz,

[aluebs-webrtc, 2016/03/18 22:51:52]

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

[peah-webrtc, 2016/03/21 08:02:07]

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 12:15:55]

You are right, my bad.

[peah-webrtc, 2016/03/23 22:06:20]

Acknowledged.

+                     AudioBuffer* capture_audio_buffer,
+                     Beamformer<float>* beamformer) {
+  if (sample_rate_hz > AudioProcessing::kSampleRate16kHz) {
+    capture_audio_buffer->SplitIntoFrequencyBands();
+  }
+
+  beamformer->ProcessChunk(*capture_audio_buffer->split_data_f(),

[aluebs-webrtc, 2016/03/18 22:51:50]

After running the beamformer, the number of channels should be set to mono:
capture_audio_buffer->set_num_channels(1);

[peah-webrtc, 2016/03/21 08:02:08]

Thanks! I saw that from the AudioProcessingImpl but it did not really make sense
to me. Why is that done? There is no comment about that.

[aluebs-webrtc, 2016/03/22 12:15:55]

That is done so that the AudioBuffer is aware about the change in number of
channels. In the APM this helps next components know the number of channels the
have to work on. But here I think it only helps when copying-to the AudioBuffer.

[peah-webrtc, 2016/03/23 22:06:20]

Acknowledged.

+                           capture_audio_buffer->split_data_f());
+
+  if (sample_rate_hz > AudioProcessing::kSampleRate16kHz) {
+    capture_audio_buffer->MergeFrequencyBands();
+  }
+}
+
+void RunBitExactnessTest(int sample_rate_hz,
+                         const std::vector<Point>& array_geometry,
+                         const SphericalPointf& target_direction,
+                         const rtc::ArrayView<const float>& output_reference) {

[aluebs-webrtc, 2016/03/18 22:51:52]

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 08:02:07]

Done.

+  NonlinearBeamformer beamformer(array_geometry, target_direction);
+  beamformer.Initialize(10, sample_rate_hz > 16000 ? 16000 : sample_rate_hz);

[aluebs-webrtc, 2016/03/18 22:51:52]

Use AudioProcessing::kChunkSizeMs instead of 10, or define your own constant.
Similarly with AudioProcessing::kSampleRate16kHz. Also, I would probably make
the split_rate calculation in a separate function with a clear name.

[peah-webrtc, 2016/03/21 08:02:07]

Done.

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

[aluebs-webrtc, 2016/03/18 22:51:52]

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

[peah-webrtc, 2016/03/21 08:02:07]

Done.

+  const StreamConfig capture_config(sample_rate_hz, 2, false);

[aluebs-webrtc, 2016/03/18 22:51:51]

Use array_geometry.size() instead of hardcoding 2.

[peah-webrtc, 2016/03/21 08:02:09]

Done.

+  AudioBuffer capture_buffer(
+      capture_config.num_frames(), capture_config.num_channels(),
+      capture_config.num_frames(), capture_config.num_channels(),

[aluebs-webrtc, 2016/03/18 22:51:51]

num_process_channels should probably be mono, when running the beamformer.

[peah-webrtc, 2016/03/21 08:02:07]

No, that does not work, as the CopyFrom then produces a mono audio buffer which
(correctly) causes an assert in the beamformer.
I don't fully understand the automated conversion in AudioBuffer but my guess is
that if num_input_channels == 2 and num_process_channels == 1 AudioBuffer will
automatically downmix the data before providing it to the user.

I think this is the reason why you explicitly need to set
capture_audio_buffer->set_num_channels(1); after running the beamformer
(right?).

[aluebs-webrtc, 2016/03/22 12:15:55]

Yes, you are right. My bad.

[peah-webrtc, 2016/03/23 22:06:20]

Acknowledged.

+      capture_config.num_frames());
+  test::InputAudioFile capture_file(
+      test::GetApmCaptureTestVectorFileName(sample_rate_hz));
+  std::vector<float> capture_input(samples_per_channel * 2);

[aluebs-webrtc, 2016/03/18 22:51:51]

Use capture_config.num_channels() or capture_buffer.num_channels() instead of 2,
and capture_config.num_frames() or capture_config.num_frames() instead of
samples_pre_channel.

[peah-webrtc, 2016/03/21 08:02:08]

Done.

+  for (size_t frame_no = 0; frame_no < kNumFramesToProcess; ++frame_no) {

[aluebs-webrtc, 2016/03/18 22:51:51]

0u?

[peah-webrtc, 2016/03/21 08:02:08]

Done.

+    ReadFloatSamplesFromStereoFile(samples_per_channel, 2, &capture_file,

[aluebs-webrtc, 2016/03/18 22:51:51]

Where does this function come from? Also use capture_config.num_channels() or
capture_buffer.num_channels() instead of 2, and capture_config.num_frames() or
capture_config.num_frames() instead of
samples_pre_channel.

[peah-webrtc, 2016/03/21 08:02:06]

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

Done.

[aluebs-webrtc, 2016/03/22 12:15:55]

Thanks for clarifying. I am not sure why this didn't appear on my initial grep.

[peah-webrtc, 2016/03/23 22:06:19]

Acknowledged.

+                                   capture_input);
+
+    test::CopyVectorToAudioBuffer(capture_config, capture_input,
+                                  &capture_buffer);
+
+    ProcessOneFrame(sample_rate_hz, &capture_buffer, &beamformer);
+  }
+
+  // Extract and verify the test results.
+  std::vector<float> capture_output;
+  test::ExtractVectorFromAudioBuffer(capture_config, &capture_buffer,
+                                     &capture_output);
+
+  const float kTolerance = 1.0f / 32768.0f;

[aluebs-webrtc, 2016/03/18 22:51:51]

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 08:02:07]

Good 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(

[aluebs-webrtc, 2016/03/18 22:51:50]

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 08:02:08]

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 12:15:56]

I think your suggestion is much more intuitive. But if it will touch many files,
you probably want to do it in a separate CL.

[peah-webrtc, 2016/03/23 22:06:19]

Good, I think that change will be for the better. But I'll do it in another CL.

Acknowledged.

[aluebs-webrtc, 2016/03/24 11:14:25]

Acknowledged.

+      capture_config.num_frames(), capture_config.num_channels(),
+      output_reference, capture_output, kTolerance));
+}
+
+std::vector<Point> CreateArrayGeometry(int variant) {
+  std::vector<Point> array_geometry;
+  switch (variant) {

[aluebs-webrtc, 2016/03/18 22:51:52]

Maybe have a variant with more than 2 points?

[peah-webrtc, 2016/03/21 08:02:08]

Done.

+    case 1:
+      array_geometry.push_back(Point(-0.025f, 0.f, 0.f));
+      array_geometry.push_back(Point(0.025f, 0.f, 0.f));
+      break;
+    case 2:
+

[aluebs-webrtc, 2016/03/18 22:51:52]

Remove blank line?

[peah-webrtc, 2016/03/21 08:02:07]

Done.

+      array_geometry.push_back(Point(-0.035f, 0.f, 0.f));
+      array_geometry.push_back(Point(0.035f, 0.f, 0.f));
+

[aluebs-webrtc, 2016/03/18 22:51:52]

Remove blank line?

[peah-webrtc, 2016/03/21 08:02:08]

Done.

+      break;
+    case 3:
+

[aluebs-webrtc, 2016/03/18 22:51:52]

Remove blank line?

[peah-webrtc, 2016/03/21 08:02:08]

Done.

+      array_geometry.push_back(Point(-0.5f, 0.f, 0.f));
+      array_geometry.push_back(Point(0.5f, 0.f, 0.f));
+      break;
+    default:
+      RTC_CHECK(false);
+  }
+  return array_geometry;
+}
+
+const SphericalPointf TargetDirection1(static_cast<float>(M_PI) / 2.f,
+                                       0.f,
+                                       1.f);
+const SphericalPointf TargetDirection2(static_cast<float>(M_PI) / 2.f,
+                                       0.f,
+                                       2.f);

[aluebs-webrtc, 2016/03/18 22:51:51]

The radius is completely ignored, but it would be interesting to add some
non-broadside direction.

[peah-webrtc, 2016/03/21 08:02:06]

Absolutely, what about the new one?

[aluebs-webrtc, 2016/03/22 12:15:56]

That is elevation, right? I meant azimuth, the first parameter.

[peah-webrtc, 2016/03/23 22:06:19]

Ah, sorry about that! I will change that.

+
+}  // namespace
+
+// TODO(peah): Investigate why the nonlinear_beamformer.cc causes a DCHECK in

[aluebs-webrtc, 2016/03/18 22:51:52]

I think this should be resolved before landing this test.

[peah-webrtc, 2016/03/21 08:02:08]

I don't agree and I cannot see why that would be desired. The test will anyway
provide test coverage for the rest of the code and I see no value at all for
delaying adding that test coverage.

I was considering removing that test case and create a bug instead, but it is
quite nice to have a test-case ready for bug recreation and as an upcoming
regression test.

[aluebs-webrtc, 2016/03/22 12:15:55]

What I am afraid of, is that this might be an issue with the test and not the
code it is testing. But if you are positive that it isn't that way, I see how
they are independent.

[peah-webrtc, 2016/03/23 22:06:19]

I don't think it is an issue with the test as the test is so simple. Regardless
of that, it should not be possible to set up the beamformer so that it DCHECKS
in the processing function so I think it is an issue with the beamformer.

[aluebs-webrtc, 2016/03/24 11:14:25]

Well, it definitively is possible to DCHECK on the processing function. Just out
of curiosity, where does it DCHECK?

[peah-webrtc, 2016/03/24 11:51:30]

True. Firstly, using the new configurations, I was able to reactivate this test.

The other two disabled tests DCHECKS in the InitHighFrequencyCorrectionRanges
function at lines 259 and 261 that is called via the Initialize call.

I first thought it was the DCHECKING in the ProcessChunk function which I don't
think should happen but in the Initialize function I guess a DCHECK is much more
fine. I have no idea about the consequenses of this or reasons for why it
DCHECKS though.

[aluebs-webrtc, 2016/03/28 23:01:17]

That is weird, since those DCHECKs only depend on the sampling_rate (and some
constants). It only checks that the frequency bins of the extremes of the range
for high and low frequency corrections are in the right order. We should look
into that, to see if it is a test-issue or and actual bug caught by this test.

+// this setup.
+TEST(BeamformerBitExactnessTest,

[aluebs-webrtc, 2016/03/18 22:51:52]

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

[peah-webrtc, 2016/03/21 08:02:08]

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 12:15:55]

You can have a function, similar to CreateArrayGeometry, that returns the
reference from the sample rate, the geometry case and the direction case.

[peah-webrtc, 2016/03/23 22:06:19]

Ah, then I see. 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:14:25]

I think using TEST_P is exactly as verbose, since you have to manually add each
case you want to test, but it avoids to repeat the code. And you could disable a
specific test by commenting out the line where you instantiate it. Did the
original reviewers evaluate using TEST_P? Because they might also prefer this,
and in that case it would be good to also port all the rest of the tests to this
mode.

[peah-webrtc, 2016/03/24 11:51:30]

Yes, the initial version of the bitexactness test for the highpass filter
(https://codereview.webrtc.org/1510493004) was using TEST_P and
INSTANTIATE_TEST_CASE_P.
The argument against that was that normal tests was easier to understand and
maintain which I agree with. Furthermore, I think it is more explicit to disable
test in the GTEST manner rather than commenting them out. I think that it makes
sense to have the bitexactness tests for all the APM submodules done in the same
way.

[aluebs-webrtc, 2016/03/28 23:01:17]

I don't see any comments of the reviewers before the patch you removed the
TEST_P, but I trust you had an offline discussion with them. I agree that all
submodule-tests should be consistent, but I personally think that if there is a
nice feature to make tests parametric, why not use it. If everybody agrees on
not using TEST_P, I will not oppose to that.

[peah-webrtc, 2016/03/29 05:19:19]

The comment is there with the timestamp 2015-12-10 08:53:49 UTC.

+     DISABLED_Stereo8kHz_ArrayGeometry1_TargetDirection1) {
+  const float kOutputReference[] = {-0.000161f, 0.000171f, -0.000096f,
+                                    0.001007f,  0.000427f, 0.000977f};
+
+  std::vector<Point> array_geometry = CreateArrayGeometry(1);

[aluebs-webrtc, 2016/03/18 22:51:51]

Do we need this intermediate variable?

[peah-webrtc, 2016/03/21 08:02:09]

Done.

+
+  RunBitExactnessTest(8000, array_geometry, TargetDirection1, kOutputReference);

[aluebs-webrtc, 2016/03/18 22:51:51]

Use AudioProcessing::kSampleRate8kHz or define your own constant.

[peah-webrtc, 2016/03/21 08:02:08]

Done.

+}
+
+TEST(BeamformerBitExactnessTest, Stereo16kHz_ArrayGeometry1_TargetDirection1) {
+  const float kOutputReference[] = {0.001144f,  -0.001026f, 0.001074f,
+                                    -0.016205f, -0.007324f, -0.015656f};
+
+  std::vector<Point> array_geometry = CreateArrayGeometry(1);

[aluebs-webrtc, 2016/03/18 22:51:52]

Do we need this intermediate variable?

[peah-webrtc, 2016/03/21 08:02:09]

Done.

+
+  RunBitExactnessTest(16000, array_geometry, TargetDirection1,

[aluebs-webrtc, 2016/03/18 22:51:51]

Use AudioProcessing::kSampleRate16kHz or define your own constant.

[peah-webrtc, 2016/03/21 08:02:08]

Done.

+                      kOutputReference);
+}
+
+TEST(BeamformerBitExactnessTest, Stereo32kHz_ArrayGeometry1_TargetDirection1) {
+  const float kOutputReference[] = {0.000732f,  -0.000397f, 0.000610f,
+                                    -0.005768f, -0.001495f, -0.007111f};
+
+  std::vector<Point> array_geometry = CreateArrayGeometry(1);

[aluebs-webrtc, 2016/03/18 22:51:52]

Do we need this intermediate variable?

[peah-webrtc, 2016/03/21 08:02:08]

Done.

+
+  RunBitExactnessTest(32000, array_geometry, TargetDirection1,

[aluebs-webrtc, 2016/03/18 22:51:51]

Use AudioProcessing::kSampleRate32kHz or define your own constant.

[peah-webrtc, 2016/03/21 08:02:08]

Done.

+                      kOutputReference);
+}
+
+TEST(BeamformerBitExactnessTest, Stereo48kHz_ArrayGeometry1_TargetDirection1) {
+  const float kOutputReference[] = {0.000106f,  -0.000464f, 0.000188f,
+                                    -0.007476f, 0.002662f,  -0.007240f};
+
+  std::vector<Point> array_geometry = CreateArrayGeometry(1);

[aluebs-webrtc, 2016/03/18 22:51:51]

Do we need this intermediate variable?

[peah-webrtc, 2016/03/21 08:02:07]

Done.

+
+  RunBitExactnessTest(48000, array_geometry, TargetDirection1,

[aluebs-webrtc, 2016/03/18 22:51:51]

Use AudioProcessing::kSampleRate48kHz or define your own constant.

[peah-webrtc, 2016/03/21 08:02:07]

Done.

+                      kOutputReference);
+}
+
+TEST(BeamformerBitExactnessTest,
+     DISABLED_Stereo8kHz_ArrayGeometry1_TargetDirection2) {
+  const float kOutputReference[] = {0.001144f,  -0.001026f, 0.001074f,
+                                    -0.016205f, -0.007324f, -0.015656f};
+
+  std::vector<Point> array_geometry = CreateArrayGeometry(1);

[aluebs-webrtc, 2016/03/18 22:51:52]

Do we need this intermediate variable?

[peah-webrtc, 2016/03/21 08:02:07]

Done.

+
+  RunBitExactnessTest(8000, array_geometry, TargetDirection2, kOutputReference);

[aluebs-webrtc, 2016/03/18 22:51:52]

Use AudioProcessing::kSampleRate8kHz or define your own constant.

[peah-webrtc, 2016/03/21 08:02:07]

Done.

+}
+
+TEST(BeamformerBitExactnessTest, Stereo16kHz_ArrayGeometry1_TargetDirection2) {
+  const float kOutputReference[] = {0.001144f,  -0.001026f, 0.001074f,
+                                    -0.016205f, -0.007324f, -0.015656f};
+
+  std::vector<Point> array_geometry = CreateArrayGeometry(1);

[aluebs-webrtc, 2016/03/18 22:51:51]

Do we need this intermediate variable?

[peah-webrtc, 2016/03/21 08:02:08]

Done.

+
+  RunBitExactnessTest(16000, array_geometry, TargetDirection2,

[aluebs-webrtc, 2016/03/18 22:51:52]

Use AudioProcessing::kSampleRate16kHz or define your own constant.

[peah-webrtc, 2016/03/21 08:02:08]

Done.

+                      kOutputReference);
+}
+
+TEST(BeamformerBitExactnessTest, Stereo32kHz_ArrayGeometry1_TargetDirection2) {
+  const float kOutputReference[] = {0.000732f,  -0.000397f, 0.000610f,
+                                    -0.005768f, -0.001495f, -0.007111f};
+
+  std::vector<Point> array_geometry = CreateArrayGeometry(1);

[aluebs-webrtc, 2016/03/18 22:51:52]

Do we need this intermediate variable?

[peah-webrtc, 2016/03/21 08:02:07]

Done.

+
+  RunBitExactnessTest(32000, array_geometry, TargetDirection2,

[aluebs-webrtc, 2016/03/18 22:51:51]

Use AudioProcessing::kSampleRate32kHz or define your own constant.

[peah-webrtc, 2016/03/21 08:02:08]

Done.

+                      kOutputReference);
+}
+
+TEST(BeamformerBitExactnessTest, Stereo48kHz_ArrayGeometry1_TargetDirection2) {
+  const float kOutputReference[] = {0.000106f,  -0.000464f, 0.000188f,
+                                    -0.007476f, 0.002662f,  -0.007240f};
+
+  std::vector<Point> array_geometry = CreateArrayGeometry(1);

[aluebs-webrtc, 2016/03/18 22:51:52]

Do we need this intermediate variable?

[peah-webrtc, 2016/03/21 08:02:09]

Done.

+
+  RunBitExactnessTest(48000, array_geometry, TargetDirection2,

[aluebs-webrtc, 2016/03/18 22:51:50]

Use AudioProcessing::kSampleRate48kHz or define your own constant.

[peah-webrtc, 2016/03/21 08:02:08]

Done.

+                      kOutputReference);
+}
+
+TEST(BeamformerBitExactnessTest, Stereo8kHz_ArrayGeometry2_TargetDirection2) {
+  const float kOutputReference[] = {-0.000649f, 0.000576f,  -0.000148f,
+                                    -0.015625f, -0.006866f, -0.014374f};
+
+  std::vector<Point> array_geometry = CreateArrayGeometry(2);

[aluebs-webrtc, 2016/03/18 22:51:51]

Do we need this intermediate variable?

[peah-webrtc, 2016/03/21 08:02:08]

Done.

+
+  RunBitExactnessTest(8000, array_geometry, TargetDirection2, kOutputReference);

[aluebs-webrtc, 2016/03/18 22:51:51]

Use AudioProcessing::kSampleRate8kHz or define your own constant.

[peah-webrtc, 2016/03/21 08:02:07]

Done.

+}
+
+TEST(BeamformerBitExactnessTest, Stereo16kHz_ArrayGeometry2_TargetDirection2) {
+  const float kOutputReference[] = {0.000808f,  -0.000695f, 0.000739f,
+                                    -0.016205f, -0.007324f, -0.015656f};
+
+  std::vector<Point> array_geometry = CreateArrayGeometry(2);

[aluebs-webrtc, 2016/03/18 22:51:50]

Do we need this intermediate variable?

[peah-webrtc, 2016/03/21 08:02:08]

Done.

+
+  RunBitExactnessTest(16000, array_geometry, TargetDirection2,

[aluebs-webrtc, 2016/03/18 22:51:52]

Use AudioProcessing::kSampleRate16kHz or define your own constant.

[peah-webrtc, 2016/03/21 08:02:09]

Done.

+                      kOutputReference);
+}
+
+TEST(BeamformerBitExactnessTest, Stereo32kHz_ArrayGeometry2_TargetDirection2) {
+  const float kOutputReference[] = {0.000580f,  -0.000183f, 0.000458f,
+                                    -0.005768f, -0.001495f, -0.007111f};
+
+  std::vector<Point> array_geometry = CreateArrayGeometry(2);

[aluebs-webrtc, 2016/03/18 22:51:52]

Do we need this intermediate variable?

[peah-webrtc, 2016/03/21 08:02:07]

Done.

+
+  RunBitExactnessTest(32000, array_geometry, TargetDirection2,

[aluebs-webrtc, 2016/03/18 22:51:52]

Use AudioProcessing::kSampleRate32kHz or define your own constant.

[peah-webrtc, 2016/03/21 08:02:07]

Done.

+                      kOutputReference);
+}
+
+TEST(BeamformerBitExactnessTest, Stereo48kHz_ArrayGeometry2_TargetDirection2) {
+  const float kOutputReference[] = {0.000075f,  -0.000288f, 0.000156f,
+                                    -0.007476f, 0.002662f,  -0.007240f};
+
+  std::vector<Point> array_geometry = CreateArrayGeometry(2);

[aluebs-webrtc, 2016/03/18 22:51:52]

Do we need this intermediate variable?

[peah-webrtc, 2016/03/21 08:02:07]

Done.

+
+  RunBitExactnessTest(48000, array_geometry, TargetDirection2,

[aluebs-webrtc, 2016/03/18 22:51:51]

Use AudioProcessing::kSampleRate48kHz or define your own constant.

[peah-webrtc, 2016/03/21 08:02:06]

Done.

+                      kOutputReference);
+}
+
+// TODO(peah): Investigate why the nonlinear_beamformer.cc causes a DCHECK in

[aluebs-webrtc, 2016/03/18 22:51:51]

I think this should be resolved before landing this test.

[peah-webrtc, 2016/03/21 08:02:08]

As for the corresponding above comment, I don't agree and I don't see the reason
for waiting with landing the test due to that.

+// this setup.
+TEST(BeamformerBitExactnessTest,
+     DISABLED_Stereo16kHz_ArrayGeometry3_TargetDirection1) {
+  const float kOutputReference[] = {-0.000161f, 0.000171f, -0.000096f,
+                                    0.001007f,  0.000427f, 0.000977f};
+
+  std::vector<Point> array_geometry = CreateArrayGeometry(3);

[aluebs-webrtc, 2016/03/18 22:51:51]

Do we need this intermediate variable?

[peah-webrtc, 2016/03/21 08:02:07]

Done.

+
+  RunBitExactnessTest(16000, array_geometry, TargetDirection1,
+                      kOutputReference);
+}
+
+}  // namespace webrtc