Simplify AudioRendererImpl by using AudioDevice.

content/renderer/media/audio_renderer_impl.cc

 // Copyright (c) 2011 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 "content/renderer/media/audio_renderer_impl.h"
 
 #include <math.h>
 
 #include <algorithm>
 
 #include "base/bind.h"
-#include "base/command_line.h"
 #include "content/common/child_process.h"
 #include "content/common/media/audio_messages.h"
-#include "content/public/common/content_switches.h"
 #include "content/renderer/render_thread_impl.h"
 #include "media/audio/audio_buffers_state.h"
-#include "media/audio/audio_output_controller.h"
 #include "media/audio/audio_util.h"
-#include "media/base/filter_host.h"
 
-// Static variable that says what code path we are using -- low or high
-// latency. Made separate variable so we don't have to go to command line
-// for every DCHECK().
-AudioRendererImpl::LatencyType AudioRendererImpl::latency_type_ =
-    AudioRendererImpl::kUninitializedLatency;
+const size_t kBufferSize = 2048;

[acolwell GONE FROM CHROMIUM, 2011/11/08 21:44:20]

Add a comment about the reasoning for this magic number value.

[Chris Rogers, 2011/11/10 02:17:22]

Added GetBufferSizeForSampleRate() method and added comments there.

On 2011/11/08 21:44:20, acolwell wrote:

 
 AudioRendererImpl::AudioRendererImpl()
     : AudioRendererBase(),
       bytes_per_second_(0),
-      stream_id_(0),
-      shared_memory_(NULL),
-      shared_memory_size_(0),
-      stopped_(false),
-      pending_request_(false) {
-  filter_ = RenderThreadImpl::current()->audio_message_filter();
-  // Figure out if we are planning to use high or low latency code path.
-  // We are initializing only one variable and double initialization is Ok,
-  // so there would not be any issues caused by CPU memory model.
-  if (latency_type_ == kUninitializedLatency) {
-    // Urgent workaround for
-    // http://code.google.com/p/chromium-os/issues/detail?id=21491
-    // TODO(enal): Fix it properly.
-#if defined(OS_CHROMEOS)
-    latency_type_ = kHighLatency;
-#else
-    if (!CommandLine::ForCurrentProcess()->HasSwitch(
-        switches::kHighLatencyAudio)) {
-      latency_type_ = kLowLatency;
-    } else {
-      latency_type_ = kHighLatency;
-    }
-#endif
-  }
+      stopped_(false) {
+  // The AudioDevice is completely initialized when we first know

[henrika (OOO until Aug 14), 2011/11/09 17:05:22]

Is it possible to make this comment more clear? Two 'when' in one sentence is
too much for me ;-)

[Chris Rogers, 2011/11/10 02:17:22]

Added what I hope is a better comment here.

On 2011/11/09 17:05:22, henrika1 wrote:

+  // the audio format when OnInitialize() is called.
+  audio_device_ = new AudioDevice(this);
 }
 
 AudioRendererImpl::~AudioRendererImpl() {
 }
 
