From 32e85fccc84f2741a17394d98175184bbf9b80ab Mon Sep 17 00:00:00 2001 From: Ajender Reddy Date: Wed, 30 Oct 2024 16:58:17 +0530 Subject: [PATCH] audio: add rollback support for streams in setAudioPatch. This rollback support isn't altering the the definition of the IModule::setAudioPatch API, in case of success. But, in case of failure, setAudioPatch tries to rollback to the old patch. Although, rollback isn't guaranteed. But the rollback attempt is ensured. Thereby, rollback support enables framework to take fail safe decisions and tries to create a deterministic behavior for HAL. Add brief comments for this logic. Reason, the Bluetooth devices (external devices) may fail to connect and thus leaving the stream with empty devices may not be desirable. Thus, Rollback attempt to previous devices may be desirable. Bug: 370242708 Test: atest VtsHalAudioCoreTargetTest Change-Id: I40627c6af1df61c6331cbadf32d259378906431c --- audio/aidl/default/Module.cpp | 150 +++++++++++++++++++++++++--------- 1 file changed, 112 insertions(+), 38 deletions(-) diff --git a/audio/aidl/default/Module.cpp b/audio/aidl/default/Module.cpp index a2a357ce88..51b6085700 100644 --- a/audio/aidl/default/Module.cpp +++ b/audio/aidl/default/Module.cpp @@ -467,58 +467,132 @@ ndk::ScopedAStatus Module::updateStreamsConnectedState(const AudioPatch& oldPatc fillConnectionsHelper(connections, patch.sinkPortConfigIds, patch.sourcePortConfigIds); } // Otherwise, there are no streams to notify. }; - fillConnections(oldConnections, oldPatch); - fillConnections(newConnections, newPatch); - - std::for_each(oldConnections.begin(), oldConnections.end(), [&](const auto& connectionPair) { - const int32_t mixPortConfigId = connectionPair.first; - if (auto it = newConnections.find(mixPortConfigId); - it == newConnections.end() || it->second != connectionPair.second) { - if (auto status = mStreams.setStreamConnectedDevices(mixPortConfigId, {}); - status.isOk()) { - LOG(DEBUG) << "updateStreamsConnectedState: The stream on port config id " - << mixPortConfigId << " has been disconnected"; - } else { - // Disconnection is tricky to roll back, just register a failure. - maybeFailure = std::move(status); + auto restoreOldConnections = [&](const std::set& mixPortIds, + const bool continueWithEmptyDevices) { + for (const auto mixPort : mixPortIds) { + if (auto it = oldConnections.find(mixPort); + continueWithEmptyDevices || it != oldConnections.end()) { + const std::vector d = + it != oldConnections.end() ? getDevicesFromDevicePortConfigIds(it->second) + : std::vector(); + if (auto status = mStreams.setStreamConnectedDevices(mixPort, d); status.isOk()) { + LOG(WARNING) << ":updateStreamsConnectedState: rollback: mix port config:" + << mixPort + << (d.empty() ? "; not connected" + : std::string("; connected to ") + + ::android::internal::ToString(d)); + } else { + // can't do much about rollback failures + LOG(ERROR) + << ":updateStreamsConnectedState: rollback: failed for mix port config:" + << mixPort; + } } } - }); - if (!maybeFailure.isOk()) return maybeFailure; - std::set idsToDisconnectOnFailure; - std::for_each(newConnections.begin(), newConnections.end(), [&](const auto& connectionPair) { - const int32_t mixPortConfigId = connectionPair.first; - if (auto it = oldConnections.find(mixPortConfigId); - it == oldConnections.end() || it->second != connectionPair.second) { - const auto connectedDevices = getDevicesFromDevicePortConfigIds(connectionPair.second); + }; + fillConnections(oldConnections, oldPatch); + fillConnections(newConnections, newPatch); + /** + * Illustration of oldConnections and newConnections + * + * oldConnections { + * a : {A,B,C}, + * b : {D}, + * d : {H,I,J}, + * e : {N,O,P}, + * f : {Q,R}, + * g : {T,U,V}, + * } + * + * newConnections { + * a : {A,B,C}, + * c : {E,F,G}, + * d : {K,L,M}, + * e : {N,P}, + * f : {Q,R,S}, + * g : {U,V,W}, + * } + * + * Expected routings: + * 'a': is ignored both in disconnect step and connect step, + * due to same devices both in oldConnections and newConnections. + * 'b': handled only in disconnect step with empty devices because 'b' is only present + * in oldConnections. + * 'c': handled only in connect step with {E,F,G} devices because 'c' is only present + * in newConnections. + * 'd': handled only in connect step with {K,L,M} devices because 'd' is also present + * in newConnections and it is ignored in disconnected step. + * 'e': handled only in connect step with {N,P} devices because 'e' is also present + * in newConnections and it is ignored in disconnect step. please note that there + * is no exclusive disconnection for device {O}. + * 'f': handled only in connect step with {Q,R,S} devices because 'f' is also present + * in newConnections and it is ignored in disconnect step. Even though stream is + * already connected with {Q,R} devices and connection happens with {Q,R,S}. + * 'g': handled only in connect step with {U,V,W} devices because 'g' is also present + * in newConnections and it is ignored in disconnect step. There is no exclusive + * disconnection with devices {T,U,V}. + * + * If, any failure, will lead to restoreOldConnections (rollback). + * The aim of the restoreOldConnections is to make connections back to oldConnections. + * Failures in restoreOldConnections aren't handled. + */ + + std::set idsToConnectBackOnFailure; + // disconnection step + for (const auto& [oldMixPortConfigId, oldDevicePortConfigIds] : oldConnections) { + if (auto it = newConnections.find(oldMixPortConfigId); it == newConnections.end()) { + idsToConnectBackOnFailure.insert(oldMixPortConfigId); + if (auto status = mStreams.setStreamConnectedDevices(oldMixPortConfigId, {}); + status.isOk()) { + LOG(DEBUG) << __func__ << ": The stream on port config id " << oldMixPortConfigId + << " has been disconnected"; + } else { + maybeFailure = std::move(status); + // proceed to rollback even on one failure + break; + } + } + } + + if (!maybeFailure.isOk()) { + restoreOldConnections(idsToConnectBackOnFailure, false /*continueWithEmptyDevices*/); + LOG(WARNING) << __func__ << ": failed to disconnect from old patch. attempted rollback"; + return maybeFailure; + } + + std::set idsToRollbackOnFailure; + // connection step + for (const auto& [newMixPortConfigId, newDevicePortConfigIds] : newConnections) { + if (auto it = oldConnections.find(newMixPortConfigId); + it == oldConnections.end() || it->second != newDevicePortConfigIds) { + const auto connectedDevices = getDevicesFromDevicePortConfigIds(newDevicePortConfigIds); + idsToRollbackOnFailure.insert(newMixPortConfigId); if (connectedDevices.empty()) { // This is important as workers use the vector size to derive the connection status. - LOG(FATAL) << "updateStreamsConnectedState: No connected devices found for port " - "config id " - << mixPortConfigId; + LOG(FATAL) << __func__ << ": No connected devices found for port config id " + << newMixPortConfigId; } - if (auto status = mStreams.setStreamConnectedDevices(mixPortConfigId, connectedDevices); + if (auto status = + mStreams.setStreamConnectedDevices(newMixPortConfigId, connectedDevices); status.isOk()) { - LOG(DEBUG) << "updateStreamsConnectedState: The stream on port config id " - << mixPortConfigId << " has been connected to: " + LOG(DEBUG) << __func__ << ": The stream on port config id " << newMixPortConfigId + << " has been connected to: " << ::android::internal::ToString(connectedDevices); } else { maybeFailure = std::move(status); - idsToDisconnectOnFailure.insert(mixPortConfigId); + // proceed to rollback even on one failure + break; } } - }); + } + if (!maybeFailure.isOk()) { - LOG(WARNING) << __func__ << ": " << mType - << ": Due to a failure, disconnecting streams on port config ids " - << ::android::internal::ToString(idsToDisconnectOnFailure); - std::for_each(idsToDisconnectOnFailure.begin(), idsToDisconnectOnFailure.end(), - [&](const auto& portConfigId) { - auto status = mStreams.setStreamConnectedDevices(portConfigId, {}); - (void)status.isOk(); // Can't do much about a failure here. - }); + restoreOldConnections(idsToConnectBackOnFailure, false /*continueWithEmptyDevices*/); + restoreOldConnections(idsToRollbackOnFailure, true /*continueWithEmptyDevices*/); + LOG(WARNING) << __func__ << ": failed to connect for new patch. attempted rollback"; return maybeFailure; } + return ndk::ScopedAStatus::ok(); }