Move some generic functions to AudioManagerBase to be inherited by platform-specific AudioManager***

media/audio/audio_manager_base.cc

Index: media/audio/audio_manager_base.cc
diff --git a/media/audio/audio_manager_base.cc b/media/audio/audio_manager_base.cc
index 2957e966f525232551c74e9b240d277973ba9ff9..ffdc5d7df28561519958a70fac1f2563ab027980 100644
--- a/media/audio/audio_manager_base.cc
+++ b/media/audio/audio_manager_base.cc
@@ -9,6 +9,8 @@
 #include "base/threading/thread.h"
 #include "media/audio/audio_output_dispatcher.h"
 #include "media/audio/audio_output_proxy.h"
+#include "media/audio/fake_audio_input_stream.h"
+#include "media/audio/fake_audio_output_stream.h"
 
 static const int kStreamCloseDelaySeconds = 5;
 
@@ -16,11 +18,15 @@ const char AudioManagerBase::kDefaultDeviceName[] = "Default";
 const char AudioManagerBase::kDefaultDeviceId[] = "default";
 
 AudioManagerBase::AudioManagerBase()
-    : num_active_input_streams_(0) {
+    : num_active_input_streams_(0),
+      max_num_output_streams_(0),

[scherkus (not reviewing), 2012/03/06 22:31:51]

so if implementors fail to set something all audio will fail?

is that intended?

[tommi (sloooow) - chröme, 2012/03/07 07:44:55]

ugh, nope, that's not what I asked for.  Shijing, see my earlier comment about a
"reasonable default".  I don't think 0 is reasonable.

[no longer working on chromium, 2012/03/07 09:57:08]

Thanks, Done.

[no longer working on chromium, 2012/03/07 09:57:08]

Sorry, I missed the comments.

+      num_output_streams_(0) {
 }
 
 AudioManagerBase::~AudioManagerBase() {
   Shutdown();
+  // All the output streams should have been deleted.
+  DCHECK_EQ(0, num_output_streams_);
 }
 
 void AudioManagerBase::Init() {
@@ -39,6 +45,54 @@ scoped_refptr<base::MessageLoopProxy> AudioManagerBase::GetMessageLoop() {
   return audio_thread_.get() ? audio_thread_->message_loop_proxy() : NULL;
 }
 
+AudioOutputStream* AudioManagerBase::MakeAudioOutputStream(
+    const AudioParameters& params) {
+  if (!params.IsValid()) {
+    DLOG(ERROR) << "Audio parameters are invalid";
+    return NULL;
+  }
+
+  // Limit the number of audio streams opened. This is to prevent using
+  // excessive resources for a large number of audio streams. More
+  // importantly it prevents instability on certain systems.
+  // See bug: http://crbug.com/30242.
+  if (num_output_streams_ >= max_num_output_streams_) {
+    return NULL;

[scherkus (not reviewing), 2012/03/06 22:31:51]

should we LOG(WARNING) here?  otherwise the resulting behaviour is very strange
(no sound)

[tommi (sloooow) - chröme, 2012/03/07 07:44:55]

+1  even LOG(ERROR)

[no longer working on chromium, 2012/03/07 09:57:08]

Good point. Done

[no longer working on chromium, 2012/03/07 09:57:08]

Done.

+  }
+
+  AudioOutputStream* stream = NULL;
+  if (params.format == AudioParameters::AUDIO_MOCK) {
+    stream = FakeAudioOutputStream::MakeFakeStream(params);
+  } else if (params.format == AudioParameters::AUDIO_PCM_LINEAR) {
+    num_output_streams_++;
+    stream = MakeLinearOutputStream(params);
+  } else if (params.format == AudioParameters::AUDIO_PCM_LOW_LATENCY) {
+    num_output_streams_++;
+    stream = MakeLowLatencyOutputStream(params);
+  }
+
+  return stream;
+}
+
+AudioInputStream* AudioManagerBase::MakeAudioInputStream(
+    const AudioParameters& params, const std::string& device_id) {
+  if (!params.IsValid() || device_id.empty()) {
+    DLOG(ERROR) << "Audio parameters are invalid for device " << device_id;
+    return NULL;
+  }
+
+  AudioInputStream* stream = NULL;
+  if (params.format == AudioParameters::AUDIO_MOCK) {
+    stream = FakeAudioInputStream::MakeFakeStream(params);
+  } else if (params.format == AudioParameters::AUDIO_PCM_LINEAR) {
+    stream = MakeLinearInputStream(params, device_id);
+  } else if (params.format == AudioParameters::AUDIO_PCM_LOW_LATENCY) {
+    stream = MakeLowLatencyInputStream(params, device_id);
+  }
+
+  return stream;
+}
+
 AudioOutputStream* AudioManagerBase::MakeAudioOutputStreamProxy(
     const AudioParameters& params) {
   DCHECK(GetMessageLoop()->BelongsToCurrentThread());
@@ -62,6 +116,17 @@ void AudioManagerBase::GetAudioInputDeviceNames(
     media::AudioDeviceNames* device_names) {
 }
 
+void AudioManagerBase::ReleaseOutputStream(AudioOutputStream* stream) {

[scherkus (not reviewing), 2012/03/06 22:31:51]

I'd really like us to see this re-done at some point

right now we have a very non-obvious destruction path via
AudioOutputStream::Close() which implementors must remember to call
AudioManagerBase::ReleaseOutputStream()

perhaps the proper way to "close" a stream is to actually pass ownership back to
AudioManager so it can close, delete, and decrement

you don't have to do this in this CL but it would be good to add a TODO+bug and
think about cleaning this up at some point

+  DCHECK(stream);
+  num_output_streams_--;
+  delete stream;
+}
+
+void AudioManagerBase::ReleaseInputStream(AudioInputStream* stream) {
+  DCHECK(stream);
+  delete stream;
+}
+
 void AudioManagerBase::IncreaseActiveInputStreamCount() {
   base::AtomicRefCountInc(&num_active_input_streams_);
 }