From c0d04987f6f7bd6e054210b1ca33a827afce650b Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Mon, 2 Mar 2020 21:02:28 +0000 Subject: [PATCH 1/3] Allow for late audio module discovery in APM Extract code that loads hardware modules and fills out mAvailable{Input|Output}Devices into a reusable function. It is used both during APM initialization and on receiving a notification that a new HAL service has been discovered. Bug: 149854039 Test: atest audiopolicy_tests Change-Id: Ifb7f0b61b06a0609802e63dccb26f3472328ef48 --- services/audiopolicy/AudioPolicyInterface.h | 4 + .../include/AudioPolicyConfig.h | 44 +++--- .../managerdefinitions/src/Serializer.cpp | 2 +- .../managerdefault/AudioPolicyManager.cpp | 142 +++++++++--------- .../managerdefault/AudioPolicyManager.h | 9 +- .../tests/AudioPolicyManagerTestClient.h | 13 +- .../tests/AudioPolicyTestManager.h | 2 + .../audiopolicy/tests/audio_health_tests.cpp | 4 +- .../tests/audiopolicymanager_tests.cpp | 47 +++++- 9 files changed, 158 insertions(+), 109 deletions(-) diff --git a/services/audiopolicy/AudioPolicyInterface.h b/services/audiopolicy/AudioPolicyInterface.h index 4d53be4915..8d0e5dbed7 100644 --- a/services/audiopolicy/AudioPolicyInterface.h +++ b/services/audiopolicy/AudioPolicyInterface.h @@ -83,6 +83,10 @@ public: // configuration functions // + // Informs APM that new HAL modules are available. This typically happens + // due to registration of an audio HAL service. + virtual void onNewAudioModulesAvailable() = 0; + // indicate a change in device connection status virtual status_t setDeviceConnectionState(audio_devices_t device, audio_policy_dev_state_t state, diff --git a/services/audiopolicy/common/managerdefinitions/include/AudioPolicyConfig.h b/services/audiopolicy/common/managerdefinitions/include/AudioPolicyConfig.h index e59386fd61..395bc7006f 100644 --- a/services/audiopolicy/common/managerdefinitions/include/AudioPolicyConfig.h +++ b/services/audiopolicy/common/managerdefinitions/include/AudioPolicyConfig.h @@ -37,13 +37,13 @@ class AudioPolicyConfig { public: AudioPolicyConfig(HwModuleCollection &hwModules, - DeviceVector &availableOutputDevices, - DeviceVector &availableInputDevices, + DeviceVector &outputDevices, + DeviceVector &inputDevices, sp &defaultOutputDevice) : mEngineLibraryNameSuffix(kDefaultEngineLibraryNameSuffix), mHwModules(hwModules), - mAvailableOutputDevices(availableOutputDevices), - mAvailableInputDevices(availableInputDevices), + mOutputDevices(outputDevices), + mInputDevices(inputDevices), mDefaultOutputDevice(defaultOutputDevice), mIsSpeakerDrcEnabled(false), mIsCallScreenModeSupported(false) @@ -70,23 +70,23 @@ public: mHwModules = hwModules; } - void addAvailableDevice(const sp &availableDevice) + void addDevice(const sp &device) { - if (audio_is_output_device(availableDevice->type())) { - mAvailableOutputDevices.add(availableDevice); - } else if (audio_is_input_device(availableDevice->type())) { - mAvailableInputDevices.add(availableDevice); + if (audio_is_output_device(device->type())) { + mOutputDevices.add(device); + } else if (audio_is_input_device(device->type())) { + mInputDevices.add(device); } } - void addAvailableInputDevices(const DeviceVector &availableInputDevices) + void addInputDevices(const DeviceVector &inputDevices) { - mAvailableInputDevices.add(availableInputDevices); + mInputDevices.add(inputDevices); } - void addAvailableOutputDevices(const DeviceVector &availableOutputDevices) + void addOutputDevices(const DeviceVector &outputDevices) { - mAvailableOutputDevices.add(availableOutputDevices); + mOutputDevices.add(outputDevices); } bool isSpeakerDrcEnabled() const { return mIsSpeakerDrcEnabled; } @@ -106,14 +106,14 @@ public: const HwModuleCollection getHwModules() const { return mHwModules; } - const DeviceVector &getAvailableInputDevices() const + const DeviceVector &getInputDevices() const { - return mAvailableInputDevices; + return mInputDevices; } - const DeviceVector &getAvailableOutputDevices() const + const DeviceVector &getOutputDevices() const { - return mAvailableOutputDevices; + return mOutputDevices; } void setDefaultOutputDevice(const sp &defaultDevice) @@ -134,13 +134,11 @@ public: sp micProfile = new AudioProfile( AUDIO_FORMAT_PCM_16_BIT, AUDIO_CHANNEL_IN_MONO, 8000); defaultInputDevice->addAudioProfile(micProfile); - mAvailableOutputDevices.add(mDefaultOutputDevice); - mAvailableInputDevices.add(defaultInputDevice); + mOutputDevices.add(mDefaultOutputDevice); + mInputDevices.add(defaultInputDevice); sp module = new HwModule(AUDIO_HARDWARE_MODULE_ID_PRIMARY, 2 /*halVersionMajor*/); mHwModules.add(module); - mDefaultOutputDevice->attach(module); - defaultInputDevice->attach(module); sp outProfile = new OutputProfile("primary"); outProfile->addAudioProfile( @@ -191,8 +189,8 @@ private: std::string mSource; std::string mEngineLibraryNameSuffix; HwModuleCollection &mHwModules; /**< Collection of Module, with Profiles, i.e. Mix Ports. */ - DeviceVector &mAvailableOutputDevices; - DeviceVector &mAvailableInputDevices; + DeviceVector &mOutputDevices; + DeviceVector &mInputDevices; sp &mDefaultOutputDevice; // TODO: remove when legacy conf file is removed. true on devices that use DRC on the // DEVICE_CATEGORY_SPEAKER path to boost soft sounds, used to adjust volume curves accordingly. diff --git a/services/audiopolicy/common/managerdefinitions/src/Serializer.cpp b/services/audiopolicy/common/managerdefinitions/src/Serializer.cpp index 1b2f7c79cc..883e713326 100644 --- a/services/audiopolicy/common/managerdefinitions/src/Serializer.cpp +++ b/services/audiopolicy/common/managerdefinitions/src/Serializer.cpp @@ -653,7 +653,7 @@ Return ModuleTraits::deserialize(const xmlNode *cur, PtrS sp device = module->getDeclaredDevices(). getDeviceFromTagName(std::string(reinterpret_cast( attachedDevice.get()))); - ctx->addAvailableDevice(device); + ctx->addDevice(device); } } } diff --git a/services/audiopolicy/managerdefault/AudioPolicyManager.cpp b/services/audiopolicy/managerdefault/AudioPolicyManager.cpp index 0aa087acb1..5a012bfcb2 100644 --- a/services/audiopolicy/managerdefault/AudioPolicyManager.cpp +++ b/services/audiopolicy/managerdefault/AudioPolicyManager.cpp @@ -4397,7 +4397,7 @@ AudioPolicyManager::AudioPolicyManager(AudioPolicyClientInterface *clientInterfa mpClientInterface(clientInterface), mLimitRingtoneVolume(false), mLastVoiceVolume(-1.0f), mA2dpSuspended(false), - mConfig(mHwModulesAll, mAvailableOutputDevices, mAvailableInputDevices, mDefaultOutputDevice), + mConfig(mHwModulesAll, mOutputDevicesAll, mInputDevicesAll, mDefaultOutputDevice), mAudioPortGeneration(1), mBeaconMuteRefCount(0), mBeaconPlayingRefCount(0), @@ -4442,9 +4442,69 @@ status_t AudioPolicyManager::initialize() { return status; } - // mAvailableOutputDevices and mAvailableInputDevices now contain all attached devices + // after parsing the config, mOutputDevicesAll and mInputDevicesAll contain all known devices; // open all output streams needed to access attached devices + onNewAudioModulesAvailable(); + + // make sure default device is reachable + if (mDefaultOutputDevice == 0 || !mAvailableOutputDevices.contains(mDefaultOutputDevice)) { + ALOGE_IF(mDefaultOutputDevice != 0, "Default device %s is unreachable", + mDefaultOutputDevice->toString().c_str()); + status = NO_INIT; + } + // If microphones address is empty, set it according to device type + for (size_t i = 0; i < mAvailableInputDevices.size(); i++) { + if (mAvailableInputDevices[i]->address().empty()) { + if (mAvailableInputDevices[i]->type() == AUDIO_DEVICE_IN_BUILTIN_MIC) { + mAvailableInputDevices[i]->setAddress(AUDIO_BOTTOM_MICROPHONE_ADDRESS); + } else if (mAvailableInputDevices[i]->type() == AUDIO_DEVICE_IN_BACK_MIC) { + mAvailableInputDevices[i]->setAddress(AUDIO_BACK_MICROPHONE_ADDRESS); + } + } + } + + if (mPrimaryOutput == 0) { + ALOGE("Failed to open primary output"); + status = NO_INIT; + } + + // Silence ALOGV statements + property_set("log.tag." LOG_TAG, "D"); + + updateDevicesAndOutputs(); + return status; +} + +AudioPolicyManager::~AudioPolicyManager() +{ + for (size_t i = 0; i < mOutputs.size(); i++) { + mOutputs.valueAt(i)->close(); + } + for (size_t i = 0; i < mInputs.size(); i++) { + mInputs.valueAt(i)->close(); + } + mAvailableOutputDevices.clear(); + mAvailableInputDevices.clear(); + mOutputs.clear(); + mInputs.clear(); + mHwModules.clear(); + mHwModulesAll.clear(); + mManualSurroundFormats.clear(); +} + +status_t AudioPolicyManager::initCheck() +{ + return hasPrimaryOutput() ? NO_ERROR : NO_INIT; +} + +// --- + +void AudioPolicyManager::onNewAudioModulesAvailable() +{ for (const auto& hwModule : mHwModulesAll) { + if (std::find(mHwModules.begin(), mHwModules.end(), hwModule) != mHwModules.end()) { + continue; + } hwModule->setHandle(mpClientInterface->loadHwModule(hwModule->getName())); if (hwModule->getHandle() == AUDIO_MODULE_HANDLE_NONE) { ALOGW("could not open HW module %s", hwModule->getName()); @@ -4473,7 +4533,7 @@ status_t AudioPolicyManager::initialize() { continue; } const DeviceVector &supportedDevices = outProfile->getSupportedDevices(); - DeviceVector availProfileDevices = supportedDevices.filter(mAvailableOutputDevices); + DeviceVector availProfileDevices = supportedDevices.filter(mOutputDevicesAll); sp supportedDevice = 0; if (supportedDevices.contains(mDefaultOutputDevice)) { supportedDevice = mDefaultOutputDevice; @@ -4485,7 +4545,7 @@ status_t AudioPolicyManager::initialize() { } supportedDevice = availProfileDevices.itemAt(0); } - if (!mAvailableOutputDevices.contains(supportedDevice)) { + if (!mOutputDevicesAll.contains(supportedDevice)) { continue; } sp outputDesc = new SwAudioOutputDescriptor(outProfile, @@ -4503,6 +4563,8 @@ status_t AudioPolicyManager::initialize() { // give a valid ID to an attached device once confirmed it is reachable if (!device->isAttached()) { device->attach(hwModule); + mAvailableOutputDevices.add(device); + setEngineDeviceConnectionState(device, AUDIO_POLICY_DEVICE_STATE_AVAILABLE); } } if (mPrimaryOutput == 0 && @@ -4531,7 +4593,7 @@ status_t AudioPolicyManager::initialize() { // chose first device present in profile's SupportedDevices also part of // available input devices const DeviceVector &supportedDevices = inProfile->getSupportedDevices(); - DeviceVector availProfileDevices = supportedDevices.filter(mAvailableInputDevices); + DeviceVector availProfileDevices = supportedDevices.filter(mInputDevicesAll); if (availProfileDevices.isEmpty()) { ALOGE("%s: Input device list is empty!", __FUNCTION__); continue; @@ -4556,81 +4618,15 @@ status_t AudioPolicyManager::initialize() { if (!device->isAttached()) { device->attach(hwModule); device->importAudioPortAndPickAudioProfile(inProfile, true); + mAvailableInputDevices.add(device); + setEngineDeviceConnectionState(device, AUDIO_POLICY_DEVICE_STATE_AVAILABLE); } } inputDesc->close(); } } - // make sure all attached devices have been allocated a unique ID - auto checkAndSetAvailable = [this](auto& devices) { - for (size_t i = 0; i < devices.size();) { - const auto &device = devices[i]; - if (!device->isAttached()) { - ALOGW("device %s is unreachable", device->toString().c_str()); - devices.remove(device); - continue; - } - // Device is now validated and can be appended to the available devices of the engine - setEngineDeviceConnectionState(device, AUDIO_POLICY_DEVICE_STATE_AVAILABLE); - i++; - } - }; - checkAndSetAvailable(mAvailableOutputDevices); - checkAndSetAvailable(mAvailableInputDevices); - - // make sure default device is reachable - if (mDefaultOutputDevice == 0 || !mAvailableOutputDevices.contains(mDefaultOutputDevice)) { - ALOGE_IF(mDefaultOutputDevice != 0, "Default device %s is unreachable", - mDefaultOutputDevice->toString().c_str()); - status = NO_INIT; - } - // If microphones address is empty, set it according to device type - for (size_t i = 0; i < mAvailableInputDevices.size(); i++) { - if (mAvailableInputDevices[i]->address().empty()) { - if (mAvailableInputDevices[i]->type() == AUDIO_DEVICE_IN_BUILTIN_MIC) { - mAvailableInputDevices[i]->setAddress(AUDIO_BOTTOM_MICROPHONE_ADDRESS); - } else if (mAvailableInputDevices[i]->type() == AUDIO_DEVICE_IN_BACK_MIC) { - mAvailableInputDevices[i]->setAddress(AUDIO_BACK_MICROPHONE_ADDRESS); - } - } - } - - if (mPrimaryOutput == 0) { - ALOGE("Failed to open primary output"); - status = NO_INIT; - } - - // Silence ALOGV statements - property_set("log.tag." LOG_TAG, "D"); - - updateDevicesAndOutputs(); - return status; -} - -AudioPolicyManager::~AudioPolicyManager() -{ - for (size_t i = 0; i < mOutputs.size(); i++) { - mOutputs.valueAt(i)->close(); - } - for (size_t i = 0; i < mInputs.size(); i++) { - mInputs.valueAt(i)->close(); - } - mAvailableOutputDevices.clear(); - mAvailableInputDevices.clear(); - mOutputs.clear(); - mInputs.clear(); - mHwModules.clear(); - mHwModulesAll.clear(); - mManualSurroundFormats.clear(); } -status_t AudioPolicyManager::initCheck() -{ - return hasPrimaryOutput() ? NO_ERROR : NO_INIT; -} - -// --- - void AudioPolicyManager::addOutput(audio_io_handle_t output, const sp& outputDesc) { diff --git a/services/audiopolicy/managerdefault/AudioPolicyManager.h b/services/audiopolicy/managerdefault/AudioPolicyManager.h index b0adbc7cba..8121f864bd 100644 --- a/services/audiopolicy/managerdefault/AudioPolicyManager.h +++ b/services/audiopolicy/managerdefault/AudioPolicyManager.h @@ -325,6 +325,8 @@ public: bool isCallScreenModeSupported() override; + void onNewAudioModulesAvailable() override; + status_t initialize(); protected: @@ -729,6 +731,8 @@ protected: SwAudioOutputCollection mPreviousOutputs; AudioInputCollection mInputs; // list of input descriptors + DeviceVector mOutputDevicesAll; // all output devices from the config + DeviceVector mInputDevicesAll; // all input devices from the config DeviceVector mAvailableOutputDevices; // all available output devices DeviceVector mAvailableInputDevices; // all available input devices @@ -739,9 +743,8 @@ protected: EffectDescriptorCollection mEffects; // list of registered audio effects sp mDefaultOutputDevice; // output device selected by default at boot time - HwModuleCollection mHwModules; // contains only modules that have been loaded successfully - HwModuleCollection mHwModulesAll; // normally not needed, used during construction and for - // dumps + HwModuleCollection mHwModules; // contains modules that have been loaded successfully + HwModuleCollection mHwModulesAll; // contains all modules declared in the config AudioPolicyConfig mConfig; diff --git a/services/audiopolicy/tests/AudioPolicyManagerTestClient.h b/services/audiopolicy/tests/AudioPolicyManagerTestClient.h index c2a92d7270..af69466482 100644 --- a/services/audiopolicy/tests/AudioPolicyManagerTestClient.h +++ b/services/audiopolicy/tests/AudioPolicyManagerTestClient.h @@ -15,6 +15,7 @@ */ #include +#include #include #include @@ -27,7 +28,10 @@ namespace android { class AudioPolicyManagerTestClient : public AudioPolicyTestClient { public: // AudioPolicyClientInterface implementation - audio_module_handle_t loadHwModule(const char * /*name*/) override { + audio_module_handle_t loadHwModule(const char* name) override { + if (!mAllowedModuleNames.empty() && !mAllowedModuleNames.count(name)) { + return AUDIO_MODULE_HANDLE_NONE; + } return mNextModuleHandle++; } @@ -101,11 +105,18 @@ public: return &it->second; }; + audio_module_handle_t peekNextModuleHandle() const { return mNextModuleHandle; } + + void swapAllowedModuleNames(std::set&& names = {}) { + mAllowedModuleNames.swap(names); + } + private: audio_module_handle_t mNextModuleHandle = AUDIO_MODULE_HANDLE_NONE + 1; audio_io_handle_t mNextIoHandle = AUDIO_IO_HANDLE_NONE + 1; audio_patch_handle_t mNextPatchHandle = AUDIO_PATCH_HANDLE_NONE + 1; std::map mActivePatches; + std::set mAllowedModuleNames; }; } // namespace android diff --git a/services/audiopolicy/tests/AudioPolicyTestManager.h b/services/audiopolicy/tests/AudioPolicyTestManager.h index bafcc63963..922a53873b 100644 --- a/services/audiopolicy/tests/AudioPolicyTestManager.h +++ b/services/audiopolicy/tests/AudioPolicyTestManager.h @@ -27,6 +27,8 @@ class AudioPolicyTestManager : public AudioPolicyManager { using AudioPolicyManager::loadConfig; using AudioPolicyManager::initialize; using AudioPolicyManager::getOutputs; + using AudioPolicyManager::getAvailableOutputDevices; + using AudioPolicyManager::getAvailableInputDevices; }; } // namespace android diff --git a/services/audiopolicy/tests/audio_health_tests.cpp b/services/audiopolicy/tests/audio_health_tests.cpp index 8736cf1425..b5c67a1040 100644 --- a/services/audiopolicy/tests/audio_health_tests.cpp +++ b/services/audiopolicy/tests/audio_health_tests.cpp @@ -67,10 +67,10 @@ TEST(AudioHealthTest, AttachedDeviceFound) { manager.loadConfig(); ASSERT_NE("AudioPolicyConfig::setDefault", manager.getConfig().getSource()); - for (auto desc : manager.getConfig().getAvailableInputDevices()) { + for (auto desc : manager.getConfig().getInputDevices()) { ASSERT_NE(attachedDevices.end(), attachedDevices.find(desc->type())); } - for (auto desc : manager.getConfig().getAvailableOutputDevices()) { + for (auto desc : manager.getConfig().getOutputDevices()) { ASSERT_NE(attachedDevices.end(), attachedDevices.find(desc->type())); } } diff --git a/services/audiopolicy/tests/audiopolicymanager_tests.cpp b/services/audiopolicy/tests/audiopolicymanager_tests.cpp index 2a8349cf22..7d92f344dc 100644 --- a/services/audiopolicy/tests/audiopolicymanager_tests.cpp +++ b/services/audiopolicy/tests/audiopolicymanager_tests.cpp @@ -298,9 +298,9 @@ TEST_F(AudioPolicyManagerTest, CreateAudioPatchFromMix) { audio_patch_handle_t handle = AUDIO_PATCH_HANDLE_NONE; uid_t uid = 42; const PatchCountCheck patchCount = snapshotPatchCount(); - ASSERT_FALSE(mManager->getConfig().getAvailableInputDevices().isEmpty()); + ASSERT_FALSE(mManager->getAvailableInputDevices().isEmpty()); PatchBuilder patchBuilder; - patchBuilder.addSource(mManager->getConfig().getAvailableInputDevices()[0]). + patchBuilder.addSource(mManager->getAvailableInputDevices()[0]). addSink(mManager->getConfig().getDefaultOutputDevice()); ASSERT_EQ(NO_ERROR, mManager->createAudioPatch(patchBuilder.patch(), &handle, uid)); ASSERT_NE(AUDIO_PATCH_HANDLE_NONE, handle); @@ -334,15 +334,13 @@ void AudioPolicyManagerTestMsd::SetUpManagerConfig() { sp pcmInputProfile = new AudioProfile( AUDIO_FORMAT_PCM_16_BIT, AUDIO_CHANNEL_IN_STEREO, 44100); mMsdInputDevice->addAudioProfile(pcmInputProfile); - config.addAvailableDevice(mMsdOutputDevice); - config.addAvailableDevice(mMsdInputDevice); + config.addDevice(mMsdOutputDevice); + config.addDevice(mMsdInputDevice); sp msdModule = new HwModule(AUDIO_HARDWARE_MODULE_ID_MSD, 2 /*halVersionMajor*/); HwModuleCollection modules = config.getHwModules(); modules.add(msdModule); config.setHwModules(modules); - mMsdOutputDevice->attach(msdModule); - mMsdInputDevice->attach(msdModule); sp msdOutputProfile = new OutputProfile("msd input"); msdOutputProfile->addAudioProfile(pcmOutputProfile); @@ -1127,3 +1125,40 @@ TEST_F(AudioPolicyManagerTVTest, MatchOutputDirectMMapNoIrq) { AUDIO_OUTPUT_FLAG_DIRECT|AUDIO_OUTPUT_FLAG_MMAP_NOIRQ), "low latency"); } + +class AudioPolicyManagerDynamicHwModulesTest : public AudioPolicyManagerTestWithConfigurationFile { +protected: + void SetUpManagerConfig() override; +}; + +void AudioPolicyManagerDynamicHwModulesTest::SetUpManagerConfig() { + AudioPolicyManagerTestWithConfigurationFile::SetUpManagerConfig(); + // Only allow successful opening of "primary" hw module during APM initialization. + mClient->swapAllowedModuleNames({"primary"}); +} + +TEST_F(AudioPolicyManagerDynamicHwModulesTest, InitSuccess) { + // SetUp must finish with no assertions. +} + +TEST_F(AudioPolicyManagerDynamicHwModulesTest, DynamicAddition) { + const auto handleBefore = mClient->peekNextModuleHandle(); + mManager->onNewAudioModulesAvailable(); + ASSERT_EQ(handleBefore, mClient->peekNextModuleHandle()); + // Reset module loading restrictions. + mClient->swapAllowedModuleNames(); + mManager->onNewAudioModulesAvailable(); + const auto handleAfter = mClient->peekNextModuleHandle(); + ASSERT_GT(handleAfter, handleBefore); + mManager->onNewAudioModulesAvailable(); + ASSERT_EQ(handleAfter, mClient->peekNextModuleHandle()); +} + +TEST_F(AudioPolicyManagerDynamicHwModulesTest, AddedDeviceAvailable) { + ASSERT_EQ(AUDIO_POLICY_DEVICE_STATE_UNAVAILABLE, mManager->getDeviceConnectionState( + AUDIO_DEVICE_IN_REMOTE_SUBMIX, "0")); + mClient->swapAllowedModuleNames({"primary", "r_submix"}); + mManager->onNewAudioModulesAvailable(); + ASSERT_EQ(AUDIO_POLICY_DEVICE_STATE_AVAILABLE, mManager->getDeviceConnectionState( + AUDIO_DEVICE_IN_REMOTE_SUBMIX, "0")); +} From 88de22ff76578985801dd7662acb07a5641298c1 Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Fri, 6 Mar 2020 10:55:34 -0800 Subject: [PATCH 2/3] libaudiohal: Allow late registration of HIDL HAL services DevicesFactoryHalHidl registers a notification callback with a service manager to get notified about registration of new HAL services. The specific code for the MSD HAL has been removed, this HAL service is now handled using the added generic mechanism. Bug: 149854039 Test: make Change-Id: I2f7f83dec11ac2390f674afd9e4451ef46dde04e --- .../impl/DevicesFactoryHalHidl.cpp | 112 ++++++++++++++---- .../libaudiohal/impl/DevicesFactoryHalHidl.h | 21 +++- .../impl/DevicesFactoryHalHybrid.cpp | 7 ++ .../impl/DevicesFactoryHalHybrid.h | 2 + .../libaudiohal/impl/DevicesFactoryHalLocal.h | 4 + .../audiohal/DevicesFactoryHalInterface.h | 10 ++ 6 files changed, 130 insertions(+), 26 deletions(-) diff --git a/media/libaudiohal/impl/DevicesFactoryHalHidl.cpp b/media/libaudiohal/impl/DevicesFactoryHalHidl.cpp index e6e96885f3..1c0eacb88b 100644 --- a/media/libaudiohal/impl/DevicesFactoryHalHidl.cpp +++ b/media/libaudiohal/impl/DevicesFactoryHalHidl.cpp @@ -15,12 +15,13 @@ */ #include -#include +#include #define LOG_TAG "DevicesFactoryHalHidl" //#define LOG_NDEBUG 0 #include +#include #include PATH(android/hardware/audio/FILE_VERSION/IDevice.h) #include #include @@ -29,33 +30,57 @@ #include "DeviceHalHidl.h" #include "DevicesFactoryHalHidl.h" -#include - using ::android::hardware::audio::CPP_VERSION::IDevice; using ::android::hardware::audio::CPP_VERSION::Result; using ::android::hardware::Return; +using ::android::hardware::Void; +using ::android::hidl::manager::V1_0::IServiceManager; +using ::android::hidl::manager::V1_0::IServiceNotification; namespace android { namespace CPP_VERSION { -DevicesFactoryHalHidl::DevicesFactoryHalHidl(sp devicesFactory) { - ALOG_ASSERT(devicesFactory != nullptr, "Provided IDevicesFactory service is NULL"); - - mDeviceFactories.push_back(devicesFactory); - if (MAJOR_VERSION >= 4) { - // The MSD factory is optional and only available starting at HAL 4.0 - sp msdFactory{IDevicesFactory::getService(AUDIO_HAL_SERVICE_NAME_MSD)}; - if (msdFactory) { - mDeviceFactories.push_back(msdFactory); +class ServiceNotificationListener : public IServiceNotification { + public: + explicit ServiceNotificationListener(sp factory) + : mFactory(factory) {} + + Return onRegistration(const hidl_string& /*fully_qualified_name*/, + const hidl_string& instance_name, + bool /*pre_existing*/) override { + if (static_cast(instance_name) == "default") return Void(); + sp factory = mFactory.promote(); + if (!factory) return Void(); + sp halFactory = IDevicesFactory::getService(instance_name); + if (halFactory) { + factory->addDeviceFactory(halFactory, true /*needToNotify*/); } + return Void(); } - for (const auto& factory : mDeviceFactories) { - // It is assumed that the DevicesFactoryHalInterface instance is owned - // by AudioFlinger and thus have the same lifespan. - factory->linkToDeath(HalDeathHandler::getInstance(), 0 /*cookie*/); - } + + private: + wp mFactory; +}; + +DevicesFactoryHalHidl::DevicesFactoryHalHidl(sp devicesFactory) { + ALOG_ASSERT(devicesFactory != nullptr, "Provided default IDevicesFactory service is NULL"); + addDeviceFactory(devicesFactory, false /*needToNotify*/); } +void DevicesFactoryHalHidl::onFirstRef() { + sp sm = IServiceManager::getService(); + ALOG_ASSERT(sm != nullptr, "Hardware service manager is not running"); + sp listener = new ServiceNotificationListener(this); + Return result = sm->registerForNotifications( + IDevicesFactory::descriptor, "", listener); + if (result.isOk()) { + ALOGE_IF(!static_cast(result), + "Hardware service manager refused to register listener"); + } else { + ALOGE("Failed to register for hardware service manager notifications: %s", + result.description().c_str()); + } +} #if MAJOR_VERSION == 2 static IDevicesFactory::Device idFromHal(const char *name, status_t* status) { @@ -83,12 +108,13 @@ static const char* idFromHal(const char *name, status_t* status) { #endif status_t DevicesFactoryHalHidl::openDevice(const char *name, sp *device) { - if (mDeviceFactories.empty()) return NO_INIT; + auto factories = copyDeviceFactories(); + if (factories.empty()) return NO_INIT; status_t status; auto hidlId = idFromHal(name, &status); if (status != OK) return status; Result retval = Result::NOT_INITIALIZED; - for (const auto& factory : mDeviceFactories) { + for (const auto& factory : factories) { Return ret = factory->openDevice( hidlId, [&](Result r, const sp& result) { @@ -113,10 +139,9 @@ status_t DevicesFactoryHalHidl::openDevice(const char *name, sp *pids) { std::set pidsSet; - - for (const auto& factory : mDeviceFactories) { + auto factories = copyDeviceFactories(); + for (const auto& factory : factories) { using ::android::hidl::base::V1_0::DebugInfo; - using android::hidl::manager::V1_0::IServiceManager; DebugInfo debugInfo; auto ret = factory->getDebugInfo([&] (const auto &info) { @@ -135,5 +160,48 @@ status_t DevicesFactoryHalHidl::getHalPids(std::vector *pids) { return NO_ERROR; } +status_t DevicesFactoryHalHidl::setCallbackOnce(sp callback) { + ALOG_ASSERT(callback != nullptr); + bool needToCallCallback = false; + { + std::lock_guard lock(mLock); + if (mCallback.unsafe_get()) return INVALID_OPERATION; + mCallback = callback; + if (mHaveUndeliveredNotifications) { + needToCallCallback = true; + mHaveUndeliveredNotifications = false; + } + } + if (needToCallCallback) { + callback->onNewDevicesAvailable(); + } + return NO_ERROR; +} + +void DevicesFactoryHalHidl::addDeviceFactory(sp factory, bool needToNotify) { + // It is assumed that the DevicesFactoryHalInterface instance is owned + // by AudioFlinger and thus have the same lifespan. + factory->linkToDeath(HalDeathHandler::getInstance(), 0 /*cookie*/); + sp callback; + { + std::lock_guard lock(mLock); + mDeviceFactories.push_back(factory); + if (needToNotify) { + callback = mCallback.promote(); + if (!callback) { + mHaveUndeliveredNotifications = true; + } + } + } + if (callback) { + callback->onNewDevicesAvailable(); + } +} + +std::vector> DevicesFactoryHalHidl::copyDeviceFactories() { + std::lock_guard lock(mLock); + return mDeviceFactories; +} + } // namespace CPP_VERSION } // namespace android diff --git a/media/libaudiohal/impl/DevicesFactoryHalHidl.h b/media/libaudiohal/impl/DevicesFactoryHalHidl.h index 52185c8641..6f84efeacc 100644 --- a/media/libaudiohal/impl/DevicesFactoryHalHidl.h +++ b/media/libaudiohal/impl/DevicesFactoryHalHidl.h @@ -17,6 +17,9 @@ #ifndef ANDROID_HARDWARE_DEVICES_FACTORY_HAL_HIDL_H #define ANDROID_HARDWARE_DEVICES_FACTORY_HAL_HIDL_H +#include +#include + #include PATH(android/hardware/audio/FILE_VERSION/IDevicesFactory.h) #include #include @@ -32,16 +35,26 @@ namespace CPP_VERSION { class DevicesFactoryHalHidl : public DevicesFactoryHalInterface { public: - DevicesFactoryHalHidl(sp devicesFactory); + explicit DevicesFactoryHalHidl(sp devicesFactory); + void onFirstRef() override; // Opens a device with the specified name. To close the device, it is // necessary to release references to the returned object. - virtual status_t openDevice(const char *name, sp *device); + status_t openDevice(const char *name, sp *device) override; + + status_t getHalPids(std::vector *pids) override; - status_t getHalPids(std::vector *pids) override; + status_t setCallbackOnce(sp callback) override; private: - std::vector> mDeviceFactories; + friend class ServiceNotificationListener; + void addDeviceFactory(sp factory, bool needToNotify); + std::vector> copyDeviceFactories(); + + std::mutex mLock; + std::vector> mDeviceFactories; // GUARDED_BY(mLock) + wp mCallback; // GUARDED_BY(mLock) + bool mHaveUndeliveredNotifications = false; // GUARDED_BY(mLock) virtual ~DevicesFactoryHalHidl() = default; }; diff --git a/media/libaudiohal/impl/DevicesFactoryHalHybrid.cpp b/media/libaudiohal/impl/DevicesFactoryHalHybrid.cpp index 52f150a3db..cde8d85e87 100644 --- a/media/libaudiohal/impl/DevicesFactoryHalHybrid.cpp +++ b/media/libaudiohal/impl/DevicesFactoryHalHybrid.cpp @@ -44,6 +44,13 @@ status_t DevicesFactoryHalHybrid::getHalPids(std::vector *pids) { return INVALID_OPERATION; } +status_t DevicesFactoryHalHybrid::setCallbackOnce(sp callback) { + if (mHidlFactory) { + return mHidlFactory->setCallbackOnce(callback); + } + return INVALID_OPERATION; +} + } // namespace CPP_VERSION extern "C" __attribute__((visibility("default"))) void* createIDevicesFactory() { diff --git a/media/libaudiohal/impl/DevicesFactoryHalHybrid.h b/media/libaudiohal/impl/DevicesFactoryHalHybrid.h index 2189b36f87..568a1fbaeb 100644 --- a/media/libaudiohal/impl/DevicesFactoryHalHybrid.h +++ b/media/libaudiohal/impl/DevicesFactoryHalHybrid.h @@ -38,6 +38,8 @@ class DevicesFactoryHalHybrid : public DevicesFactoryHalInterface status_t getHalPids(std::vector *pids) override; + status_t setCallbackOnce(sp callback) override; + private: sp mLocalFactory; sp mHidlFactory; diff --git a/media/libaudiohal/impl/DevicesFactoryHalLocal.h b/media/libaudiohal/impl/DevicesFactoryHalLocal.h index 2b011f47b8..32bf362001 100644 --- a/media/libaudiohal/impl/DevicesFactoryHalLocal.h +++ b/media/libaudiohal/impl/DevicesFactoryHalLocal.h @@ -37,6 +37,10 @@ class DevicesFactoryHalLocal : public DevicesFactoryHalInterface return INVALID_OPERATION; } + status_t setCallbackOnce(sp callback __unused) override { + return INVALID_OPERATION; + } + private: friend class DevicesFactoryHalHybrid; diff --git a/media/libaudiohal/include/media/audiohal/DevicesFactoryHalInterface.h b/media/libaudiohal/include/media/audiohal/DevicesFactoryHalInterface.h index e9ac1ce34d..5091558ce4 100644 --- a/media/libaudiohal/include/media/audiohal/DevicesFactoryHalInterface.h +++ b/media/libaudiohal/include/media/audiohal/DevicesFactoryHalInterface.h @@ -24,6 +24,12 @@ namespace android { +class DevicesFactoryHalCallback : public RefBase +{ + public: + virtual void onNewDevicesAvailable() = 0; +}; + class DevicesFactoryHalInterface : public RefBase { public: @@ -33,6 +39,10 @@ class DevicesFactoryHalInterface : public RefBase virtual status_t getHalPids(std::vector *pids) = 0; + // Sets a DevicesFactoryHalCallback to notify the client. + // The callback can be only set once. + virtual status_t setCallbackOnce(sp callback) = 0; + static sp create(); protected: From 88b30d2c1520112670472ae879b20cc33a1b6eb0 Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Mon, 9 Mar 2020 19:43:13 +0000 Subject: [PATCH 3/3] Plumb the notification about audio HAL services update to APM AudioFlinger registers a callback with libaudiohal to receive notifications when new HAL services get registered. It relays the notification to AudioPolicyManager via AudioSystem / IAudioPolicyService interface. Because AF / APM only interact via Binder interfaces and APM's interface gets registered later than AF's, the notification from AF is made asynchronous. Bug: 149854039 Test: audio test on a regular phone configuration audio test on a phone with MSD audio HAL module Change-Id: I158e941b8f75e2a4614b9d84ca798b0f1f47aa6a --- media/libaudioclient/AudioSystem.cpp | 7 ++++++ media/libaudioclient/IAudioPolicyService.cpp | 17 +++++++++++++- .../include/media/AudioSystem.h | 1 + .../include/media/IAudioPolicyService.h | 1 + services/audioflinger/AudioFlinger.cpp | 17 ++++++++++++++ services/audioflinger/AudioFlinger.h | 2 ++ .../service/AudioPolicyInterfaceImpl.cpp | 8 +++++++ .../service/AudioPolicyService.cpp | 22 +++++++++++++++++++ .../audiopolicy/service/AudioPolicyService.h | 4 ++++ 9 files changed, 78 insertions(+), 1 deletion(-) diff --git a/media/libaudioclient/AudioSystem.cpp b/media/libaudioclient/AudioSystem.cpp index 2f67a18297..f030ab0e88 100644 --- a/media/libaudioclient/AudioSystem.cpp +++ b/media/libaudioclient/AudioSystem.cpp @@ -792,6 +792,13 @@ const sp AudioSystem::get_audio_policy_service() // --------------------------------------------------------------------------- +void AudioSystem::onNewAudioModulesAvailable() +{ + const sp& aps = AudioSystem::get_audio_policy_service(); + if (aps == 0) return; + aps->onNewAudioModulesAvailable(); +} + status_t AudioSystem::setDeviceConnectionState(audio_devices_t device, audio_policy_dev_state_t state, const char *device_address, diff --git a/media/libaudioclient/IAudioPolicyService.cpp b/media/libaudioclient/IAudioPolicyService.cpp index cccb131405..f1213a381a 100644 --- a/media/libaudioclient/IAudioPolicyService.cpp +++ b/media/libaudioclient/IAudioPolicyService.cpp @@ -113,6 +113,7 @@ enum { REMOVE_PREFERRED_DEVICE_FOR_PRODUCT_STRATEGY, GET_PREFERRED_DEVICE_FOR_PRODUCT_STRATEGY, GET_DEVICES_FOR_ATTRIBUTES, + AUDIO_MODULES_UPDATED, // oneway }; #define MAX_ITEMS_PER_LIST 1024 @@ -1451,6 +1452,13 @@ public: } return NO_ERROR; } + + virtual void onNewAudioModulesAvailable() + { + Parcel data, reply; + data.writeInterfaceToken(IAudioPolicyService::getInterfaceDescriptor()); + remote()->transact(AUDIO_MODULES_UPDATED, data, &reply, IBinder::FLAG_ONEWAY); + } }; IMPLEMENT_META_INTERFACE(AudioPolicyService, "android.media.IAudioPolicyService"); @@ -1522,7 +1530,8 @@ status_t BnAudioPolicyService::onTransact( case REMOVE_PREFERRED_DEVICE_FOR_PRODUCT_STRATEGY: case GET_PREFERRED_DEVICE_FOR_PRODUCT_STRATEGY: case GET_DEVICES_FOR_ATTRIBUTES: - case SET_ALLOWED_CAPTURE_POLICY: { + case SET_ALLOWED_CAPTURE_POLICY: + case AUDIO_MODULES_UPDATED: { if (!isServiceUid(IPCThreadState::self()->getCallingUid())) { ALOGW("%s: transaction %d received from PID %d unauthorized UID %d", __func__, code, IPCThreadState::self()->getCallingPid(), @@ -2672,6 +2681,12 @@ status_t BnAudioPolicyService::onTransact( return NO_ERROR; } + case AUDIO_MODULES_UPDATED: { + CHECK_INTERFACE(IAudioPolicyService, data, reply); + onNewAudioModulesAvailable(); + return NO_ERROR; + } break; + default: return BBinder::onTransact(code, data, reply, flags); } diff --git a/media/libaudioclient/include/media/AudioSystem.h b/media/libaudioclient/include/media/AudioSystem.h index 07a2292262..aebc875f4a 100644 --- a/media/libaudioclient/include/media/AudioSystem.h +++ b/media/libaudioclient/include/media/AudioSystem.h @@ -221,6 +221,7 @@ public: // // IAudioPolicyService interface (see AudioPolicyInterface for method descriptions) // + static void onNewAudioModulesAvailable(); static status_t setDeviceConnectionState(audio_devices_t device, audio_policy_dev_state_t state, const char *device_address, const char *device_name, audio_format_t encodedFormat); diff --git a/media/libaudioclient/include/media/IAudioPolicyService.h b/media/libaudioclient/include/media/IAudioPolicyService.h index 779ca4379e..ec3461e936 100644 --- a/media/libaudioclient/include/media/IAudioPolicyService.h +++ b/media/libaudioclient/include/media/IAudioPolicyService.h @@ -42,6 +42,7 @@ public: // // IAudioPolicyService interface (see AudioPolicyInterface for method descriptions) // + virtual void onNewAudioModulesAvailable() = 0; virtual status_t setDeviceConnectionState(audio_devices_t device, audio_policy_dev_state_t state, const char *device_address, diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp index e5a6e27fad..e1b5bf82a1 100644 --- a/services/audioflinger/AudioFlinger.cpp +++ b/services/audioflinger/AudioFlinger.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include @@ -144,6 +145,19 @@ static sp getExternalVibratorService() { return sExternalVibratorService; } +class DevicesFactoryHalCallbackImpl : public DevicesFactoryHalCallback { + public: + void onNewDevicesAvailable() override { + // Start a detached thread to execute notification in parallel. + // This is done to prevent mutual blocking of audio_flinger and + // audio_policy services during system initialization. + std::thread notifier([]() { + AudioSystem::onNewAudioModulesAvailable(); + }); + notifier.detach(); + } +}; + // ---------------------------------------------------------------------------- std::string formatToString(audio_format_t format) { @@ -227,6 +241,9 @@ void AudioFlinger::onFirstRef() mMode = AUDIO_MODE_NORMAL; gAudioFlinger = this; + + mDevicesFactoryHalCallback = new DevicesFactoryHalCallbackImpl; + mDevicesFactoryHal->setCallbackOnce(mDevicesFactoryHalCallback); } status_t AudioFlinger::setAudioHalPids(const std::vector& pids) { diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h index a16fa94b04..ef218fa44f 100644 --- a/services/audioflinger/AudioFlinger.h +++ b/services/audioflinger/AudioFlinger.h @@ -104,6 +104,7 @@ class AudioMixer; class AudioBuffer; class AudioResampler; class DeviceHalInterface; +class DevicesFactoryHalCallback; class DevicesFactoryHalInterface; class EffectsFactoryHalInterface; class FastMixer; @@ -836,6 +837,7 @@ using effect_buffer_t = int16_t; DefaultKeyedVector mAudioHwDevs; sp mDevicesFactoryHal; + sp mDevicesFactoryHalCallback; // for dump, indicates which hardware operation is currently in progress (but not stream ops) enum hardware_call_state { diff --git a/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp b/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp index 38801ec5b2..0da3b9cb01 100644 --- a/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp +++ b/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp @@ -66,6 +66,14 @@ status_t AudioPolicyService::validateUsage(audio_usage_t usage, pid_t pid, uid_t // ---------------------------------------------------------------------------- +void AudioPolicyService::doOnNewAudioModulesAvailable() +{ + if (mAudioPolicyManager == NULL) return; + Mutex::Autolock _l(mLock); + AutoCallerClear acc; + mAudioPolicyManager->onNewAudioModulesAvailable(); +} + status_t AudioPolicyService::setDeviceConnectionState(audio_devices_t device, audio_policy_dev_state_t state, const char *device_address, diff --git a/services/audiopolicy/service/AudioPolicyService.cpp b/services/audiopolicy/service/AudioPolicyService.cpp index 99cec5a0a5..cd0637cfa0 100644 --- a/services/audiopolicy/service/AudioPolicyService.cpp +++ b/services/audiopolicy/service/AudioPolicyService.cpp @@ -1277,6 +1277,16 @@ bool AudioPolicyService::AudioCommandThread::threadLoop() mLock.lock(); } } break; + case AUDIO_MODULES_UPDATE: { + ALOGV("AudioCommandThread() processing audio modules update"); + svc = mService.promote(); + if (svc == 0) { + break; + } + mLock.unlock(); + svc->doOnNewAudioModulesAvailable(); + mLock.lock(); + } break; default: ALOGW("AudioCommandThread() unknown command %d", command->mCommand); @@ -1566,6 +1576,13 @@ void AudioPolicyService::AudioCommandThread::recordingConfigurationUpdateCommand sendCommand(command); } +void AudioPolicyService::AudioCommandThread::audioModulesUpdateCommand() +{ + sp command = new AudioCommand(); + command->mCommand = AUDIO_MODULES_UPDATE; + sendCommand(command); +} + status_t AudioPolicyService::AudioCommandThread::sendCommand(sp& command, int delayMs) { { @@ -1813,6 +1830,11 @@ void AudioPolicyService::setEffectSuspended(int effectId, mAudioCommandThread->setEffectSuspendedCommand(effectId, sessionId, suspended); } +void AudioPolicyService::onNewAudioModulesAvailable() +{ + mAudioCommandThread->audioModulesUpdateCommand(); +} + extern "C" { audio_module_handle_t aps_load_hw_module(void *service __unused, diff --git a/services/audiopolicy/service/AudioPolicyService.h b/services/audiopolicy/service/AudioPolicyService.h index c3c87f1576..ff99124a26 100644 --- a/services/audiopolicy/service/AudioPolicyService.h +++ b/services/audiopolicy/service/AudioPolicyService.h @@ -60,6 +60,7 @@ public: // BnAudioPolicyService (see AudioPolicyInterface for method descriptions) // + void onNewAudioModulesAvailable() override; virtual status_t setDeviceConnectionState(audio_devices_t device, audio_policy_dev_state_t state, const char *device_address, @@ -277,6 +278,7 @@ public: bool isCallScreenModeSupported() override; + void doOnNewAudioModulesAvailable(); status_t doStopOutput(audio_port_handle_t portId); void doReleaseOutput(audio_port_handle_t portId); @@ -463,6 +465,7 @@ private: DYN_POLICY_MIX_STATE_UPDATE, RECORDING_CONFIGURATION_UPDATE, SET_EFFECT_SUSPENDED, + AUDIO_MODULES_UPDATE, }; AudioCommandThread (String8 name, const wp& service); @@ -508,6 +511,7 @@ private: void setEffectSuspendedCommand(int effectId, audio_session_t sessionId, bool suspended); + void audioModulesUpdateCommand(); void insertCommand_l(AudioCommand *command, int delayMs = 0); private: class AudioCommandData;