From 80e5850d82f1b311a89367a77bab2c801ac16b37 Mon Sep 17 00:00:00 2001 From: Shunkai Yao Date: Fri, 14 Jul 2023 22:10:46 +0000 Subject: [PATCH] Support custom effect type UUID in audio effect AIDL example service Parse custom effect type uuid in audio_effects_config.xml, so OEM can add effect types without changing code Bug: 271500140 Test: atest --test-mapping hardware/interfaces/audio/aidl/vts:presubmit Change-Id: I558958cc42c6c4a304e0ab1239ddefec9575a5af --- audio/aidl/default/EffectConfig.cpp | 52 ++++++++++++------- audio/aidl/default/EffectFactory.cpp | 9 ++-- audio/aidl/default/audio_effects_config.xml | 9 ++-- .../include/effectFactory-impl/EffectConfig.h | 15 +++--- .../effectFactory-impl/EffectFactory.h | 2 +- 5 files changed, 52 insertions(+), 35 deletions(-) diff --git a/audio/aidl/default/EffectConfig.cpp b/audio/aidl/default/EffectConfig.cpp index f3f674f6ef..730c0bf758 100644 --- a/audio/aidl/default/EffectConfig.cpp +++ b/audio/aidl/default/EffectConfig.cpp @@ -117,53 +117,59 @@ bool EffectConfig::parseLibrary(const tinyxml2::XMLElement& xml) { bool EffectConfig::parseEffect(const tinyxml2::XMLElement& xml) { struct EffectLibraries effectLibraries; - std::vector libraryUuids; + std::vector libraries; std::string name = xml.Attribute("name"); RETURN_VALUE_IF(name == "", false, "effectsNoName"); LOG(DEBUG) << __func__ << dump(xml); - struct LibraryUuid libraryUuid; + struct Library library; if (std::strcmp(xml.Name(), "effectProxy") == 0) { // proxy lib and uuid - RETURN_VALUE_IF(!parseLibraryUuid(xml, libraryUuid, true), false, "parseProxyLibFailed"); - effectLibraries.proxyLibrary = libraryUuid; + RETURN_VALUE_IF(!parseLibrary(xml, library, true), false, "parseProxyLibFailed"); + effectLibraries.proxyLibrary = library; // proxy effect libs and UUID auto xmlProxyLib = xml.FirstChildElement(); RETURN_VALUE_IF(!xmlProxyLib, false, "noLibForProxy"); while (xmlProxyLib) { - struct LibraryUuid tempLibraryUuid; - RETURN_VALUE_IF(!parseLibraryUuid(*xmlProxyLib, tempLibraryUuid), false, + struct Library tempLibrary; + RETURN_VALUE_IF(!parseLibrary(*xmlProxyLib, tempLibrary), false, "parseEffectLibFailed"); - libraryUuids.push_back(std::move(tempLibraryUuid)); + libraries.push_back(std::move(tempLibrary)); xmlProxyLib = xmlProxyLib->NextSiblingElement(); } } else { // expect only one library if not proxy - RETURN_VALUE_IF(!parseLibraryUuid(xml, libraryUuid), false, "parseEffectLibFailed"); - libraryUuids.push_back(std::move(libraryUuid)); + RETURN_VALUE_IF(!parseLibrary(xml, library), false, "parseEffectLibFailed"); + libraries.push_back(std::move(library)); } - effectLibraries.libraries = std::move(libraryUuids); + effectLibraries.libraries = std::move(libraries); mEffectsMap[name] = std::move(effectLibraries); return true; } -bool EffectConfig::parseLibraryUuid(const tinyxml2::XMLElement& xml, - struct LibraryUuid& libraryUuid, bool isProxy) { +bool EffectConfig::parseLibrary(const tinyxml2::XMLElement& xml, struct Library& library, + bool isProxy) { // Retrieve library name only if not effectProxy element if (!isProxy) { const char* name = xml.Attribute("library"); RETURN_VALUE_IF(!name, false, "noLibraryAttribute"); - libraryUuid.name = name; + library.name = name; } const char* uuidStr = xml.Attribute("uuid"); RETURN_VALUE_IF(!uuidStr, false, "noUuidAttribute"); - libraryUuid.uuid = stringToUuid(uuidStr); - RETURN_VALUE_IF((libraryUuid.uuid == getEffectUuidZero()), false, "invalidUuidAttribute"); + library.uuid = stringToUuid(uuidStr); + if (const char* typeUuidStr = xml.Attribute("type")) { + library.type = stringToUuid(typeUuidStr); + } + RETURN_VALUE_IF((library.uuid == getEffectUuidZero()), false, "invalidUuidAttribute"); - LOG(DEBUG) << __func__ << (isProxy ? " proxy " : libraryUuid.name) << " : " - << ::android::audio::utils::toString(libraryUuid.uuid); + LOG(DEBUG) << __func__ << (isProxy ? " proxy " : library.name) << " : uuid " + << ::android::audio::utils::toString(library.uuid) + << (library.type.has_value() + ? ::android::audio::utils::toString(library.type.value()) + : ""); return true; } @@ -241,7 +247,8 @@ EffectConfig::getProcessingMap() const { return mProcessingMap; } -bool EffectConfig::findUuid(const std::string& xmlEffectName, AudioUuid* uuid) { +bool EffectConfig::findUuid(const std::pair& effectElem, + AudioUuid* uuid) { // Difference from EFFECT_TYPE_LIST_DEF, there could be multiple name mapping to same Effect Type #define EFFECT_XML_TYPE_LIST_DEF(V) \ V("acoustic_echo_canceler", AcousticEchoCanceler) \ @@ -268,6 +275,7 @@ bool EffectConfig::findUuid(const std::string& xmlEffectName, AudioUuid* uuid) { #define GENERATE_MAP_ENTRY_V(s, symbol) {s, &getEffectTypeUuid##symbol}, + const std::string xmlEffectName = effectElem.first; typedef const AudioUuid& (*UuidGetter)(void); static const std::map uuidMap{ // std::make_pair("s", &getEffectTypeUuidExtension)}; @@ -276,6 +284,14 @@ bool EffectConfig::findUuid(const std::string& xmlEffectName, AudioUuid* uuid) { *uuid = (*it->second)(); return true; } + + const auto& libs = effectElem.second.libraries; + for (const auto& lib : libs) { + if (lib.type.has_value()) { + *uuid = lib.type.value(); + return true; + } + } return false; } diff --git a/audio/aidl/default/EffectFactory.cpp b/audio/aidl/default/EffectFactory.cpp index 8ed62c9c7a..62f3c7ee47 100644 --- a/audio/aidl/default/EffectFactory.cpp +++ b/audio/aidl/default/EffectFactory.cpp @@ -105,7 +105,7 @@ ndk::ScopedAStatus Factory::queryProcessing(const std::optional */) { - for (const auto& lib : libs.libraries /* std::vector */) { + for (const auto& lib : libs.libraries /* std::vector */) { Descriptor desc; if (libs.proxyLibrary.has_value()) { desc.common.id.proxy = libs.proxyLibrary.value().uuid; @@ -219,7 +219,7 @@ bool Factory::openEffectLibrary(const AudioUuid& impl, const std::string& path) return true; } -void Factory::createIdentityWithConfig(const EffectConfig::LibraryUuid& configLib, +void Factory::createIdentityWithConfig(const EffectConfig::Library& configLib, const AudioUuid& typeUuid, const std::optional proxyUuid) { static const auto& libMap = mConfig.getLibraryMap(); @@ -246,8 +246,7 @@ void Factory::createIdentityWithConfig(const EffectConfig::LibraryUuid& configLi void Factory::loadEffectLibs() { const auto& configEffectsMap = mConfig.getEffectsMap(); for (const auto& configEffects : configEffectsMap) { - if (AudioUuid uuid; - EffectConfig::findUuid(configEffects.first /* xml effect name */, &uuid)) { + if (AudioUuid type; EffectConfig::findUuid(configEffects /* xml effect */, &type)) { const auto& configLibs = configEffects.second; std::optional proxyUuid; if (configLibs.proxyLibrary.has_value()) { @@ -255,7 +254,7 @@ void Factory::loadEffectLibs() { proxyUuid = proxyLib.uuid; } for (const auto& configLib : configLibs.libraries) { - createIdentityWithConfig(configLib, uuid, proxyUuid); + createIdentityWithConfig(configLib, type, proxyUuid); } } else { LOG(ERROR) << __func__ << ": can not find type UUID for effect " << configEffects.first diff --git a/audio/aidl/default/audio_effects_config.xml b/audio/aidl/default/audio_effects_config.xml index 6627ae7533..00de797764 100644 --- a/audio/aidl/default/audio_effects_config.xml +++ b/audio/aidl/default/audio_effects_config.xml @@ -50,7 +50,8 @@ @@ -94,7 +95,7 @@ - + diff --git a/audio/aidl/default/include/effectFactory-impl/EffectConfig.h b/audio/aidl/default/include/effectFactory-impl/EffectConfig.h index f8a86e1515..344846a4ca 100644 --- a/audio/aidl/default/include/effectFactory-impl/EffectConfig.h +++ b/audio/aidl/default/include/effectFactory-impl/EffectConfig.h @@ -40,14 +40,15 @@ class EffectConfig { public: explicit EffectConfig(const std::string& file); - struct LibraryUuid { + struct Library { std::string name; // library name - ::aidl::android::media::audio::common::AudioUuid uuid; + ::aidl::android::media::audio::common::AudioUuid uuid; // implementation UUID + std::optional<::aidl::android::media::audio::common::AudioUuid> type; // optional type UUID }; // struct EffectLibraries { - std::optional proxyLibrary; - std::vector libraries; + std::optional proxyLibrary; + std::vector libraries; }; int getSkippedElements() const { return mSkippedElements; } @@ -56,7 +57,7 @@ class EffectConfig { return mEffectsMap; } - static bool findUuid(const std::string& xmlEffectName, + static bool findUuid(const std::pair& effectElem, ::aidl::android::media::audio::common::AudioUuid* uuid); using ProcessingLibrariesMap = std::map>; @@ -96,8 +97,8 @@ class EffectConfig { bool parseProcessing(Processing::Type::Tag typeTag, const tinyxml2::XMLElement& xml); // Function to parse effect.library name and effect.uuid from xml - bool parseLibraryUuid(const tinyxml2::XMLElement& xml, struct LibraryUuid& libraryUuid, - bool isProxy = false); + bool parseLibrary(const tinyxml2::XMLElement& xml, struct Library& library, + bool isProxy = false); const char* dump(const tinyxml2::XMLElement& element, tinyxml2::XMLPrinter&& printer = {}) const; diff --git a/audio/aidl/default/include/effectFactory-impl/EffectFactory.h b/audio/aidl/default/include/effectFactory-impl/EffectFactory.h index ad59ca799c..1401db03e5 100644 --- a/audio/aidl/default/include/effectFactory-impl/EffectFactory.h +++ b/audio/aidl/default/include/effectFactory-impl/EffectFactory.h @@ -104,7 +104,7 @@ class Factory : public BnFactory { bool openEffectLibrary(const ::aidl::android::media::audio::common::AudioUuid& impl, const std::string& path); void createIdentityWithConfig( - const EffectConfig::LibraryUuid& configLib, + const EffectConfig::Library& configLib, const ::aidl::android::media::audio::common::AudioUuid& typeUuidStr, const std::optional<::aidl::android::media::audio::common::AudioUuid> proxyUuid);