diff --git a/audio/aidl/default/Module.cpp b/audio/aidl/default/Module.cpp index ab71ef4ed2..c0c6c4889d 100644 --- a/audio/aidl/default/Module.cpp +++ b/audio/aidl/default/Module.cpp @@ -340,30 +340,61 @@ void Module::registerPatch(const AudioPatch& patch) { do_insert(patch.sinkPortConfigIds); } -void Module::updateStreamsConnectedState(const AudioPatch& oldPatch, const AudioPatch& newPatch) { +ndk::ScopedAStatus Module::updateStreamsConnectedState(const AudioPatch& oldPatch, + const AudioPatch& newPatch) { // Streams from the old patch need to be disconnected, streams from the new // patch need to be connected. If the stream belongs to both patches, no need // to update it. - std::set idsToDisconnect, idsToConnect; + auto maybeFailure = ndk::ScopedAStatus::ok(); + std::set idsToDisconnect, idsToConnect, idsToDisconnectOnFailure; idsToDisconnect.insert(oldPatch.sourcePortConfigIds.begin(), oldPatch.sourcePortConfigIds.end()); idsToDisconnect.insert(oldPatch.sinkPortConfigIds.begin(), oldPatch.sinkPortConfigIds.end()); idsToConnect.insert(newPatch.sourcePortConfigIds.begin(), newPatch.sourcePortConfigIds.end()); idsToConnect.insert(newPatch.sinkPortConfigIds.begin(), newPatch.sinkPortConfigIds.end()); std::for_each(idsToDisconnect.begin(), idsToDisconnect.end(), [&](const auto& portConfigId) { - if (idsToConnect.count(portConfigId) == 0) { - LOG(DEBUG) << "The stream on port config id " << portConfigId << " is not connected"; - mStreams.setStreamIsConnected(portConfigId, {}); + if (idsToConnect.count(portConfigId) == 0 && mStreams.count(portConfigId) != 0) { + if (auto status = mStreams.setStreamConnectedDevices(portConfigId, {}); status.isOk()) { + LOG(DEBUG) << "updateStreamsConnectedState: The stream on port config id " + << portConfigId << " has been disconnected"; + } else { + // Disconnection is tricky to roll back, just register a failure. + maybeFailure = std::move(status); + } } }); + if (!maybeFailure.isOk()) return maybeFailure; std::for_each(idsToConnect.begin(), idsToConnect.end(), [&](const auto& portConfigId) { - if (idsToDisconnect.count(portConfigId) == 0) { + if (idsToDisconnect.count(portConfigId) == 0 && mStreams.count(portConfigId) != 0) { const auto connectedDevices = findConnectedDevices(portConfigId); - LOG(DEBUG) << "The stream on port config id " << portConfigId - << " is connected to: " << ::android::internal::ToString(connectedDevices); - mStreams.setStreamIsConnected(portConfigId, connectedDevices); + 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 " + << portConfigId; + } + if (auto status = mStreams.setStreamConnectedDevices(portConfigId, connectedDevices); + status.isOk()) { + LOG(DEBUG) << "updateStreamsConnectedState: The stream on port config id " + << portConfigId << " has been connected to: " + << ::android::internal::ToString(connectedDevices); + } else { + maybeFailure = std::move(status); + idsToDisconnectOnFailure.insert(portConfigId); + } } }); + if (!maybeFailure.isOk()) { + LOG(WARNING) << __func__ << ": 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. + }); + return maybeFailure; + } + return ndk::ScopedAStatus::ok(); } ndk::ScopedAStatus Module::setModuleDebug( @@ -659,12 +690,12 @@ ndk::ScopedAStatus Module::openInputStream(const OpenInputStreamArguments& in_ar RETURN_STATUS_IF_ERROR(getStreamInCreator(mType)(in_args.sinkMetadata, std::move(context), mConfig->microphones, &stream)); StreamWrapper streamWrapper(stream); + if (auto patchIt = mPatches.find(in_args.portConfigId); patchIt != mPatches.end()) { + RETURN_STATUS_IF_ERROR( + streamWrapper.setConnectedDevices(findConnectedDevices(in_args.portConfigId))); + } AIBinder_setMinSchedulerPolicy(streamWrapper.getBinder().get(), SCHED_NORMAL, ANDROID_PRIORITY_AUDIO); - auto patchIt = mPatches.find(in_args.portConfigId); - if (patchIt != mPatches.end()) { - streamWrapper.setStreamIsConnected(findConnectedDevices(in_args.portConfigId)); - } mStreams.insert(port->id, in_args.portConfigId, std::move(streamWrapper)); _aidl_return->stream = std::move(stream); return ndk::ScopedAStatus::ok(); @@ -705,12 +736,12 @@ ndk::ScopedAStatus Module::openOutputStream(const OpenOutputStreamArguments& in_ RETURN_STATUS_IF_ERROR(getStreamOutCreator(mType)(in_args.sourceMetadata, std::move(context), in_args.offloadInfo, &stream)); StreamWrapper streamWrapper(stream); + if (auto patchIt = mPatches.find(in_args.portConfigId); patchIt != mPatches.end()) { + RETURN_STATUS_IF_ERROR( + streamWrapper.setConnectedDevices(findConnectedDevices(in_args.portConfigId))); + } AIBinder_setMinSchedulerPolicy(streamWrapper.getBinder().get(), SCHED_NORMAL, ANDROID_PRIORITY_AUDIO); - auto patchIt = mPatches.find(in_args.portConfigId); - if (patchIt != mPatches.end()) { - streamWrapper.setStreamIsConnected(findConnectedDevices(in_args.portConfigId)); - } mStreams.insert(port->id, in_args.portConfigId, std::move(streamWrapper)); _aidl_return->stream = std::move(stream); return ndk::ScopedAStatus::ok(); @@ -813,13 +844,20 @@ ndk::ScopedAStatus Module::setAudioPatch(const AudioPatch& in_requested, AudioPa if (existing == patches.end()) { _aidl_return->id = getConfig().nextPatchId++; patches.push_back(*_aidl_return); - existing = patches.begin() + (patches.size() - 1); } else { oldPatch = *existing; - *existing = *_aidl_return; } - registerPatch(*existing); - updateStreamsConnectedState(oldPatch, *_aidl_return); + patchesBackup = mPatches; + registerPatch(*_aidl_return); + if (auto status = updateStreamsConnectedState(oldPatch, *_aidl_return); !status.isOk()) { + mPatches = std::move(*patchesBackup); + if (existing == patches.end()) { + patches.pop_back(); + } else { + *existing = oldPatch; + } + return status; + } LOG(DEBUG) << __func__ << ": " << (oldPatch.id == 0 ? "created" : "updated") << " patch " << _aidl_return->toString(); @@ -971,8 +1009,12 @@ ndk::ScopedAStatus Module::resetAudioPatch(int32_t in_patchId) { auto& patches = getConfig().patches; auto patchIt = findById(patches, in_patchId); if (patchIt != patches.end()) { + auto patchesBackup = mPatches; cleanUpPatch(patchIt->id); - updateStreamsConnectedState(*patchIt, AudioPatch{}); + if (auto status = updateStreamsConnectedState(*patchIt, AudioPatch{}); !status.isOk()) { + mPatches = std::move(patchesBackup); + return status; + } patches.erase(patchIt); LOG(DEBUG) << __func__ << ": erased patch " << in_patchId; return ndk::ScopedAStatus::ok(); diff --git a/audio/aidl/default/include/core-impl/Module.h b/audio/aidl/default/include/core-impl/Module.h index e8b3d899c3..212cfd9570 100644 --- a/audio/aidl/default/include/core-impl/Module.h +++ b/audio/aidl/default/include/core-impl/Module.h @@ -219,7 +219,8 @@ class Module : public BnModule { template std::set portIdsFromPortConfigIds(C portConfigIds); void registerPatch(const AudioPatch& patch); - void updateStreamsConnectedState(const AudioPatch& oldPatch, const AudioPatch& newPatch); + ndk::ScopedAStatus updateStreamsConnectedState(const AudioPatch& oldPatch, + const AudioPatch& newPatch); }; } // namespace aidl::android::hardware::audio::core diff --git a/audio/aidl/default/include/core-impl/Stream.h b/audio/aidl/default/include/core-impl/Stream.h index 4f84de900b..826b0f1ec9 100644 --- a/audio/aidl/default/include/core-impl/Stream.h +++ b/audio/aidl/default/include/core-impl/Stream.h @@ -395,11 +395,11 @@ class StreamCommonImpl : public StreamCommonInterface { : ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); } bool isClosed() const { return mWorker->isClosed(); } - void setIsConnected( + ndk::ScopedAStatus setConnectedDevices( const std::vector<::aidl::android::media::audio::common::AudioDevice>& devices) { mWorker->setIsConnected(!devices.empty()); mConnectedDevices = devices; - mDriver->setConnectedDevices(devices); + return ndk::ScopedAStatus::fromStatus(mDriver->setConnectedDevices(devices)); } ndk::ScopedAStatus updateMetadata(const Metadata& metadata); @@ -547,12 +547,13 @@ class StreamWrapper { }, mStream); } - void setStreamIsConnected( + ndk::ScopedAStatus setConnectedDevices( const std::vector<::aidl::android::media::audio::common::AudioDevice>& devices) { - std::visit( + return std::visit( [&](auto&& ws) { auto s = ws.lock(); - if (s) s->setIsConnected(devices); + if (s) return s->setConnectedDevices(devices); + return ndk::ScopedAStatus::ok(); }, mStream); } @@ -576,12 +577,13 @@ class Streams { mStreams.insert(std::pair{portConfigId, sw}); mStreams.insert(std::pair{portId, std::move(sw)}); } - void setStreamIsConnected( + ndk::ScopedAStatus setStreamConnectedDevices( int32_t portConfigId, const std::vector<::aidl::android::media::audio::common::AudioDevice>& devices) { if (auto it = mStreams.find(portConfigId); it != mStreams.end()) { - it->second.setStreamIsConnected(devices); + return it->second.setConnectedDevices(devices); } + return ndk::ScopedAStatus::ok(); } private: diff --git a/audio/aidl/default/usb/StreamUsb.cpp b/audio/aidl/default/usb/StreamUsb.cpp index 9ac1cc9b77..d2ee484c2d 100644 --- a/audio/aidl/default/usb/StreamUsb.cpp +++ b/audio/aidl/default/usb/StreamUsb.cpp @@ -72,7 +72,7 @@ DriverUsb::DriverUsb(const StreamContext& context, bool isInput) if (mIsInput && connectedDevices.size() > 1) { LOG(ERROR) << __func__ << ": wrong device size(" << connectedDevices.size() << ") for input stream"; - return ::android::BAD_VALUE; + return ::android::INVALID_OPERATION; } for (const auto& connectedDevice : connectedDevices) { if (connectedDevice.address.getTag() != AudioDeviceAddress::alsa) { diff --git a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp index 825b865ef1..0012cd559e 100644 --- a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp +++ b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp @@ -2763,6 +2763,7 @@ TEST_P(AudioStreamIn, ActiveMicrophones) { ASSERT_NO_FATAL_FAILURE(patch.SetUp(module.get())); std::vector activeMics; EXPECT_IS_OK(stream.get()->getActiveMicrophones(&activeMics)); + EXPECT_FALSE(activeMics.empty()); for (const auto& mic : activeMics) { EXPECT_NE(micInfos.end(), std::find_if(micInfos.begin(), micInfos.end(),