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

Merge "audio: Fix handling of external devices disconnection" into main

Browse source
This commit is contained in:
Mikhail Naganov 2023-09-14 20:02:48 +00:00 committed by Gerrit Code Review
commit a39e854ef5
6 changed files with 224 additions and 73 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

29
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()) {
if (portsIt != ports.end()) {
if (portsIt->profiles.empty()) {
portsIt->profiles = connectedPort.profiles;
connectedPortsIt->second.push_back(portsIt->id);
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",

27
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,
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;
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);

220
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) {
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()));
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";
RoutedPortsProfilesSnapshot connected(portConnected.getId());
ASSERT_NO_FATAL_FAILURE(connected.Capture(module.get()));
EXPECT_NO_FATAL_FAILURE(connected.VerifyProfilesNonEmpty());
}
} 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";
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());
}
}