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..20c92ac78d35db89f0fd452d6b8f65ce72cd73a6 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,14 @@ const char AudioManagerBase::kDefaultDeviceName[] = "Default";
 const char AudioManagerBase::kDefaultDeviceId[] = "default";
 
 AudioManagerBase::AudioManagerBase()
-    : num_active_input_streams_(0) {
+    : num_active_input_streams_(0),
+      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 +44,50 @@ 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())
+    return NULL;

[tommi (sloooow) - chröme, 2012/03/05 14:28:28]

NOTREACHED?

[no longer working on chromium, 2012/03/06 15:27:07]

We have some failed unittests which will try the invalid parameters, adding a
NOTREACHED will break those tests, so I added a DLOG(ERROR) instead.

+
+  // 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_ >= GetMaxAudioOutputStreamsAllowed()) {
+    return NULL;
+  }
+
+  AudioOutputStream* stream = NULL;
+  if (params.format == AudioParameters::AUDIO_MOCK) {
+    stream = FakeAudioOutputStream::MakeFakeStream(params);

[tommi (sloooow) - chröme, 2012/03/05 14:28:28]

missing increment?

[no longer working on chromium, 2012/03/06 15:27:07]

We do not need to limit the max number of the fake streams.

+  } else if (params.format == AudioParameters::AUDIO_PCM_LINEAR) {
+    num_output_streams_++;

[tommi (sloooow) - chröme, 2012/03/05 14:28:28]

what about not incrementing inside every if/else and instead add an extra if()
just before return:

if (stream)
  ++num_output_streams_;

That way you also avoid erroneously incrementing if the return value was NULL.

[no longer working on chromium, 2012/03/06 15:27:07]

it may not be a good idea since fake stream needs to be out of the counting.

+    stream = MakeAudioLinearOutputStream(params);
+  } else if (params.format == AudioParameters::AUDIO_PCM_LOW_LATENCY) {
+    num_output_streams_++;
+    stream = MakeAudioLowLatencyOutputStream(params);
+  }
+
+  return stream;
+}
+
+AudioInputStream* AudioManagerBase::MakeAudioInputStream(
+    const AudioParameters& params, const std::string& device_id) {
+  if (!params.IsValid() || device_id.empty())
+    return NULL;

[tommi (sloooow) - chröme, 2012/03/05 14:28:28]

NOTREACHED?

[no longer working on chromium, 2012/03/06 15:27:07]

The same here, added a DLOG instead.

+
+  AudioInputStream* stream = NULL;
+  if (params.format == AudioParameters::AUDIO_MOCK) {
+    stream = FakeAudioInputStream::MakeFakeStream(params);
+  } else if (params.format == AudioParameters::AUDIO_PCM_LINEAR) {
+    stream = MakeAudioLinearInputStream(params, device_id);
+  } else if (params.format == AudioParameters::AUDIO_PCM_LOW_LATENCY) {
+    stream = MakeAudioLowLatencyInputStream(params, device_id);
+  }
+
+  return stream;

[tommi (sloooow) - chröme, 2012/03/05 14:28:28]

just thinking out loud here... 

we're not counting the input streams here as we do for output streams.  Yet we
have special functions below for counting the input streams via
IncreaseActiveInputStreamCount etc.

Can't we simplify this and increment the input stream count here and decrement
in ReleaseInputStream, the same way as we do for output streams, and get rid of
num_active_input_streams_ and related functions?  Is there something I'm missing
perhaps?

[no longer working on chromium, 2012/03/06 15:27:07]

These ActiveInputStreamCount APIs are added by speech recognition team to keep
track if there is any active recording stream, so they are called in Start/Stop,
which is completely different from our num_output_streams_.

We should keep it for now, and may refactor it in a follow up CL.

+}
+
 AudioOutputStream* AudioManagerBase::MakeAudioOutputStreamProxy(
     const AudioParameters& params) {
   DCHECK(GetMessageLoop()->BelongsToCurrentThread());
@@ -62,6 +111,17 @@ void AudioManagerBase::GetAudioInputDeviceNames(
     media::AudioDeviceNames* device_names) {
 }
 
+void AudioManagerBase::ReleaseOutputStream(AudioOutputStream* stream) {
+  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_);
 }