From ad8d175b405373b9614ad4278f274eae706df588 Mon Sep 17 00:00:00 2001 From: Glenn Kasten Date: Thu, 2 Feb 2012 14:05:20 -0800 Subject: [PATCH] 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 | 41 +++++++++++++------------- services/audioflinger/AudioFlinger.h | 9 ++++-- 2 files changed, 27 insertions(+), 23 deletions(-) diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp index aaf73ea6a6e89..bbc919669cbc7 100644 --- a/services/audioflinger/AudioFlinger.cpp +++ b/services/audioflinger/AudioFlinger.cpp @@ -188,29 +188,32 @@ void AudioFlinger::onFirstRef() 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 @@ void AudioFlinger::onFirstRef() } } -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 @@ AudioFlinger::~AudioFlinger() 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 AudioFlinger::getMicMute() const } 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 @@ size_t AudioFlinger::getInputBufferSize(uint32_t sampleRate, audio_format_t form 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 diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h index c66b19e83b73e..fdcd9166e2092 100644 --- a/services/audioflinger/AudioFlinger.h +++ b/services/audioflinger/AudioFlinger.h @@ -199,7 +199,9 @@ private: 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 @@ mutable Mutex mLock; // mutex for process, commands and handl DefaultKeyedVector< pid_t, wp > 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 mAudioHwDevs; enum hardware_call_state { @@ -1411,6 +1415,7 @@ mutable Mutex mLock; // mutex for process, commands and handl 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