Add MultiChannelResampler wrapper for SincResampler.

media/base/multi_channel_resampler.cc

Index: media/base/multi_channel_resampler.cc
diff --git a/media/base/multi_channel_resampler.cc b/media/base/multi_channel_resampler.cc
new file mode 100644
index 0000000000000000000000000000000000000000..0236aa59c4f0362be5a7c2198a730a42a0b46694
--- /dev/null
+++ b/media/base/multi_channel_resampler.cc
@@ -0,0 +1,98 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "media/base/multi_channel_resampler.h"
+
+#include "base/logging.h"
+
+namespace media {
+
+MultiChannelResampler::MultiChannelResampler(
+    MultiChannelAudioSourceProvider* provider, double scale_factor,
+    int number_of_channels)
+    : number_of_channels_(number_of_channels),
+      channel_index_(0),
+      last_frame_count_(0),
+      callback_count_(0),
+      provider_(provider) {
+  DCHECK_GT(number_of_channels, 0);
+
+  // Preallocate staging arrays based on SincResampler's buffer size.
+  resampler_audio_data_.reserve(number_of_channels);
+  for (int i = 0; i < number_of_channels; ++i)
+    resampler_audio_data_.push_back(new float[SincResampler::kBufferSize]);
+
+  // Create each channel's resampler.
+  resamplers_.reserve(number_of_channels);
+  for (int i = 0; i < number_of_channels; ++i)
+    resamplers_.push_back(new SincResampler(this, scale_factor));
+}
+
+MultiChannelResampler::~MultiChannelResampler() {
+  // Clean up |resampler_audio_data_|.

[Ami GONE FROM CHROMIUM, 2012/06/30 20:12:14]

With my scoped* suggestions in the .h file, this dtor should become empty.

[DaleCurtis, 2012/07/03 03:02:57]

Removed clean up of resamplers_

+  for (size_t i = 0; i < resampler_audio_data_.size(); ++i)
+    delete [] resampler_audio_data_[i];
+  resampler_audio_data_.clear();
+
+  // Clean up |resamplers_|.
+  for (size_t i = 0; i < resamplers_.size(); ++i)
+    delete resamplers_[i];
+  resamplers_.clear();
+}
+
+void MultiChannelResampler::Resample(const std::vector<float*>& audio_data,
+                                     int number_of_frames) {
+  // We need to ensure that SincResampler only calls ProvideInput once for each

[Ami GONE FROM CHROMIUM, 2012/06/30 20:12:14]

An alternative, of course, is to allow SR to call PI as many times as it wants
to satisfy the entire number_of_frames in one shot, and manage the buffering in
this class (by tracking per-channel the first not-yet-handed-to-SR offset in the
buffered audio data (and asking for more multichannel data when a channel's
offset reaches its current buffered size).

I actually suspect that would result in clearer code so if I were you I'd try
that out and see whether my suspicion is correct.  Your choice.

[DaleCurtis, 2012/07/01 23:27:46]

Yeah, I'm not super happy with the chunking design here since it relies on
internal details of SincResampler. That's how it's implemented in WebKit though.

I didn't go the internal management route because that means reallocating
buffers on every call unless I fix number_of_frames at construction which is
contrary (and a little confusing due to scale_factor / SincResampler
implementation) to the RenderCallback design.

[Ami GONE FROM CHROMIUM, 2012/07/01 23:44:44]

I don't follow.  Why would you need to reallocate on every call?

[DaleCurtis, 2012/07/02 17:43:45]

If SR calls PI as many times as it wants for a single channel we have to buffer
the data for all the other channels (at the time of call) for as many times as
SR needs to call. There's no way to get just a single channel of audio data back
with the current RenderCallback design.

I could lazy-initialize and resize on the first channel assuming every call to
MCR::PI would ask for the same amount. That's not very clean either though.

Am I missing something?

[DaleCurtis, 2012/07/03 03:02:57]

After discussion and partial implementation this is a bit tricky and certainly
more complicated than the current implementation since we'd reallocate buffers,
track both read pointers and write pointers in a secondary vector<float*> for
calling into read_cb_.

Keeping the current implementation for now.

+  // channel.  To ensure this, we chunk the number of requested frames into
+  // SincResampler::kBlockSize chunks for which SincResampler guarantees it will
+  // only call ProvideInput() once.
+  int frames_done = 0;
+  while (frames_done < number_of_frames) {
+    int frames_this_time = std::min(
+        number_of_frames - frames_done, SincResampler::kBlockSize);
+
+    // Sanity check to ensure ProvideInput is only called once per channel.  The
+    // value should track |channel_index_| when checked inside ProvideInput().
+    callback_count_ = 0;
+
+    // Resample each channel.
+    for (channel_index_ = 0; channel_index_ < resamplers_.size();
+         ++channel_index_) {
+      // Depending on the sample-rate scale factor, and the internal buffering
+      // used in a SincResampler kernel, this call to Resample() will only
+      // sometimes call ProvideInput().  However, if it calls ProvideInput() for
+      // the first channel, then it will call it for the remaining channels,
+      // since they all buffer in the same way and are processing the same
+      // number of frames.
+      resamplers_[channel_index_]->Resample(
+          audio_data[channel_index_] + frames_done, frames_this_time);
+    }
+
+    frames_done += frames_this_time;
+  }
+}
+
+void MultiChannelResampler::ProvideInput(float* destination,
+                                         int number_of_frames) {
+  // Get the data from the multi-channel provider when the first channel asks
+  // for it.  For subsequent channels, we can just dish out the channel data
+  // from that (stored in |resampler_audio_data_|).
+  if (!channel_index_) {
+    DCHECK_LE(number_of_frames, SincResampler::kBufferSize);
+    last_frame_count_ = number_of_frames;
+    provider_->ProvideInput(resampler_audio_data_, number_of_frames);
+  }
+
+  // All channels must ask for the same amount.  This should always be the case,
+  // but let's just make sure.
+  DCHECK_EQ(number_of_frames, last_frame_count_);
+
+  // Copy the channel data from what we received from |provider_|.
+  DCHECK_LT(channel_index_, static_cast<size_t>(number_of_channels_));
+  DCHECK_EQ(channel_index_, callback_count_++);

[Ami GONE FROM CHROMIUM, 2012/06/30 20:12:14]

Bug: this line is compiled out of Release builds, so your ++ will disappear.

[DaleCurtis, 2012/07/01 23:27:46]

Whoops! I wonder if that's the source of my memory corruption in the CL that
ties this all together.

[DaleCurtis, 2012/07/03 03:02:57]

Actually this doesn't matter as it's only used for DCHECK'ing anyways. I can
move it out if you prefer, but I think it makes sense to keep in the DCHECK
since it's unused elsewhere.

+  memcpy(destination, resampler_audio_data_[channel_index_],
+         sizeof(*resampler_audio_data_[0]) * number_of_frames);
+}
+
+}  // namespace media