Make the separation between target and interferer scenario depend on microphone spacing in Nonlinea…

Make the separation between target and interferer scenario depend on microphone spacing in NonlinearBeamformer

Depends on this CL: https://codereview.webrtc.org/1378973003/

Committed: https://crrev.com/4a66e4a4d8847cef1e61aec6978f64dddece3d96
Cr-Commit-Position: refs/heads/master@{#10330}

[aluebs-webrtc, 2015-10-06 02:32:21]

aluebs@webrtc.org changed reviewers:
+ andrew@webrtc.org

[Andrew MacDonald, 2015-10-14 23:38:53]

lgtm

Consider adding a unit test for GetMinimumSpacing.

https://codereview.webrtc.org/1388033002/diff/40001/webrtc/modules/audio_proc...
File webrtc/modules/audio_processing/beamformer/nonlinear_beamformer.cc (right):

https://codereview.webrtc.org/1388033002/diff/40001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/beamformer/nonlinear_beamformer.cc:39: // The
minimum separation in radians between the target direction and a
and an interferer

https://codereview.webrtc.org/1388033002/diff/40001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/beamformer/nonlinear_beamformer.cc:194: float
GetMinimumSpacing(std::vector<Point> array_geometry) {
const reference

https://codereview.webrtc.org/1388033002/diff/40001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/beamformer/nonlinear_beamformer.cc:198: float
distance = Distance(array_geometry[i], array_geometry[j]);
Not a big deal, but you're computing the first distance twice. Perhaps:
float mic_spacing = std::numeric_limits<float>::max();

OTOH, the very common two mic case will then have to enter the loop. Up to you.

https://codereview.webrtc.org/1388033002/diff/40001/webrtc/modules/audio_proc...
File webrtc/modules/audio_processing/beamformer/nonlinear_beamformer.h (right):

https://codereview.webrtc.org/1388033002/diff/40001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/beamformer/nonlinear_beamformer.h:120: const
float mic_spacing_;
min_mic_spacing_

[aluebs-webrtc, 2015-10-17 00:54:50]

Added GetMinimumSpacing unittest
PTAL

https://codereview.webrtc.org/1388033002/diff/40001/webrtc/modules/audio_proc...
File webrtc/modules/audio_processing/beamformer/nonlinear_beamformer.cc (right):

https://codereview.webrtc.org/1388033002/diff/40001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/beamformer/nonlinear_beamformer.cc:39: // The
minimum separation in radians between the target direction and a
On 2015/10/14 23:38:53, Andrew MacDonald wrote:
> and an interferer

Done.

https://codereview.webrtc.org/1388033002/diff/40001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/beamformer/nonlinear_beamformer.cc:194: float
GetMinimumSpacing(std::vector<Point> array_geometry) {
On 2015/10/14 23:38:53, Andrew MacDonald wrote:
> const reference

Done.

https://codereview.webrtc.org/1388033002/diff/40001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/beamformer/nonlinear_beamformer.cc:198: float
distance = Distance(array_geometry[i], array_geometry[j]);
On 2015/10/14 23:38:53, Andrew MacDonald wrote:
> Not a big deal, but you're computing the first distance twice. Perhaps:
> float mic_spacing = std::numeric_limits<float>::max();
> 
> OTOH, the very common two mic case will then have to enter the loop. Up to
you.

Good suggestion. Done.

https://codereview.webrtc.org/1388033002/diff/40001/webrtc/modules/audio_proc...
File webrtc/modules/audio_processing/beamformer/nonlinear_beamformer.h (right):

https://codereview.webrtc.org/1388033002/diff/40001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/beamformer/nonlinear_beamformer.h:120: const
float mic_spacing_;
On 2015/10/14 23:38:53, Andrew MacDonald wrote:
> min_mic_spacing_

Done.

[Andrew MacDonald, 2015-10-19 22:35:07]

lgtm

https://codereview.webrtc.org/1388033002/diff/120001/webrtc/modules/audio_pro...
File webrtc/modules/audio_processing/beamformer/array_util.cc (right):

https://codereview.webrtc.org/1388033002/diff/120001/webrtc/modules/audio_pro...
webrtc/modules/audio_processing/beamformer/array_util.cc:17: float
GetMinimumSpacing(const std::vector<Point>& array_geometry) {
CHECK_GT(array_geometry.size(), 1);

https://codereview.webrtc.org/1388033002/diff/120001/webrtc/modules/audio_pro...
webrtc/modules/audio_processing/beamformer/array_util.cc:21: float distance =
Distance(array_geometry[i], array_geometry[j]);
A bit neater maybe:
mic_spacing = std::min(mic_spacing, Distance(...));

https://codereview.webrtc.org/1388033002/diff/120001/webrtc/modules/audio_pro...
File webrtc/modules/audio_processing/beamformer/array_util.h (right):

https://codereview.webrtc.org/1388033002/diff/120001/webrtc/modules/audio_pro...
webrtc/modules/audio_processing/beamformer/array_util.h:35: float
GetMinimumSpacing(const std::vector<Point>& array_geometry);
Add a comment.

[aluebs-webrtc, 2015-10-20 00:16:23]

The CQ bit was checked by aluebs@webrtc.org

[aluebs-webrtc, 2015-10-20 00:16:23]

The patchset sent to the CQ was uploaded after l-g-t-m from andrew@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1388033002/#ps160001 (title:
"Formatting")

[commit-bot: I haz the power, 2015-10-20 00:16:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1388033002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1388033002/160001

[aluebs-webrtc, 2015-10-20 00:17:08]

https://codereview.webrtc.org/1388033002/diff/120001/webrtc/modules/audio_pro...
File webrtc/modules/audio_processing/beamformer/array_util.cc (right):

https://codereview.webrtc.org/1388033002/diff/120001/webrtc/modules/audio_pro...
webrtc/modules/audio_processing/beamformer/array_util.cc:17: float
GetMinimumSpacing(const std::vector<Point>& array_geometry) {
On 2015/10/19 22:35:07, Andrew MacDonald wrote:
> CHECK_GT(array_geometry.size(), 1);

Done.

https://codereview.webrtc.org/1388033002/diff/120001/webrtc/modules/audio_pro...
webrtc/modules/audio_processing/beamformer/array_util.cc:21: float distance =
Distance(array_geometry[i], array_geometry[j]);
On 2015/10/19 22:35:07, Andrew MacDonald wrote:
> A bit neater maybe:
> mic_spacing = std::min(mic_spacing, Distance(...));

Done.

https://codereview.webrtc.org/1388033002/diff/120001/webrtc/modules/audio_pro...
File webrtc/modules/audio_processing/beamformer/array_util.h (right):

https://codereview.webrtc.org/1388033002/diff/120001/webrtc/modules/audio_pro...
webrtc/modules/audio_processing/beamformer/array_util.h:35: float
GetMinimumSpacing(const std::vector<Point>& array_geometry);
On 2015/10/19 22:35:07, Andrew MacDonald wrote:
> Add a comment.

Done.

[commit-bot: I haz the power, 2015-10-20 01:02:43]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2015-10-20 01:02:49]

Patchset 9 (id:??) landed as
https://crrev.com/4a66e4a4d8847cef1e61aec6978f64dddece3d96
Cr-Commit-Position: refs/heads/master@{#10330}