audio: Propagate errors from Driver::setConnectedDevices

Plumb propagation of errors reported by the implementations
of DriverInterface::setConnectedDevices up to the Module.
This allows returning the reported errors from the corresponding
IModule interface methods. Implement handling of errors
from connection state update by the Module implementation.

When updating streams about the connection status
ensure that the list of connected devices is not empty.

Also, add an extra check to AudioStreamIn#ActiveMicrophones
to validate the size of the returned active microphones
list.

Bug: 282568751
Test: atest VtsHalAudioCoreTargetTest
Merged-In: I62a422d95c37a672fce4ad221bea435cc7b4ebfa
Change-Id: I62a422d95c37a672fce4ad221bea435cc7b4ebfa
(cherry picked from commit 75b59dfb4e)

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<int32_t> idsToDisconnect, idsToConnect;
+    auto maybeFailure = ndk::ScopedAStatus::ok();
+    std::set<int32_t> 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<AudioPatch>(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();

audio/aidl/default/include/core-impl/Module.h

@@ -219,7 +219,8 @@ class Module : public BnModule {
     template <typename C>
     std::set<int32_t> 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

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:

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) {

audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp

@@ -2763,6 +2763,7 @@ TEST_P(AudioStreamIn, ActiveMicrophones) {
             ASSERT_NO_FATAL_FAILURE(patch.SetUp(module.get()));
             std::vector<MicrophoneDynamicInfo> 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(),