Allow more than 2 input channels in AudioProcessing.

webrtc/modules/audio_processing/audio_processing_impl.cc

Index: webrtc/modules/audio_processing/audio_processing_impl.cc
diff --git a/webrtc/modules/audio_processing/audio_processing_impl.cc b/webrtc/modules/audio_processing/audio_processing_impl.cc
index 87b82a6a3509131adae9ed698cc0f896fd01d4c0..2b11b48b186b261f0b71bc9026053c065b87b047 100644
--- a/webrtc/modules/audio_processing/audio_processing_impl.cc
+++ b/webrtc/modules/audio_processing/audio_processing_impl.cc
@@ -11,6 +11,7 @@
 #include "webrtc/modules/audio_processing/audio_processing_impl.h"
 
 #include <assert.h>
+#include <algorithm>
 
 #include "webrtc/base/checks.h"
 #include "webrtc/base/platform_file.h"
@@ -57,6 +58,23 @@ extern "C" {
   } while (0)
 
 namespace webrtc {
+namespace {
+
+static bool LayoutHasKeyboard(AudioProcessing::ChannelLayout layout) {
+  switch (layout) {
+    case AudioProcessing::kMono:
+    case AudioProcessing::kStereo:
+      return false;
+    case AudioProcessing::kMonoAndKeyboard:
+    case AudioProcessing::kStereoAndKeyboard:
+      return true;
+  }
+
+  assert(false);
+  return false;
+}
+
+}  // namespace
 
 // Throughout webrtc, it's assumed that success is represented by zero.
 static_assert(AudioProcessing::kNoError == 0, "kNoError must be zero");
@@ -75,9 +93,7 @@ static_assert(AudioProcessing::kNoError == 0, "kNoError must be zero");
 class GainControlForNewAgc : public GainControl, public VolumeCallbacks {
  public:
   explicit GainControlForNewAgc(GainControlImpl* gain_control)
-      : real_gain_control_(gain_control),
-        volume_(0) {
-  }
+      : real_gain_control_(gain_control), volume_(0) {}
 
   // GainControl implementation.
   int Enable(bool enable) override {
@@ -166,10 +182,10 @@ AudioProcessingImpl::AudioProcessingImpl(const Config& config,
       debug_file_(FileWrapper::Create()),
       event_msg_(new audioproc::Event()),
 #endif
-      fwd_in_format_(kSampleRate16kHz, 1),
+      api_format_({{{kSampleRate16kHz, 1, false},
+                    {kSampleRate16kHz, 1, false},
+                    {kSampleRate16kHz, 1, false}}}),
       fwd_proc_format_(kSampleRate16kHz),
-      fwd_out_format_(kSampleRate16kHz, 1),
-      rev_in_format_(kSampleRate16kHz, 1),
       rev_proc_format_(kSampleRate16kHz, 1),
       split_rate_(kSampleRate16kHz),
       stream_delay_ms_(0),
@@ -253,12 +269,11 @@ int AudioProcessingImpl::Initialize() {
 
 int AudioProcessingImpl::set_sample_rate_hz(int rate) {
   CriticalSectionScoped crit_scoped(crit_);
-  return InitializeLocked(rate,
-                          rate,
-                          rev_in_format_.rate(),
-                          fwd_in_format_.num_channels(),
-                          fwd_out_format_.num_channels(),
-                          rev_in_format_.num_channels());
+
+  ProcessingConfig processing_config = api_format_;
+  processing_config.input_stream().set_sample_rate_hz(rate);
+  processing_config.output_stream().set_sample_rate_hz(rate);
+  return InitializeLocked(processing_config);
 }
 
 int AudioProcessingImpl::Initialize(int input_sample_rate_hz,

[aluebs-webrtc, 2015/07/14 23:12:43]

Will this interface be replaced by one with a ProcessingConfig?

[mgraczyk, 2015/07/15 01:12:46]

Yes, eventually. It'll be easiest to first commit this, then swap clients over
to the other Initialize() overload, then remove this overload in 3 separate CLs.

[aluebs-webrtc, 2015/07/15 18:04:06]

Agreed. Just making sure that that is the plan.

@@ -267,29 +282,40 @@ int AudioProcessingImpl::Initialize(int input_sample_rate_hz,
                                     ChannelLayout input_layout,
                                     ChannelLayout output_layout,
                                     ChannelLayout reverse_layout) {
+  const ProcessingConfig processing_config = {
+      {{input_sample_rate_hz, ChannelsFromLayout(input_layout),
+        LayoutHasKeyboard(input_layout)},
+       {output_sample_rate_hz, ChannelsFromLayout(output_layout),
+        LayoutHasKeyboard(output_layout)},
+       {reverse_sample_rate_hz, ChannelsFromLayout(reverse_layout),
+        LayoutHasKeyboard(reverse_layout)}}};
+
+  return Initialize(processing_config);
+}
+
+int AudioProcessingImpl::Initialize(const ProcessingConfig& processing_config) {
   CriticalSectionScoped crit_scoped(crit_);
-  return InitializeLocked(input_sample_rate_hz,
-                          output_sample_rate_hz,
-                          reverse_sample_rate_hz,
-                          ChannelsFromLayout(input_layout),
-                          ChannelsFromLayout(output_layout),
-                          ChannelsFromLayout(reverse_layout));
+  return InitializeLocked(processing_config);
 }
 
 int AudioProcessingImpl::InitializeLocked() {
-  const int fwd_audio_buffer_channels = beamformer_enabled_ ?
-                                        fwd_in_format_.num_channels() :
-                                        fwd_out_format_.num_channels();
-  render_audio_.reset(new AudioBuffer(rev_in_format_.samples_per_channel(),
-                                      rev_in_format_.num_channels(),
-                                      rev_proc_format_.samples_per_channel(),
-                                      rev_proc_format_.num_channels(),
-                                      rev_proc_format_.samples_per_channel()));
-  capture_audio_.reset(new AudioBuffer(fwd_in_format_.samples_per_channel(),
-                                       fwd_in_format_.num_channels(),
-                                       fwd_proc_format_.samples_per_channel(),
-                                       fwd_audio_buffer_channels,
-                                       fwd_out_format_.samples_per_channel()));
+  const int fwd_audio_buffer_channels =
+      beamformer_enabled_ ? api_format_.input_stream().num_channels()
+                          : api_format_.output_stream().num_channels();
+  if (api_format_.reverse_stream().num_channels() > 0) {
+    render_audio_.reset(new AudioBuffer(
+        api_format_.reverse_stream().samples_per_channel(),
+        api_format_.reverse_stream().num_channels(),
+        rev_proc_format_.samples_per_channel(), rev_proc_format_.num_channels(),
+        rev_proc_format_.samples_per_channel()));
+  } else {
+    render_audio_.reset(nullptr);

[aluebs-webrtc, 2015/07/14 23:12:43]

When do we want this case?

[mgraczyk, 2015/07/15 01:12:46]

This is just saying "When there is no reverse stream, don't bother allocating a
buffer for it".

[aluebs-webrtc, 2015/07/15 18:04:06]

Ack.

+  }
+  capture_audio_.reset(new AudioBuffer(
+      api_format_.input_stream().samples_per_channel(),
+      api_format_.input_stream().num_channels(),
+      fwd_proc_format_.samples_per_channel(), fwd_audio_buffer_channels,
+      api_format_.output_stream().samples_per_channel()));
 
   // Initialize all components.
   for (auto item : component_list_) {
@@ -317,38 +343,38 @@ int AudioProcessingImpl::InitializeLocked() {
   return kNoError;
 }
 
-int AudioProcessingImpl::InitializeLocked(int input_sample_rate_hz,
-                                          int output_sample_rate_hz,
-                                          int reverse_sample_rate_hz,
-                                          int num_input_channels,
-                                          int num_output_channels,
-                                          int num_reverse_channels) {
-  if (input_sample_rate_hz <= 0 ||
-      output_sample_rate_hz <= 0 ||
-      reverse_sample_rate_hz <= 0) {
-    return kBadSampleRateError;
-  }
-  if (num_output_channels > num_input_channels) {
-    return kBadNumberChannelsError;
+int AudioProcessingImpl::InitializeLocked(const ProcessingConfig& config) {
+  for (const auto& stream : config.streams) {
+    if (stream.sample_rate_hz() < 0) {

[aluebs-webrtc, 2015/07/14 23:12:43]

<=

[mgraczyk, 2015/07/15 01:12:46]

The reverse stream can have zero sampling rate and zero channels here.  I check
that the reverse channel is not zero in AnalyzeReverseStream.

[aluebs-webrtc, 2015/07/15 18:04:06]

But here it looks like all streams can (although you check the num_channels_
below).

[mgraczyk, 2015/07/15 20:03:19]

Yeah num_in_channels is checked to be nonzero below, and the output has to have
1 channel or the same (nonzero) number of channels as the input.

[aluebs-webrtc, 2015/07/15 21:29:17]

Yes, but what about the sample rate?

[mgraczyk, 2015/07/15 21:53:56]

I changed the check so that sample_rate only matters if num_channels is
positive.

+      return kBadSampleRateError;
+    }
+    if (stream.num_channels() < 0) {

[aluebs-webrtc, 2015/07/14 23:12:43]

< 1

[mgraczyk, 2015/07/15 01:12:46]

See above

+      return kBadNumberChannelsError;
+    }
   }
-  // Only mono and stereo supported currently.
-  if (num_input_channels > 2 || num_input_channels < 1 ||
-      num_output_channels > 2 || num_output_channels < 1 ||
-      num_reverse_channels > 2 || num_reverse_channels < 1) {
+
+  const int num_in_channels = config.input_stream().num_channels();
+  const int num_out_channels = config.output_stream().num_channels();
+
+  // Need at least one input channel.
+  // Need either one output channel or as many outputs as there are inputs.
+  if (num_in_channels == 0 ||
+      !(num_out_channels == 1 || num_out_channels == num_in_channels)) {
     return kBadNumberChannelsError;
   }
+
   if (beamformer_enabled_ &&
-      (static_cast<size_t>(num_input_channels) != array_geometry_.size() ||
-       num_output_channels > 1)) {
+      (static_cast<size_t>(config.input_stream().num_channels()) !=

[aluebs-webrtc, 2015/07/14 23:12:43]

You can use num_in_channels here.

[mgraczyk, 2015/07/15 01:12:46]

Done.

+           array_geometry_.size() || num_out_channels > 1)) {
     return kBadNumberChannelsError;
   }
 
-  fwd_in_format_.set(input_sample_rate_hz, num_input_channels);
-  fwd_out_format_.set(output_sample_rate_hz, num_output_channels);
-  rev_in_format_.set(reverse_sample_rate_hz, num_reverse_channels);
+  api_format_ = config;
 
   // We process at the closest native rate >= min(input rate, output rate)...
-  int min_proc_rate = std::min(fwd_in_format_.rate(), fwd_out_format_.rate());
+  const int min_proc_rate =
+      std::min(api_format_.input_stream().sample_rate_hz(),
+               api_format_.output_stream().sample_rate_hz());
   int fwd_proc_rate;
   if (supports_48kHz_ && min_proc_rate > kSampleRate32kHz) {
     fwd_proc_rate = kSampleRate48kHz;
@@ -364,15 +390,15 @@ int AudioProcessingImpl::InitializeLocked(int input_sample_rate_hz,
     fwd_proc_rate = kSampleRate16kHz;
   }
 
-  fwd_proc_format_.set(fwd_proc_rate);
+  fwd_proc_format_ = StreamConfig(fwd_proc_rate);
 
   // We normally process the reverse stream at 16 kHz. Unless...
   int rev_proc_rate = kSampleRate16kHz;
-  if (fwd_proc_format_.rate() == kSampleRate8kHz) {
+  if (fwd_proc_format_.sample_rate_hz() == kSampleRate8kHz) {
     // ...the forward stream is at 8 kHz.
     rev_proc_rate = kSampleRate8kHz;
   } else {
-    if (rev_in_format_.rate() == kSampleRate32kHz) {
+    if (api_format_.reverse_stream().sample_rate_hz() == kSampleRate32kHz) {
       // ...or the input is at 32 kHz, in which case we use the splitting
       // filter rather than the resampler.
       rev_proc_rate = kSampleRate32kHz;
@@ -381,13 +407,13 @@ int AudioProcessingImpl::InitializeLocked(int input_sample_rate_hz,
 
   // Always downmix the reverse stream to mono for analysis. This has been
   // demonstrated to work well for AEC in most practical scenarios.
-  rev_proc_format_.set(rev_proc_rate, 1);
+  rev_proc_format_ = StreamConfig(rev_proc_rate, 1);
 
-  if (fwd_proc_format_.rate() == kSampleRate32kHz ||
-      fwd_proc_format_.rate() == kSampleRate48kHz) {
+  if (fwd_proc_format_.sample_rate_hz() == kSampleRate32kHz ||
+      fwd_proc_format_.sample_rate_hz() == kSampleRate48kHz) {
     split_rate_ = kSampleRate16kHz;
   } else {
-    split_rate_ = fwd_proc_format_.rate();
+    split_rate_ = fwd_proc_format_.sample_rate_hz();
   }
 
   return InitializeLocked();
@@ -395,26 +421,12 @@ int AudioProcessingImpl::InitializeLocked(int input_sample_rate_hz,
 
 // Calls InitializeLocked() if any of the audio parameters have changed from
 // their current values.
-int AudioProcessingImpl::MaybeInitializeLocked(int input_sample_rate_hz,
-                                               int output_sample_rate_hz,
-                                               int reverse_sample_rate_hz,
-                                               int num_input_channels,
-                                               int num_output_channels,
-                                               int num_reverse_channels) {
-  if (input_sample_rate_hz == fwd_in_format_.rate() &&
-      output_sample_rate_hz == fwd_out_format_.rate() &&
-      reverse_sample_rate_hz == rev_in_format_.rate() &&
-      num_input_channels == fwd_in_format_.num_channels() &&
-      num_output_channels == fwd_out_format_.num_channels() &&
-      num_reverse_channels == rev_in_format_.num_channels()) {
+int AudioProcessingImpl::MaybeInitializeLocked(
+    const ProcessingConfig& processing_config) {
+  if (processing_config == api_format_) {
     return kNoError;
   }
-  return InitializeLocked(input_sample_rate_hz,
-                          output_sample_rate_hz,
-                          reverse_sample_rate_hz,
-                          num_input_channels,
-                          num_output_channels,
-                          num_reverse_channels);
+  return InitializeLocked(processing_config);
 }
 
 void AudioProcessingImpl::SetExtraOptions(const Config& config) {
@@ -431,16 +443,16 @@ void AudioProcessingImpl::SetExtraOptions(const Config& config) {
 
 int AudioProcessingImpl::input_sample_rate_hz() const {
   CriticalSectionScoped crit_scoped(crit_);
-  return fwd_in_format_.rate();
+  return api_format_.input_stream().sample_rate_hz();
 }
 
 int AudioProcessingImpl::sample_rate_hz() const {
   CriticalSectionScoped crit_scoped(crit_);
-  return fwd_in_format_.rate();
+  return api_format_.input_stream().sample_rate_hz();
 }
 
 int AudioProcessingImpl::proc_sample_rate_hz() const {
-  return fwd_proc_format_.rate();
+  return fwd_proc_format_.sample_rate_hz();
 }
 
 int AudioProcessingImpl::proc_split_sample_rate_hz() const {
@@ -452,11 +464,11 @@ int AudioProcessingImpl::num_reverse_channels() const {
 }
 
 int AudioProcessingImpl::num_input_channels() const {
-  return fwd_in_format_.num_channels();
+  return api_format_.input_stream().num_channels();
 }
 
 int AudioProcessingImpl::num_output_channels() const {
-  return fwd_out_format_.num_channels();
+  return api_format_.output_stream().num_channels();
 }
 
 void AudioProcessingImpl::set_output_will_be_muted(bool muted) {
@@ -479,44 +491,58 @@ int AudioProcessingImpl::ProcessStream(const float* const* src,
                                        int output_sample_rate_hz,
                                        ChannelLayout output_layout,
                                        float* const* dest) {
+  const ProcessingConfig processing_config = {
+      {{
+           input_sample_rate_hz, ChannelsFromLayout(input_layout),
+           LayoutHasKeyboard(input_layout),
+       },
+       {
+           output_sample_rate_hz, ChannelsFromLayout(output_layout),
+           LayoutHasKeyboard(output_layout),
+       },
+       api_format_.reverse_stream()}};
+
+  if (samples_per_channel !=
+      processing_config.input_stream().samples_per_channel()) {
+    return kBadDataLengthError;
+  }
+  return ProcessStream(src, processing_config, dest);
+}
+
+int AudioProcessingImpl::ProcessStream(
+    const float* const* src,
+    const ProcessingConfig& processing_config,
+    float* const* dest) {
   CriticalSectionScoped crit_scoped(crit_);
   if (!src || !dest) {
     return kNullPointerError;
   }
 
-  RETURN_ON_ERR(MaybeInitializeLocked(input_sample_rate_hz,
-                                      output_sample_rate_hz,
-                                      rev_in_format_.rate(),
-                                      ChannelsFromLayout(input_layout),
-                                      ChannelsFromLayout(output_layout),
-                                      rev_in_format_.num_channels()));
-  if (samples_per_channel != fwd_in_format_.samples_per_channel()) {
-    return kBadDataLengthError;
-  }
+  RETURN_ON_ERR(MaybeInitializeLocked(processing_config));
+  assert(processing_config.input_stream().samples_per_channel() ==
+         api_format_.input_stream().samples_per_channel());
 
 #ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP
   if (debug_file_->Open()) {
     event_msg_->set_type(audioproc::Event::STREAM);
     audioproc::Stream* msg = event_msg_->mutable_stream();
     const size_t channel_size =
-        sizeof(float) * fwd_in_format_.samples_per_channel();
-    for (int i = 0; i < fwd_in_format_.num_channels(); ++i)
+        sizeof(float) * processing_config.input_stream().samples_per_channel();

[aluebs-webrtc, 2015/07/14 23:12:43]

How about using api_format_.input_stream().samples_per_channel() to be
consistent with next line?

[mgraczyk, 2015/07/15 01:12:46]

Done.

+    for (int i = 0; i < api_format_.input_stream().num_channels(); ++i)
       msg->add_input_channel(src[i], channel_size);
   }
 #endif
 
-  capture_audio_->CopyFrom(src, samples_per_channel, input_layout);
+  capture_audio_->CopyFrom(src, api_format_.input_stream());
   RETURN_ON_ERR(ProcessStreamLocked());
-  capture_audio_->CopyTo(fwd_out_format_.samples_per_channel(),
-                         output_layout,
-                         dest);
+  capture_audio_->CopyTo(api_format_.output_stream(), dest);
 
 #ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP
   if (debug_file_->Open()) {
     audioproc::Stream* msg = event_msg_->mutable_stream();
     const size_t channel_size =
-        sizeof(float) * fwd_out_format_.samples_per_channel();
-    for (int i = 0; i < fwd_out_format_.num_channels(); ++i)
+        sizeof(float) * processing_config.input_stream().samples_per_channel();

[aluebs-webrtc, 2015/07/14 23:12:43]

How about using api_format_.input_stream().samples_per_channel() to be
consistent with next line?

[mgraczyk, 2015/07/15 01:12:46]

Done.

+    for (int i = 0; i < api_format_.input_stream().num_channels(); ++i)
       msg->add_output_channel(dest[i], channel_size);
     RETURN_ON_ERR(WriteMessageToDebugFile());
   }
@@ -545,13 +571,15 @@ int AudioProcessingImpl::ProcessStream(AudioFrame* frame) {
 
   // TODO(ajm): The input and output rates and channels are currently
   // constrained to be identical in the int16 interface.
-  RETURN_ON_ERR(MaybeInitializeLocked(frame->sample_rate_hz_,
-                                      frame->sample_rate_hz_,
-                                      rev_in_format_.rate(),
-                                      frame->num_channels_,
-                                      frame->num_channels_,
-                                      rev_in_format_.num_channels()));
-  if (frame->samples_per_channel_ != fwd_in_format_.samples_per_channel()) {
+  ProcessingConfig processing_config = api_format_;

[aluebs-webrtc, 2015/07/14 23:12:43]

To be consistent, how about creating the ProcessingConfig like:
const ProcessingConfig processing_config = { ... }

[mgraczyk, 2015/07/15 01:12:46]

I did this to be defensive against the possibility of new fields in
ProcessingConfig in the future.  We want to copy api_format_, only changing the
sampling rate and channel count.  If we add new fields, those should be copied
as well rather than set to default values (which is what the aggregate
initialization syntax would result in).

WDYT?

[aluebs-webrtc, 2015/07/15 18:04:06]

Good point! But then shouldn't you do the same in the other cases where you copy
it?

[mgraczyk, 2015/07/15 20:03:19]

True, Done.

+  processing_config.input_stream().set_sample_rate_hz(frame->sample_rate_hz_);
+  processing_config.input_stream().set_num_channels(frame->num_channels_);
+  processing_config.output_stream().set_sample_rate_hz(frame->sample_rate_hz_);
+  processing_config.output_stream().set_num_channels(frame->num_channels_);
+
+  RETURN_ON_ERR(MaybeInitializeLocked(processing_config));
+  if (frame->samples_per_channel_ !=
+      api_format_.input_stream().samples_per_channel()) {
     return kBadDataLengthError;
   }
 
@@ -559,9 +587,8 @@ int AudioProcessingImpl::ProcessStream(AudioFrame* frame) {
   if (debug_file_->Open()) {
     event_msg_->set_type(audioproc::Event::STREAM);
     audioproc::Stream* msg = event_msg_->mutable_stream();
-    const size_t data_size = sizeof(int16_t) *
-                             frame->samples_per_channel_ *
-                             frame->num_channels_;
+    const size_t data_size =
+        sizeof(int16_t) * frame->samples_per_channel_ * frame->num_channels_;
     msg->set_input_data(frame->data_, data_size);
   }
 #endif
@@ -573,9 +600,8 @@ int AudioProcessingImpl::ProcessStream(AudioFrame* frame) {
 #ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP
   if (debug_file_->Open()) {
     audioproc::Stream* msg = event_msg_->mutable_stream();
-    const size_t data_size = sizeof(int16_t) *
-                             frame->samples_per_channel_ *
-                             frame->num_channels_;
+    const size_t data_size =
+        sizeof(int16_t) * frame->samples_per_channel_ * frame->num_channels_;
     msg->set_output_data(frame->data_, data_size);
     RETURN_ON_ERR(WriteMessageToDebugFile());
   }
@@ -584,7 +610,6 @@ int AudioProcessingImpl::ProcessStream(AudioFrame* frame) {
   return kNoError;
 }
 
-
 int AudioProcessingImpl::ProcessStreamLocked() {
 #ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP
   if (debug_file_->Open()) {
@@ -600,8 +625,7 @@ int AudioProcessingImpl::ProcessStreamLocked() {
 
   AudioBuffer* ca = capture_audio_.get();  // For brevity.
   if (use_new_agc_ && gain_control_->is_enabled()) {
-    agc_manager_->AnalyzePreProcess(ca->channels()[0],
-                                    ca->num_channels(),
+    agc_manager_->AnalyzePreProcess(ca->channels()[0], ca->num_channels(),
                                     fwd_proc_format_.samples_per_channel());
   }
 
@@ -627,12 +651,10 @@ int AudioProcessingImpl::ProcessStreamLocked() {
   RETURN_ON_ERR(echo_control_mobile_->ProcessCaptureAudio(ca));
   RETURN_ON_ERR(voice_detection_->ProcessCaptureAudio(ca));
 
-  if (use_new_agc_ &&
-      gain_control_->is_enabled() &&
+  if (use_new_agc_ && gain_control_->is_enabled() &&
       (!beamformer_enabled_ || beamformer_->is_target_present())) {
     agc_manager_->Process(ca->split_bands_const(0)[kBand0To8kHz],
-                          ca->num_frames_per_band(),
-                          split_rate_);
+                          ca->num_frames_per_band(), split_rate_);
   }
   RETURN_ON_ERR(gain_control_->ProcessCaptureAudio(ca));
 
@@ -646,15 +668,11 @@ int AudioProcessingImpl::ProcessStreamLocked() {
     float voice_probability =
         agc_manager_.get() ? agc_manager_->voice_probability() : 1.f;
 
-    transient_suppressor_->Suppress(ca->channels_f()[0],
-                                    ca->num_frames(),
-                                    ca->num_channels(),
-                                    ca->split_bands_const_f(0)[kBand0To8kHz],
-                                    ca->num_frames_per_band(),
-                                    ca->keyboard_data(),
-                                    ca->num_keyboard_frames(),
-                                    voice_probability,
-                                    key_pressed_);
+    transient_suppressor_->Suppress(
+        ca->channels_f()[0], ca->num_frames(), ca->num_channels(),
+        ca->split_bands_const_f(0)[kBand0To8kHz], ca->num_frames_per_band(),
+        ca->keyboard_data(), ca->num_keyboard_frames(), voice_probability,
+        key_pressed_);
   }
 
   // The level estimator operates on the recombined data.
@@ -668,19 +686,30 @@ int AudioProcessingImpl::AnalyzeReverseStream(const float* const* data,
                                               int samples_per_channel,
                                               int sample_rate_hz,
                                               ChannelLayout layout) {
+  const StreamConfig reverse_config = {
+      sample_rate_hz, ChannelsFromLayout(layout), LayoutHasKeyboard(layout),
+  };
+  return AnalyzeReverseStream(data, reverse_config);
+}
+
+int AudioProcessingImpl::AnalyzeReverseStream(
+    const float* const* data,
+    const StreamConfig& reverse_config) {
   CriticalSectionScoped crit_scoped(crit_);
   if (data == NULL) {
     return kNullPointerError;
   }
 
-  const int num_channels = ChannelsFromLayout(layout);
-  RETURN_ON_ERR(MaybeInitializeLocked(fwd_in_format_.rate(),
-                                      fwd_out_format_.rate(),
-                                      sample_rate_hz,
-                                      fwd_in_format_.num_channels(),
-                                      fwd_out_format_.num_channels(),
-                                      num_channels));
-  if (samples_per_channel != rev_in_format_.samples_per_channel()) {
+  if (reverse_config.num_channels() <= 0) {

[aluebs-webrtc, 2015/07/14 23:12:43]

Why is this needed? Is this because you don't check it in the initialization?

[mgraczyk, 2015/07/15 01:12:46]

Yes.

[aluebs-webrtc, 2015/07/15 18:04:06]

Do you think it is worth it to add these to be able to not allocate the reverse
stream for some test cases (in all real applications, there is a reverse
stream)?

[mgraczyk, 2015/07/15 20:03:19]

What about clients that only use APM for it's one way processing? Noise
suppression, beamforming, AGC, etc?

[aluebs-webrtc, 2015/07/15 21:29:16]

Ok, I was not thinking of any 1 way clients, but you are right. Agreed on having
this.

+    return kBadNumberChannelsError;
+  }
+
+  const ProcessingConfig processing_config = {{api_format_.input_stream(),
+                                               api_format_.output_stream(),
+                                               reverse_config}};
+  RETURN_ON_ERR(MaybeInitializeLocked(processing_config));
+  if (reverse_config.samples_per_channel() !=

[aluebs-webrtc, 2015/07/14 23:12:43]

Move this if statement to the API which receives sample_per_channel and assert
directly here, as you did in the ProcessStream interface.

[mgraczyk, 2015/07/15 01:12:46]

Done.

+      api_format_.reverse_stream().samples_per_channel()) {
     return kBadDataLengthError;
   }
 
@@ -689,14 +718,14 @@ int AudioProcessingImpl::AnalyzeReverseStream(const float* const* data,
     event_msg_->set_type(audioproc::Event::REVERSE_STREAM);
     audioproc::ReverseStream* msg = event_msg_->mutable_reverse_stream();
     const size_t channel_size =
-        sizeof(float) * rev_in_format_.samples_per_channel();
-    for (int i = 0; i < num_channels; ++i)
+        sizeof(float) * api_format_.reverse_stream().samples_per_channel();
+    for (int i = 0; i < api_format_.reverse_stream().num_channels(); ++i)
       msg->add_channel(data[i], channel_size);
     RETURN_ON_ERR(WriteMessageToDebugFile());
   }
 #endif
 
-  render_audio_->CopyFrom(data, samples_per_channel, layout);
+  render_audio_->CopyFrom(data, api_format_.reverse_stream());
   return AnalyzeReverseStreamLocked();
 }
 
@@ -713,17 +742,25 @@ int AudioProcessingImpl::AnalyzeReverseStream(AudioFrame* frame) {
     return kBadSampleRateError;
   }
   // This interface does not tolerate different forward and reverse rates.
-  if (frame->sample_rate_hz_ != fwd_in_format_.rate()) {
+  if (frame->sample_rate_hz_ != api_format_.input_stream().sample_rate_hz()) {
     return kBadSampleRateError;
   }
 
-  RETURN_ON_ERR(MaybeInitializeLocked(fwd_in_format_.rate(),
-                                      fwd_out_format_.rate(),
-                                      frame->sample_rate_hz_,
-                                      fwd_in_format_.num_channels(),
-                                      fwd_in_format_.num_channels(),
-                                      frame->num_channels_));
-  if (frame->samples_per_channel_ != rev_in_format_.samples_per_channel()) {
+  if (frame->num_channels_ <= 0) {

[aluebs-webrtc, 2015/07/14 23:12:43]

Why is this needed? Is this because you don't check it in the initialization?

[mgraczyk, 2015/07/15 01:12:46]

yes, same thing.

+    return kBadNumberChannelsError;
+  }
+
+  const ProcessingConfig processing_config = {{
+      api_format_.input_stream(),
+      api_format_.output_stream(),
+      {
+          frame->sample_rate_hz_, frame->num_channels_,
+          api_format_.reverse_stream().has_keyboard(),
+      },
+  }};
+  RETURN_ON_ERR(MaybeInitializeLocked(processing_config));
+  if (frame->samples_per_channel_ !=
+      api_format_.reverse_stream().samples_per_channel()) {
     return kBadDataLengthError;
   }
 
@@ -731,9 +768,8 @@ int AudioProcessingImpl::AnalyzeReverseStream(AudioFrame* frame) {
   if (debug_file_->Open()) {
     event_msg_->set_type(audioproc::Event::REVERSE_STREAM);
     audioproc::ReverseStream* msg = event_msg_->mutable_reverse_stream();
-    const size_t data_size = sizeof(int16_t) *
-                             frame->samples_per_channel_ *
-                             frame->num_channels_;
+    const size_t data_size =
+        sizeof(int16_t) * frame->samples_per_channel_ * frame->num_channels_;
     msg->set_data(frame->data_, data_size);
     RETURN_ON_ERR(WriteMessageToDebugFile());
   }
@@ -745,7 +781,7 @@ int AudioProcessingImpl::AnalyzeReverseStream(AudioFrame* frame) {
 
 int AudioProcessingImpl::AnalyzeReverseStreamLocked() {
   AudioBuffer* ra = render_audio_.get();  // For brevity.
-  if (rev_proc_format_.rate() == kSampleRate32kHz) {
+  if (rev_proc_format_.sample_rate_hz() == kSampleRate32kHz) {
     ra->SplitIntoFrequencyBands();
   }
 
@@ -947,13 +983,15 @@ bool AudioProcessingImpl::is_data_processed() const {
 
 bool AudioProcessingImpl::output_copy_needed(bool is_data_processed) const {
   // Check if we've upmixed or downmixed the audio.
-  return ((fwd_out_format_.num_channels() != fwd_in_format_.num_channels()) ||
+  return ((api_format_.output_stream().num_channels() !=
+           api_format_.input_stream().num_channels()) ||
           is_data_processed || transient_suppressor_enabled_);
 }
 
 bool AudioProcessingImpl::synthesis_needed(bool is_data_processed) const {
-  return (is_data_processed && (fwd_proc_format_.rate() == kSampleRate32kHz ||
-          fwd_proc_format_.rate() == kSampleRate48kHz));
+  return (is_data_processed &&
+          (fwd_proc_format_.sample_rate_hz() == kSampleRate32kHz ||
+           fwd_proc_format_.sample_rate_hz() == kSampleRate48kHz));
 }
 
 bool AudioProcessingImpl::analysis_needed(bool is_data_processed) const {
@@ -961,8 +999,8 @@ bool AudioProcessingImpl::analysis_needed(bool is_data_processed) const {
       !transient_suppressor_enabled_) {
     // Only level_estimator_ is enabled.
     return false;
-  } else if (fwd_proc_format_.rate() == kSampleRate32kHz ||
-             fwd_proc_format_.rate() == kSampleRate48kHz) {
+  } else if (fwd_proc_format_.sample_rate_hz() == kSampleRate32kHz ||
+             fwd_proc_format_.sample_rate_hz() == kSampleRate48kHz) {
     // Something besides level_estimator_ is enabled, and we have super-wb.
     return true;
   }
@@ -986,9 +1024,9 @@ void AudioProcessingImpl::InitializeTransient() {
     if (!transient_suppressor_.get()) {
       transient_suppressor_.reset(new TransientSuppressor());
     }
-    transient_suppressor_->Initialize(fwd_proc_format_.rate(),
-                                      split_rate_,
-                                      fwd_out_format_.num_channels());
+    transient_suppressor_->Initialize(
+        fwd_proc_format_.sample_rate_hz(), split_rate_,
+        api_format_.output_stream().num_channels());
   }
 }
 
@@ -1031,8 +1069,8 @@ void AudioProcessingImpl::MaybeUpdateHistograms() {
     const int frames_per_ms = rtc::CheckedDivExact(split_rate_, 1000);
     const int aec_system_delay_ms =
         WebRtcAec_system_delay(echo_cancellation()->aec_core()) / frames_per_ms;
-    const int diff_aec_system_delay_ms = aec_system_delay_ms -
-        last_aec_system_delay_ms_;
+    const int diff_aec_system_delay_ms =
+        aec_system_delay_ms - last_aec_system_delay_ms_;
     if (diff_aec_system_delay_ms > kMinDiffDelayMs &&
         last_aec_system_delay_ms_ != 0) {
       RTC_HISTOGRAM_COUNTS("WebRTC.Audio.AecSystemDelayJump",
@@ -1072,8 +1110,8 @@ int AudioProcessingImpl::WriteMessageToDebugFile() {
     return kUnspecifiedError;
   }
 #if defined(WEBRTC_ARCH_BIG_ENDIAN)
-  // TODO(ajm): Use little-endian "on the wire". For the moment, we can be
-  //            pretty safe in assuming little-endian.
+// TODO(ajm): Use little-endian "on the wire". For the moment, we can be
+//            pretty safe in assuming little-endian.
 #endif
 
   if (!event_msg_->SerializeToString(&event_str_)) {
@@ -1096,12 +1134,12 @@ int AudioProcessingImpl::WriteMessageToDebugFile() {
 int AudioProcessingImpl::WriteInitMessage() {
   event_msg_->set_type(audioproc::Event::INIT);
   audioproc::Init* msg = event_msg_->mutable_init();
-  msg->set_sample_rate(fwd_in_format_.rate());
-  msg->set_num_input_channels(fwd_in_format_.num_channels());
-  msg->set_num_output_channels(fwd_out_format_.num_channels());
-  msg->set_num_reverse_channels(rev_in_format_.num_channels());
-  msg->set_reverse_sample_rate(rev_in_format_.rate());
-  msg->set_output_sample_rate(fwd_out_format_.rate());
+  msg->set_sample_rate(api_format_.input_stream().sample_rate_hz());
+  msg->set_num_input_channels(api_format_.input_stream().num_channels());
+  msg->set_num_output_channels(api_format_.output_stream().num_channels());
+  msg->set_num_reverse_channels(api_format_.reverse_stream().num_channels());
+  msg->set_reverse_sample_rate(api_format_.reverse_stream().sample_rate_hz());
+  msg->set_output_sample_rate(api_format_.output_stream().sample_rate_hz());
 
   int err = WriteMessageToDebugFile();
   if (err != kNoError) {