audiopolicy: Add more tests, fix issues found
Added tests that ensure correct initialization of APM.
Fixed issues:
-- UB in VolumeCurvesForStream::volIndexToDb due to not
checking that the pointer is non-null;
-- AudioInputDescriptor::open and SwAudioOutputDescriptor::open
were not checking that the returned IO handle is valid,
this was causing an infinite loop in
AudioPolicyManager::selectOutputForMusicEffects, added an
assertion.
-- Memory leak of VolumeCurves collection in APM.
Test: audiopolicy_tests
Change-Id: Ia4ecca1dd03b74d7f93720f042da05d5a0c74a6b
diff --git a/services/audiopolicy/common/managerdefinitions/include/IVolumeCurvesCollection.h b/services/audiopolicy/common/managerdefinitions/include/IVolumeCurvesCollection.h
index 7b35f66..e1f6b08 100644
--- a/services/audiopolicy/common/managerdefinitions/include/IVolumeCurvesCollection.h
+++ b/services/audiopolicy/common/managerdefinitions/include/IVolumeCurvesCollection.h
@@ -25,6 +25,8 @@
class IVolumeCurvesCollection
{
public:
+ virtual ~IVolumeCurvesCollection() = default;
+
virtual void clearCurrentVolumeIndex(audio_stream_type_t stream) = 0;
virtual void addCurrentVolumeIndex(audio_stream_type_t stream, audio_devices_t device,
int index) = 0;
@@ -46,9 +48,6 @@
audio_devices_t device) const = 0;
virtual status_t dump(int fd) const = 0;
-
-protected:
- virtual ~IVolumeCurvesCollection() {}
};
} // namespace android
diff --git a/services/audiopolicy/common/managerdefinitions/include/VolumeCurve.h b/services/audiopolicy/common/managerdefinitions/include/VolumeCurve.h
index db25ed8..3e6b2b4 100644
--- a/services/audiopolicy/common/managerdefinitions/include/VolumeCurve.h
+++ b/services/audiopolicy/common/managerdefinitions/include/VolumeCurve.h
@@ -135,7 +135,13 @@
float volIndexToDb(device_category deviceCat, int indexInUi) const
{
- return getCurvesFor(deviceCat)->volIndexToDb(indexInUi, mIndexMin, mIndexMax);
+ sp<VolumeCurve> vc = getCurvesFor(deviceCat);
+ if (vc != 0) {
+ return vc->volIndexToDb(indexInUi, mIndexMin, mIndexMax);
+ } else {
+ ALOGE("Invalid device category %d for Volume Curve", deviceCat);
+ return 0.0f;
+ }
}
void dump(int fd, int spaces, bool curvePoints = false) const;
diff --git a/services/audiopolicy/common/managerdefinitions/src/AudioInputDescriptor.cpp b/services/audiopolicy/common/managerdefinitions/src/AudioInputDescriptor.cpp
index f589743..92332fb 100644
--- a/services/audiopolicy/common/managerdefinitions/src/AudioInputDescriptor.cpp
+++ b/services/audiopolicy/common/managerdefinitions/src/AudioInputDescriptor.cpp
@@ -223,6 +223,9 @@
__FUNCTION__, mDevice, device);
if (status == NO_ERROR) {
+ LOG_ALWAYS_FATAL_IF(*input == AUDIO_IO_HANDLE_NONE,
+ "%s openInput returned input handle %d for device %08x",
+ __FUNCTION__, *input, device);
mSamplingRate = lConfig.sample_rate;
mChannelMask = lConfig.channel_mask;
mFormat = lConfig.format;
diff --git a/services/audiopolicy/common/managerdefinitions/src/AudioOutputDescriptor.cpp b/services/audiopolicy/common/managerdefinitions/src/AudioOutputDescriptor.cpp
index 6e677e4..044d6db 100644
--- a/services/audiopolicy/common/managerdefinitions/src/AudioOutputDescriptor.cpp
+++ b/services/audiopolicy/common/managerdefinitions/src/AudioOutputDescriptor.cpp
@@ -431,6 +431,9 @@
__FUNCTION__, mDevice, device);
if (status == NO_ERROR) {
+ LOG_ALWAYS_FATAL_IF(*output == AUDIO_IO_HANDLE_NONE,
+ "%s openOutput returned output handle %d for device %08x",
+ __FUNCTION__, *output, device);
mSamplingRate = lConfig.sample_rate;
mChannelMask = lConfig.channel_mask;
mFormat = lConfig.format;
diff --git a/services/audiopolicy/managerdefault/AudioPolicyManager.cpp b/services/audiopolicy/managerdefault/AudioPolicyManager.cpp
index 25c5e7a..ae3dd08 100644
--- a/services/audiopolicy/managerdefault/AudioPolicyManager.cpp
+++ b/services/audiopolicy/managerdefault/AudioPolicyManager.cpp
@@ -3484,7 +3484,7 @@
#ifdef USE_XML_AUDIO_POLICY_CONF
mVolumeCurves(new VolumeCurvesCollection()),
mConfig(mHwModulesAll, mAvailableOutputDevices, mAvailableInputDevices,
- mDefaultOutputDevice, static_cast<VolumeCurvesCollection*>(mVolumeCurves)),
+ mDefaultOutputDevice, static_cast<VolumeCurvesCollection*>(mVolumeCurves.get())),
#else
mVolumeCurves(new StreamDescriptorCollection()),
mConfig(mHwModulesAll, mAvailableOutputDevices, mAvailableInputDevices,
diff --git a/services/audiopolicy/managerdefault/AudioPolicyManager.h b/services/audiopolicy/managerdefault/AudioPolicyManager.h
index 3659e34..611edec 100644
--- a/services/audiopolicy/managerdefault/AudioPolicyManager.h
+++ b/services/audiopolicy/managerdefault/AudioPolicyManager.h
@@ -17,6 +17,7 @@
#pragma once
#include <atomic>
+#include <memory>
#include <stdint.h>
#include <sys/types.h>
@@ -547,7 +548,7 @@
float mLastVoiceVolume; // last voice volume value sent to audio HAL
bool mA2dpSuspended; // true if A2DP output is suspended
- IVolumeCurvesCollection *mVolumeCurves; // Volume Curves per use case and device category
+ std::unique_ptr<IVolumeCurvesCollection> mVolumeCurves; // Volume Curves per use case and device category
EffectDescriptorCollection mEffects; // list of registered audio effects
sp<DeviceDescriptor> mDefaultOutputDevice; // output device selected by default at boot time
HwModuleCollection mHwModules; // contains only modules that have been loaded successfully
diff --git a/services/audiopolicy/tests/AudioPolicyTestClient.h b/services/audiopolicy/tests/AudioPolicyTestClient.h
index c4da98a..7e0450d 100644
--- a/services/audiopolicy/tests/AudioPolicyTestClient.h
+++ b/services/audiopolicy/tests/AudioPolicyTestClient.h
@@ -22,7 +22,7 @@
class AudioPolicyTestClient : public AudioPolicyClientInterface
{
public:
- virtual ~AudioPolicyTestClient() {}
+ virtual ~AudioPolicyTestClient() = default;
// AudioPolicyClientInterface Implementation
audio_module_handle_t loadHwModule(const char* /*name*/) override { return 0; }
diff --git a/services/audiopolicy/tests/audiopolicymanager_tests.cpp b/services/audiopolicy/tests/audiopolicymanager_tests.cpp
index 33b15c7..de26ab0 100644
--- a/services/audiopolicy/tests/audiopolicymanager_tests.cpp
+++ b/services/audiopolicy/tests/audiopolicymanager_tests.cpp
@@ -21,9 +21,62 @@
using namespace android;
-TEST(AudioPolicyManager, CreateWithDummyClient) {
+TEST(AudioPolicyManager, InitFailure) {
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());
+}
+
+
+// A client that provides correct module and IO handles for inputs and outputs.
+class AudioPolicyTestClientWithModulesIoHandles : public AudioPolicyTestClient {
+ public:
+ audio_module_handle_t loadHwModule(const char* /*name*/) override {
+ return mNextModule++;
+ }
+ status_t openOutput(audio_module_handle_t module,
+ audio_io_handle_t* output,
+ audio_config_t* /*config*/,
+ audio_devices_t* /*devices*/,
+ const String8& /*address*/,
+ uint32_t* /*latencyMs*/,
+ audio_output_flags_t /*flags*/) override {
+ if (module >= mNextModule) {
+ ALOGE("%s: Module handle %d has not been allocated yet (next is %d)",
+ __func__, module, mNextModule);
+ return BAD_VALUE;
+ }
+ *output = mNextIoHandle++;
+ return NO_ERROR;
+ }
+ status_t openInput(audio_module_handle_t module,
+ audio_io_handle_t* input,
+ audio_config_t* /*config*/,
+ audio_devices_t* /*device*/,
+ const String8& /*address*/,
+ audio_source_t /*source*/,
+ audio_input_flags_t /*flags*/) override {
+ if (module >= mNextModule) {
+ ALOGE("%s: Module handle %d has not been allocated yet (next is %d)",
+ __func__, module, mNextModule);
+ return BAD_VALUE;
+ }
+ *input = mNextIoHandle++;
+ return NO_ERROR;
+ }
+ private:
+ audio_module_handle_t mNextModule = AUDIO_MODULE_HANDLE_NONE + 1;
+ audio_io_handle_t mNextIoHandle = AUDIO_IO_HANDLE_NONE + 1;
+};
+
+TEST(AudioPolicyManager, InitSuccess) {
+ AudioPolicyTestClientWithModulesIoHandles client;
+ AudioPolicyTestManager manager(&client);
+ manager.getConfig().setDefault();
+ ASSERT_EQ(NO_ERROR, manager.initialize());
+ ASSERT_EQ(NO_ERROR, manager.initCheck());
}