Collapse AudioRendererMixer and OnMoreDataResampler into AudioTransform.

media/audio/audio_output_resampler.cc

Index: media/audio/audio_output_resampler.cc
diff --git a/media/audio/audio_output_resampler.cc b/media/audio/audio_output_resampler.cc
index 75b703b2fc62630a63598719e0cf8b8fb52ae913..6e3ef5eda879518928379acd9f014a4e3c70b880 100644
--- a/media/audio/audio_output_resampler.cc
+++ b/media/audio/audio_output_resampler.cc
@@ -16,20 +16,19 @@
 #include "media/audio/audio_output_proxy.h"
 #include "media/audio/audio_util.h"
 #include "media/audio/sample_rates.h"
-#include "media/base/audio_pull_fifo.h"
-#include "media/base/channel_mixer.h"
+#include "media/base/audio_transform.h"
 #include "media/base/limits.h"
 #include "media/base/media_switches.h"
-#include "media/base/multi_channel_resampler.h"
 
 namespace media {
 
-class OnMoreDataResampler : public AudioOutputStream::AudioSourceCallback {
+class OnMoreDataTransform
+    : public AudioOutputStream::AudioSourceCallback,
+      public AudioTransform::AudioTransformInput {
  public:
-  OnMoreDataResampler(double io_ratio,
-                      const AudioParameters& input_params,
+  OnMoreDataTransform(const AudioParameters& input_params,
                       const AudioParameters& output_params);
-  virtual ~OnMoreDataResampler();
+  virtual ~OnMoreDataTransform();
 
   // AudioSourceCallback interface.
   virtual int OnMoreData(AudioBus* dest,
@@ -48,15 +47,9 @@ class OnMoreDataResampler : public AudioOutputStream::AudioSourceCallback {
   void Stop();
 
  private:
-  // Called by MultiChannelResampler when more data is necessary.
-  void ProvideInput(AudioBus* audio_bus);
-
-  // Called by AudioPullFifo when more data is necessary.  Requires
-  // |source_lock_| to have been acquired.
-  void SourceCallback_Locked(AudioBus* audio_bus);
-
-  // Passes through |source| to the |source_callback_| OnMoreIOData() call.
-  void SourceIOCallback_Locked(AudioBus* source, AudioBus* dest);
+  // AudioTransform::ProvideAudioTransformInput implementation.

[Chris Rogers, 2012/11/14 23:50:49]

class name is wrong

[DaleCurtis, 2012/11/16 23:51:05]

Done.

+  virtual float ProvideAudioTransformInput(
+      AudioBus* audio_bus, base::TimeDelta buffer_delay) OVERRIDE;
 
   // Ratio of input bytes to output bytes used to correct playback delay with
   // regard to buffering and resampling.
@@ -66,36 +59,20 @@ class OnMoreDataResampler : public AudioOutputStream::AudioSourceCallback {
   base::Lock source_lock_;
   AudioOutputStream::AudioSourceCallback* source_callback_;
 
+  // |source| passed to OnMoreIOData() which should be passed downstream.
+  AudioBus* source_bus_;
+
   // Last AudioBuffersState object received via OnMoreData(), used to correct
   // playback delay by ProvideInput() and passed on to |source_callback_|.
   AudioBuffersState current_buffers_state_;
 
-  // Total number of bytes (in terms of output parameters) stored in resampler
-  // or FIFO buffers which have not been sent to the audio device.
-  int outstanding_audio_bytes_;
-
-  // Used to buffer data between the client and the output device in cases where
-  // the client buffer size is not the same as the output device buffer size.
-  // Bound to SourceCallback_Locked() so must only be used when |source_lock_|
-  // has already been acquired.
-  scoped_ptr<AudioPullFifo> audio_fifo_;
-
-  // Handles resampling.
-  scoped_ptr<MultiChannelResampler> resampler_;
+  int input_bytes_per_second_;

[miu, 2012/11/12 20:51:59]

nit: could be "const int"

[DaleCurtis, 2012/11/16 23:51:05]

Done.

 
-  // Handles channel transforms.  |unmixed_audio_| is a temporary destination
-  // for audio data before it goes into the channel mixer.
-  scoped_ptr<ChannelMixer> channel_mixer_;
-  scoped_ptr<AudioBus> unmixed_audio_;
+  // Handles resampling, rebuffering, and channel mixing between input and

[Chris Rogers, 2012/11/14 23:50:49]

rebuffering?  How about just "buffering"

[DaleCurtis, 2012/11/16 23:51:05]

Done.

+  // output parameters.
+  AudioTransform audio_transform_;
 
-  int output_bytes_per_frame_;
-  int input_bytes_per_frame_;
-
-  // Since resampling is expensive, figure out if we should downmix channels
-  // before resampling.
-  bool downmix_early_;
-
-  DISALLOW_COPY_AND_ASSIGN(OnMoreDataResampler);
+  DISALLOW_COPY_AND_ASSIGN(OnMoreDataTransform);
 };
 
 // Record UMA statistics for hardware output configuration.
@@ -170,7 +147,6 @@ AudioOutputResampler::AudioOutputResampler(AudioManager* audio_manager,
                                            const AudioParameters& output_params,
                                            const base::TimeDelta& close_delay)
     : AudioOutputDispatcher(audio_manager, input_params),
-      io_ratio_(1),
       close_delay_(close_delay),
       output_params_(output_params),
       streams_opened_(false) {
@@ -191,37 +167,6 @@ AudioOutputResampler::~AudioOutputResampler() {
 void AudioOutputResampler::Initialize() {
   DCHECK(!streams_opened_);
   DCHECK(callbacks_.empty());
-
-  io_ratio_ = 1;
-
-  // Only resample or rebuffer if the input parameters don't match the output
-  // parameters to avoid any unnecessary work.
-  if (params_.channels() != output_params_.channels() ||
-      params_.sample_rate() != output_params_.sample_rate() ||
-      params_.bits_per_sample() != output_params_.bits_per_sample() ||
-      params_.frames_per_buffer() != output_params_.frames_per_buffer()) {
-    if (params_.sample_rate() != output_params_.sample_rate()) {
-      double io_sample_rate_ratio = params_.sample_rate() /
-          static_cast<double>(output_params_.sample_rate());
-      // Include the I/O resampling ratio in our global I/O ratio.
-      io_ratio_ *= io_sample_rate_ratio;
-    }
-
-    // Include bits per channel differences.
-    io_ratio_ *= static_cast<double>(params_.bits_per_sample()) /
-        output_params_.bits_per_sample();
-
-    // Include channel count differences.
-    io_ratio_ *= static_cast<double>(params_.channels()) /
-        output_params_.channels();
-
-    DVLOG(1) << "I/O ratio is " << io_ratio_;
-  } else {
-    DVLOG(1) << "Input and output params are the same; in pass-through mode.";
-  }
-
-  // TODO(dalecurtis): All this code should be merged into AudioOutputMixer once
-  // we've stabilized the issues there.
   dispatcher_ = new AudioOutputDispatcherImpl(
       audio_manager_, output_params_, close_delay_);
 }
@@ -274,11 +219,10 @@ bool AudioOutputResampler::StartStream(
     AudioOutputProxy* stream_proxy) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
 
-  OnMoreDataResampler* resampler_callback = NULL;
+  OnMoreDataTransform* resampler_callback = NULL;
   CallbackMap::iterator it = callbacks_.find(stream_proxy);
   if (it == callbacks_.end()) {
-    resampler_callback = new OnMoreDataResampler(
-        io_ratio_, params_, output_params_);
+    resampler_callback = new OnMoreDataTransform(params_, output_params_);
     callbacks_[stream_proxy] = resampler_callback;
   } else {
     resampler_callback = it->second;
@@ -290,6 +234,8 @@ bool AudioOutputResampler::StartStream(
 void AudioOutputResampler::StreamVolumeSet(AudioOutputProxy* stream_proxy,
                                            double volume) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
+  // TODO(dalecurtis): Remove this and instead perform volume adjustment during
+  // the OnMoreDataTransform::ProvideInput_Locked() call.

[Chris Rogers, 2012/11/14 23:50:49]

Not that we need to change anything for this CL, but don't we want to get rid of
SetVolume?  I really hoped we could get rid of the volume and pause stuff on the
browser-side, since that was just an artifact of older <audio> implementation.

[DaleCurtis, 2012/11/15 00:30:59]

Hmmm, is <audio> really the only person using stream volume adjustment? I guess
I thought WebAudio/WebRTC were passing on renderer side volume information too.

If that's the case, this TODO can just be dropped.  I'll also add a
vector_math::FAC() method in my optimization pass to skip the multiply step if
we're not going to be handling volume adjustment in most cases.

   dispatcher_->StreamVolumeSet(stream_proxy, volume);
 }
 
@@ -299,7 +245,7 @@ void AudioOutputResampler::StopStream(AudioOutputProxy* stream_proxy) {
 
   // Now that StopStream() has completed the underlying physical stream should
   // be stopped and no longer calling OnMoreData(), making it safe to Stop() the
-  // OnMoreDataResampler.
+  // OnMoreDataTransform.
   CallbackMap::iterator it = callbacks_.find(stream_proxy);
   if (it != callbacks_.end())
     it->second->Stop();
@@ -310,7 +256,7 @@ void AudioOutputResampler::CloseStream(AudioOutputProxy* stream_proxy) {
   dispatcher_->CloseStream(stream_proxy);
 
   // We assume that StopStream() is always called prior to CloseStream(), so
-  // that it is safe to delete the OnMoreDataResampler here.
+  // that it is safe to delete the OnMoreDataTransform here.
   CallbackMap::iterator it = callbacks_.find(stream_proxy);
   if (it != callbacks_.end()) {
     delete it->second;
@@ -329,95 +275,39 @@ void AudioOutputResampler::Shutdown() {
   DCHECK(callbacks_.empty());
 }
 
-OnMoreDataResampler::OnMoreDataResampler(
-    double io_ratio, const AudioParameters& input_params,
-    const AudioParameters& output_params)
-    : io_ratio_(io_ratio),
-      source_callback_(NULL),
-      outstanding_audio_bytes_(0),
-      output_bytes_per_frame_(output_params.GetBytesPerFrame()),
-      input_bytes_per_frame_(input_params.GetBytesPerFrame()),
-      downmix_early_(false) {
-  // Handle different input and output channel layouts.
-  if (input_params.channel_layout() != output_params.channel_layout()) {
-    DVLOG(1) << "Remixing channel layout from " << input_params.channel_layout()
-             << " to " << output_params.channel_layout() << "; from "
-             << input_params.channels() << " channels to "
-             << output_params.channels() << " channels.";
-    channel_mixer_.reset(new ChannelMixer(
-        input_params.channel_layout(), output_params.channel_layout()));
-
-    // Pare off data as early as we can for efficiency.
-    downmix_early_ = input_params.channels() > output_params.channels();
-    if (downmix_early_) {
-      DVLOG(1) << "Remixing channel layout prior to resampling.";
-      // If we're downmixing early we need a temporary AudioBus which matches
-      // the the input channel count and input frame size since we're passing
-      // |unmixed_audio_| directly to the |source_callback_|.
-      unmixed_audio_ = AudioBus::Create(input_params);
-    } else {
-      // Instead, if we're not downmixing early we need a temporary AudioBus
-      // which matches the input channel count but uses the output frame size
-      // since we'll mix into the AudioBus from the output stream.
-      unmixed_audio_ = AudioBus::Create(
-          input_params.channels(), output_params.frames_per_buffer());
-    }
-  }
-
-  // Only resample if necessary since it's expensive.
-  if (input_params.sample_rate() != output_params.sample_rate()) {
-    DVLOG(1) << "Resampling from " << input_params.sample_rate() << " to "
-             << output_params.sample_rate();
-    double io_sample_rate_ratio = input_params.sample_rate() /
-        static_cast<double>(output_params.sample_rate());
-    resampler_.reset(new MultiChannelResampler(
-        downmix_early_ ? output_params.channels() :
-            input_params.channels(),
-        io_sample_rate_ratio, base::Bind(
-            &OnMoreDataResampler::ProvideInput, base::Unretained(this))));
-  }
-
-  // Since the resampler / output device may want a different buffer size than
-  // the caller asked for, we need to use a FIFO to ensure that both sides
-  // read in chunk sizes they're configured for.
-  if (input_params.sample_rate() != output_params.sample_rate() ||
-      input_params.frames_per_buffer() != output_params.frames_per_buffer()) {
-    DVLOG(1) << "Rebuffering from " << input_params.frames_per_buffer()
-             << " to " << output_params.frames_per_buffer();
-    audio_fifo_.reset(new AudioPullFifo(
-        downmix_early_ ? output_params.channels() :
-            input_params.channels(),
-        input_params.frames_per_buffer(), base::Bind(
-            &OnMoreDataResampler::SourceCallback_Locked,
-            base::Unretained(this))));
-  }
+OnMoreDataTransform::OnMoreDataTransform(const AudioParameters& input_params,
+                                         const AudioParameters& output_params)
+    : source_callback_(NULL),
+      source_bus_(NULL),
+      input_bytes_per_second_(input_params.GetBytesPerSecond()),
+      audio_transform_(input_params, output_params) {
+  io_ratio_ = static_cast<double>(

[miu, 2012/11/12 20:51:59]

The indentation here makes it look like you're casting the entire RHS to double.
 Also, instead of multiplying, why not use params.GetBytesPerSecond()?

[DaleCurtis, 2012/11/16 23:51:05]

Done.

+      input_params.sample_rate() * input_params.GetBytesPerFrame()) /
+      (output_params.sample_rate() * output_params.GetBytesPerFrame());

[Chris Rogers, 2012/11/14 23:50:49]

Not that we have to change this right now, but can we at least add a TODO here
that we want to get rid of relying on |input_bytes_per_second_|
(bytes-per-frame) at this level of the audio stack?  After all, both the input
and output parameters are dealing with AudioBus's with float32 data, regardless
of any wacky legacy info we are still propagating about bit-depth in the
AudioParameters

Instead we should always be dealing with number of frames at canonical float32

[DaleCurtis, 2012/11/15 00:30:59]

Yes I think that's a good idea, we need to stop passing AudioBufferState() in
bytes before that can happen though.

 }
 
-OnMoreDataResampler::~OnMoreDataResampler() {}
+OnMoreDataTransform::~OnMoreDataTransform() {}
 
-void OnMoreDataResampler::Start(
+void OnMoreDataTransform::Start(
     AudioOutputStream::AudioSourceCallback* callback) {
   base::AutoLock auto_lock(source_lock_);
   DCHECK(!source_callback_);
   source_callback_ = callback;
+  audio_transform_.AddInput(this);

[Chris Rogers, 2012/11/14 23:50:49]

It seems like AudioTransform is capable of doing mixing (multiple inputs) but
we're just using it in a simple way right now with one single input.  It's
probably worth a comment right here about this.

[DaleCurtis, 2012/11/16 23:51:05]

Done.

 }
 
-void OnMoreDataResampler::Stop() {
+void OnMoreDataTransform::Stop() {
   base::AutoLock auto_lock(source_lock_);
   source_callback_ = NULL;
-  outstanding_audio_bytes_ = 0;
-  if (audio_fifo_)
-    audio_fifo_->Clear();
-  if (resampler_)
-    resampler_->Flush();
+  audio_transform_.RemoveInput(this);
 }
 
-int OnMoreDataResampler::OnMoreData(AudioBus* dest,
+int OnMoreDataTransform::OnMoreData(AudioBus* dest,
                                     AudioBuffersState buffers_state) {
   return OnMoreIOData(NULL, dest, buffers_state);
 }
 
-int OnMoreDataResampler::OnMoreIOData(AudioBus* source,
+int OnMoreDataTransform::OnMoreIOData(AudioBus* source,
                                       AudioBus* dest,
                                       AudioBuffersState buffers_state) {
   base::AutoLock auto_lock(source_lock_);
@@ -427,91 +317,47 @@ int OnMoreDataResampler::OnMoreIOData(AudioBus* source,
     return dest->frames();
   }
 
+  source_bus_ = source;
   current_buffers_state_ = buffers_state;
+  audio_transform_.Transform(dest);
 
-  bool needs_mixing = channel_mixer_ && !downmix_early_;
-  AudioBus* temp_dest = needs_mixing ? unmixed_audio_.get() : dest;
-
-  if (!resampler_ && !audio_fifo_) {
-    // We have no internal buffers, so clear any outstanding audio data.
-    outstanding_audio_bytes_ = 0;
-    SourceIOCallback_Locked(source, temp_dest);
-  } else {
-    if (resampler_)
-      resampler_->Resample(temp_dest, temp_dest->frames());
-    else
-      ProvideInput(temp_dest);
-
-    // Calculate how much data is left in the internal FIFO and resampler.
-    outstanding_audio_bytes_ -= temp_dest->frames() * output_bytes_per_frame_;
-  }
-
-  if (needs_mixing) {
-    DCHECK_EQ(temp_dest->frames(), dest->frames());
-    channel_mixer_->Transform(temp_dest, dest);
-  }
-
-  // Due to rounding errors while multiplying against |io_ratio_|,
-  // |outstanding_audio_bytes_| might (rarely) slip below zero.
-  if (outstanding_audio_bytes_ < 0) {
-    DLOG(ERROR) << "Outstanding audio bytes went negative! Value: "
-                << outstanding_audio_bytes_;
-    outstanding_audio_bytes_ = 0;
-  }
-
-  // Always return the full number of frames requested, ProvideInput() will pad
-  // with silence if it wasn't able to acquire enough data.
+  // Always return the full number of frames requested, ProvideInput_Locked()
+  // will pad with silence if it wasn't able to acquire enough data.
   return dest->frames();
 }
 
-void OnMoreDataResampler::SourceCallback_Locked(AudioBus* dest) {
-  SourceIOCallback_Locked(NULL, dest);
-}
-
-void OnMoreDataResampler::SourceIOCallback_Locked(AudioBus* source,
-                                                  AudioBus* dest) {
+float OnMoreDataTransform::ProvideAudioTransformInput(
+    AudioBus* dest, base::TimeDelta buffer_delay) {
   source_lock_.AssertAcquired();
 
-  // Adjust playback delay to include the state of the internal buffers used by
-  // the resampler and/or the FIFO.  Since the sample rate and bits per channel
-  // may be different, we need to scale this value appropriately.
+  // Adjust playback delay to include |buffer_delay|.
   AudioBuffersState new_buffers_state;
-  new_buffers_state.pending_bytes = io_ratio_ *
-      (current_buffers_state_.total_bytes() + outstanding_audio_bytes_);
-
-  bool needs_downmix = channel_mixer_ && downmix_early_;
-  AudioBus* temp_dest = needs_downmix ? unmixed_audio_.get() : dest;
+  new_buffers_state.pending_bytes =
+      io_ratio_ * (current_buffers_state_.total_bytes() +
+                   buffer_delay.InSecondsF() * input_bytes_per_second_);
 
-  // Retrieve data from the original callback.  Zero any unfilled frames.
+  // Retrieve data from the original callback.
   int frames = source_callback_->OnMoreIOData(
-      source, temp_dest, new_buffers_state);
-  if (frames < temp_dest->frames())
-    temp_dest->ZeroFramesPartial(frames, temp_dest->frames() - frames);
-
-  // Scale the number of frames we got back in terms of input bytes to output
-  // bytes accordingly.
-  outstanding_audio_bytes_ +=
-      (temp_dest->frames() * input_bytes_per_frame_) / io_ratio_;
-
-  if (needs_downmix) {
-    DCHECK_EQ(temp_dest->frames(), dest->frames());
-    channel_mixer_->Transform(temp_dest, dest);
-  }
-}
+      source_bus_, dest, new_buffers_state);
 

[Chris Rogers, 2012/11/14 23:50:49]

If ProvideAudioTransformInput() is called multiple times (if there's a FIFO or
sample-rate conversion) then we'll call OnMoreIOData() multiple times with the
same |source_bus|.  We don't have to solve this problem right now, but I think
we need to add a warning comment here that the "synchronized" I/O needs more
work and that our use of |source_bus_| here is not quite fully baked.

[DaleCurtis, 2012/11/15 00:30:59]

Hmmm, should I just NULL the source_bus_ after the first call?

-void OnMoreDataResampler::ProvideInput(AudioBus* audio_bus) {
-  audio_fifo_->Consume(audio_bus, audio_bus->frames());
+  // Zero any unfilled frames if anything was filled, otherwise we'll just
+  // return a volume of zero and let AudioTransform drop the output.
+  if (frames > 0 && frames < dest->frames())
+    dest->ZeroFramesPartial(frames, dest->frames() - frames);
+
+  // TODO(dalecurtis): Return the correct volume here.
+  return frames > 0 ? 1 : 0;
 }
 
-void OnMoreDataResampler::OnError(AudioOutputStream* stream, int code) {
+void OnMoreDataTransform::OnError(AudioOutputStream* stream, int code) {
   base::AutoLock auto_lock(source_lock_);
   if (source_callback_)
     source_callback_->OnError(stream, code);
 }
 
-void OnMoreDataResampler::WaitTillDataReady() {
+void OnMoreDataTransform::WaitTillDataReady() {
   base::AutoLock auto_lock(source_lock_);
-  if (source_callback_ && !outstanding_audio_bytes_)
+  if (source_callback_)
     source_callback_->WaitTillDataReady();
 }