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
Signed-off-by: Dmitry Shmidt <dimitrysh@google.com>
Merged-In: I7c9449bb705a6fbebdc0642166e58348d47b7ee8
Change-Id: I1c2e1cbfdb16c408c7368e2e51838c299c22ec7b
diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp
index 3377c17..474313f 100644
--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -134,7 +134,7 @@
static sp<os::IExternalVibratorService> getExternalVibratorService() {
if (sExternalVibratorService == 0) {
- sp <IBinder> binder = defaultServiceManager()->getService(
+ sp<IBinder> binder = defaultServiceManager()->getService(
String16("external_vibrator_service"));
if (binder != 0) {
sExternalVibratorService =
@@ -402,6 +402,7 @@
status_t AudioFlinger::addEffectToHal(audio_port_handle_t deviceId,
audio_module_handle_t hwModuleId, sp<EffectHalInterface> effect) {
+ AutoMutex lock(mHardwareLock);
AudioHwDevice *audioHwDevice = mAudioHwDevs.valueFor(hwModuleId);
if (audioHwDevice == nullptr) {
return NO_INIT;
@@ -411,6 +412,7 @@
status_t AudioFlinger::removeEffectFromHal(audio_port_handle_t deviceId,
audio_module_handle_t hwModuleId, sp<EffectHalInterface> effect) {
+ AutoMutex lock(mHardwareLock);
AudioHwDevice *audioHwDevice = mAudioHwDevs.valueFor(hwModuleId);
if (audioHwDevice == nullptr) {
return NO_INIT;
@@ -430,6 +432,7 @@
{
// 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++) {
@@ -1069,17 +1072,18 @@
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
@@ -1146,6 +1150,9 @@
{ // scope for the lock
AutoMutex lock(mHardwareLock);
+ if (mPrimaryHardwareDev == nullptr) {
+ return INVALID_OPERATION;
+ }
sp<DeviceHalInterface> dev = mPrimaryHardwareDev->hwDevice();
mHardwareStatus = AUDIO_HW_SET_MODE;
ret = dev->setMode(mode);
@@ -1175,6 +1182,9 @@
}
AutoMutex lock(mHardwareLock);
+ if (mPrimaryHardwareDev == nullptr) {
+ return INVALID_OPERATION;
+ }
sp<DeviceHalInterface> primaryDev = mPrimaryHardwareDev->hwDevice();
if (primaryDev == nullptr) {
ALOGW("%s: no primary HAL device", __func__);
@@ -1200,6 +1210,9 @@
return false;
}
AutoMutex lock(mHardwareLock);
+ if (mPrimaryHardwareDev == nullptr) {
+ return false;
+ }
sp<DeviceHalInterface> primaryDev = mPrimaryHardwareDev->hwDevice();
if (primaryDev == nullptr) {
ALOGW("%s: no primary HAL device", __func__);
@@ -1242,15 +1255,17 @@
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
@@ -1581,16 +1596,13 @@
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<DeviceHalInterface> 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;
@@ -1623,6 +1635,9 @@
}
AutoMutex lock(mHardwareLock);
+ if (mPrimaryHardwareDev == nullptr) {
+ return 0;
+ }
mHardwareStatus = AUDIO_HW_GET_INPUT_BUFFER_SIZE;
audio_config_t config, proposed;
memset(&proposed, 0, sizeof(proposed));
@@ -1684,6 +1699,9 @@
}
AutoMutex lock(mHardwareLock);
+ if (mPrimaryHardwareDev == nullptr) {
+ return INVALID_OPERATION;
+ }
sp<DeviceHalInterface> dev = mPrimaryHardwareDev->hwDevice();
mHardwareStatus = AUDIO_HW_SET_VOICE_VOLUME;
ret = dev->setVoiceVolume(value);
@@ -2088,10 +2106,11 @@
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++) {
@@ -2123,44 +2142,49 @@
// master mute and volume settings.
AudioHwDevice::Flags flags = static_cast<AudioHwDevice::Flags>(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<AudioHwDevice::Flags>(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<AudioHwDevice::Flags>(flags |
- AudioHwDevice::AHWD_CAN_SET_MASTER_MUTE);
- }
-
- mHardwareStatus = AUDIO_HW_IDLE;
}
+
+ mHardwareStatus = AUDIO_HW_SET_MASTER_VOLUME;
+ if (OK == dev->setMasterVolume(mMasterVolume)) {
+ flags = static_cast<AudioHwDevice::Flags>(flags |
+ AudioHwDevice::AHWD_CAN_SET_MASTER_VOLUME);
+ }
+
+ mHardwareStatus = AUDIO_HW_SET_MASTER_MUTE;
+ if (OK == dev->setMasterMute(mMasterMute)) {
+ flags = static_cast<AudioHwDevice::Flags>(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<AudioHwDevice::Flags>(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);
@@ -2246,6 +2270,7 @@
}
Mutex::Autolock _l(mLock);
+ AutoMutex lock(mHardwareLock);
ssize_t index = mAudioHwDevs.indexOfKey(module);
if (index < 0) {
ALOGW("%s() bad hw module %d", __func__, module);
@@ -2267,8 +2292,15 @@
return mHwAvSyncIds.valueAt(index);
}
- sp<DeviceHalInterface> dev = mPrimaryHardwareDev->hwDevice();
- if (dev == NULL) {
+ sp<DeviceHalInterface> 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;
@@ -2336,8 +2368,21 @@
status_t AudioFlinger::getMicrophones(std::vector<media::MicrophoneInfo> *microphones)
{
AutoMutex lock(mHardwareLock);
- sp<DeviceHalInterface> 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<media::MicrophoneInfo> 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;
}
@@ -2484,12 +2529,13 @@
// 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;
@@ -3157,6 +3203,10 @@
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()) {
@@ -3167,7 +3217,7 @@
return thread;
}
}
- return NULL;
+ return nullptr;
}
DeviceTypeSet AudioFlinger::primaryOutputDevice_l() const
diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h
index 115bbb8..e9fb5a1 100644
--- a/services/audioflinger/AudioFlinger.h
+++ b/services/audioflinger/AudioFlinger.h
@@ -823,10 +823,11 @@
// 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<audio_module_handle_t, AudioHwDevice*> mAudioHwDevs;
+ // These two fields are immutable after onFirstRef(), so no lock needed to access
sp<DevicesFactoryHalInterface> mDevicesFactoryHal;
sp<DevicesFactoryHalCallback> mDevicesFactoryHalCallback;
@@ -853,6 +854,7 @@
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