From dc41773ba323196ffba37325f50519beae9c578e Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Fri, 29 Sep 2023 15:49:35 -0700 Subject: [PATCH] audio: Fix update of an existing patch The code for updating the existing patch did not modify the patch stored in the module's list of patches. Added a test which switches the patch to another port config and validates that 'Module.getAudioPatches' returns the updated patch. Bug: 302573756 Test: atest VtsHalAudioCoreTargetTest Change-Id: I0e3412b9387cd451436a48af116dc5a940d868cf --- audio/aidl/default/Module.cpp | 3 +- .../vts/VtsHalAudioCoreModuleTargetTest.cpp | 54 ++++++++++++++++++- 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/audio/aidl/default/Module.cpp b/audio/aidl/default/Module.cpp index 3117134f4d..0d4e21f311 100644 --- a/audio/aidl/default/Module.cpp +++ b/audio/aidl/default/Module.cpp @@ -515,7 +515,7 @@ ndk::ScopedAStatus Module::connectExternalDevice(const AudioPort& in_templateIdA } } - connectedPort.id = ++getConfig().nextPortId; + connectedPort.id = getConfig().nextPortId++; auto [connectedPortsIt, _] = mConnectedDevicePorts.insert(std::pair(connectedPort.id, std::set())); LOG(DEBUG) << __func__ << ": template port " << templateId << " external device connected, " @@ -865,6 +865,7 @@ ndk::ScopedAStatus Module::setAudioPatch(const AudioPatch& in_requested, AudioPa patches.push_back(*_aidl_return); } else { oldPatch = *existing; + *existing = *_aidl_return; } patchesBackup = mPatches; registerPatch(*_aidl_return); diff --git a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp index cd765d2225..d86e171465 100644 --- a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp +++ b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp @@ -1237,11 +1237,25 @@ class WithAudioPatch { const AudioPortConfig& portConfig2) : mSrcPortConfig(sinkIsCfg1 ? portConfig2 : portConfig1), mSinkPortConfig(sinkIsCfg1 ? portConfig1 : portConfig2) {} + WithAudioPatch(const WithAudioPatch& patch, const AudioPortConfig& srcPortConfig, + const AudioPortConfig& sinkPortConfig) + : mInitialPatch(patch.mPatch), + mSrcPortConfig(srcPortConfig), + mSinkPortConfig(sinkPortConfig), + mModule(patch.mModule), + mPatch(patch.mPatch) {} WithAudioPatch(const WithAudioPatch&) = delete; WithAudioPatch& operator=(const WithAudioPatch&) = delete; ~WithAudioPatch() { if (mModule != nullptr && mPatch.id != 0) { - EXPECT_IS_OK(mModule->resetAudioPatch(mPatch.id)) << "patch id " << getId(); + if (mInitialPatch.has_value()) { + AudioPatch ignored; + // This releases our port configs so that they can be reset. + EXPECT_IS_OK(mModule->setAudioPatch(*mInitialPatch, &ignored)) + << "patch id " << mInitialPatch->id; + } else { + EXPECT_IS_OK(mModule->resetAudioPatch(mPatch.id)) << "patch id " << getId(); + } } } void SetUpPortConfigs(IModule* module) { @@ -1263,6 +1277,16 @@ class WithAudioPatch { EXPECT_GT(latencyMs, 0) << "patch id " << getId(); } } + void VerifyAgainstAllPatches(IModule* module) { + std::vector allPatches; + ASSERT_IS_OK(module->getAudioPatches(&allPatches)); + const auto& patchIt = findById(allPatches, getId()); + ASSERT_NE(patchIt, allPatches.end()) << "patch id " << getId(); + if (get() != *patchIt) { + FAIL() << "Stored patch: " << get().toString() << " is not the same as returned " + << "by the HAL module: " << patchIt->toString(); + } + } int32_t getId() const { return mPatch.id; } const AudioPatch& get() const { return mPatch; } const AudioPortConfig& getSinkPortConfig() const { return mSinkPortConfig.get(); } @@ -1272,6 +1296,7 @@ class WithAudioPatch { } private: + std::optional mInitialPatch; WithAudioPortConfig mSrcPortConfig; WithAudioPortConfig mSinkPortConfig; IModule* mModule = nullptr; @@ -3630,9 +3655,12 @@ class AudioModulePatch : public AudioCoreModule { patches.push_back( std::make_unique(srcSink.first, srcSink.second)); EXPECT_NO_FATAL_FAILURE(patches[patches.size() - 1]->SetUp(module.get())); + EXPECT_NO_FATAL_FAILURE( + patches[patches.size() - 1]->VerifyAgainstAllPatches(module.get())); } else { WithAudioPatch patch(srcSink.first, srcSink.second); EXPECT_NO_FATAL_FAILURE(patch.SetUp(module.get())); + EXPECT_NO_FATAL_FAILURE(patch.VerifyAgainstAllPatches(module.get())); } } } @@ -3653,6 +3681,29 @@ class AudioModulePatch : public AudioCoreModule { } } + void UpdatePatchPorts(bool isInput) { + auto srcSinkGroups = moduleConfig->getRoutableSrcSinkGroups(isInput); + if (srcSinkGroups.empty()) { + GTEST_SKIP() << "No routes to any attached " << direction(isInput, false) << " devices"; + } + bool hasAtLeastOnePair = false; + for (const auto& srcSinkGroup : srcSinkGroups) { + const auto& srcSinks = srcSinkGroup.second; + if (srcSinks.size() < 2) continue; + hasAtLeastOnePair = true; + const auto& pair1 = srcSinks[0]; + const auto& pair2 = srcSinks[1]; + WithAudioPatch patch(pair1.first, pair1.second); + ASSERT_NO_FATAL_FAILURE(patch.SetUp(module.get())); + WithAudioPatch update(patch, pair2.first, pair2.second); + EXPECT_NO_FATAL_FAILURE(update.SetUp(module.get())); + EXPECT_NO_FATAL_FAILURE(update.VerifyAgainstAllPatches(module.get())); + } + if (!hasAtLeastOnePair) { + GTEST_SKIP() << "No routes with multiple sources"; + } + } + void UpdateInvalidPatchId(bool isInput) { auto srcSinkGroups = moduleConfig->getRoutableSrcSinkGroups(isInput); if (srcSinkGroups.empty()) { @@ -3692,6 +3743,7 @@ TEST_PATCH_BOTH_DIRECTIONS(SetNonRoutablePatch); TEST_PATCH_BOTH_DIRECTIONS(SetPatch); TEST_PATCH_BOTH_DIRECTIONS(UpdateInvalidPatchId); TEST_PATCH_BOTH_DIRECTIONS(UpdatePatch); +TEST_PATCH_BOTH_DIRECTIONS(UpdatePatchPorts); TEST_P(AudioModulePatch, ResetInvalidPatchId) { std::set patchIds;