Pull out the PostFilter to its own NonlinearBeamformer API

webrtc/modules/audio_processing/beamformer/beamformer.h

 /*
  *  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.
  */
 
@@ skipping 10 matching lines @@
  public:
   virtual ~Beamformer() {}
 
   // Process one time-domain chunk of audio. The audio is expected to be split
   // into frequency bands inside the ChannelBuffer. The number of frames and
   // channels must correspond to the constructor parameters. The same
   // ChannelBuffer can be passed in as |input| and |output|.
   virtual void ProcessChunk(const ChannelBuffer<T>& input,
                             ChannelBuffer<T>* output) = 0;
 
+  // Applies the postfilter mask to one chunk of audio. The audio is expected to
+  // be split into frequency bands inside the ChannelBuffer. The number of
+  // frames must correspond to the constructor parameters and the number of
+  // channels is expected to be 1, since that is the output number of channels
+  // of ProcessChunk(). The same ChannelBuffer can be passed in as |input| and
+  // |output|.
+  virtual void PostFilter(const ChannelBuffer<float>& input,

[peah-webrtc, 2016/05/22 21:06:48]

Why not change this to have one input/output parameter as that is how it is
used? That would mean less code and would also save one memcopy.

[peah-webrtc, 2016/05/22 21:06:48]

Is there a reason why this is not implemented in the .cc file?

[aluebs-webrtc, 2016/05/26 01:04:45]

Beamformer is an interface, it has no .cc file.

[aluebs-webrtc, 2016/05/26 01:04:45]

To be consistent with the ProcessChunk interface and give the user the freedom
to *not* override the input. But I see your point, so I changed it.

[peah-webrtc, 2016/05/26 08:48:52]

My skills on this are limited, but to me, an interface is something that the
user of the interface needs to implement. If it has a default implementation, it
is not a pure interface. Is there a reason for why this method cannot be
declared pure virtual so that this is a proper interface that enforces the
implementer of the interface to supply a interface implementation?

[aluebs-webrtc, 2016/05/28 03:00:00]

It already is not a pure interface, because IsInBeam() has a default
implementation. This has a default empty implementation because not all
Beamformers have postfilters.

[peah-webrtc, 2016/05/30 11:49:25]

Do you mean that there are more beamformers that use this interface? 

Would it not be better then to let the actual beamformer implementing the
interface to explicitly provide an empty implementation, rather to have it
pre-provided?

[aluebs-webrtc, 2016/06/01 00:16:34]

At this point there is only the non-linear Beamformer in WebRTC, but there were
plans of porting other types if we think  they provide a benefit. I think having
a default implementation is more intuitive and adds less code-complexity. First,
because it doesn't force all Beamformers, even the ones that have nothing to do
with postfilters, implement it. But also, because this means that every
Beamformer needs it's own empty implementation.

[peah-webrtc, 2016/06/01 14:51:01]

If there are no other beamformer solutions that should implement this interface
at the moment, I really don't think we should imposing restrictions on the
interface or add any code at all at the moment based on the possible behavior of
future beamformer  solutions. 

As it is now, the default implementation only adds code-complexity (as it is not
needed). It may possibly reduce code complexity in a future implementation, but
that we don't know until we have seen that implementation. 

[aluebs-webrtc, 2016/06/01 22:13:20]

I think we should keep the code scalable, specially if the "code-complexity
addition" is literally "{}". But if you insist, I dropped the whole interface,
since it makes no sense to have one for only one Beamformer implementation in
WebRTC.

+                          ChannelBuffer<float>* output) {
+    for (size_t i = 0u; i < input.num_channels(); ++i) {
+      for (size_t j = 0u; j < input.num_bands(); ++j) {
+        memcpy(output->channels(j)[i],
+               input.channels(j)[i],
+               input.num_frames_per_band() * sizeof(output->channels(j)[i][0]));
+      }
+    }
+  };
+
   // Sample rate corresponds to the lower band.
   // Needs to be called before the the Beamformer can be used.
   virtual void Initialize(int chunk_size_ms, int sample_rate_hz) = 0;
 
   // Aim the beamformer at a point in space.
   virtual void AimAt(const SphericalPointf& spherical_point) = 0;
 
   // Indicates whether a given point is inside of the beam.
   virtual bool IsInBeam(const SphericalPointf& spherical_point) { return true; }
 
   // Returns true if the current data contains the target signal.
   // Which signals are considered "targets" is implementation dependent.
   virtual bool is_target_present() = 0;
 };
 
 }  // namespace webrtc
 
 #endif  // WEBRTC_MODULES_AUDIO_PROCESSING_BEAMFORMER_BEAMFORMER_H_