Refactor audio manager for Android to avoid heavy tasks at startup

media/audio/android/audio_manager_android.cc

Index: media/audio/android/audio_manager_android.cc
diff --git a/media/audio/android/audio_manager_android.cc b/media/audio/android/audio_manager_android.cc
index b588768bbd08c1fcfbb75df70248a9da752a28a4..cfc4ec2d45785c4d585bb185a0b03b456f708d2e 100644
--- a/media/audio/android/audio_manager_android.cc
+++ b/media/audio/android/audio_manager_android.cc
@@ -124,8 +124,11 @@ AudioOutputStream* AudioManagerAndroid::MakeAudioOutputStream(
   AudioOutputStream* stream =
       AudioManagerBase::MakeAudioOutputStream(params, std::string(),
           std::string());
-  if (stream && output_stream_count() == 1) {
-    SetAudioMode(kAudioModeInCommunication);
+
+  // The audio manager for Android creates streams intended for real-time
+  // VoIP sessions and therefore sets the audio mode to MODE_IN_COMMUNICATION.
+  if (stream && IsFirstCreatedAudioStream()) {

[tommi (sloooow) - chröme, 2013/12/10 21:19:11]

you don't really need both IsFirst (or HasOne) and IsLast methods (or HasNo).
Instead, you can just implement HasNoAudioStreams() and do:

bool has_no_streams = HasNoAudioStreams();
AudioOutputStream* stream = AudioManagerBase::MakeAudioOutputStream(...);
if (stream && has_no_streams) {
  // We just created the first stream.
  SetCommunicationAudioModeOn(true);
}

[henrika (OOO until Aug 14), 2013/12/11 13:16:38]

Done.

+    SetCommunicationAudioModeOn(true);

[tommi (sloooow) - chröme, 2013/12/10 21:19:11]

Here, we should store what the previous mode is. 
SetCommunicationAudioModeOn(false) should restore the mode to what it previously
was.  Can you file a bug for that?

Also, it would be good to have a comment here that explains that the current
implementation of the AudioManager on Android is for voice only.  As is, I'm
assuming wide band audio quality would suffer if Android used the AudioManager
for rendering audio in general (which is what I expect will happen down the
line), so this will need to change.

[henrika (OOO until Aug 14), 2013/12/11 13:16:38]

I do save the state and restore the state in Java. OK?

I have not noticed any degradation in BW due this mode. Note that we use
MODE_IN_COMMUNICATION: "In communication audio mode. An audio/video chat or VoIP
call is established." and *not* MODE_IN_CALL: the voice/in-call mode "A
telephony call is established.". Hence, name of mode does not imply quality
reduction IMHO.

Still want comment?

[tommi (sloooow) - chröme, 2013/12/11 17:25:05]

Yes that's great.  I meant to update this comment but didn't get around to it.
If "MODE_IN_COMMUNICATION" means that we'll be using SCO over bluetooth (as I
think it does), then we may be switching from A2DP to SCO here and subsequently
get a drop in audio quality.

A2DP is typically a uni-directional stereo audio stream with focus on wide band
codecs (if I'm not mistaken) whereas SCO is typically a low sample rate (16kHz
or lower), bidirectional, mono, connection.

[henrika (OOO until Aug 14), 2013/12/12 10:58:00]

OK, get your point but note that BT is just one case and the only case where
Comm mode will lead to degradation. Will make a note about it; please check
again.

And i think that some hands-free profiles actually supports 16k as well.
"Version 1.6 adds optional support for wide band speech with the mSBC codec, a
16 kHz monaural configuration of the SBC codec mandated by the A2DP profile."
(http://en.wikipedia.org/wiki/Bluetooth_profile)

The Android spec actually states that it supports 16k out and 8k in.
It is actually not clear to me what profile we select when we say "switch to
SCO".

[tommi (sloooow) - chröme, 2013/12/12 11:43:15]

I'm not convinced that bluetooth is and forever will be the only case where
communication mode will have different attributes than e.g. music mode.  When
you tested this, did you test stereo sources or only voice? I can understand
that voice would sound more or less the same since the source signal would be
tailored to the communication mode.

In any case, my concern is that the AudioManager on android is currently
implemented with the assumption that it will only be used for "communication
mode".  There can be differences in both routing and quality from 'normal' mode,
so I think it's a good thing to document that somewhere.

[henrika (OOO until Aug 14), 2013/12/12 12:53:11]

Discussed offline; think we are in phase now.

   }
 
   {
@@ -140,13 +143,22 @@ AudioInputStream* AudioManagerAndroid::MakeAudioInputStream(
     const AudioParameters& params, const std::string& device_id) {
   AudioInputStream* stream =
       AudioManagerBase::MakeAudioInputStream(params, device_id);
+
+  // The audio manager for Android creates streams intended for real-time
+  // VoIP sessions and therefore sets the audio mode to MODE_IN_COMMUNICATION.
+  if (stream && IsFirstCreatedAudioStream()) {
+    SetCommunicationAudioModeOn(true);
+  }
   return stream;
 }
 
 void AudioManagerAndroid::ReleaseOutputStream(AudioOutputStream* stream) {
   AudioManagerBase::ReleaseOutputStream(stream);
-  if (!output_stream_count()) {
-    SetAudioMode(kAudioModeNormal);
+
+  // Restore the audio mode which was used before the first communication-
+  // mode stream was created.
+  if (IsLastDestroyedAudioStream()) {
+    SetCommunicationAudioModeOn(false);
   }
   base::AutoLock lock(streams_lock_);
   streams_.erase(static_cast<OpenSLESOutputStream*>(stream));
@@ -154,6 +166,12 @@ void AudioManagerAndroid::ReleaseOutputStream(AudioOutputStream* stream) {
 
 void AudioManagerAndroid::ReleaseInputStream(AudioInputStream* stream) {
   AudioManagerBase::ReleaseInputStream(stream);
+
+  // Restore the audio mode which was used before the first communication-
+  // mode stream was created.
+  if (IsLastDestroyedAudioStream()) {
+    SetCommunicationAudioModeOn(false);
+  }
 }
 
 AudioOutputStream* AudioManagerAndroid::MakeLinearOutputStream(
@@ -188,7 +206,10 @@ AudioInputStream* AudioManagerAndroid::MakeLowLatencyInputStream(
   // Note that the input device is always associated with a certain output
   // device, i.e., this selection does also switch the output device.
   // All input and output streams will be affected by the device selection.
-  SetAudioDevice(device_id);
+  if (!SetAudioDevice(device_id)) {
+    LOG(ERROR) << "Unable to select audio device!";
+    return NULL;
+  }
   return new OpenSLESInputStream(this, params);
 }
 
@@ -233,6 +254,16 @@ AudioParameters AudioManagerAndroid::GetPreferredOutputStreamParameters(
       sample_rate, bits_per_sample, buffer_size);
 }
 
+bool AudioManagerAndroid::IsFirstCreatedAudioStream() {
+  const int out_count = output_stream_count();
+  const int in_count = input_stream_count();
+  return (out_count == 1 && in_count == 0) || (out_count == 0 && in_count == 1);

[tommi (sloooow) - chröme, 2013/12/10 21:19:11]

return (output_stream_count() + input_stream_count()) == 1;

(but I don't think you need this method actually. see above)

[henrika (OOO until Aug 14), 2013/12/11 13:16:38]

removed

+}
+
+bool AudioManagerAndroid::IsLastDestroyedAudioStream() {

[tommi (sloooow) - chröme, 2013/12/10 21:19:11]

HasNoAudioStreams?  I don't see a stream being checked so 'last destroyed'
doesn't have much meaning.

[henrika (OOO until Aug 14), 2013/12/11 13:16:38]

Done.

+  return (output_stream_count() == 0 && input_stream_count() == 0);

[tommi (sloooow) - chröme, 2013/12/10 21:19:11]

nit: remove superflous parenthesis

[henrika (OOO until Aug 14), 2013/12/11 13:16:38]

Done.

+}
+
 // static
 bool AudioManagerAndroid::RegisterAudioManager(JNIEnv* env) {
   return RegisterNativesImpl(env);
@@ -267,13 +298,13 @@ void AudioManagerAndroid::DoSetMuteOnAudioThread(bool muted) {
   }
 }
 
-void AudioManagerAndroid::SetAudioMode(int mode) {
-  Java_AudioManagerAndroid_setMode(
+void AudioManagerAndroid::SetCommunicationAudioModeOn(bool on) {
+  Java_AudioManagerAndroid_setCommunicationAudioModeOn(
       base::android::AttachCurrentThread(),
-      j_audio_manager_.obj(), mode);
+      j_audio_manager_.obj(), on);

[tommi (sloooow) - chröme, 2013/12/10 21:19:11]

see comment on this above.  since this doesn't remember from what mode we
changed the mode, this looks like a bug to me.

[henrika (OOO until Aug 14), 2013/12/11 13:16:38]

So you want me to move the cache to this class instead? As mentioned above, I do
that in Java today.

[tommi (sloooow) - chröme, 2013/12/11 17:25:05]

Doing it in Java as you're currently doing, is fine.  I meant to remove my
comment but forgot :)

[henrika (OOO until Aug 14), 2013/12/12 10:58:00]

Great.

 }
 
-void AudioManagerAndroid::SetAudioDevice(const std::string& device_id) {
+bool AudioManagerAndroid::SetAudioDevice(const std::string& device_id) {
   JNIEnv* env = AttachCurrentThread();
 
   // Send the unique device ID to the Java audio manager and make the
@@ -283,7 +314,7 @@ void AudioManagerAndroid::SetAudioDevice(const std::string& device_id) {
       env,
       device_id == AudioManagerBase::kDefaultDeviceId ?
           std::string() : device_id);
-  Java_AudioManagerAndroid_setDevice(
+  return Java_AudioManagerAndroid_setDevice(
       env, j_audio_manager_.obj(), j_device_id.obj());
 }