Fix parsing of legacy engine config XML files am: bada1f5e01
Original change: https://android-review.googlesource.com/c/platform/frameworks/av/+/3391045
Change-Id: I94c137cf574aabb5a4725be24ec6cc8b8e8e52d9
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
diff --git a/services/audiopolicy/common/include/policy.h b/services/audiopolicy/common/include/policy.h
index d499222..0c03900 100644
--- a/services/audiopolicy/common/include/policy.h
+++ b/services/audiopolicy/common/include/policy.h
@@ -22,31 +22,77 @@
#include <media/AudioContainers.h>
+#include <string.h>
+
namespace android {
using StreamTypeVector = std::vector<audio_stream_type_t>;
+#define AUDIO_ENUM_QUOTE(x) #x
+#define AUDIO_ENUM_STRINGIFY(x) AUDIO_ENUM_QUOTE(x)
+#define AUDIO_DEFINE_ENUM_SYMBOL_V(symbol, value) symbol = value,
+#define AUDIO_DEFINE_STRINGIFY_CASE_V(symbol, _) case symbol: return AUDIO_ENUM_STRINGIFY(symbol);
+#define AUDIO_DEFINE_PARSE_CASE_V(symbol, _) \
+ if (strcmp(s, AUDIO_ENUM_STRINGIFY(symbol)) == 0) { *t = symbol; return true; } else
+#define AUDIO_DEFINE_MAP_ENTRY_V(symbol, _) { AUDIO_ENUM_STRINGIFY(symbol), symbol },
+
/**
* Legacy audio policy product strategies IDs. These strategies are supported by the default
* policy engine.
* IMPORTANT NOTE: the order of this enum is important as it determines the priority
- * between active strategies for routing decisions: lower enum value => higher prioriy
+ * between active strategies for routing decisions: lower enum value => higher priority
*/
+#define AUDIO_LEGACY_STRATEGY_LIST_DEF(V) \
+ V(STRATEGY_NONE, -1) \
+ V(STRATEGY_PHONE, 0) \
+ V(STRATEGY_SONIFICATION, 1) \
+ V(STRATEGY_ENFORCED_AUDIBLE, 2) \
+ V(STRATEGY_ACCESSIBILITY, 3) \
+ V(STRATEGY_SONIFICATION_RESPECTFUL, 4) \
+ V(STRATEGY_MEDIA, 5) \
+ V(STRATEGY_DTMF, 6) \
+ V(STRATEGY_CALL_ASSISTANT, 7) \
+ V(STRATEGY_TRANSMITTED_THROUGH_SPEAKER, 8) \
+ V(STRATEGY_REROUTING, 9) \
+ V(STRATEGY_PATCH, 10)
+
enum legacy_strategy {
- STRATEGY_NONE = -1,
- STRATEGY_PHONE,
- STRATEGY_SONIFICATION,
- STRATEGY_ENFORCED_AUDIBLE,
- STRATEGY_ACCESSIBILITY,
- STRATEGY_SONIFICATION_RESPECTFUL,
- STRATEGY_MEDIA,
- STRATEGY_DTMF,
- STRATEGY_CALL_ASSISTANT,
- STRATEGY_TRANSMITTED_THROUGH_SPEAKER,
- STRATEGY_REROUTING,
- STRATEGY_PATCH,
+ AUDIO_LEGACY_STRATEGY_LIST_DEF(AUDIO_DEFINE_ENUM_SYMBOL_V)
};
+inline const char* legacy_strategy_to_string(legacy_strategy t) {
+ switch (t) {
+ AUDIO_LEGACY_STRATEGY_LIST_DEF(AUDIO_DEFINE_STRINGIFY_CASE_V)
+ }
+ return "";
+}
+
+inline bool legacy_strategy_from_string(const char* s, legacy_strategy* t) {
+ AUDIO_LEGACY_STRATEGY_LIST_DEF(AUDIO_DEFINE_PARSE_CASE_V)
+ return false;
+}
+
+namespace audio_policy {
+
+struct legacy_strategy_map { const char *name; legacy_strategy id; };
+
+inline std::vector<legacy_strategy_map> getLegacyStrategyMap() {
+ return std::vector<legacy_strategy_map> {
+ AUDIO_LEGACY_STRATEGY_LIST_DEF(AUDIO_DEFINE_MAP_ENTRY_V)
+ };
+}
+
+} // namespace audio_policy
+
+#undef AUDIO_LEGACY_STRATEGY_LIST_DEF
+
+#undef AUDIO_DEFINE_MAP_ENTRY_V
+#undef AUDIO_DEFINE_PARSE_CASE_V
+#undef AUDIO_DEFINE_STRINGIFY_CASE_V
+#undef AUDIO_DEFINE_ENUM_SYMBOL_V
+#undef AUDIO_ENUM_STRINGIFY
+#undef AUDIO_ENUM_QUOTE
+
static const audio_attributes_t defaultAttr = AUDIO_ATTRIBUTES_INITIALIZER;
static const std::set<audio_usage_t > gHighPriorityUseCases = {
diff --git a/services/audiopolicy/engine/common/include/EngineBase.h b/services/audiopolicy/engine/common/include/EngineBase.h
index edb2e29..4445b66 100644
--- a/services/audiopolicy/engine/common/include/EngineBase.h
+++ b/services/audiopolicy/engine/common/include/EngineBase.h
@@ -117,9 +117,10 @@
AudioDeviceTypeAddrVector &devices) const override;
engineConfig::ParsingResult loadAudioPolicyEngineConfig(
- const media::audio::common::AudioHalEngineConfig& aidlConfig);
+ const media::audio::common::AudioHalEngineConfig& aidlConfig, bool);
- engineConfig::ParsingResult loadAudioPolicyEngineConfig(const std::string& xmlFilePath = "");
+ engineConfig::ParsingResult loadAudioPolicyEngineConfig(
+ const std::string& xmlFilePath = "", bool isConfigurable = false);
const ProductStrategyMap &getProductStrategies() const { return mProductStrategies; }
diff --git a/services/audiopolicy/engine/common/src/EngineBase.cpp b/services/audiopolicy/engine/common/src/EngineBase.cpp
index fb8379e..0799399 100644
--- a/services/audiopolicy/engine/common/src/EngineBase.cpp
+++ b/services/audiopolicy/engine/common/src/EngineBase.cpp
@@ -126,7 +126,7 @@
}
engineConfig::ParsingResult EngineBase::loadAudioPolicyEngineConfig(
- const media::audio::common::AudioHalEngineConfig& aidlConfig)
+ const media::audio::common::AudioHalEngineConfig& aidlConfig, bool)
{
engineConfig::ParsingResult result = engineConfig::convert(aidlConfig);
if (result.parsedConfig == nullptr) {
@@ -141,7 +141,8 @@
return processParsingResult(std::move(result));
}
-engineConfig::ParsingResult EngineBase::loadAudioPolicyEngineConfig(const std::string& xmlFilePath)
+engineConfig::ParsingResult EngineBase::loadAudioPolicyEngineConfig(
+ const std::string& xmlFilePath, bool isConfigurable)
{
auto fileExists = [](const char* path) {
struct stat fileStat;
@@ -150,7 +151,7 @@
const std::string filePath = xmlFilePath.empty() ? engineConfig::DEFAULT_PATH : xmlFilePath;
engineConfig::ParsingResult result =
fileExists(filePath.c_str()) ?
- engineConfig::parse(filePath.c_str()) : engineConfig::ParsingResult{};
+ engineConfig::parse(filePath.c_str(), isConfigurable) : engineConfig::ParsingResult{};
if (result.parsedConfig == nullptr) {
ALOGD("%s: No configuration found, using default matching phone experience.", __FUNCTION__);
engineConfig::Config config = gDefaultEngineConfig;
diff --git a/services/audiopolicy/engine/config/include/EngineConfig.h b/services/audiopolicy/engine/config/include/EngineConfig.h
index 054bdae..8a4fc88 100644
--- a/services/audiopolicy/engine/config/include/EngineConfig.h
+++ b/services/audiopolicy/engine/config/include/EngineConfig.h
@@ -116,7 +116,7 @@
/** Parses the provided audio policy usage configuration.
* @return audio policy usage @see Config
*/
-ParsingResult parse(const char* path = DEFAULT_PATH);
+ParsingResult parse(const char* path = DEFAULT_PATH, bool isConfigurable = false);
android::status_t parseLegacyVolumes(VolumeGroups &volumeGroups);
ParsingResult convert(const ::android::media::audio::common::AudioHalEngineConfig& aidlConfig);
// Exposed for testing.
diff --git a/services/audiopolicy/engine/config/src/EngineConfig.cpp b/services/audiopolicy/engine/config/src/EngineConfig.cpp
index 714ab78..b8d95ee 100644
--- a/services/audiopolicy/engine/config/src/EngineConfig.cpp
+++ b/services/audiopolicy/engine/config/src/EngineConfig.cpp
@@ -52,6 +52,8 @@
namespace {
+static bool gIsConfigurableEngine = false;
+
ConversionResult<std::string> aidl2legacy_AudioHalProductStrategy_ProductStrategyType(int id) {
using AudioProductStrategyType = media::audio::common::AudioProductStrategyType;
@@ -547,9 +549,16 @@
if (!convertTo(idLiteral, id)) {
return BAD_VALUE;
}
- ALOGV("%s: %s, %s = %d", __FUNCTION__, name.c_str(), Attributes::id, id);
+ } else {
+ legacy_strategy legacyId;
+ if (legacy_strategy_from_string(name.c_str(), &legacyId)) {
+ id = legacyId;
+ } else if (!gIsConfigurableEngine) {
+ return BAD_VALUE;
+ }
+ // With a configurable engine it can be a vendor-provided strategy name.
}
- ALOGV("%s: %s = %s", __FUNCTION__, Attributes::name, name.c_str());
+ ALOGV("%s: %s, %s = %d", __FUNCTION__, name.c_str(), Attributes::id, id);
size_t skipped = 0;
AttributesGroups attrGroups;
@@ -776,7 +785,7 @@
} // namespace
-ParsingResult parse(const char* path) {
+ParsingResult parse(const char* path, bool isConfigurable) {
XmlErrorHandler errorHandler;
auto doc = make_xmlUnique(xmlParseFile(path));
if (doc == NULL) {
@@ -801,6 +810,7 @@
ALOGE("%s: No version found", __func__);
return {nullptr, 0};
}
+ gIsConfigurableEngine = isConfigurable;
size_t nbSkippedElements = 0;
auto config = std::make_unique<Config>();
config->version = std::stof(version);
diff --git a/services/audiopolicy/engineconfigurable/src/Engine.cpp b/services/audiopolicy/engineconfigurable/src/Engine.cpp
index 45da7b0..ad49b19 100644
--- a/services/audiopolicy/engineconfigurable/src/Engine.cpp
+++ b/services/audiopolicy/engineconfigurable/src/Engine.cpp
@@ -112,7 +112,7 @@
template<typename T>
status_t Engine::loadWithFallback(const T& configSource) {
- auto result = EngineBase::loadAudioPolicyEngineConfig(configSource);
+ auto result = EngineBase::loadAudioPolicyEngineConfig(configSource, true /*isConfigurable*/);
ALOGE_IF(result.nbSkippedElement != 0,
"Policy Engine configuration is partially invalid, skipped %zu elements",
result.nbSkippedElement);
diff --git a/services/audiopolicy/enginedefault/src/Engine.cpp b/services/audiopolicy/enginedefault/src/Engine.cpp
index 7de6939..d42f2dc 100644
--- a/services/audiopolicy/enginedefault/src/Engine.cpp
+++ b/services/audiopolicy/enginedefault/src/Engine.cpp
@@ -38,22 +38,8 @@
namespace android::audio_policy {
-struct legacy_strategy_map { const char *name; legacy_strategy id; };
static const std::vector<legacy_strategy_map>& getLegacyStrategy() {
- static const std::vector<legacy_strategy_map> legacyStrategy = {
- { "STRATEGY_NONE", STRATEGY_NONE },
- { "STRATEGY_MEDIA", STRATEGY_MEDIA },
- { "STRATEGY_PHONE", STRATEGY_PHONE },
- { "STRATEGY_SONIFICATION", STRATEGY_SONIFICATION },
- { "STRATEGY_SONIFICATION_RESPECTFUL", STRATEGY_SONIFICATION_RESPECTFUL },
- { "STRATEGY_DTMF", STRATEGY_DTMF },
- { "STRATEGY_ENFORCED_AUDIBLE", STRATEGY_ENFORCED_AUDIBLE },
- { "STRATEGY_TRANSMITTED_THROUGH_SPEAKER", STRATEGY_TRANSMITTED_THROUGH_SPEAKER },
- { "STRATEGY_ACCESSIBILITY", STRATEGY_ACCESSIBILITY },
- { "STRATEGY_REROUTING", STRATEGY_REROUTING },
- { "STRATEGY_PATCH", STRATEGY_PATCH }, // boiler to manage stream patch volume
- { "STRATEGY_CALL_ASSISTANT", STRATEGY_CALL_ASSISTANT },
- };
+ static const std::vector<legacy_strategy_map> legacyStrategy = getLegacyStrategyMap();
return legacyStrategy;
}
@@ -68,7 +54,7 @@
template<typename T>
status_t Engine::loadWithFallback(const T& configSource) {
- auto result = EngineBase::loadAudioPolicyEngineConfig(configSource);
+ auto result = EngineBase::loadAudioPolicyEngineConfig(configSource, false /*isConfigurable*/);
ALOGE_IF(result.nbSkippedElement != 0,
"Policy Engine configuration is partially invalid, skipped %zu elements",
result.nbSkippedElement);
diff --git a/services/audiopolicy/tests/audiopolicymanager_tests.cpp b/services/audiopolicy/tests/audiopolicymanager_tests.cpp
index 1649099..fa6e6c4 100644
--- a/services/audiopolicy/tests/audiopolicymanager_tests.cpp
+++ b/services/audiopolicy/tests/audiopolicymanager_tests.cpp
@@ -186,6 +186,7 @@
void SetUp() override;
void TearDown() override;
virtual void SetUpManagerConfig();
+ virtual std::string getEngineConfigFilePath() const { return sTestEngineConfig; }
void dumpToLog();
// When explicit routing is needed, selectedDeviceId needs to be set as the wanted port
@@ -224,6 +225,7 @@
const std::string &address, audio_port_v7 *foundPort);
static audio_port_handle_t getDeviceIdFromPatch(const struct audio_patch* patch);
virtual AudioPolicyManagerTestClient* getClient() { return new AudioPolicyManagerTestClient; }
+ void verifyBuiltInStrategyIdsAreValid();
sp<AudioPolicyConfig> mConfig;
std::unique_ptr<AudioPolicyManagerTestClient> mClient;
@@ -238,7 +240,7 @@
void AudioPolicyManagerTest::SetUp() {
mClient.reset(getClient());
ASSERT_NO_FATAL_FAILURE(SetUpManagerConfig()); // Subclasses may want to customize the config.
- mManager.reset(new AudioPolicyTestManager(mConfig, mClient.get(), sTestEngineConfig));
+ mManager.reset(new AudioPolicyTestManager(mConfig, mClient.get(), getEngineConfigFilePath()));
ASSERT_EQ(NO_ERROR, mManager->initialize());
ASSERT_EQ(NO_ERROR, mManager->initCheck());
}
@@ -399,6 +401,16 @@
return AUDIO_PORT_HANDLE_NONE;
}
+void AudioPolicyManagerTest::verifyBuiltInStrategyIdsAreValid() {
+ AudioProductStrategyVector strategies;
+ ASSERT_EQ(NO_ERROR, mManager->listAudioProductStrategies(strategies));
+ for (const auto& strategy : strategies) {
+ // Since ids are unsigned, this will also cover the case when the id is 'NONE' which is -1.
+ EXPECT_LT(strategy.getId(),
+ media::audio::common::AudioHalProductStrategy::VENDOR_STRATEGY_ID_START)
+ << strategy.getName();
+ }
+}
TEST_F(AudioPolicyManagerTest, InitSuccess) {
// SetUp must finish with no assertions.
@@ -456,6 +468,20 @@
// TODO: Add patch creation tests that involve already existing patch
+TEST_F(AudioPolicyManagerTest, BuiltInStrategyIdsAreValid) {
+ verifyBuiltInStrategyIdsAreValid();
+}
+
+class AudioPolicyManagerTestWithDefaultEngineConfig : public AudioPolicyManagerTest {
+ protected:
+ // The APM will use the default engine config from EngineDefaultConfig.h.
+ std::string getEngineConfigFilePath() const override { return ""; }
+};
+
+TEST_F(AudioPolicyManagerTestWithDefaultEngineConfig, BuiltInStrategyIdsAreValid) {
+ verifyBuiltInStrategyIdsAreValid();
+}
+
enum
{
MSD_AUDIO_PATCH_COUNT_NUM_AUDIO_PATCHES_INDEX = 0,