-// static
-void AudioRendererImpl::set_latency_type(LatencyType latency_type) {
-  DCHECK_EQ(kUninitializedLatency, latency_type_);
-  latency_type_ = latency_type;
-}
-
 base::TimeDelta AudioRendererImpl::ConvertToDuration(int bytes) {
   if (bytes_per_second_) {
     return base::TimeDelta::FromMicroseconds(
         base::Time::kMicrosecondsPerSecond * bytes / bytes_per_second_);
   }
   return base::TimeDelta();
 }
 
 void AudioRendererImpl::UpdateEarliestEndTime(int bytes_filled,
                                               base::TimeDelta request_delay,
                                               base::Time time_now) {
   if (bytes_filled != 0) {
     base::TimeDelta predicted_play_time = ConvertToDuration(bytes_filled);
     float playback_rate = GetPlaybackRate();
     if (playback_rate != 1.0f) {
       predicted_play_time = base::TimeDelta::FromMicroseconds(
           static_cast<int64>(ceil(predicted_play_time.InMicroseconds() *
                                   playback_rate)));
     }
     earliest_end_time_ =
         std::max(earliest_end_time_,
                  time_now + request_delay + predicted_play_time);
   }
 }
 
 bool AudioRendererImpl::OnInitialize(int bits_per_channel,
                                      ChannelLayout channel_layout,
                                      int sample_rate) {
-  AudioParameters params(AudioParameters::AUDIO_PCM_LINEAR, channel_layout,
-                         sample_rate, bits_per_channel, 0);
+  // We use the AUDIO_PCM_LINEAR flag because AUDIO_PCM_LOW_LATENCY
+  // does not currently support all the sample-rates that we require.

[scherkus (not reviewing), 2011/11/09 02:39:05]

do we have a bug tracking this work?

[Chris Rogers, 2011/11/10 02:17:22]

Yes, good to track this.  I've created bug:
http://code.google.com/p/chromium/issues/detail?id=103627

and added this link in the comment itself.

We could consider this a sort of "meta-bug" or technical note, where other more
specific bugs can be written to address this issue.

On 2011/11/09 02:39:05, scherkus wrote:

[henrika (OOO until Aug 14), 2011/11/10 11:45:07]

Great summary.

+  audio_parameters_ = AudioParameters(AudioParameters::AUDIO_PCM_LINEAR,
+                                      channel_layout,
+                                      sample_rate,
+                                      bits_per_channel,
+                                      0);
 
-  bytes_per_second_ = params.GetBytesPerSecond();
+  bytes_per_second_ = audio_parameters_.GetBytesPerSecond();
 
-  ChildProcess::current()->io_message_loop()->PostTask(
-      FROM_HERE,
-      base::Bind(&AudioRendererImpl::CreateStreamTask, this, params));
+  if (audio_device_.get() && !audio_device_->IsInitialized()) {
+    audio_device_->Initialize(
+        kBufferSize,
+        audio_parameters_.channels,
+        audio_parameters_.sample_rate,
+        audio_parameters_.format);
+
+    audio_device_->Start();
+  }
+
   return true;
 }
 
 void AudioRendererImpl::OnStop() {
-  // Since joining with the audio thread can acquire lock_, we make sure to
-  // Join() with it not under lock.
-  base::DelegateSimpleThread* audio_thread = NULL;
-  {
-    base::AutoLock auto_lock(lock_);
-    if (stopped_)
-      return;
-    stopped_ = true;
+  if (stopped_)
+    return;
 
-    DCHECK_EQ(!audio_thread_.get(), !socket_.get());
-    if (socket_.get())
-      socket_->Close();
-    if (audio_thread_.get())
-      audio_thread = audio_thread_.get();
-
-    ChildProcess::current()->io_message_loop()->PostTask(
-        FROM_HERE,
-        base::Bind(&AudioRendererImpl::DestroyTask, this));
+  if (audio_device_.get()) {
+    audio_device_->Stop();
+    audio_device_ = NULL;
   }
-
-  if (audio_thread)
-    audio_thread->Join();
-}
-
-void AudioRendererImpl::NotifyDataAvailableIfNecessary() {
-  if (latency_type_ == kHighLatency) {
-    // Post a task to render thread to notify a packet reception.
-    ChildProcess::current()->io_message_loop()->PostTask(
-        FROM_HERE,
-        base::Bind(&AudioRendererImpl::NotifyPacketReadyTask, this));
-  }
+  stopped_ = true;
 }
 
 void AudioRendererImpl::ConsumeAudioSamples(
     scoped_refptr<media::Buffer> buffer_in) {
-  base::AutoLock auto_lock(lock_);
   if (stopped_)
     return;
 
   // TODO(hclam): handle end of stream here.
 
   // Use the base class to queue the buffer.
   AudioRendererBase::ConsumeAudioSamples(buffer_in);
-
-  NotifyDataAvailableIfNecessary();
 }
 
 void AudioRendererImpl::SetPlaybackRate(float rate) {
   DCHECK_LE(0.0f, rate);
 
-  base::AutoLock auto_lock(lock_);
   // Handle the case where we stopped due to IO message loop dying.
   if (stopped_) {
     AudioRendererBase::SetPlaybackRate(rate);
     return;
   }
 
   // We have two cases here:
   // Play: GetPlaybackRate() == 0.0 && rate != 0.0
   // Pause: GetPlaybackRate() != 0.0 && rate == 0.0
   if (GetPlaybackRate() == 0.0f && rate != 0.0f) {
-    ChildProcess::current()->io_message_loop()->PostTask(
-        FROM_HERE,
-        base::Bind(&AudioRendererImpl::PlayTask, this));
+    DoPlay();
   } else if (GetPlaybackRate() != 0.0f && rate == 0.0f) {
     // Pause is easy, we can always pause.
-    ChildProcess::current()->io_message_loop()->PostTask(
-        FROM_HERE,
-        base::Bind(&AudioRendererImpl::PauseTask, this));
+    DoPause();
   }
   AudioRendererBase::SetPlaybackRate(rate);
-
-  // If we are playing, give a kick to try fulfilling the packet request as
-  // the previous packet request may be stalled by a pause.
-  if (rate > 0.0f) {
-    NotifyDataAvailableIfNecessary();
-  }
 }
 
 void AudioRendererImpl::Pause(const base::Closure& callback) {
   AudioRendererBase::Pause(callback);
-  base::AutoLock auto_lock(lock_);
   if (stopped_)
     return;
 
-  ChildProcess::current()->io_message_loop()->PostTask(
-      FROM_HERE,
-      base::Bind(&AudioRendererImpl::PauseTask, this));
+  DoPause();
 }
 
 void AudioRendererImpl::Seek(base::TimeDelta time,
                              const media::FilterStatusCB& cb) {
   AudioRendererBase::Seek(time, cb);
-  base::AutoLock auto_lock(lock_);
   if (stopped_)
     return;
 
-  ChildProcess::current()->io_message_loop()->PostTask(
-      FROM_HERE,
-      base::Bind(&AudioRendererImpl::SeekTask, this));
+  DoSeek();
 }
 
-
 void AudioRendererImpl::Play(const base::Closure& callback) {
   AudioRendererBase::Play(callback);
-  base::AutoLock auto_lock(lock_);
   if (stopped_)
     return;
 
   if (GetPlaybackRate() != 0.0f) {
-    ChildProcess::current()->io_message_loop()->PostTask(
-        FROM_HERE,
-        base::Bind(&AudioRendererImpl::PlayTask, this));
+    DoPlay();
   } else {
-    ChildProcess::current()->io_message_loop()->PostTask(
-        FROM_HERE,
-        base::Bind(&AudioRendererImpl::PauseTask, this));
+    DoPause();
   }
 }
 
 void AudioRendererImpl::SetVolume(float volume) {
-  base::AutoLock auto_lock(lock_);
   if (stopped_)
     return;
-  ChildProcess::current()->io_message_loop()->PostTask(
-      FROM_HERE,
-      base::Bind(&AudioRendererImpl::SetVolumeTask, this, volume));
+  if (audio_device_.get())
+    audio_device_->SetVolume(volume);
 }
 
-void AudioRendererImpl::OnCreated(base::SharedMemoryHandle handle,
-                                  uint32 length) {
-  DCHECK(MessageLoop::current() == ChildProcess::current()->io_message_loop());
-  DCHECK_EQ(kHighLatency, latency_type_);
-
-  base::AutoLock auto_lock(lock_);
-  if (stopped_)
-    return;
-
-  shared_memory_.reset(new base::SharedMemory(handle, false));
-  shared_memory_->Map(length);
-  shared_memory_size_ = length;
+void AudioRendererImpl::DoPlay() {
+  earliest_end_time_ = base::Time::Now();
+  if (audio_device_.get())

[acolwell GONE FROM CHROMIUM, 2011/11/08 21:44:20]

Shouldn't this be a DCHECK?

[Chris Rogers, 2011/11/10 02:17:22]

Done.

+    audio_device_->Play();
 }
 
-void AudioRendererImpl::CreateSocket(base::SyncSocket::Handle socket_handle) {
-  DCHECK_EQ(kLowLatency, latency_type_);
-#if defined(OS_WIN)
-  DCHECK(socket_handle);
-#else
-  DCHECK_GE(socket_handle, 0);
-#endif
-  socket_.reset(new base::SyncSocket(socket_handle));
+void AudioRendererImpl::DoPause() {
+  if (audio_device_.get())

[acolwell GONE FROM CHROMIUM, 2011/11/08 21:44:20]

DCHECK

[Chris Rogers, 2011/11/10 02:17:22]

Done.

+    audio_device_->Pause(false);
 }
 
-void AudioRendererImpl::CreateAudioThread() {
-  DCHECK_EQ(kLowLatency, latency_type_);
-  audio_thread_.reset(
-      new base::DelegateSimpleThread(this, "renderer_audio_thread"));
-  audio_thread_->Start();
-}
+void AudioRendererImpl::DoSeek() {
+  earliest_end_time_ = base::Time::Now();
 
-void AudioRendererImpl::OnLowLatencyCreated(
-    base::SharedMemoryHandle handle,
-    base::SyncSocket::Handle socket_handle,
-    uint32 length) {
-  DCHECK(MessageLoop::current() == ChildProcess::current()->io_message_loop());
-  DCHECK_EQ(kLowLatency, latency_type_);
-#if defined(OS_WIN)
-  DCHECK(handle);
-#else
-  DCHECK_GE(handle.fd, 0);
-#endif
-  DCHECK_NE(0u, length);
-
-  base::AutoLock auto_lock(lock_);
-  if (stopped_)
-    return;
-
-  shared_memory_.reset(new base::SharedMemory(handle, false));
-  shared_memory_->Map(media::TotalSharedMemorySizeInBytes(length));
-  shared_memory_size_ = length;
-
-  CreateSocket(socket_handle);
-  CreateAudioThread();
-}
-
-void AudioRendererImpl::OnRequestPacket(AudioBuffersState buffers_state) {
-  DCHECK(MessageLoop::current() == ChildProcess::current()->io_message_loop());
-  DCHECK_EQ(kHighLatency, latency_type_);
-  {
-    base::AutoLock auto_lock(lock_);
-    DCHECK(!pending_request_);
-    pending_request_ = true;
-    request_buffers_state_ = buffers_state;
-  }
-
-  // Try to fill in the fulfill the packet request.
-  NotifyPacketReadyTask();
-}
-
-void AudioRendererImpl::OnStateChanged(AudioStreamState state) {
-  DCHECK(MessageLoop::current() == ChildProcess::current()->io_message_loop());
-
-  base::AutoLock auto_lock(lock_);
-  if (stopped_)
-    return;
-
-  switch (state) {
-    case kAudioStreamError:
-      // We receive this error if we counter an hardware error on the browser
-      // side. We can proceed with ignoring the audio stream.
-      // TODO(hclam): We need more handling of these kind of error. For example
-      // re-try creating the audio output stream on the browser side or fail
-      // nicely and report to demuxer that the whole audio stream is discarded.
-      host()->DisableAudioRenderer();
-      break;
-    // TODO(hclam): handle these events.
-    case kAudioStreamPlaying:
-    case kAudioStreamPaused:
-      break;
-    default:
-      NOTREACHED();
-      break;
-  }
-}
-
-void AudioRendererImpl::OnVolume(double volume) {
-  // TODO(hclam): decide whether we need to report the current volume to
-  // pipeline.
-}
-
-void AudioRendererImpl::CreateStreamTask(const AudioParameters& audio_params) {
-  DCHECK(MessageLoop::current() == ChildProcess::current()->io_message_loop());
-
-  base::AutoLock auto_lock(lock_);
-  if (stopped_)
-    return;
-
-  // Make sure we don't call create more than once.
-  DCHECK_EQ(0, stream_id_);
-  stream_id_ = filter_->AddDelegate(this);
-  ChildProcess::current()->io_message_loop()->AddDestructionObserver(this);
-
-  AudioParameters params_to_send(audio_params);
-  // Let the browser choose packet size.
-  params_to_send.samples_per_packet = 0;
-
-  Send(new AudioHostMsg_CreateStream(stream_id_,
-                                     params_to_send,
-                                     latency_type_ == kLowLatency));
-}
-
-void AudioRendererImpl::PlayTask() {
-  DCHECK(MessageLoop::current() == ChildProcess::current()->io_message_loop());
-
-  earliest_end_time_ = base::Time::Now();
-  Send(new AudioHostMsg_PlayStream(stream_id_));
-}
-
-void AudioRendererImpl::PauseTask() {
-  DCHECK(MessageLoop::current() == ChildProcess::current()->io_message_loop());
-
-  Send(new AudioHostMsg_PauseStream(stream_id_));
-}
-
-void AudioRendererImpl::SeekTask() {
-  DCHECK(MessageLoop::current() == ChildProcess::current()->io_message_loop());
-
-  earliest_end_time_ = base::Time::Now();
-  // We have to pause the audio stream before we can flush.
-  Send(new AudioHostMsg_PauseStream(stream_id_));
-  Send(new AudioHostMsg_FlushStream(stream_id_));
-}
-
-void AudioRendererImpl::DestroyTask() {
-  DCHECK(MessageLoop::current() == ChildProcess::current()->io_message_loop());
-
-  // Make sure we don't call destroy more than once.
-  DCHECK_NE(0, stream_id_);
-  filter_->RemoveDelegate(stream_id_);
-  Send(new AudioHostMsg_CloseStream(stream_id_));
-  // During shutdown this may be NULL; don't worry about deregistering in that
-  // case.
-  if (ChildProcess::current())
-    ChildProcess::current()->io_message_loop()->RemoveDestructionObserver(this);
-  stream_id_ = 0;
-}
-
-void AudioRendererImpl::SetVolumeTask(double volume) {
-  DCHECK(MessageLoop::current() == ChildProcess::current()->io_message_loop());
-
-  base::AutoLock auto_lock(lock_);
-  if (stopped_)
-    return;
-  Send(new AudioHostMsg_SetVolume(stream_id_, volume));
-}
-
-void AudioRendererImpl::NotifyPacketReadyTask() {
-  DCHECK(MessageLoop::current() == ChildProcess::current()->io_message_loop());
-  DCHECK_EQ(kHighLatency, latency_type_);
-
-  base::AutoLock auto_lock(lock_);
-  if (stopped_)
-    return;
-  if (pending_request_ && GetPlaybackRate() > 0.0f) {
-    DCHECK(shared_memory_.get());
-
-    // Adjust the playback delay.
-    base::Time current_time = base::Time::Now();
-
-    base::TimeDelta request_delay =
-        ConvertToDuration(request_buffers_state_.total_bytes());
-
-    // Add message delivery delay.
-    if (current_time > request_buffers_state_.timestamp) {
-      base::TimeDelta receive_latency =
-          current_time - request_buffers_state_.timestamp;
-
-      // If the receive latency is too much it may offset all the delay.
-      if (receive_latency >= request_delay) {
-        request_delay = base::TimeDelta();
-      } else {
-        request_delay -= receive_latency;
-      }
-    }
-
-    // Finally we need to adjust the delay according to playback rate.
-    if (GetPlaybackRate() != 1.0f) {
-      request_delay = base::TimeDelta::FromMicroseconds(
-          static_cast<int64>(ceil(request_delay.InMicroseconds() *
-                                  GetPlaybackRate())));
-    }
-
-    bool buffer_empty = (request_buffers_state_.pending_bytes == 0) &&
-                        (current_time >= earliest_end_time_);
-
-    // For high latency mode we don't write length into shared memory,
-    // it is explicit part of AudioHostMsg_NotifyPacketReady() message,
-    // so no need to reserve first word of buffer for length.
-    uint32 filled = FillBuffer(static_cast<uint8*>(shared_memory_->memory()),
-                               shared_memory_size_, request_delay,
-                               buffer_empty);
-    UpdateEarliestEndTime(filled, request_delay, current_time);
-    pending_request_ = false;
-
-    // Then tell browser process we are done filling into the buffer.
-    Send(new AudioHostMsg_NotifyPacketReady(stream_id_, filled));
+  if (audio_device_.get()) {

[acolwell GONE FROM CHROMIUM, 2011/11/08 21:44:20]

DCHECK

[Chris Rogers, 2011/11/10 02:17:22]

Done.

+    // Pause and flush the stream when we seek to a new location.
+    audio_device_->Pause(true);
   }
 }
 
 void AudioRendererImpl::WillDestroyCurrentMessageLoop() {
   DCHECK(!ChildProcess::current() ||  // During shutdown.
          (MessageLoop::current() ==
           ChildProcess::current()->io_message_loop()));
 
   // We treat the IO loop going away the same as stopping.
-  base::AutoLock auto_lock(lock_);
+  // base::AutoLock auto_lock(lock_);

[acolwell GONE FROM CHROMIUM, 2011/11/08 21:44:20]

Why isn't this needed anymore? If it isn't needed delete the line.

[Chris Rogers, 2011/11/10 02:17:22]

Good point.  I've removed this lock and the commented out line.  The details of
the IO thread safety issues are now being handled in AudioDevice.  The only last
question I have for you is the thread safety of FillBuffer().  I think we're OK
because AudioDevice::Stop() is synchronous, but can you sanity check this for
me?
On 2011/11/08 21:44:20, acolwell wrote:

   if (stopped_)
     return;
 
   stopped_ = true;
-  DestroyTask();
+
+  // During shutdown this may be NULL; don't worry about deregistering in that
+  // case.
+  if (ChildProcess::current())
+    ChildProcess::current()->io_message_loop()->RemoveDestructionObserver(this);
+
+  if (audio_device_)
+    audio_device_->Stop();

[acolwell GONE FROM CHROMIUM, 2011/11/08 21:44:20]

add audio_device_ = NULL so Stop() can't accidentally get called twice.

[Chris Rogers, 2011/11/10 02:17:22]

Done.

On 2011/11/08 21:44:20, acolwell wrote:
Done.

 }
 
-// Our audio thread runs here. We receive requests for more data and send it
-// on this thread.
-void AudioRendererImpl::Run() {
-  DCHECK_EQ(kLowLatency, latency_type_);
-  audio_thread_->SetThreadPriority(base::kThreadPriority_RealtimeAudio);
+void AudioRendererImpl::Render(const std::vector<float*>& audio_data,
+                               size_t number_of_frames,
+                               size_t audio_delay_milliseconds) {
+  if (stopped_ || GetPlaybackRate() == 0.0f) {
+    // Output silence if stopped.
+    for (size_t i = 0; i < audio_data.size(); ++i)
+        memset(audio_data[i], 0, sizeof(float) * number_of_frames);

[scherkus (not reviewing), 2011/11/09 02:39:05]

de-indent by 2

[Chris Rogers, 2011/11/10 02:17:22]

Done.

+    return;
+  }
 
-  int bytes;
-  while (sizeof(bytes) == socket_->Receive(&bytes, sizeof(bytes))) {
-    if (bytes == media::AudioOutputController::kPauseMark) {
-      // When restarting playback, host should get new data,
-      // not what is currently in the buffer.
-      media::SetActualDataSizeInBytes(shared_memory_.get(),
-                                      shared_memory_size_,
-                                      0);
-      continue;
-    }
-    else if (bytes < 0)
-      break;
-    base::AutoLock auto_lock(lock_);
-    if (stopped_)
-      break;
-    float playback_rate = GetPlaybackRate();
-    if (playback_rate <= 0.0f)
-      continue;
-    DCHECK(shared_memory_.get());
-    base::TimeDelta request_delay = ConvertToDuration(bytes);
+  // Adjust the playback delay.
+  base::Time current_time = base::Time::Now();
 
-    // We need to adjust the delay according to playback rate.
-    if (playback_rate != 1.0f) {
-      request_delay = base::TimeDelta::FromMicroseconds(
-          static_cast<int64>(ceil(request_delay.InMicroseconds() *
-                                  playback_rate)));
-    }
-    base::Time time_now = base::Time::Now();
-    uint32 size = FillBuffer(static_cast<uint8*>(shared_memory_->memory()),
-                             shared_memory_size_,
+  base::TimeDelta request_delay =
+      base::TimeDelta::FromMilliseconds(audio_delay_milliseconds);
+
+  // Finally we need to adjust the delay according to playback rate.
+  if (GetPlaybackRate() != 1.0f) {
+    request_delay = base::TimeDelta::FromMicroseconds(
+        static_cast<int64>(ceil(request_delay.InMicroseconds() *
+                                GetPlaybackRate())));
+  }
+
+  uint32 bytes_per_frame =
+      audio_parameters_.bits_per_sample * audio_parameters_.channels / 8;
+
+  const size_t buf_size = number_of_frames * bytes_per_frame;
+  scoped_array<uint8> buf(new uint8[buf_size]);
+
+  base::Time time_now = base::Time::Now();
+  uint32 filled = FillBuffer(buf.get(),
+                             buf_size,
                              request_delay,
                              time_now >= earliest_end_time_);
-    media::SetActualDataSizeInBytes(shared_memory_.get(),
-                                    shared_memory_size_,
-                                    size);
-    UpdateEarliestEndTime(size, request_delay, time_now);
+  DCHECK_LE(filled, buf_size);
+
+  uint32 filled_frames = filled / bytes_per_frame;
+
+  // Deinterleave each audio channel.
+  int channels = audio_data.size();
+  for (int channelIndex = 0; channelIndex < channels; ++channelIndex) {

[scherkus (not reviewing), 2011/11/09 02:39:05]

channel_index

[Chris Rogers, 2011/11/10 02:17:22]

FIXED - I'm spending too much time in WebKit ;)

On 2011/11/09 02:39:05, scherkus wrote:

+    media::DeinterleaveAudioChannel(buf.get(),

[scherkus (not reviewing), 2011/11/09 02:39:05]

We're decoding to PCM data but audio_data is float -- anything to be concerned
about?

[Chris Rogers, 2011/11/10 02:17:22]

the AudioDevice class takes care of the proper conversion so we're good.

On 2011/11/09 02:39:05, scherkus wrote:

[acolwell GONE FROM CHROMIUM, 2011/11/10 21:58:15]

Have you quantified the performance hit of deinterleaving & then reinterleaving?
It seems unfortunate that this code and WebRTC has to do this just because the
WebAudio code uses deinterleaved channels. Maybe not in this CL, but in a
follow-up one we should remedy this so deinterleaving only happens when the data
goes to WebAudio.
On 2011/11/10 02:17:22, Chris Rogers wrote:

[Chris Rogers, 2011/11/15 22:48:29]

Having spent a few years optimizing code and profiling at the assembly level
using tools like "Shark", I can pretty confidently say that the overhead  here
is very very far down in the noise, especially considering the cost of audio
decoding, and the IPC involved.

The AudioDevice API was designed to abstract away audio format details, and use
a canonical representation (nominal -1 -> +1 non-interleaved floating-point). 
It'll be more and more important to have this abstraction as we do more
renderer-side mixing, sample-rate conversion, and other processing.  We want to
be able to pass higher precision sample data to the browser where the audio
hardware may often have DACS > 16bit.  What if we're playing back a 24bit audio
file?

That said, I totally agree that it seems odd to convert to float here, then back
to int16 in AudioDevice.  So I've added a TODO in AudioDevice about passing the
floating-point data all the way to the browser.

On 2011/11/10 21:58:15, acolwell wrote:

+                                    audio_data[channelIndex],
+                                    channels,
+                                    channelIndex,
+                                    bytes_per_frame / channels,
+                                    filled_frames);
+
+    // If FillBuffer() didn't give us enough data then zero out the remainder.
+    if (filled_frames < number_of_frames) {
+      int frames_to_zero = number_of_frames - filled_frames;
+      memset(audio_data[channelIndex], 0, sizeof(float) * frames_to_zero);
+    }
   }
 }
-
-void AudioRendererImpl::Send(IPC::Message* message) {
-  filter_->Send(message);
-}