From 00abf0d08363d063bd5ea6e35a71c54f16f3ed31 Mon Sep 17 00:00:00 2001 From: Eric Laurent Date: Wed, 22 Apr 2020 19:28:22 -0700 Subject: [PATCH] AudioFlinger: fix getMicrophones implementation getMicrophones() should return aggregated mic information from all HW modules, not just primary. Also: - Fix assignment of mPrimaryHardwareDev that should be first from HW module name and then according to primary output if no module with name "primary" is loaded. - Make sure we do not dereference mPrimaryHardwareDev if null. Note that this should not happen with current rule that a primary module must be present. - Implement consistent locking scheme where both mPrimaryHardwareDev and mAudioHwDevs are guarded by mHardwareLock Bug: 154772890 Test: AudioManagerTest#testGetMicrophones Change-Id: I7c9449bb705a6fbebdc0642166e58348d47b7ee8 --- services/audioflinger/AudioFlinger.cpp | 158 ++++++++++++++++--------- services/audioflinger/AudioFlinger.h | 6 +- 2 files changed, 108 insertions(+), 56 deletions(-) diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp index 03c16f38c7..de7ae40635 100644 --- a/services/audioflinger/AudioFlinger.cpp +++ b/services/audioflinger/AudioFlinger.cpp @@ -135,7 +135,7 @@ static sp sExternalVibratorService; static sp getExternalVibratorService() { if (sExternalVibratorService == 0) { - sp binder = defaultServiceManager()->getService( + sp binder = defaultServiceManager()->getService( String16("external_vibrator_service")); if (binder != 0) { sExternalVibratorService = @@ -411,6 +411,7 @@ void AudioFlinger::onExternalVibrationStop(const sp& exte status_t AudioFlinger::addEffectToHal(audio_port_handle_t deviceId, audio_module_handle_t hwModuleId, sp effect) { + AutoMutex lock(mHardwareLock); AudioHwDevice *audioHwDevice = mAudioHwDevs.valueFor(hwModuleId); if (audioHwDevice == nullptr) { return NO_INIT; @@ -420,6 +421,7 @@ status_t AudioFlinger::addEffectToHal(audio_port_handle_t deviceId, status_t AudioFlinger::removeEffectFromHal(audio_port_handle_t deviceId, audio_module_handle_t hwModuleId, sp effect) { + AutoMutex lock(mHardwareLock); AudioHwDevice *audioHwDevice = mAudioHwDevs.valueFor(hwModuleId); if (audioHwDevice == nullptr) { return NO_INIT; @@ -439,6 +441,7 @@ AudioHwDevice* AudioFlinger::findSuitableHwDev_l( { // if module is 0, the request comes from an old policy manager and we should load // well known modules + AutoMutex lock(mHardwareLock); if (module == 0) { ALOGW("findSuitableHwDev_l() loading well know audio hw modules"); for (size_t i = 0; i < arraysize(audio_interfaces); i++) { @@ -1079,17 +1082,18 @@ status_t AudioFlinger::setMasterVolume(float value) mMasterVolume = value; // Set master volume in the HALs which support it. - for (size_t i = 0; i < mAudioHwDevs.size(); i++) { + { AutoMutex lock(mHardwareLock); - AudioHwDevice *dev = mAudioHwDevs.valueAt(i); + for (size_t i = 0; i < mAudioHwDevs.size(); i++) { + AudioHwDevice *dev = mAudioHwDevs.valueAt(i); - mHardwareStatus = AUDIO_HW_SET_MASTER_VOLUME; - if (dev->canSetMasterVolume()) { - dev->hwDevice()->setMasterVolume(value); + mHardwareStatus = AUDIO_HW_SET_MASTER_VOLUME; + if (dev->canSetMasterVolume()) { + dev->hwDevice()->setMasterVolume(value); + } + mHardwareStatus = AUDIO_HW_IDLE; } - mHardwareStatus = AUDIO_HW_IDLE; } - // Now set the master volume in each playback thread. Playback threads // assigned to HALs which do not have master volume support will apply // master volume during the mix operation. Threads with HALs which do @@ -1156,6 +1160,9 @@ status_t AudioFlinger::setMode(audio_mode_t mode) { // scope for the lock AutoMutex lock(mHardwareLock); + if (mPrimaryHardwareDev == nullptr) { + return INVALID_OPERATION; + } sp dev = mPrimaryHardwareDev->hwDevice(); mHardwareStatus = AUDIO_HW_SET_MODE; ret = dev->setMode(mode); @@ -1185,6 +1192,9 @@ status_t AudioFlinger::setMicMute(bool state) } AutoMutex lock(mHardwareLock); + if (mPrimaryHardwareDev == nullptr) { + return INVALID_OPERATION; + } sp primaryDev = mPrimaryHardwareDev->hwDevice(); if (primaryDev == nullptr) { ALOGW("%s: no primary HAL device", __func__); @@ -1210,6 +1220,9 @@ bool AudioFlinger::getMicMute() const return false; } AutoMutex lock(mHardwareLock); + if (mPrimaryHardwareDev == nullptr) { + return false; + } sp primaryDev = mPrimaryHardwareDev->hwDevice(); if (primaryDev == nullptr) { ALOGW("%s: no primary HAL device", __func__); @@ -1252,15 +1265,17 @@ status_t AudioFlinger::setMasterMute(bool muted) mMasterMute = muted; // Set master mute in the HALs which support it. - for (size_t i = 0; i < mAudioHwDevs.size(); i++) { + { AutoMutex lock(mHardwareLock); - AudioHwDevice *dev = mAudioHwDevs.valueAt(i); + for (size_t i = 0; i < mAudioHwDevs.size(); i++) { + AudioHwDevice *dev = mAudioHwDevs.valueAt(i); - mHardwareStatus = AUDIO_HW_SET_MASTER_MUTE; - if (dev->canSetMasterMute()) { - dev->hwDevice()->setMasterMute(muted); + mHardwareStatus = AUDIO_HW_SET_MASTER_MUTE; + if (dev->canSetMasterMute()) { + dev->hwDevice()->setMasterMute(muted); + } + mHardwareStatus = AUDIO_HW_IDLE; } - mHardwareStatus = AUDIO_HW_IDLE; } // Now set the master mute in each playback thread. Playback threads @@ -1591,16 +1606,13 @@ String8 AudioFlinger::getParameters(audio_io_handle_t ioHandle, const String8& k if (ioHandle == AUDIO_IO_HANDLE_NONE) { String8 out_s8; + AutoMutex lock(mHardwareLock); for (size_t i = 0; i < mAudioHwDevs.size(); i++) { String8 s; - status_t result; - { - AutoMutex lock(mHardwareLock); mHardwareStatus = AUDIO_HW_GET_PARAMETER; sp dev = mAudioHwDevs.valueAt(i)->hwDevice(); - result = dev->getParameters(keys, &s); + status_t result = dev->getParameters(keys, &s); mHardwareStatus = AUDIO_HW_IDLE; - } if (result == OK) out_s8 += s; } return out_s8; @@ -1633,6 +1645,9 @@ size_t AudioFlinger::getInputBufferSize(uint32_t sampleRate, audio_format_t form } AutoMutex lock(mHardwareLock); + if (mPrimaryHardwareDev == nullptr) { + return 0; + } mHardwareStatus = AUDIO_HW_GET_INPUT_BUFFER_SIZE; sp dev = mPrimaryHardwareDev->hwDevice(); @@ -1718,6 +1733,9 @@ status_t AudioFlinger::setVoiceVolume(float value) } AutoMutex lock(mHardwareLock); + if (mPrimaryHardwareDev == nullptr) { + return INVALID_OPERATION; + } sp dev = mPrimaryHardwareDev->hwDevice(); mHardwareStatus = AUDIO_HW_SET_VOICE_VOLUME; ret = dev->setVoiceVolume(value); @@ -2126,10 +2144,11 @@ audio_module_handle_t AudioFlinger::loadHwModule(const char *name) return AUDIO_MODULE_HANDLE_NONE; } Mutex::Autolock _l(mLock); + AutoMutex lock(mHardwareLock); return loadHwModule_l(name); } -// loadHwModule_l() must be called with AudioFlinger::mLock held +// loadHwModule_l() must be called with AudioFlinger::mLock and AudioFlinger::mHardwareLock held audio_module_handle_t AudioFlinger::loadHwModule_l(const char *name) { for (size_t i = 0; i < mAudioHwDevs.size(); i++) { @@ -2161,44 +2180,49 @@ audio_module_handle_t AudioFlinger::loadHwModule_l(const char *name) // master mute and volume settings. AudioHwDevice::Flags flags = static_cast(0); - { // scope for auto-lock pattern - AutoMutex lock(mHardwareLock); - - if (0 == mAudioHwDevs.size()) { - mHardwareStatus = AUDIO_HW_GET_MASTER_VOLUME; - float mv; - if (OK == dev->getMasterVolume(&mv)) { - mMasterVolume = mv; - } - - mHardwareStatus = AUDIO_HW_GET_MASTER_MUTE; - bool mm; - if (OK == dev->getMasterMute(&mm)) { - mMasterMute = mm; - } + if (0 == mAudioHwDevs.size()) { + mHardwareStatus = AUDIO_HW_GET_MASTER_VOLUME; + float mv; + if (OK == dev->getMasterVolume(&mv)) { + mMasterVolume = mv; } - mHardwareStatus = AUDIO_HW_SET_MASTER_VOLUME; - if (OK == dev->setMasterVolume(mMasterVolume)) { - flags = static_cast(flags | - AudioHwDevice::AHWD_CAN_SET_MASTER_VOLUME); + mHardwareStatus = AUDIO_HW_GET_MASTER_MUTE; + bool mm; + if (OK == dev->getMasterMute(&mm)) { + mMasterMute = mm; } + } - mHardwareStatus = AUDIO_HW_SET_MASTER_MUTE; - if (OK == dev->setMasterMute(mMasterMute)) { - flags = static_cast(flags | - AudioHwDevice::AHWD_CAN_SET_MASTER_MUTE); - } + mHardwareStatus = AUDIO_HW_SET_MASTER_VOLUME; + if (OK == dev->setMasterVolume(mMasterVolume)) { + flags = static_cast(flags | + AudioHwDevice::AHWD_CAN_SET_MASTER_VOLUME); + } - mHardwareStatus = AUDIO_HW_IDLE; + mHardwareStatus = AUDIO_HW_SET_MASTER_MUTE; + if (OK == dev->setMasterMute(mMasterMute)) { + flags = static_cast(flags | + AudioHwDevice::AHWD_CAN_SET_MASTER_MUTE); } + + mHardwareStatus = AUDIO_HW_IDLE; + if (strcmp(name, AUDIO_HARDWARE_MODULE_ID_MSD) == 0) { // An MSD module is inserted before hardware modules in order to mix encoded streams. flags = static_cast(flags | AudioHwDevice::AHWD_IS_INSERT); } audio_module_handle_t handle = (audio_module_handle_t) nextUniqueId(AUDIO_UNIQUE_ID_USE_MODULE); - mAudioHwDevs.add(handle, new AudioHwDevice(handle, name, dev, flags)); + AudioHwDevice *audioDevice = new AudioHwDevice(handle, name, dev, flags); + if (strcmp(name, AUDIO_HARDWARE_MODULE_ID_PRIMARY) == 0) { + mPrimaryHardwareDev = audioDevice; + mHardwareStatus = AUDIO_HW_SET_MODE; + mPrimaryHardwareDev->hwDevice()->setMode(mMode); + mHardwareStatus = AUDIO_HW_IDLE; + } + + mAudioHwDevs.add(handle, audioDevice); ALOGI("loadHwModule() Loaded %s audio interface, handle %d", name, handle); @@ -2284,6 +2308,7 @@ status_t AudioFlinger::setAudioPortConfig(const struct audio_port_config *config } Mutex::Autolock _l(mLock); + AutoMutex lock(mHardwareLock); ssize_t index = mAudioHwDevs.indexOfKey(module); if (index < 0) { ALOGW("%s() bad hw module %d", __func__, module); @@ -2305,8 +2330,15 @@ audio_hw_sync_t AudioFlinger::getAudioHwSyncForSession(audio_session_t sessionId return mHwAvSyncIds.valueAt(index); } - sp dev = mPrimaryHardwareDev->hwDevice(); - if (dev == NULL) { + sp dev; + { + AutoMutex lock(mHardwareLock); + if (mPrimaryHardwareDev == nullptr) { + return AUDIO_HW_SYNC_INVALID; + } + dev = mPrimaryHardwareDev->hwDevice(); + } + if (dev == nullptr) { return AUDIO_HW_SYNC_INVALID; } String8 reply; @@ -2374,8 +2406,21 @@ status_t AudioFlinger::systemReady() status_t AudioFlinger::getMicrophones(std::vector *microphones) { AutoMutex lock(mHardwareLock); - sp dev = mPrimaryHardwareDev->hwDevice(); - status_t status = dev->getMicrophones(microphones); + status_t status = INVALID_OPERATION; + + for (size_t i = 0; i < mAudioHwDevs.size(); i++) { + std::vector mics; + AudioHwDevice *dev = mAudioHwDevs.valueAt(i); + mHardwareStatus = AUDIO_HW_GET_MICROPHONES; + status_t devStatus = dev->hwDevice()->getMicrophones(&mics); + mHardwareStatus = AUDIO_HW_IDLE; + if (devStatus == NO_ERROR) { + microphones->insert(microphones->begin(), mics.begin(), mics.end()); + // report success if at least one HW module supports the function. + status = NO_ERROR; + } + } + return status; } @@ -2522,12 +2567,13 @@ status_t AudioFlinger::openOutput(audio_module_handle_t module, // notify client processes of the new output creation playbackThread->ioConfigChanged(AUDIO_OUTPUT_OPENED); - // the first primary output opened designates the primary hw device - if ((mPrimaryHardwareDev == NULL) && (flags & AUDIO_OUTPUT_FLAG_PRIMARY)) { + // the first primary output opened designates the primary hw device if no HW module + // named "primary" was already loaded. + AutoMutex lock(mHardwareLock); + if ((mPrimaryHardwareDev == nullptr) && (flags & AUDIO_OUTPUT_FLAG_PRIMARY)) { ALOGI("Using module %d as the primary audio interface", module); mPrimaryHardwareDev = playbackThread->getOutput()->audioHwDev; - AutoMutex lock(mHardwareLock); mHardwareStatus = AUDIO_HW_SET_MODE; mPrimaryHardwareDev->hwDevice()->setMode(mMode); mHardwareStatus = AUDIO_HW_IDLE; @@ -3196,6 +3242,10 @@ audio_unique_id_t AudioFlinger::nextUniqueId(audio_unique_id_use_t use) AudioFlinger::PlaybackThread *AudioFlinger::primaryPlaybackThread_l() const { + AutoMutex lock(mHardwareLock); + if (mPrimaryHardwareDev == nullptr) { + return nullptr; + } for (size_t i = 0; i < mPlaybackThreads.size(); i++) { PlaybackThread *thread = mPlaybackThreads.valueAt(i).get(); if(thread->isDuplicating()) { @@ -3206,7 +3256,7 @@ AudioFlinger::PlaybackThread *AudioFlinger::primaryPlaybackThread_l() const return thread; } } - return NULL; + return nullptr; } DeviceTypeSet AudioFlinger::primaryOutputDevice_l() const diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h index 5c20a2d514..6afbd4ff6f 100644 --- a/services/audioflinger/AudioFlinger.h +++ b/services/audioflinger/AudioFlinger.h @@ -835,10 +835,11 @@ using effect_buffer_t = int16_t; // NOTE: If both mLock and mHardwareLock mutexes must be held, // always take mLock before mHardwareLock - // These two fields are immutable after onFirstRef(), so no lock needed to access - AudioHwDevice* mPrimaryHardwareDev; // mAudioHwDevs[0] or NULL + // guarded by mHardwareLock + AudioHwDevice* mPrimaryHardwareDev; DefaultKeyedVector mAudioHwDevs; + // These two fields are immutable after onFirstRef(), so no lock needed to access sp mDevicesFactoryHal; sp mDevicesFactoryHalCallback; @@ -865,6 +866,7 @@ using effect_buffer_t = int16_t; AUDIO_HW_GET_PARAMETER, // get_parameters AUDIO_HW_SET_MASTER_MUTE, // set_master_mute AUDIO_HW_GET_MASTER_MUTE, // get_master_mute + AUDIO_HW_GET_MICROPHONES, // getMicrophones }; mutable hardware_call_state mHardwareStatus; // for dump only