Clean up LappedTransform and Blocker.

- Remove unnecessary window member from lapped_transform.
  - Add comment indicated that Blocker does not take ownership of
    the window passed to its constructor.
  - Streamline LappedTransform constructor so members can be const.

Also use a range-based for loop in audio_processing_impl.cc for clarity.

R=aluebs@webrtc.org, andrew@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/41229004

Cr-Commit-Position: refs/heads/master@{#8708}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8708 4adac7df-926f-26a2-2b94-8c16560cd09d

webrtc/common_audio/blocker.cc

@@ -43,7 +43,7 @@ void CopyFrames(const float* const* src,
   for (int i = 0; i < num_channels; ++i) {
     memcpy(&dst[i][dst_start_index],
            &src[i][src_start_index],
-           num_frames * sizeof(float));
+           num_frames * sizeof(dst[i][dst_start_index]));
   }
 }
 
@@ -57,7 +57,7 @@ void MoveFrames(const float* const* src,
   for (int i = 0; i < num_channels; ++i) {
     memmove(&dst[i][dst_start_index],
             &src[i][src_start_index],
-            num_frames * sizeof(float));
+            num_frames * sizeof(dst[i][dst_start_index]));
   }
 }
 
@@ -66,7 +66,8 @@ void ZeroOut(float* const* buffer,
              int num_frames,
              int num_channels) {
   for (int i = 0; i < num_channels; ++i) {
-    memset(&buffer[i][starting_idx], 0, num_frames * sizeof(float));
+    memset(&buffer[i][starting_idx], 0,
+           num_frames * sizeof(buffer[i][starting_idx]));
   }
 }
 
