Pull the Voice Activity Detector out from the AGC

webrtc/modules/audio_processing/vad/voice_activity_detector.cc

+/*
+ *  Copyright (c) 2015 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 "webrtc/modules/audio_processing/vad/voice_activity_detector.h"
+
+#include "webrtc/base/checks.h"
+
+namespace webrtc {
+namespace {
+
+const int kMaxLength = 320;
+const int kNumChannels = 1;

[bloch, 2015/06/12 23:34:38]

Just wondering, is there a VAD implementation that processes a multi-channel
input?

[aluebs-webrtc, 2015/06/16 01:17:52]

Not right now, and I don't think we will need it, so I want to keep this as
simple as possible. Also, if multi-channel is needed, you can always have a
vector of VADs.

+
+const double kDefaultVoiceValue = 1.0;
+const double kNeutralProbability = 0.5;
+const double kLowProbability = 0.01;
+
+}  // namespace
+
+VoiceActivityDetector::VoiceActivityDetector()
+    : last_voice_probability_(kDefaultVoiceValue),
+      // Initialize to the most common resampling situation.
+      resampler_(kMaxLength, kLength10Ms, kNumChannels),
+      standalone_vad_(StandaloneVad::Create()) {
+}
+
+void VoiceActivityDetector::ProcessCaptureAudio(const int16_t* audio,
+                                                int length) {
+  CHECK_LE(length, kMaxLength);
+  // Resample to the required rate.
+  const int16_t* resampled_ptr = audio;
+  if (length != kLength10Ms) {

[Andrew MacDonald, 2015/06/15 04:32:25]

This assumes that chunks of 10 ms are passed in. I think you should continue to
take the sample rate as input.

[aluebs-webrtc, 2015/06/16 01:17:52]

Agreed.

+    CHECK_EQ(resampler_.ResetIfNeeded(length, kLength10Ms, kNumChannels), 0);
+    resampler_.Push(audio, length, resampled_, kLength10Ms, length);
+    resampled_ptr = resampled_;
+  }
+  assert(length == kLength10Ms);

[Andrew MacDonald, 2015/06/15 04:32:25]

DCHECK_EQ

[aluebs-webrtc, 2015/06/16 01:17:52]

Done.

+
+  CHECK_EQ(standalone_vad_->AddAudio(audio, length), 0);
+
+  audio_processing_.ExtractFeatures(resampled_ptr, length, &features_);
+
+  chunkwise_voice_probabilities_.clear();

[Andrew MacDonald, 2015/06/15 04:32:25]

minor: It might be overall more clear if you resize both vectors to num_frames
here.
chunkwise_voice_probabilities_.resize(features_.num_frames);
chunkwise_rms_.num_frames(features_.num_frames);
std::copy(features_.rms, features_.rms + chunkwise_rms_.size(),
chunkwise_rms_.begin());
...
std::fill(chunkwise_voice_probabilities_.begin(),
chunkwise_voice_probabilities_.end(), kLowProbability);
...

wdyt?

[aluebs-webrtc, 2015/06/16 01:17:52]

But the point in clearing here is that it returns a zero-size vector when there
is no information. WDYT?

[Andrew MacDonald, 2015/06/16 22:18:31]

It will work, assuming features_.num_frames == 0 when there are no features. The
vectors will be resized to zero.

Still this is more a preference; whatever you want to do.

[aluebs-webrtc, 2015/06/17 01:44:31]

Good point. I agree it is clearer, so I changed it to your suggestion.

+  chunkwise_rms_.clear();
+  if (features_.num_frames > 0) {
+    if (features_.silence) {
+      // The other features are invalid, so set the voice probabilities to an
+      // arbitrary low value.
+      chunkwise_voice_probabilities_.assign(features_.num_frames,
+                                            kLowProbability);
+    } else {
+      chunkwise_voice_probabilities_.assign(features_.num_frames,
+                                            kNeutralProbability);

[Andrew MacDonald, 2015/06/15 04:32:25]

Move the call to standalone_vad_->AddAudio to here? Looks like it's not needed
otherwise.

[aluebs-webrtc, 2015/06/16 01:17:52]

But here it will not be run on each chunk, which is the idea. Internally it
buffers the audio every "AddAudio" call and runs the VAD on the buffered audio
on each "GetActivity" call.

[Andrew MacDonald, 2015/06/16 22:18:31]

Ah OK. Can you put a comment to that effect on the call above?

[aluebs-webrtc, 2015/06/17 01:44:31]

Done.

+      CHECK_GE(standalone_vad_->GetActivity(&chunkwise_voice_probabilities_[0],
+                                            kMaxNumFrames),

[Andrew MacDonald, 2015/06/15 04:32:25]

I'm worried about this call. You're only guaranteed that
chunkwise_voice_probabilities_.size() == features_.num_frames, which could be
less than kMaxNumFrames, right?

Is kMaxNumFrames needed at all? Perhaps just use
chunkwise_voice_probabilities_.size() here?

[aluebs-webrtc, 2015/06/16 01:17:52]

That is an excellent point! I think in practice they are always equal, but it es
better to be sure.

[Andrew MacDonald, 2015/06/16 22:18:31]

Where is kMaxNumFrames defined? Can you remove it?

[aluebs-webrtc, 2015/06/17 01:44:30]

It is defined in vad/common.h and used to define the sizes of the arrays in
AudioFeatures and in:
* activity_metric.cc
* vad_audio_proc.cc
* standalone_vad_unittest.cc
* vad_audio_proc_unittest.cc
So I guess I can't remove it for now.

+               0);
+      CHECK_GE(pitch_based_vad_.VoicingProbability(
+                   features_, &chunkwise_voice_probabilities_[0]),
+               0);
+    }
+    last_voice_probability_ =
+        chunkwise_voice_probabilities_[features_.num_frames - 1];

[Andrew MacDonald, 2015/06/15 04:32:25]

chunkwise_voice_probabilities_.last()

[aluebs-webrtc, 2015/06/16 01:17:52]

Done.

+  }
+  for (int i = 0; i < features_.num_frames; ++i) {
+    chunkwise_rms_.push_back(features_.rms[i]);
+  }
+}
+
+}  // namespace webrtc