commit 2b213bc220768d2b984239511cd4554a96bc0079
author Glenn Kasten <gkasten@google.com> Thu Feb 02 14:05:20 2012 -0800
committer Glenn Kasten <gkasten@google.com> Fri Feb 10 15:36:46 2012 -0800
tree 06053ae20ae309df04caa0ad5a63563fe72f5a15
parent b6333aa8317ce5162ab006c4baed6b0890936dc7

mAudioHwDevs and related cleanup

Inline AudioFlinger::initCheck and remove unnecessary lock.

Remove redundant check of mAudioHwDevs.size().

No need to lock mHardwareLock for each device separately
during initialization.

Use size_t not int to loop through Vector, since size() returns size_t.

Add missing hardware lock for get_mic_mute() and get_input_buffer_size().

Add comments.

Change-Id: Iafae78ef78bbf65f703d99fcc27c2f4ff221aedc

services/audioflinger/AudioFlinger.cpp

diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp
index aaf73ea..bbc9196 100644
--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -188,29 +188,32 @@
              mod->name, mod->id);
         mAudioHwDevs.push(dev);
 
-        if (!mPrimaryHardwareDev) {
+        if (mPrimaryHardwareDev == NULL) {
             mPrimaryHardwareDev = dev;
             ALOGI("Using '%s' (%s.%s) as the primary audio interface",
                  mod->name, mod->id, audio_interfaces[i]);
         }
     }
 
-    mHardwareStatus = AUDIO_HW_INIT;
-
-    if (!mPrimaryHardwareDev || mAudioHwDevs.size() == 0) {
+    if (mPrimaryHardwareDev == NULL) {
         ALOGE("Primary audio interface not found");
-        return;
+        // proceed, all later accesses to mPrimaryHardwareDev verify it's safe with initCheck()
     }
 
+    // Currently (mPrimaryHardwareDev == NULL) == (mAudioHwDevs.size() == 0), but the way the
+    // primary HW dev is selected can change so these conditions might not always be equivalent.
+    // When that happens, re-visit all the code that assumes this.
+
+    AutoMutex lock(mHardwareLock);
+
     for (size_t i = 0; i < mAudioHwDevs.size(); i++) {
         audio_hw_device_t *dev = mAudioHwDevs[i];
 
         mHardwareStatus = AUDIO_HW_INIT;
         rc = dev->init_check(dev);
+        mHardwareStatus = AUDIO_HW_IDLE;
         if (rc == 0) {
-            AutoMutex lock(mHardwareLock);
-
-            mMode = AUDIO_MODE_NORMAL;
+            mMode = AUDIO_MODE_NORMAL;  // assigned multiple times with same value
             mHardwareStatus = AUDIO_HW_SET_MODE;
             dev->set_mode(dev, mMode);
             mHardwareStatus = AUDIO_HW_SET_MASTER_VOLUME;
@@ -220,17 +223,8 @@
     }
 }
 
-status_t AudioFlinger::initCheck() const
-{
-    Mutex::Autolock _l(mLock);
-    if (mPrimaryHardwareDev == NULL || mAudioHwDevs.size() == 0)
-        return NO_INIT;
-    return NO_ERROR;
-}
-
 AudioFlinger::~AudioFlinger()
 {
-    int num_devs = mAudioHwDevs.size();
 
     while (!mRecordThreads.isEmpty()) {
         // closeInput() will remove first entry from mRecordThreads
@@ -241,9 +235,9 @@
         closeOutput(mPlaybackThreads.keyAt(0));
     }
 
-    for (int i = 0; i < num_devs; i++) {
-        audio_hw_device_t *dev = mAudioHwDevs[i];
-        audio_hw_device_close(dev);
+    for (size_t i = 0; i < mAudioHwDevs.size(); i++) {
+        // no mHardwareLock needed, as there are no other references to this
+        audio_hw_device_close(mAudioHwDevs[i]);
     }
 }
 
@@ -625,6 +619,7 @@
     }
 
     bool state = AUDIO_MODE_INVALID;
+    AutoMutex lock(mHardwareLock);
     mHardwareStatus = AUDIO_HW_GET_MIC_MUTE;
     mPrimaryHardwareDev->get_mic_mute(mPrimaryHardwareDev, &state);
     mHardwareStatus = AUDIO_HW_IDLE;
@@ -856,7 +851,11 @@
         return 0;
     }
 
-    return mPrimaryHardwareDev->get_input_buffer_size(mPrimaryHardwareDev, sampleRate, format, channelCount);
+    AutoMutex lock(mHardwareLock);
+    mHardwareStatus = AUDIO_HW_GET_INPUT_BUFFER_SIZE;
+    size_t size = mPrimaryHardwareDev->get_input_buffer_size(mPrimaryHardwareDev, sampleRate, format, channelCount);
+    mHardwareStatus = AUDIO_HW_IDLE;
+    return size;
 }
 
 unsigned int AudioFlinger::getInputFramesLost(audio_io_handle_t ioHandle) const

services/audioflinger/AudioFlinger.h

diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h
index c66b19e..fdcd916 100644
--- a/services/audioflinger/AudioFlinger.h
+++ b/services/audioflinger/AudioFlinger.h
@@ -199,7 +199,9 @@
                             AudioFlinger();
     virtual                 ~AudioFlinger();
 
-    status_t                initCheck() const;
+    // call in any IAudioFlinger method that accesses mPrimaryHardwareDev
+    status_t                initCheck() const { return mPrimaryHardwareDev == NULL ? NO_INIT : NO_ERROR; }
+
     virtual     void        onFirstRef();
     audio_hw_device_t*      findSuitableHwDev_l(uint32_t devices);
     void                    purgeStaleEffects_l();
@@ -1391,7 +1393,9 @@
                 DefaultKeyedVector< pid_t, wp<Client> >     mClients;   // see ~Client()
 
                 mutable     Mutex                   mHardwareLock;
-                audio_hw_device_t*                  mPrimaryHardwareDev;
+
+                // These two fields are immutable after onFirstRef(), so no lock needed to access
+                audio_hw_device_t*                  mPrimaryHardwareDev; // mAudioHwDevs[0] or NULL
                 Vector<audio_hw_device_t*>          mAudioHwDevs;
 
     enum hardware_call_state {
@@ -1411,6 +1415,7 @@
         AUDIO_HW_SET_MIC_MUTE,
         AUDIO_SET_VOICE_VOLUME,
         AUDIO_SET_PARAMETER,
+        AUDIO_HW_GET_INPUT_BUFFER_SIZE,
     };
 
     mutable     hardware_call_state                 mHardwareStatus;    // for dump only