Audio software mixer.

media/audio/audio_output_mixer.cc

Index: media/audio/audio_output_mixer.cc
===================================================================
--- media/audio/audio_output_mixer.cc	(revision 0)
+++ media/audio/audio_output_mixer.cc	(revision 0)
@@ -0,0 +1,222 @@
+// 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/audio/audio_output_mixer.h"
+
+#include <algorithm>
+
+#include "base/bind.h"
+#include "base/compiler_specific.h"
+#include "base/message_loop.h"
+#include "base/time.h"
+#include "media/audio/audio_io.h"
+#include "media/audio/audio_output_proxy.h"
+#include "media/audio/audio_util.h"
+
+AudioOutputMixer::AudioOutputMixer(AudioManager* audio_manager,
+                                   const AudioParameters& params,
+                                   base::TimeDelta close_delay)
+    : AudioOutputDispatcher(audio_manager, params),
+      ALLOW_THIS_IN_INITIALIZER_LIST(weak_this_(this)),
+      close_timer_(FROM_HERE,
+                   close_delay,
+                   weak_this_.GetWeakPtr(),
+                   &AudioOutputMixer::ClosePhysicalStream) {
+  // TODO(enal): align data.
+  mixer_data_.reset(new uint8[params_.GetBytesPerBuffer()]);
+}
+
+AudioOutputMixer::~AudioOutputMixer() {
+}
+
+bool AudioOutputMixer::StreamOpened() {
+  DCHECK_EQ(MessageLoop::current(), message_loop_);
+
+  if (physical_stream_.get())
+    return true;
+  AudioOutputStream* stream = audio_manager_->MakeAudioOutputStream(params_);
+  if (!stream)
+    return false;
+  if (!stream->Open()) {
+    stream->Close();
+    return false;
+  }
+  physical_stream_.reset(stream);
+  close_timer_.Reset();
+  return true;
+}
+
+AudioOutputStream* AudioOutputMixer::StreamStarted(
+    AudioOutputStream::AudioSourceCallback* callback,
+    AudioOutputProxy* stream_proxy) {
+  DCHECK_EQ(MessageLoop::current(), message_loop_);
+
+  // May need to re-open the physical stream if no active proxies and
+  // enough time had pass.
+  StreamOpened();
+  if (!physical_stream_.get())
+    return NULL;
+
+  bool should_start = proxies_.empty();
+  {
+    base::AutoLock lock(lock_);
+    proxies_.push_back(stream_proxy);
+  }
+  // We cannot start physical stream under the lock,
+  // OnMoreData() would try acquiring it...
+  if (should_start) {
+    physical_stream_->SetVolume(1.0);
+    physical_stream_->Start(this);
+  }
+  return stream_proxy;
+}
+
+void AudioOutputMixer::StreamStopped(AudioOutputStream* physical_stream,
+                                     AudioOutputProxy* stream_proxy) {
+  DCHECK_EQ(MessageLoop::current(), message_loop_);
+  DCHECK_EQ(stream_proxy, physical_stream);
+
+  // Because of possible deadlock we cannot stop physical stream
+  // under the lock, so acquire the lock, update proxy list, release the lock,
+  // and only then stop physical stream if necessary.
+  bool stop_physical_stream = false;
+  {
+    base::AutoLock lock(lock_);
+    ProxyList::iterator it = std::find(proxies_.begin(),
+                                       proxies_.end(),
+                                       stream_proxy);
+    if (it != proxies_.end()) {
+      proxies_.erase(it);
+      stop_physical_stream = proxies_.empty();
+    }
+  }
+  if (physical_stream_.get()) {
+    if (stop_physical_stream)
+      physical_stream_->Stop();
+    close_timer_.Reset();
+  }
+}
+
+void AudioOutputMixer::StreamVolumeSet(AudioOutputStream* physical_stream,
+                                       double volume) {
+}
+
+void AudioOutputMixer::StreamClosed(AudioOutputProxy* stream_proxy) {
+  DCHECK_EQ(MessageLoop::current(), message_loop_);
+  StreamStopped(stream_proxy, stream_proxy);
+}
+
+void AudioOutputMixer::Shutdown() {
+  DCHECK_EQ(MessageLoop::current(), message_loop_);
+
+  // Cancel any pending tasks to close physical stream.
+  weak_this_.InvalidateWeakPtrs();
+
+  while (!proxies_.empty()) {
+    StreamStopped(proxies_[0], proxies_[0]);
+  }
+  ClosePhysicalStream();
+
+  // No AudioOutputProxy objects should hold a reference to us when we get
+  // to this stage.
+  DCHECK(HasOneRef()) << "Only the AudioManager should hold a reference";
+}
+
+void AudioOutputMixer::ClosePhysicalStream() {
+  DCHECK_EQ(MessageLoop::current(), message_loop_);
+  if (proxies_.empty() && physical_stream_.get() != NULL)
+    physical_stream_.release()->Close();
+}
+
+// AudioSourceCallback implementation.
+uint32 AudioOutputMixer::OnMoreData(AudioOutputStream* stream,
+                                    uint8* dest,
+                                    uint32 max_size,
+                                    AudioBuffersState buffers_state) {
+  max_size = std::min(max_size,
+                      static_cast<uint32>(params_.GetBytesPerBuffer()));
+  base::AutoLock lock(lock_);

[vrk (LEFT CHROMIUM), 2012/03/30 22:25:58]

Hmm, I'm a little worried about lock contention. This is a potentially
time-intensive method call, and if I understand correctly, many different
threads may be calling StreamStarted().

Since this is the first implementation, I think it's right to do the safe thing
by locking and not worry about hypothetical problems. But please add a TODO to
revisit this and see if it causes problems in practice.

[enal1, 2012/04/04 18:46:55]

Done.

+  if (proxies_.empty())
+    return 0;
+  uint32 actual_total_size = 0;
+  uint32 bytes_per_sample = params_.bits_per_sample() >> 3;
+
+  // Go through all the streams, getting data for every one of them
+  // and mixing it into destination.
+  // Minor optimization: for the first stream we are writing data directly into
+  // destination. This way we don't have to mix the data when there is only one
+  // active stream, and net win in other cases, too.
+  bool first_stream = true;
+  uint8* actual_dest = dest;
+  for (ProxyList::iterator it = proxies_.begin(); it != proxies_.end(); ++it) {
+    AudioOutputProxy* stream_proxy = *it;
+    int pending_bytes = std::min(stream_proxy->pending_bytes(),

[vrk (LEFT CHROMIUM), 2012/03/30 22:25:58]

|pending_bytes| isn't actually used for anything useful -- did you mean to use
it in line 162?

[enal1, 2012/04/04 18:46:55]

Done.

+                                 buffers_state.pending_bytes);
+    // Note: there is no way we can deduce hardware_delay_bytes for the
+    // particular proxy stream. Use zero instead.
+    uint32 actual_size = stream_proxy->audio_source_callback()->OnMoreData(

[vrk (LEFT CHROMIUM), 2012/03/30 22:25:58]

Exposing audio_source_callback() is leaking implementation details. Executing
|stream_proxy|'s audio_source_callback should be abstracted away having
AudioOutputProxy implement the AudioSourceCallback interface that will call its
callback under the covers.

[enal1, 2012/04/04 18:46:55]

Done.

+        stream,
+        actual_dest,
+        max_size,
+        AudioBuffersState(buffers_state.pending_bytes, 0));

[vrk (LEFT CHROMIUM), 2012/03/30 22:25:58]

You're right that there's no way to get the exact amount of
hardware_delay_bytes, but I'm worried about using "0" here. AV sync relies on
hardware_delay_bytes being accurate, or at least relatively accurate.

I think this is what you were trying to solve with the |pending_bytes|
calculation? Can you explain more what you were doing with |pending_bytes|?

[enal1, 2012/04/04 18:46:55]

There are 2 different problems here:
(1) We don't know |pending _bytes| for individual stream, and we are giving that
value to stream's OnMoreData(), and it can be used there -- I believe it is used
to evaluate exact position of data it should return. Current code "sorta works"
if everything works perfectly, but would have problems if some of the buffers
are only partially filled -- we don't know how how much data was in the buffer
OS returned to us, so we cannot correctly calculate new value. If we know number
of buffers we can solve the problem by storing not one value but vector of them.
(2) There is no way to get |hardware_delay_bytes| after we switched to the low
latency mode. We should either change the interface, passing not one but 2 words
via sockets (I tried that, but at the moment it was very hard to change native
client; now they are in better shape, so maybe it can be done), or get rid of it
completely -- I changed AudioRendererBase to depend on timing instead of
|hardware_delay_bytes|.

Added comment

+
+    // Should update pending_bytes for each proxy.
+    // If stream ended, pending_bytes goes down by max_size.
+    if (actual_size == 0) {
+      pending_bytes -= max_size;
+      stream_proxy->set_pending_bytes(std::max(pending_bytes, 0));
+      continue;
+    }
+
+    // Otherwise, it goes up by amount of data. It cannot exceed max amount of
+    // data we can buffer, but we don't know that value. So we increment
+    // pending_bytes unconditionally but adjust it before actual use (which
+    // would be on a next OnMoreData() call).
+    stream_proxy->set_pending_bytes(pending_bytes + actual_size);
+
+    // No need to mix muted stream.
+    double volume = 0.0;
+    stream_proxy->GetVolume(&volume);
+    if (volume == 0.0)
+      continue;
+
+    // Different handling for first and all subsequent streams.
+    if (first_stream) {

[vrk (LEFT CHROMIUM), 2012/03/30 22:25:58]

Instead of the "first_stream" bool, please handle the first stream case before
the for-loop, and begin the for loop on the 2nd element of the list.

[enal1, 2012/04/04 18:46:55]

I thought about it, but decided against it -- (1) there is some common code, and
(2) there is extra "continue" in case volume is zero, and having separate loop
to search for first non-muted stream looks awkward.

Of course we can assume that muting case is rare and accept any first stream,
even if it is muted, but I believe current code is clean enough.

+      if (volume != 1.0) {
+        media::AdjustVolume(static_cast<void*>(actual_dest),

[tommi (sloooow) - chröme, 2012/04/03 13:47:51]

cast not necessary

[enal1, 2012/04/04 18:46:55]

Done.

+                            actual_size,
+                            params_.channels(),
+                            bytes_per_sample,
+                            volume);
+      }
+      if (actual_size < max_size)
+        memset(dest + actual_size, 0, max_size - actual_size);
+      first_stream = false;
+      actual_dest = mixer_data_.get();
+      actual_total_size = actual_size;
+    } else {
+      media::MixStreams(static_cast<void*>(dest),

[tommi (sloooow) - chröme, 2012/04/03 13:47:51]

casts not necessary and static_cast to void* seems a bit odd

[enal1, 2012/04/04 18:46:55]

Done.

+                        static_cast<void*>(actual_dest),
+                        actual_size,
+                        bytes_per_sample,
+                        volume);
+      actual_total_size = std::max(actual_size, actual_total_size);
+    }
+  }
+  return actual_total_size;
+}
+
+void AudioOutputMixer::OnError(AudioOutputStream* stream, int code) {
+  base::AutoLock lock(lock_);
+  for (ProxyList::iterator it = proxies_.begin(); it != proxies_.end(); ++it) {
+    (*it)->audio_source_callback()->OnError(stream, code);

[vrk (LEFT CHROMIUM), 2012/03/30 22:25:58]

In the same vein as line 158, do not expose audio_source_callback() and instead
have AudioOutputProxy implement the AudioSourceCallback interface.

[enal1, 2012/04/04 18:46:55]

Done.

+  }
+}
+
+void AudioOutputMixer::WaitTillDataReady() {
+  base::AutoLock lock(lock_);
+  for (ProxyList::iterator it = proxies_.begin(); it != proxies_.end(); ++it) {
+    (*it)->audio_source_callback()->WaitTillDataReady();

[vrk (LEFT CHROMIUM), 2012/03/30 22:25:58]

In the same vein as line 158, do not expose audio_source_callback() and instead
have AudioOutputProxy implement the AudioSourceCallback interface.

[enal1, 2012/04/04 18:46:55]

Done.

+  }
+}