@@ -118,7 +119,9 @@ Blocker::Blocker(int chunk_size,
       shift_amount_(shift_amount),
       callback_(callback) {
   CHECK_LE(num_output_channels_, num_input_channels_);
-  memcpy(window_.get(), window, block_size_ * sizeof(float));
+  CHECK(window);
+
+  memcpy(window_.get(), window, block_size_ * sizeof(*window_.get()));
   input_buffer_.MoveReadPosition(-initial_delay_);
 }
 

webrtc/common_audio/blocker.h

@@ -58,6 +58,9 @@ class BlockerCallback {
 //
 // A small amount of delay is added to the first received chunk to deal with
 // the difference in chunk/block sizes. This delay is <= chunk_size.
+//
+// Ownership of window is retained by the caller.  That is, Blocker makes a
+// copy of window and does not attempt to delete it.
 class Blocker {
  public:
   Blocker(int chunk_size,

webrtc/common_audio/channel_buffer.h

@@ -42,14 +42,13 @@ class ChannelBuffer {
   ChannelBuffer(int num_frames,
                 int num_channels,
                 int num_bands = 1)
-      : data_(new T[num_frames * num_channels]),
+      : data_(new T[num_frames * num_channels]()),
         channels_(new T*[num_channels * num_bands]),
         bands_(new T*[num_channels * num_bands]),
         num_frames_(num_frames),
         num_frames_per_band_(num_frames / num_bands),
         num_channels_(num_channels),
         num_bands_(num_bands) {
-    memset(data_.get(), 0, size() * sizeof(T));
     for (int i = 0; i < num_channels_; ++i) {
       for (int j = 0; j < num_bands_; ++j) {
         channels_[j * num_channels_ + i] =

webrtc/common_audio/lapped_transform.cc

@@ -10,6 +10,7 @@
 
 #include "webrtc/common_audio/lapped_transform.h"
 
+#include <algorithm>
 #include <cstdlib>
 #include <cstring>
 
@@ -57,49 +58,29 @@ LappedTransform::LappedTransform(int in_channels, int out_channels,
     : blocker_callback_(this),
       in_channels_(in_channels),
       out_channels_(out_channels),
-      window_(window),
-      own_window_(false),
-      window_shift_amount_(shift_amount),
       block_length_(block_length),
       chunk_length_(chunk_length),
       block_processor_(callback),
-      blocker_(nullptr),
+      blocker_(
+        chunk_length_, block_length_, in_channels_, out_channels_, window,
+        shift_amount, &blocker_callback_),
       fft_(RealFourier::FftOrder(block_length_)),
       cplx_length_(RealFourier::ComplexLength(fft_.order())),
-      real_buf_(in_channels, block_length, RealFourier::kFftBufferAlignment),
+      real_buf_(in_channels, block_length_, RealFourier::kFftBufferAlignment),
       cplx_pre_(in_channels, cplx_length_, RealFourier::kFftBufferAlignment),
       cplx_post_(out_channels, cplx_length_, RealFourier::kFftBufferAlignment) {
   CHECK(in_channels_ > 0 && out_channels_ > 0);
   CHECK_GT(block_length_, 0);
   CHECK_GT(chunk_length_, 0);
   CHECK(block_processor_);
-  CHECK_EQ(0, block_length & (block_length - 1));  // block_length_ power of 2?
 
-  if (!window_) {
-    own_window_ = true;
-    window_ = new float[block_length_];
-    CHECK(window_ != nullptr);
-    window_shift_amount_ = block_length_;
-    float* temp = const_cast<float*>(window_);
-    for (int i = 0; i < block_length_; ++i) {
-      temp[i] = 1.0f;
-    }
-  }
-
-  blocker_.reset(new Blocker(chunk_length_, block_length_, in_channels_,
-                             out_channels_, window_, window_shift_amount_,
-                             &blocker_callback_));
-}
-
-LappedTransform::~LappedTransform() {
-  if (own_window_) {
-    delete [] window_;
-  }
+  // block_length_ power of 2?
+  CHECK_EQ(0, block_length_ & (block_length_ - 1));
 }
 
 void LappedTransform::ProcessChunk(const float* const* in_chunk,
                                    float* const* out_chunk) {
-  blocker_->ProcessChunk(in_chunk, chunk_length_, in_channels_, out_channels_,
+  blocker_.ProcessChunk(in_chunk, chunk_length_, in_channels_, out_channels_,
                          out_chunk);
 }
 

webrtc/common_audio/lapped_transform.h

@@ -43,15 +43,14 @@ class LappedTransform {
 
   // Construct a transform instance. |chunk_length| is the number of samples in
   // each channel. |window| defines the window, owned by the caller (a copy is
-  // made internally); can be NULL to disable windowing entirely.
-  // |block_length| defines the length of a block, in samples, even when
-  // windowing is disabled. |shift_length| is in samples. |callback| is the
-  // caller-owned audio processing function called for each block of the input
-  // chunk.
+  // made internally); |window| should have length equal to |block_length|.
+  // |block_length| defines the length of a block, in samples.
+  // |shift_amount| is in samples. |callback| is the caller-owned audio
+  // processing function called for each block of the input chunk.
   LappedTransform(int in_channels, int out_channels, int chunk_length,
                   const float* window, int block_length, int shift_amount,
                   Callback* callback);
-  ~LappedTransform();
+  ~LappedTransform() {}
 
   // Main audio processing helper method. Internally slices |in_chunk| into
   // blocks, transforms them to frequency domain, calls the callback for each
@@ -62,34 +61,29 @@ class LappedTransform {
  private:
   // Internal middleware callback, given to the blocker. Transforms each block
   // and hands it over to the processing method given at construction time.
-  friend class BlockThunk;
   class BlockThunk : public BlockerCallback {
    public:
     explicit BlockThunk(LappedTransform* parent) : parent_(parent) {}
-    virtual ~BlockThunk() {}
 
     virtual void ProcessBlock(const float* const* input, int num_frames,
                               int num_input_channels, int num_output_channels,
                               float* const* output);
 
    private:
-    LappedTransform* parent_;
+    LappedTransform* const parent_;
   } blocker_callback_;
 
-  int in_channels_;
-  int out_channels_;
+  const int in_channels_;
+  const int out_channels_;
 
-  const float* window_;
-  bool own_window_;
-  int window_shift_amount_;
+  const int block_length_;
+  const int chunk_length_;
 
-  int block_length_;
-  int chunk_length_;
-  Callback* block_processor_;
-  rtc::scoped_ptr<Blocker> blocker_;
+  Callback* const block_processor_;
+  Blocker blocker_;
 
   RealFourier fft_;
-  int cplx_length_;
+  const int cplx_length_;
   AlignedArray<float> real_buf_;
   AlignedArray<std::complex<float> > cplx_pre_;
   AlignedArray<std::complex<float> > cplx_post_;

webrtc/common_audio/lapped_transform_unittest.cc

@@ -10,6 +10,7 @@
 
 #include "webrtc/common_audio/lapped_transform.h"
 
+#include <algorithm>
 #include <cmath>
 #include <cstring>
 
@@ -87,9 +88,14 @@ TEST(LappedTransformTest, Windowless) {
   const int kChannels = 3;
   const int kChunkLength = 512;
   const int kBlockLength = 64;
-  const int kShiftAmount = 32;
+  const int kShiftAmount = 64;
   NoopCallback noop;
-  LappedTransform trans(kChannels, kChannels, kChunkLength, nullptr,
+
+  // Rectangular window.
+  float window[kBlockLength];
+  std::fill(window, &window[kBlockLength], 1.0f);
+
+  LappedTransform trans(kChannels, kChannels, kChunkLength, window,
                         kBlockLength, kShiftAmount, &noop);
   float in_buffer[kChannels][kChunkLength];
   float* in_chunk[kChannels];
@@ -121,11 +127,10 @@ TEST(LappedTransformTest, IdentityProcessor) {
   const int kBlockLength = 64;
   const int kShiftAmount = 32;
   NoopCallback noop;
-  float window[kBlockLength];
-  float* window_ptr = window;
 
   // Identity window for |overlap = block_size / 2|.
-  SetFloatArray(sqrtf(0.5f), 1, kBlockLength, &window_ptr);
+  float window[kBlockLength];
+  std::fill(window, &window[kBlockLength], std::sqrt(0.5f));
 
   LappedTransform trans(1, 1, kChunkLength, window, kBlockLength, kShiftAmount,
                         &noop);
@@ -152,7 +157,12 @@ TEST(LappedTransformTest, Callbacks) {
   const int kChunkLength = 512;
   const int kBlockLength = 64;
   FftCheckerCallback call;
-  LappedTransform trans(1, 1, kChunkLength, nullptr, kBlockLength,
+
+  // Rectangular window.
+  float window[kBlockLength];
+  std::fill(window, &window[kBlockLength], 1.0f);
+
+  LappedTransform trans(1, 1, kChunkLength, window, kBlockLength,
                         kBlockLength, &call);
   float in_buffer[kChunkLength];
   float* in_chunk = in_buffer;

webrtc/modules/audio_processing/audio_processing_impl.cc

@@ -278,9 +278,8 @@ int AudioProcessingImpl::InitializeLocked() {
                                        fwd_out_format_.samples_per_channel()));
 
   // Initialize all components.
-  std::list<ProcessingComponent*>::iterator it;
-  for (it = component_list_.begin(); it != component_list_.end(); ++it) {
-    int err = (*it)->Initialize();
+  for (auto item : component_list_) {
+    int err = item->Initialize();
     if (err != kNoError) {
       return err;
     }
@@ -412,9 +411,9 @@ int AudioProcessingImpl::MaybeInitializeLocked(int input_sample_rate_hz,
 
 void AudioProcessingImpl::SetExtraOptions(const Config& config) {
   CriticalSectionScoped crit_scoped(crit_);
-  std::list<ProcessingComponent*>::iterator it;
-  for (it = component_list_.begin(); it != component_list_.end(); ++it)
-    (*it)->SetExtraOptions(config);
+  for (auto item : component_list_) {
+    item->SetExtraOptions(config);
+  }
 
   if (transient_suppressor_enabled_ != config.Get<ExperimentalNs>().enabled) {
     transient_suppressor_enabled_ = config.Get<ExperimentalNs>().enabled;
@@ -915,9 +914,8 @@ bool AudioProcessingImpl::is_data_processed() const {
   }
 
   int enabled_count = 0;
-  std::list<ProcessingComponent*>::const_iterator it;
-  for (it = component_list_.begin(); it != component_list_.end(); it++) {
-    if ((*it)->is_component_enabled()) {
+  for (auto item : component_list_) {
+    if (item->is_component_enabled()) {
       enabled_count++;
     }
   }