tequilaOS/platform_hardware_interfaces
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

audio: Fix handling of external devices disconnection

Browse source 
A mix port can be patched to multiple connected device ports. Thus, when
disconnecting an external device and removing the connected port, the
profiles of the mix port can only be cleared iff there are no more
connected device ports patched to it, and it did not have profiles prior to
connection of the first device.

Enhanced VTS tests to catch this problem in the HAL implementations. Also,
ensure that audio ports and audio routes do not change after the test
finishes. This ensures that tests can't affect each other.

Bug: 298175108
Test: atest audiosystem_tests
Test: atest VtsHalAudioCoreTargetTest
Change-Id: Ia666b874958fb260513fc2b8cd20a823953ec679
This commit is contained in:
Mikhail Naganov 2023-09-13 18:01:18 -07:00
parent ecc3e1af82
commit 0e128dd3fe
6 changed files with 224 additions and 73 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

31
audio/aidl/default/Module.cpp
View file

@ -517,7 +517,7 @@ ndk::ScopedAStatus Module::connectExternalDevice(const AudioPort& in_templateIdA
connectedPort.id = ++getConfig().nextPortId;
auto [connectedPortsIt, _] =
mConnectedDevicePorts.insert(std::pair(connectedPort.id, std::vector<int32_t>()));
mConnectedDevicePorts.insert(std::pair(connectedPort.id, std::set<int32_t>()));
LOG(DEBUG) << __func__ << ": template port " << templateId << " external device connected, "
<< "connected port ID " << connectedPort.id;
ports.push_back(connectedPort);
@ -550,9 +550,21 @@ ndk::ScopedAStatus Module::connectExternalDevice(const AudioPort& in_templateIdA
// of all profiles from all routable dynamic device ports would be more involved.
for (const auto mixPortId : routablePortIds) {
auto portsIt = findById<AudioPort>(ports, mixPortId);
if (portsIt != ports.end() && portsIt->profiles.empty()) {
portsIt->profiles = connectedPort.profiles;
connectedPortsIt->second.push_back(portsIt->id);
if (portsIt != ports.end()) {
if (portsIt->profiles.empty()) {
portsIt->profiles = connectedPort.profiles;
connectedPortsIt->second.insert(portsIt->id);
} else {
// Check if profiles are non empty because they were populated by
// a previous connection. Otherwise, it means that they are not empty because
// the mix port has static profiles.
for (const auto cp : mConnectedDevicePorts) {
if (cp.second.count(portsIt->id) > 0) {
connectedPortsIt->second.insert(portsIt->id);
break;
}
}
}
}
}
*_aidl_return = std::move(connectedPort);
@ -607,13 +619,20 @@ ndk::ScopedAStatus Module::disconnectExternalDevice(int32_t in_portId) {
}
}
for (const auto mixPortId : connectedPortsIt->second) {
// Clear profiles for mix ports that are not connected to any other ports.
std::set<int32_t> mixPortsToClear = std::move(connectedPortsIt->second);
mConnectedDevicePorts.erase(connectedPortsIt);
for (const auto& connectedPort : mConnectedDevicePorts) {
for (int32_t mixPortId : connectedPort.second) {
mixPortsToClear.erase(mixPortId);
}
}
for (int32_t mixPortId : mixPortsToClear) {
auto mixPortIt = findById<AudioPort>(ports, mixPortId);
if (mixPortIt != ports.end()) {
mixPortIt->profiles = {};
}
}
mConnectedDevicePorts.erase(connectedPortsIt);
return ndk::ScopedAStatus::ok();
}

2
audio/aidl/default/include/core-impl/Module.h
View file

@ -142,7 +142,7 @@ class Module : public BnModule {
// ids of device ports created at runtime via 'connectExternalDevice'.
// Also stores a list of ids of mix ports with dynamic profiles that were populated from
// the connected port. This list can be empty, thus an int->int multimap can't be used.
using ConnectedDevicePorts = std::map<int32_t, std::vector<int32_t>>;
using ConnectedDevicePorts = std::map<int32_t, std::set<int32_t>>;
// Maps port ids and port config ids to patch ids.
// Multimap because both ports and configs can be used by multiple patches.
using Patches = std::multimap<int32_t, int32_t>;

5
audio/aidl/vts/Android.bp
View file

@ -26,7 +26,10 @@ cc_defaults {
"libaudioaidlcommon",
"libaidlcommonsupport",
],
header_libs: ["libaudioaidl_headers"],
header_libs: [
"libaudioaidl_headers",
"libexpectedutils_headers",
],
cflags: [
"-Wall",
"-Wextra",

29
audio/aidl/vts/ModuleConfig.cpp
View file

@ -21,6 +21,7 @@
#include <aidl/android/media/audio/common/AudioInputFlags.h>
#include <aidl/android/media/audio/common/AudioIoFlags.h>
#include <aidl/android/media/audio/common/AudioOutputFlags.h>
#include <error/expected_utils.h>
#include "ModuleConfig.h"
@ -499,18 +500,13 @@ std::vector<AudioPortConfig> ModuleConfig::generateAudioDevicePortConfigs(
return result;
}
const ndk::ScopedAStatus& ModuleConfig::onExternalDeviceConnected(IModule* module,
const AudioPort& port) {
// Update ports and routes
mStatus = module->getAudioPorts(&mPorts);
if (!mStatus.isOk()) return mStatus;
mStatus = module->getAudioRoutes(&mRoutes);
if (!mStatus.isOk()) return mStatus;
ndk::ScopedAStatus ModuleConfig::onExternalDeviceConnected(IModule* module, const AudioPort& port) {
RETURN_STATUS_IF_ERROR(module->getAudioPorts(&mPorts));
RETURN_STATUS_IF_ERROR(module->getAudioRoutes(&mRoutes));
// Validate port is present in module
if (std::find(mPorts.begin(), mPorts.end(), port) == mPorts.end()) {
mStatus = ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE);
return mStatus;
return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT);
}
if (port.flags.getTag() == aidl::android::media::audio::common::AudioIoFlags::Tag::input) {
@ -518,23 +514,20 @@ const ndk::ScopedAStatus& ModuleConfig::onExternalDeviceConnected(IModule* modul
} else {
mConnectedExternalSinkDevicePorts.insert(port.id);
}
return mStatus;
return ndk::ScopedAStatus::ok();
}
const ndk::ScopedAStatus& ModuleConfig::onExternalDeviceDisconnected(IModule* module,
const AudioPort& port) {
// Update ports and routes
mStatus = module->getAudioPorts(&mPorts);
if (!mStatus.isOk()) return mStatus;
mStatus = module->getAudioRoutes(&mRoutes);
if (!mStatus.isOk()) return mStatus;
ndk::ScopedAStatus ModuleConfig::onExternalDeviceDisconnected(IModule* module,
const AudioPort& port) {
RETURN_STATUS_IF_ERROR(module->getAudioPorts(&mPorts));
RETURN_STATUS_IF_ERROR(module->getAudioRoutes(&mRoutes));
if (port.flags.getTag() == aidl::android::media::audio::common::AudioIoFlags::Tag::input) {
mConnectedExternalSourceDevicePorts.erase(port.id);
} else {
mConnectedExternalSinkDevicePorts.erase(port.id);
}
return mStatus;
return ndk::ScopedAStatus::ok();
}
bool ModuleConfig::isMmapSupported() const {

4
audio/aidl/vts/ModuleConfig.h
View file

@ -157,10 +157,10 @@ class ModuleConfig {
return *config.begin();
}
const ndk::ScopedAStatus& onExternalDeviceConnected(
ndk::ScopedAStatus onExternalDeviceConnected(
aidl::android::hardware::audio::core::IModule* module,
const aidl::android::media::audio::common::AudioPort& port);
const ndk::ScopedAStatus& onExternalDeviceDisconnected(
ndk::ScopedAStatus onExternalDeviceDisconnected(
aidl::android::hardware::audio::core::IModule* module,
const aidl::android::media::audio::common::AudioPort& port);

226
audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp
View file

@ -46,6 +46,7 @@
#include <aidl/android/media/audio/common/AudioOutputFlags.h>
#include <android-base/chrono_utils.h>
#include <android/binder_enums.h>
#include <error/expected_utils.h>
#include <fmq/AidlMessageQueue.h>
#include "AudioHalBinderServiceUtil.h"
@ -412,9 +413,21 @@ class AudioCoreModuleBase {
void SetUpImpl(const std::string& moduleName) {
ASSERT_NO_FATAL_FAILURE(ConnectToService(moduleName));
ASSERT_IS_OK(module->getAudioPorts(&initialPorts));
ASSERT_IS_OK(module->getAudioRoutes(&initialRoutes));
}
void TearDownImpl() { debug.reset(); }
void TearDownImpl() {
debug.reset();
std::vector<AudioPort> finalPorts;
ASSERT_IS_OK(module->getAudioPorts(&finalPorts));
EXPECT_NO_FATAL_FAILURE(VerifyVectorsAreEqual<AudioPort>(initialPorts, finalPorts))
<< "The list of audio ports was not restored to the initial state";
std::vector<AudioRoute> finalRoutes;
ASSERT_IS_OK(module->getAudioRoutes(&finalRoutes));
EXPECT_NO_FATAL_FAILURE(VerifyVectorsAreEqual<AudioRoute>(initialRoutes, finalRoutes))
<< "The list of audio routes was not restored to the initial state";
}
void ConnectToService(const std::string& moduleName) {
ASSERT_EQ(module, nullptr);
@ -502,17 +515,24 @@ class AudioCoreModuleBase {
}
}
// Warning: modifies the vectors!
template <typename T>
void VerifyVectorsAreEqual(std::vector<T>& v1, std::vector<T>& v2) {
ASSERT_EQ(v1.size(), v2.size());
std::sort(v1.begin(), v1.end());
std::sort(v2.begin(), v2.end());
if (v1 != v2) {
FAIL() << "Vectors are not equal: v1 = " << ::android::internal::ToString(v1)
<< ", v2 = " << ::android::internal::ToString(v2);
}
}
std::shared_ptr<IModule> module;
std::unique_ptr<ModuleConfig> moduleConfig;
AudioHalBinderServiceUtil binderUtil;
std::unique_ptr<WithDebugFlags> debug;
};
class AudioCoreModule : public AudioCoreModuleBase, public testing::TestWithParam<std::string> {
public:
void SetUp() override { ASSERT_NO_FATAL_FAILURE(SetUpImpl(GetParam())); }
void TearDown() override { ASSERT_NO_FATAL_FAILURE(TearDownImpl()); }
std::vector<AudioPort> initialPorts;
std::vector<AudioRoute> initialRoutes;
};
class WithDevicePortConnectedState {
@ -530,16 +550,19 @@ class WithDevicePortConnectedState {
<< "when external device disconnected";
}
}
ScopedAStatus SetUpNoChecks(IModule* module, ModuleConfig* moduleConfig) {
RETURN_STATUS_IF_ERROR(module->connectExternalDevice(mIdAndData, &mConnectedPort));
RETURN_STATUS_IF_ERROR(moduleConfig->onExternalDeviceConnected(module, mConnectedPort));
mModule = module;
mModuleConfig = moduleConfig;
return ScopedAStatus::ok();
}
void SetUp(IModule* module, ModuleConfig* moduleConfig) {
ASSERT_IS_OK(module->connectExternalDevice(mIdAndData, &mConnectedPort))
ASSERT_NE(moduleConfig, nullptr);
ASSERT_IS_OK(SetUpNoChecks(module, moduleConfig))
<< "when connecting device port ID & data " << mIdAndData.toString();
ASSERT_NE(mIdAndData.id, getId())
<< "ID of the connected port must not be the same as the ID of the template port";
ASSERT_NE(moduleConfig, nullptr);
ASSERT_IS_OK(moduleConfig->onExternalDeviceConnected(module, mConnectedPort))
<< "when external device connected";
mModule = module;
mModuleConfig = moduleConfig;
}
int32_t getId() const { return mConnectedPort.id; }
const AudioPort& get() { return mConnectedPort; }
@ -551,6 +574,13 @@ class WithDevicePortConnectedState {
AudioPort mConnectedPort;
};
class AudioCoreModule : public AudioCoreModuleBase, public testing::TestWithParam<std::string> {
public:
void SetUp() override { ASSERT_NO_FATAL_FAILURE(SetUpImpl(GetParam())); }
void TearDown() override { ASSERT_NO_FATAL_FAILURE(TearDownImpl()); }
};
class StreamContext {
public:
typedef AidlMessageQueue<StreamDescriptor::Command, SynchronizedReadWrite> CommandMQ;
@ -1258,11 +1288,8 @@ TEST_P(AudioCoreModule, GetAudioPortsIsStable) {
ASSERT_IS_OK(module->getAudioPorts(&ports1));
std::vector<AudioPort> ports2;
ASSERT_IS_OK(module->getAudioPorts(&ports2));
ASSERT_EQ(ports1.size(), ports2.size())
<< "Sizes of audio port arrays do not match across consequent calls to getAudioPorts";
std::sort(ports1.begin(), ports1.end());
std::sort(ports2.begin(), ports2.end());
EXPECT_EQ(ports1, ports2);
EXPECT_NO_FATAL_FAILURE(VerifyVectorsAreEqual<AudioPort>(ports1, ports2))
<< "Audio port arrays do not match across consequent calls to getAudioPorts";
}
TEST_P(AudioCoreModule, GetAudioRoutesIsStable) {
@ -1270,11 +1297,8 @@ TEST_P(AudioCoreModule, GetAudioRoutesIsStable) {
ASSERT_IS_OK(module->getAudioRoutes(&routes1));
std::vector<AudioRoute> routes2;
ASSERT_IS_OK(module->getAudioRoutes(&routes2));
ASSERT_EQ(routes1.size(), routes2.size())
<< "Sizes of audio route arrays do not match across consequent calls to getAudioRoutes";
std::sort(routes1.begin(), routes1.end());
std::sort(routes2.begin(), routes2.end());
EXPECT_EQ(routes1, routes2);
EXPECT_NO_FATAL_FAILURE(VerifyVectorsAreEqual<AudioRoute>(routes1, routes2))
<< " Audio route arrays do not match across consequent calls to getAudioRoutes";
}
TEST_P(AudioCoreModule, GetAudioRoutesAreValid) {
@ -1792,39 +1816,151 @@ TEST_P(AudioCoreModule, ExternalDevicePortRoutes) {
}
}
class RoutedPortsProfilesSnapshot {
public:
explicit RoutedPortsProfilesSnapshot(int32_t portId) : mPortId(portId) {}
void Capture(IModule* module) {
std::vector<AudioRoute> routes;
ASSERT_IS_OK(module->getAudioRoutesForAudioPort(mPortId, &routes));
std::vector<AudioPort> allPorts;
ASSERT_IS_OK(module->getAudioPorts(&allPorts));
ASSERT_NO_FATAL_FAILURE(GetAllRoutedPorts(routes, allPorts));
ASSERT_NO_FATAL_FAILURE(GetProfileSizes());
}
void VerifyNoProfilesChanges(const RoutedPortsProfilesSnapshot& before) {
for (const auto& p : before.mRoutedPorts) {
auto beforeIt = before.mPortProfileSizes.find(p.id);
ASSERT_NE(beforeIt, before.mPortProfileSizes.end())
<< "port ID " << p.id << " not found in the initial profile sizes";
EXPECT_EQ(beforeIt->second, mPortProfileSizes[p.id])
<< " port " << p.toString() << " has an unexpected profile size change"
<< " following an external device connection and disconnection";
}
}
void VerifyProfilesNonEmpty() {
for (const auto& p : mRoutedPorts) {
EXPECT_NE(0UL, mPortProfileSizes[p.id])
<< " port " << p.toString() << " must have had its profiles"
<< " populated while having a connected external device";
}
}
const std::vector<AudioPort>& getRoutedPorts() const { return mRoutedPorts; }
private:
void GetAllRoutedPorts(const std::vector<AudioRoute>& routes,
std::vector<AudioPort>& allPorts) {
for (const auto& r : routes) {
if (r.sinkPortId == mPortId) {
for (const auto& srcPortId : r.sourcePortIds) {
const auto srcPortIt = findById(allPorts, srcPortId);
ASSERT_NE(allPorts.end(), srcPortIt) << "port ID " << srcPortId;
mRoutedPorts.push_back(*srcPortIt);
}
} else {
const auto sinkPortIt = findById(allPorts, r.sinkPortId);
ASSERT_NE(allPorts.end(), sinkPortIt) << "port ID " << r.sinkPortId;
mRoutedPorts.push_back(*sinkPortIt);
}
}
}
void GetProfileSizes() {
std::transform(
mRoutedPorts.begin(), mRoutedPorts.end(),
std::inserter(mPortProfileSizes, mPortProfileSizes.end()),
[](const auto& port) { return std::make_pair(port.id, port.profiles.size()); });
}
const int32_t mPortId;
std::vector<AudioPort> mRoutedPorts;
std::map<int32_t, size_t> mPortProfileSizes;
};
// Note: This test relies on simulation of external device connections by the HAL module.
TEST_P(AudioCoreModule, ExternalDeviceMixPortConfigs) {
// After an external device has been connected, all mix ports that can be routed
// to the device port for the connected device must have non-empty profiles.
// Since the test connects and disconnects a single device each time, the size
// of profiles for all mix ports routed to the device port under test must get back
// to the original count once the external device is disconnected.
ASSERT_NO_FATAL_FAILURE(SetUpModuleConfig());
std::vector<AudioPort> externalDevicePorts = moduleConfig->getExternalDevicePorts();
if (externalDevicePorts.empty()) {
GTEST_SKIP() << "No external devices in the module.";
}
for (const auto& port : externalDevicePorts) {
WithDevicePortConnectedState portConnected(GenerateUniqueDeviceAddress(port));
ASSERT_NO_FATAL_FAILURE(portConnected.SetUp(module.get(), moduleConfig.get()));
std::vector<AudioRoute> routes;
ASSERT_IS_OK(module->getAudioRoutesForAudioPort(portConnected.getId(), &routes));
std::vector<AudioPort> allPorts;
ASSERT_IS_OK(module->getAudioPorts(&allPorts));
for (const auto& r : routes) {
if (r.sinkPortId == portConnected.getId()) {
for (const auto& srcPortId : r.sourcePortIds) {
const auto srcPortIt = findById(allPorts, srcPortId);
ASSERT_NE(allPorts.end(), srcPortIt) << "port ID " << srcPortId;
EXPECT_NE(0UL, srcPortIt->profiles.size())
<< " source port " << srcPortIt->toString() << " must have its profiles"
<< " populated following external device connection";
}
} else {
const auto sinkPortIt = findById(allPorts, r.sinkPortId);
ASSERT_NE(allPorts.end(), sinkPortIt) << "port ID " << r.sinkPortId;
EXPECT_NE(0UL, sinkPortIt->profiles.size())
<< " source port " << sinkPortIt->toString() << " must have its"
<< " profiles populated following external device connection";
SCOPED_TRACE(port.toString());
RoutedPortsProfilesSnapshot before(port.id);
ASSERT_NO_FATAL_FAILURE(before.Capture(module.get()));
if (before.getRoutedPorts().empty()) continue;
{
WithDevicePortConnectedState portConnected(GenerateUniqueDeviceAddress(port));
ASSERT_NO_FATAL_FAILURE(portConnected.SetUp(module.get(), moduleConfig.get()));
RoutedPortsProfilesSnapshot connected(portConnected.getId());
ASSERT_NO_FATAL_FAILURE(connected.Capture(module.get()));
EXPECT_NO_FATAL_FAILURE(connected.VerifyProfilesNonEmpty());
}
RoutedPortsProfilesSnapshot after(port.id);
ASSERT_NO_FATAL_FAILURE(after.Capture(module.get()));
EXPECT_NO_FATAL_FAILURE(after.VerifyNoProfilesChanges(before));
}
}
// Note: This test relies on simulation of external device connections by the HAL module.
TEST_P(AudioCoreModule, TwoExternalDevicesMixPortConfigsNested) {
// Ensure that in the case when two external devices are connected to the same
// device port, disconnecting one of them does not erase the profiles of routed mix ports.
// In this scenario, the connections are "nested."
ASSERT_NO_FATAL_FAILURE(SetUpModuleConfig());
std::vector<AudioPort> externalDevicePorts = moduleConfig->getExternalDevicePorts();
if (externalDevicePorts.empty()) {
GTEST_SKIP() << "No external devices in the module.";
}
for (const auto& port : externalDevicePorts) {
SCOPED_TRACE(port.toString());
WithDevicePortConnectedState portConnected1(GenerateUniqueDeviceAddress(port));
ASSERT_NO_FATAL_FAILURE(portConnected1.SetUp(module.get(), moduleConfig.get()));
{
// Connect and disconnect another device, if possible. It might not be possible
// for point-to-point connections, like analog or SPDIF.
WithDevicePortConnectedState portConnected2(GenerateUniqueDeviceAddress(port));
if (auto status = portConnected2.SetUpNoChecks(module.get(), moduleConfig.get());
!status.isOk()) {
continue;
}
}
RoutedPortsProfilesSnapshot connected(portConnected1.getId());
ASSERT_NO_FATAL_FAILURE(connected.Capture(module.get()));
EXPECT_NO_FATAL_FAILURE(connected.VerifyProfilesNonEmpty());
}
}
// Note: This test relies on simulation of external device connections by the HAL module.
TEST_P(AudioCoreModule, TwoExternalDevicesMixPortConfigsInterleaved) {
// Ensure that in the case when two external devices are connected to the same
// device port, disconnecting one of them does not erase the profiles of routed mix ports.
// In this scenario, the connections are "interleaved."
ASSERT_NO_FATAL_FAILURE(SetUpModuleConfig());
std::vector<AudioPort> externalDevicePorts = moduleConfig->getExternalDevicePorts();
if (externalDevicePorts.empty()) {
GTEST_SKIP() << "No external devices in the module.";
}
for (const auto& port : externalDevicePorts) {
SCOPED_TRACE(port.toString());
auto portConnected1 =
std::make_unique<WithDevicePortConnectedState>(GenerateUniqueDeviceAddress(port));
ASSERT_NO_FATAL_FAILURE(portConnected1->SetUp(module.get(), moduleConfig.get()));
WithDevicePortConnectedState portConnected2(GenerateUniqueDeviceAddress(port));
// Connect another device, if possible. It might not be possible for point-to-point
// connections, like analog or SPDIF.
if (auto status = portConnected2.SetUpNoChecks(module.get(), moduleConfig.get());
!status.isOk()) {
continue;
}
portConnected1.reset();
RoutedPortsProfilesSnapshot connected(portConnected2.getId());
ASSERT_NO_FATAL_FAILURE(connected.Capture(module.get()));
EXPECT_NO_FATAL_FAILURE(connected.VerifyProfilesNonEmpty());
}
}