Decouple the configuration from the AudioPolicyManager
Promote AudioPolicyConfig to a full class, not just a "view"
on the APM data. This allows separating initialization of
the APM config from the initialization of the APM itself.
Thus, the APM receives a fully initialized config upon
its creation, regardless of the way the config data has been
obtained.
Bug: 205884982
Test: atest audiopolicy_tests
Test: atest audiosystem_tests
Test: atest audio_health_tests
Test: m audiopolicy_fuzzer
Change-Id: I334d16e762499a0292540e6abae4e177480d6235
diff --git a/services/audiopolicy/tests/Android.bp b/services/audiopolicy/tests/Android.bp
index 6813587..8499c1c 100644
--- a/services/audiopolicy/tests/Android.bp
+++ b/services/audiopolicy/tests/Android.bp
@@ -19,18 +19,19 @@
],
shared_libs: [
+ "framework-permission-aidl-cpp",
"libaudioclient",
"libaudiofoundation",
"libaudiopolicy",
"libaudiopolicymanagerdefault",
"libbase",
+ "libbinder",
+ "libcutils",
"libhidlbase",
"liblog",
"libmedia_helper",
"libutils",
"libxml2",
- "framework-permission-aidl-cpp",
- "libbinder",
],
static_libs: [
@@ -69,17 +70,19 @@
require_root: true,
shared_libs: [
- "libaudiofoundation",
+ "audioclient-types-aidl-cpp",
"libaudioclient",
+ "libaudioclient_aidl_conversion",
+ "libaudiofoundation",
"libaudiopolicymanagerdefault",
+ "libcutils",
"liblog",
"libmedia_helper",
- "libutils",
- "libaudioclient_aidl_conversion",
- "libstagefright_foundation",
"libshmemcompat",
"libshmemutil",
- "audioclient-types-aidl-cpp",
+ "libstagefright_foundation",
+ "libutils",
+ "libxml2",
],
static_libs: ["libaudiopolicycomponents"],
diff --git a/services/audiopolicy/tests/AudioPolicyTestManager.h b/services/audiopolicy/tests/AudioPolicyTestManager.h
index 2a7a060..9ad9849 100644
--- a/services/audiopolicy/tests/AudioPolicyTestManager.h
+++ b/services/audiopolicy/tests/AudioPolicyTestManager.h
@@ -22,9 +22,11 @@
class AudioPolicyTestManager : public AudioPolicyManager {
public:
explicit AudioPolicyTestManager(AudioPolicyClientInterface *clientInterface)
- : AudioPolicyManager(clientInterface, true /*forTesting*/) { }
+ : AudioPolicyManager(AudioPolicyConfig::createDefault(), clientInterface) {}
+ AudioPolicyTestManager(const sp<const AudioPolicyConfig>& config,
+ AudioPolicyClientInterface *clientInterface)
+ : AudioPolicyManager(config, clientInterface) {}
using AudioPolicyManager::getConfig;
- using AudioPolicyManager::loadConfig;
using AudioPolicyManager::initialize;
using AudioPolicyManager::getOutputs;
using AudioPolicyManager::getAvailableOutputDevices;
diff --git a/services/audiopolicy/tests/audio_health_tests.cpp b/services/audiopolicy/tests/audio_health_tests.cpp
index 798332c..70a3022 100644
--- a/services/audiopolicy/tests/audio_health_tests.cpp
+++ b/services/audiopolicy/tests/audio_health_tests.cpp
@@ -21,6 +21,7 @@
#include <gtest/gtest.h>
+#include <AudioPolicyConfig.h>
#include <media/AudioSystem.h>
#include <media/TypeConverter.h>
#include <system/audio.h>
@@ -65,19 +66,17 @@
}
free(audioPorts);
- AudioPolicyManagerTestClient client;
- AudioPolicyTestManager manager(&client);
- manager.loadConfig();
- ASSERT_NE("AudioPolicyConfig::setDefault", manager.getConfig().getSource());
+ auto config = AudioPolicyConfig::loadFromApmXmlConfigWithFallback();
+ ASSERT_NE(AudioPolicyConfig::kDefaultConfigSource, config->getSource());
- for (auto desc : manager.getConfig().getInputDevices()) {
+ for (const auto& desc : config->getInputDevices()) {
if (attachedDevices.find(desc->type()) == attachedDevices.end()) {
std::string deviceType;
(void)DeviceConverter::toString(desc->type(), deviceType);
ADD_FAILURE() << "Input device \"" << deviceType << "\" not found";
}
}
- for (auto desc : manager.getConfig().getOutputDevices()) {
+ for (const auto& desc : config->getOutputDevices()) {
if (attachedDevices.find(desc->type()) == attachedDevices.end()) {
std::string deviceType;
(void)DeviceConverter::toString(desc->type(), deviceType);
@@ -87,13 +86,13 @@
}
TEST(AudioHealthTest, ConnectSupportedDevice) {
+ auto config = AudioPolicyConfig::loadFromApmXmlConfigWithFallback();
+ ASSERT_NE(AudioPolicyConfig::kDefaultConfigSource, config->getSource());
AudioPolicyManagerTestClient client;
- AudioPolicyTestManager manager(&client);
- manager.loadConfig();
- ASSERT_NE("AudioPolicyConfig::setDefault", manager.getConfig().getSource());
+ AudioPolicyTestManager manager(config, &client);
DeviceVector devices;
- for (const auto& hwModule : manager.getConfig().getHwModules()) {
+ for (const auto& hwModule : config->getHwModules()) {
for (const auto& profile : hwModule->getOutputProfiles()) {
devices.merge(profile->getSupportedDevices());
}
diff --git a/services/audiopolicy/tests/audiopolicymanager_tests.cpp b/services/audiopolicy/tests/audiopolicymanager_tests.cpp
index 3821f97..4486ce6 100644
--- a/services/audiopolicy/tests/audiopolicymanager_tests.cpp
+++ b/services/audiopolicy/tests/audiopolicymanager_tests.cpp
@@ -68,11 +68,44 @@
} // namespace
+TEST(AudioPolicyConfigTest, DefaultConfigForTestsIsEmpty) {
+ auto config = AudioPolicyConfig::createWritableForTests();
+ EXPECT_TRUE(config->getSource().empty());
+ EXPECT_TRUE(config->getHwModules().isEmpty());
+ EXPECT_TRUE(config->getInputDevices().isEmpty());
+ EXPECT_TRUE(config->getOutputDevices().isEmpty());
+}
+
+TEST(AudioPolicyConfigTest, FallbackToDefault) {
+ auto config = AudioPolicyConfig::loadFromApmXmlConfigWithFallback(
+ base::GetExecutableDirectory() + "/test_invalid_audio_policy_configuration.xml");
+ EXPECT_EQ(AudioPolicyConfig::kDefaultConfigSource, config->getSource());
+}
+
+TEST(AudioPolicyConfigTest, LoadForTests) {
+ {
+ auto result = AudioPolicyConfig::loadFromCustomXmlConfigForTests(
+ base::GetExecutableDirectory() + "/test_invalid_audio_policy_configuration.xml");
+ EXPECT_FALSE(result.ok());
+ }
+ {
+ const std::string source =
+ base::GetExecutableDirectory() + "/test_audio_policy_configuration.xml";
+ auto result = AudioPolicyConfig::loadFromCustomXmlConfigForTests(source);
+ ASSERT_TRUE(result.ok());
+ EXPECT_EQ(source, result.value()->getSource());
+ EXPECT_FALSE(result.value()->getHwModules().isEmpty());
+ EXPECT_FALSE(result.value()->getInputDevices().isEmpty());
+ EXPECT_FALSE(result.value()->getOutputDevices().isEmpty());
+ }
+}
+
TEST(AudioPolicyManagerTestInit, EngineFailure) {
AudioPolicyTestClient client;
- AudioPolicyTestManager manager(&client);
- manager.getConfig().setDefault();
- manager.getConfig().setEngineLibraryNameSuffix("non-existent");
+ auto config = AudioPolicyConfig::createWritableForTests();
+ config->setDefault();
+ config->setEngineLibraryNameSuffix("non-existent");
+ AudioPolicyTestManager manager(config, &client);
ASSERT_EQ(NO_INIT, manager.initialize());
ASSERT_EQ(NO_INIT, manager.initCheck());
}
@@ -80,41 +113,12 @@
TEST(AudioPolicyManagerTestInit, ClientFailure) {
AudioPolicyTestClient client;
AudioPolicyTestManager manager(&client);
- manager.getConfig().setDefault();
// Since the default client fails to open anything,
// APM should indicate that the initialization didn't succeed.
ASSERT_EQ(NO_INIT, manager.initialize());
ASSERT_EQ(NO_INIT, manager.initCheck());
}
-// Verifies that a failure while loading a config doesn't leave
-// APM config in a "dirty" state. Since AudioPolicyConfig object
-// is a proxy for the data hosted by APM, it isn't possible
-// to "deep copy" it, and thus we have to test its elements
-// individually.
-TEST(AudioPolicyManagerTestInit, ConfigLoadingIsTransactional) {
- AudioPolicyTestClient client;
- AudioPolicyTestManager manager(&client);
- ASSERT_TRUE(manager.getConfig().getHwModules().isEmpty());
- ASSERT_TRUE(manager.getConfig().getInputDevices().isEmpty());
- ASSERT_TRUE(manager.getConfig().getOutputDevices().isEmpty());
- status_t status = deserializeAudioPolicyFile(
- (base::GetExecutableDirectory() +
- "/test_invalid_audio_policy_configuration.xml").c_str(),
- &manager.getConfig());
- ASSERT_NE(NO_ERROR, status);
- EXPECT_TRUE(manager.getConfig().getHwModules().isEmpty());
- EXPECT_TRUE(manager.getConfig().getInputDevices().isEmpty());
- EXPECT_TRUE(manager.getConfig().getOutputDevices().isEmpty());
- status = deserializeAudioPolicyFile(
- (base::GetExecutableDirectory() + "/test_audio_policy_configuration.xml").c_str(),
- &manager.getConfig());
- ASSERT_EQ(NO_ERROR, status);
- EXPECT_FALSE(manager.getConfig().getHwModules().isEmpty());
- EXPECT_FALSE(manager.getConfig().getInputDevices().isEmpty());
- EXPECT_FALSE(manager.getConfig().getOutputDevices().isEmpty());
-}
-
class PatchCountCheck {
public:
@@ -172,6 +176,7 @@
static audio_port_handle_t getDeviceIdFromPatch(const struct audio_patch* patch);
virtual AudioPolicyManagerTestClient* getClient() { return new AudioPolicyManagerTestClient; }
+ sp<AudioPolicyConfig> mConfig;
std::unique_ptr<AudioPolicyManagerTestClient> mClient;
std::unique_ptr<AudioPolicyTestManager> mManager;
@@ -180,8 +185,8 @@
void AudioPolicyManagerTest::SetUp() {
mClient.reset(getClient());
- mManager.reset(new AudioPolicyTestManager(mClient.get()));
ASSERT_NO_FATAL_FAILURE(SetUpManagerConfig()); // Subclasses may want to customize the config.
+ mManager.reset(new AudioPolicyTestManager(mConfig, mClient.get()));
ASSERT_EQ(NO_ERROR, mManager->initialize());
ASSERT_EQ(NO_ERROR, mManager->initCheck());
}
@@ -192,7 +197,8 @@
}
void AudioPolicyManagerTest::SetUpManagerConfig() {
- mManager->getConfig().setDefault();
+ mConfig = AudioPolicyConfig::createWritableForTests();
+ mConfig->setDefault();
}
void AudioPolicyManagerTest::dumpToLog() {
@@ -439,7 +445,6 @@
void AudioPolicyManagerTestMsd::SetUpManagerConfig() {
// TODO: Consider using Serializer to load part of the config from a string.
ASSERT_NO_FATAL_FAILURE(AudioPolicyManagerTest::SetUpManagerConfig());
- AudioPolicyConfig& config = mManager->getConfig();
mMsdOutputDevice = new DeviceDescriptor(AUDIO_DEVICE_OUT_BUS);
sp<AudioProfile> pcmOutputProfile = new AudioProfile(
AUDIO_FORMAT_PCM_16_BIT, AUDIO_CHANNEL_OUT_STEREO, k48000SamplingRate);
@@ -455,26 +460,26 @@
sp<AudioProfile> pcmInputProfile = new AudioProfile(
AUDIO_FORMAT_PCM_16_BIT, AUDIO_CHANNEL_IN_STEREO, 44100);
mMsdInputDevice->addAudioProfile(pcmInputProfile);
- config.addDevice(mMsdOutputDevice);
- config.addDevice(mMsdInputDevice);
+ mConfig->addDevice(mMsdOutputDevice);
+ mConfig->addDevice(mMsdInputDevice);
if (mExpectedAudioPatchCount == 2) {
// Add SPDIF device with PCM output profile as a second device for dual MSD audio patching.
mSpdifDevice = new DeviceDescriptor(AUDIO_DEVICE_OUT_SPDIF);
mSpdifDevice->addAudioProfile(pcmOutputProfile);
- config.addDevice(mSpdifDevice);
+ mConfig->addDevice(mSpdifDevice);
sp<OutputProfile> spdifOutputProfile = new OutputProfile("spdif output");
spdifOutputProfile->addAudioProfile(pcmOutputProfile);
spdifOutputProfile->addSupportedDevice(mSpdifDevice);
- config.getHwModules().getModuleFromName(AUDIO_HARDWARE_MODULE_ID_PRIMARY)->
+ mConfig->getHwModules().getModuleFromName(AUDIO_HARDWARE_MODULE_ID_PRIMARY)->
addOutputProfile(spdifOutputProfile);
}
sp<HwModule> msdModule = new HwModule(AUDIO_HARDWARE_MODULE_ID_MSD, 2 /*halVersionMajor*/);
- HwModuleCollection modules = config.getHwModules();
+ HwModuleCollection modules = mConfig->getHwModules();
modules.add(msdModule);
- config.setHwModules(modules);
+ mConfig->setHwModules(modules);
sp<OutputProfile> msdOutputProfile = new OutputProfile("msd input");
msdOutputProfile->addAudioProfile(pcmOutputProfile);
@@ -502,15 +507,15 @@
// of streams that are not supported by MSD.
sp<AudioProfile> dtsOutputProfile = new AudioProfile(
AUDIO_FORMAT_DTS, AUDIO_CHANNEL_OUT_5POINT1, k48000SamplingRate);
- config.getDefaultOutputDevice()->addAudioProfile(dtsOutputProfile);
+ mConfig->getDefaultOutputDevice()->addAudioProfile(dtsOutputProfile);
sp<OutputProfile> primaryEncodedOutputProfile = new OutputProfile("encoded");
primaryEncodedOutputProfile->addAudioProfile(dtsOutputProfile);
primaryEncodedOutputProfile->setFlags(AUDIO_OUTPUT_FLAG_DIRECT);
- primaryEncodedOutputProfile->addSupportedDevice(config.getDefaultOutputDevice());
- config.getHwModules().getModuleFromName(AUDIO_HARDWARE_MODULE_ID_PRIMARY)->
+ primaryEncodedOutputProfile->addSupportedDevice(mConfig->getDefaultOutputDevice());
+ mConfig->getHwModules().getModuleFromName(AUDIO_HARDWARE_MODULE_ID_PRIMARY)->
addOutputProfile(primaryEncodedOutputProfile);
- mDefaultOutputDevice = config.getDefaultOutputDevice();
+ mDefaultOutputDevice = mConfig->getDefaultOutputDevice();
if (mExpectedAudioPatchCount == 2) {
mSpdifDevice->addAudioProfile(dtsOutputProfile);
primaryEncodedOutputProfile->addSupportedDevice(mSpdifDevice);
@@ -521,12 +526,12 @@
sp<AudioProfile> iec958InputProfile = new AudioProfile(
AUDIO_FORMAT_IEC60958, AUDIO_CHANNEL_INDEX_MASK_24, k48000SamplingRate);
mHdmiInputDevice->addAudioProfile(iec958InputProfile);
- config.addDevice(mHdmiInputDevice);
+ mConfig->addDevice(mHdmiInputDevice);
sp<InputProfile> hdmiInputProfile = new InputProfile("hdmi input");
hdmiInputProfile->addAudioProfile(iec958InputProfile);
hdmiInputProfile->setFlags(AUDIO_INPUT_FLAG_DIRECT);
hdmiInputProfile->addSupportedDevice(mHdmiInputDevice);
- config.getHwModules().getModuleFromName(AUDIO_HARDWARE_MODULE_ID_PRIMARY)->
+ mConfig->getHwModules().getModuleFromName(AUDIO_HARDWARE_MODULE_ID_PRIMARY)->
addInputProfile(hdmiInputProfile);
}
@@ -693,7 +698,7 @@
int countDirectProfilesPrimary = 0;
const auto& primary = mManager->getConfig().getHwModules()
.getModuleFromName(AUDIO_HARDWARE_MODULE_ID_PRIMARY);
- for (const auto outputProfile : primary->getOutputProfiles()) {
+ for (const auto& outputProfile : primary->getOutputProfiles()) {
if (outputProfile->asAudioPort()->isDirectOutput()) {
countDirectProfilesPrimary += outputProfile->asAudioPort()->getAudioProfiles().size();
}
@@ -703,7 +708,7 @@
int countDirectProfilesMsd = 0;
const auto& msd = mManager->getConfig().getHwModules()
.getModuleFromName(AUDIO_HARDWARE_MODULE_ID_MSD);
- for (const auto outputProfile : msd->getOutputProfiles()) {
+ for (const auto& outputProfile : msd->getOutputProfiles()) {
if (outputProfile->asAudioPort()->isDirectOutput()) {
countDirectProfilesMsd += outputProfile->asAudioPort()->getAudioProfiles().size();
}
@@ -894,9 +899,9 @@
sExecutableDir + "test_audio_policy_configuration.xml";
void AudioPolicyManagerTestWithConfigurationFile::SetUpManagerConfig() {
- status_t status = deserializeAudioPolicyFile(getConfigFile().c_str(), &mManager->getConfig());
- ASSERT_EQ(NO_ERROR, status);
- mManager->getConfig().setSource(getConfigFile());
+ auto result = AudioPolicyConfig::loadFromCustomXmlConfigForTests(getConfigFile());
+ ASSERT_TRUE(result.ok());
+ mConfig = result.value();
}
TEST_F(AudioPolicyManagerTestWithConfigurationFile, InitSuccess) {