Revert "libaudiohal@aidl: Fix handling of stream config suggestion"

Revert submission 2862771-cuttlefish_aidl_audio-2

Reason for revert: b/316027906

Reverted changes: /q/submissionid:2862771-cuttlefish_aidl_audio-2

Change-Id: I6affbc1f0b87b2d58934bf86b32d0545d91ef1ba
diff --git a/media/libaudiohal/impl/DeviceHalAidl.cpp b/media/libaudiohal/impl/DeviceHalAidl.cpp
index 7575a6f..8a843ed 100644
--- a/media/libaudiohal/impl/DeviceHalAidl.cpp
+++ b/media/libaudiohal/impl/DeviceHalAidl.cpp
@@ -448,7 +448,6 @@
     }
     *config = VALUE_OR_RETURN_STATUS(
             ::aidl::android::aidl2legacy_AudioConfig_audio_config_t(aidlConfig, isInput));
-    if (mixPortConfig.id == 0) return BAD_VALUE;  // HAL suggests a different config.
     ::aidl::android::hardware::audio::core::IModule::OpenOutputStreamArguments args;
     args.portConfigId = mixPortConfig.id;
     const bool isOffload = isBitPositionFlagSet(
@@ -521,7 +520,6 @@
     }
     *config = VALUE_OR_RETURN_STATUS(
             ::aidl::android::aidl2legacy_AudioConfig_audio_config_t(aidlConfig, isInput));
-    if (mixPortConfig.id == 0) return BAD_VALUE;  // HAL suggests a different config.
     ::aidl::android::hardware::audio::core::IModule::OpenInputStreamArguments args;
     args.portConfigId = mixPortConfig.id;
     RecordTrackMetadata aidlTrackMetadata{
@@ -706,7 +704,8 @@
                     *config, isInput, 0 /*portId*/));
     AudioPortConfig portConfig;
     std::lock_guard l(mLock);
-    return mMapper.setPortConfig(requestedPortConfig, std::set<int32_t>(), &portConfig);
+    return mMapper.findOrCreatePortConfig(
+            requestedPortConfig, std::set<int32_t>(), &portConfig);
 }
 
 MicrophoneInfoProvider::Info const* DeviceHalAidl::getMicrophoneInfo() {
@@ -778,7 +777,7 @@
     Hal2AidlMapper::Cleanups cleanups(mMapperAccessor);
     {
         std::lock_guard l(mLock);
-        RETURN_STATUS_IF_ERROR(mMapper.setPortConfig(
+        RETURN_STATUS_IF_ERROR(mMapper.findOrCreatePortConfig(
                     requestedPortConfig, {} /*destinationPortIds*/, &devicePortConfig, &cleanups));
     }
     auto aidlEffect = sp<effect::EffectHalAidl>::cast(effect);
diff --git a/media/libaudiohal/impl/Hal2AidlMapper.cpp b/media/libaudiohal/impl/Hal2AidlMapper.cpp
index 413a1f8..47fcd27 100644
--- a/media/libaudiohal/impl/Hal2AidlMapper.cpp
+++ b/media/libaudiohal/impl/Hal2AidlMapper.cpp
@@ -30,13 +30,11 @@
 using aidl::android::aidl_utils::statusTFromBinderStatus;
 using aidl::android::media::audio::common::AudioChannelLayout;
 using aidl::android::media::audio::common::AudioConfig;
-using aidl::android::media::audio::common::AudioConfigBase;
 using aidl::android::media::audio::common::AudioDevice;
 using aidl::android::media::audio::common::AudioDeviceAddress;
 using aidl::android::media::audio::common::AudioDeviceDescription;
 using aidl::android::media::audio::common::AudioDeviceType;
 using aidl::android::media::audio::common::AudioFormatDescription;
-using aidl::android::media::audio::common::AudioFormatType;
 using aidl::android::media::audio::common::AudioInputFlags;
 using aidl::android::media::audio::common::AudioIoFlags;
 using aidl::android::media::audio::common::AudioOutputFlags;
@@ -66,11 +64,10 @@
             portConfig.format.value() == config.base.format;
 }
 
-AudioConfig* setConfigFromPortConfig(AudioConfig* config, const AudioPortConfig& portConfig) {
+void setConfigFromPortConfig(AudioConfig* config, const AudioPortConfig& portConfig) {
     config->base.sampleRate = portConfig.sampleRate.value().value;
     config->base.channelMask = portConfig.channelMask.value();
     config->base.format = portConfig.format.value();
-    return config;
 }
 
 void setPortConfigFromConfig(AudioPortConfig* portConfig, const AudioConfig& config) {
@@ -145,11 +142,8 @@
             std::vector<int32_t>* ids, std::set<int32_t>* portIds) -> status_t {
         for (const auto& s : configs) {
             AudioPortConfig portConfig;
-            RETURN_STATUS_IF_ERROR(setPortConfig(
+            RETURN_STATUS_IF_ERROR(findOrCreatePortConfig(
                             s, destinationPortIds, &portConfig, cleanups));
-            LOG_ALWAYS_FATAL_IF(portConfig.id == 0,
-                    "fillPortConfigs: initial config: %s, port config: %s",
-                    s.toString().c_str(), portConfig.toString().c_str());
             ids->push_back(portConfig.id);
             if (portIds != nullptr) {
                 portIds->insert(portConfig.portId);
@@ -195,23 +189,30 @@
 }
 
 status_t Hal2AidlMapper::createOrUpdatePortConfig(
-        const AudioPortConfig& requestedPortConfig, AudioPortConfig* result, bool* created) {
+        const AudioPortConfig& requestedPortConfig, PortConfigs::iterator* result, bool* created) {
+    AudioPortConfig appliedPortConfig;
     bool applied = false;
     RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mModule->setAudioPortConfig(
-                            requestedPortConfig, result, &applied)));
+                            requestedPortConfig, &appliedPortConfig, &applied)));
     if (!applied) {
-        result->id = 0;
-        *created = false;
-        return OK;
+        RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mModule->setAudioPortConfig(
+                                appliedPortConfig, &appliedPortConfig, &applied)));
+        if (!applied) {
+            ALOGE("%s: module %s did not apply suggested config %s",
+                    __func__, mInstance.c_str(), appliedPortConfig.toString().c_str());
+            return NO_INIT;
+        }
     }
 
-    int32_t id = result->id;
+    int32_t id = appliedPortConfig.id;
     if (requestedPortConfig.id != 0 && requestedPortConfig.id != id) {
         LOG_ALWAYS_FATAL("%s: requested port config id %d changed to %d", __func__,
                 requestedPortConfig.id, id);
     }
 
-    auto [_, inserted] = mPortConfigs.insert_or_assign(id, *result);
+    auto [it, inserted] = mPortConfigs.insert_or_assign(std::move(id),
+            std::move(appliedPortConfig));
+    *result = it;
     *created = inserted;
     return OK;
 }
@@ -257,10 +258,11 @@
     return OK;
 }
 
-status_t Hal2AidlMapper::findOrCreateDevicePortConfig(
+status_t Hal2AidlMapper::findOrCreatePortConfig(
         const AudioDevice& device, const AudioConfig* config, AudioPortConfig* portConfig,
         bool* created) {
-    if (auto portConfigIt = findPortConfig(device); portConfigIt == mPortConfigs.end()) {
+    auto portConfigIt = findPortConfig(device);
+    if (portConfigIt == mPortConfigs.end()) {
         auto portsIt = findPort(device);
         if (portsIt == mPorts.end()) {
             ALOGE("%s: device port for device %s is not found in the module %s",
@@ -272,40 +274,24 @@
         if (config != nullptr) {
             setPortConfigFromConfig(&requestedPortConfig, *config);
         }
-        AudioPortConfig suggestedOrAppliedPortConfig;
-        RETURN_STATUS_IF_ERROR(createOrUpdatePortConfig(requestedPortConfig,
-                        &suggestedOrAppliedPortConfig, created));
-        if (suggestedOrAppliedPortConfig.id == 0) {
-            // Try again with the suggested config
-            suggestedOrAppliedPortConfig.id = requestedPortConfig.id;
-            AudioPortConfig appliedPortConfig;
-            RETURN_STATUS_IF_ERROR(createOrUpdatePortConfig(suggestedOrAppliedPortConfig,
-                            &appliedPortConfig, created));
-            if (appliedPortConfig.id == 0) {
-                ALOGE("%s: module %s did not apply suggested config %s", __func__,
-                        mInstance.c_str(), suggestedOrAppliedPortConfig.toString().c_str());
-                return NO_INIT;
-            }
-            *portConfig = appliedPortConfig;
-        } else {
-            *portConfig = suggestedOrAppliedPortConfig;
-        }
+        RETURN_STATUS_IF_ERROR(createOrUpdatePortConfig(requestedPortConfig, &portConfigIt,
+                created));
     } else {
-        *portConfig = portConfigIt->second;
         *created = false;
     }
+    *portConfig = portConfigIt->second;
     return OK;
 }
 
-status_t Hal2AidlMapper::findOrCreateMixPortConfig(
+status_t Hal2AidlMapper::findOrCreatePortConfig(
         const AudioConfig& config, const std::optional<AudioIoFlags>& flags, int32_t ioHandle,
         AudioSource source, const std::set<int32_t>& destinationPortIds,
         AudioPortConfig* portConfig, bool* created) {
     // These flags get removed one by one in this order when retrying port finding.
     static const std::vector<AudioInputFlags> kOptionalInputFlags{
         AudioInputFlags::FAST, AudioInputFlags::RAW, AudioInputFlags::VOIP_TX };
-    if (auto portConfigIt = findPortConfig(config, flags, ioHandle);
-            portConfigIt == mPortConfigs.end() && flags.has_value()) {
+    auto portConfigIt = findPortConfig(config, flags, ioHandle);
+    if (portConfigIt == mPortConfigs.end() && flags.has_value()) {
         auto optionalInputFlagsIt = kOptionalInputFlags.begin();
         AudioIoFlags matchFlags = flags.value();
         auto portsIt = findPort(config, matchFlags, destinationPortIds);
@@ -333,14 +319,14 @@
         AudioPortConfig requestedPortConfig;
         requestedPortConfig.portId = portsIt->first;
         setPortConfigFromConfig(&requestedPortConfig, config);
-        requestedPortConfig.flags = portsIt->second.flags;
         requestedPortConfig.ext = AudioPortMixExt{ .handle = ioHandle };
         if (matchFlags.getTag() == AudioIoFlags::Tag::input
                 && source != AudioSource::SYS_RESERVED_INVALID) {
             requestedPortConfig.ext.get<AudioPortExt::Tag::mix>().usecase =
                     AudioPortMixExtUseCase::make<AudioPortMixExtUseCase::Tag::source>(source);
         }
-        return createOrUpdatePortConfig(requestedPortConfig, portConfig, created);
+        RETURN_STATUS_IF_ERROR(createOrUpdatePortConfig(requestedPortConfig, &portConfigIt,
+                created));
     } else if (portConfigIt == mPortConfigs.end() && !flags.has_value()) {
         ALOGW("%s: mix port config for %s, handle %d not found in the module %s, "
                 "and was not created as flags are not specified",
@@ -357,12 +343,13 @@
         }
 
         if (requestedPortConfig != portConfigIt->second) {
-            return createOrUpdatePortConfig(requestedPortConfig, portConfig, created);
+            RETURN_STATUS_IF_ERROR(createOrUpdatePortConfig(requestedPortConfig, &portConfigIt,
+                    created));
         } else {
-            *portConfig = portConfigIt->second;
             *created = false;
         }
     }
+    *portConfig = portConfigIt->second;
     return OK;
 }
 
@@ -384,11 +371,11 @@
                 AudioPortMixExtUseCase::Tag::source ?
                 requestedPortConfig.ext.get<Tag::mix>().usecase.
                 get<AudioPortMixExtUseCase::Tag::source>() : AudioSource::SYS_RESERVED_INVALID;
-        return findOrCreateMixPortConfig(config, requestedPortConfig.flags,
+        return findOrCreatePortConfig(config, requestedPortConfig.flags,
                 requestedPortConfig.ext.get<Tag::mix>().handle, source, destinationPortIds,
                 portConfig, created);
     } else if (requestedPortConfig.ext.getTag() == Tag::device) {
-        return findOrCreateDevicePortConfig(
+        return findOrCreatePortConfig(
                 requestedPortConfig.ext.get<Tag::device>().device, nullptr /*config*/,
                 portConfig, created);
     }
@@ -397,6 +384,18 @@
     return BAD_VALUE;
 }
 
+status_t Hal2AidlMapper::findOrCreatePortConfig(
+        const AudioPortConfig& requestedPortConfig, const std::set<int32_t>& destinationPortIds,
+        AudioPortConfig* portConfig, Cleanups* cleanups) {
+    bool created = false;
+    RETURN_STATUS_IF_ERROR(findOrCreatePortConfig(
+                    requestedPortConfig, destinationPortIds, portConfig, &created));
+    if (created && cleanups != nullptr) {
+        cleanups->add(&Hal2AidlMapper::resetPortConfig, portConfig->id);
+    }
+    return OK;
+}
+
 status_t Hal2AidlMapper::findPortConfig(const AudioDevice& device, AudioPortConfig* portConfig) {
     if (auto it = findPortConfig(device); it != mPortConfigs.end()) {
         *portConfig = it->second;
@@ -676,56 +675,21 @@
             config->toString().c_str(), mixPortConfig->toString().c_str());
     resetUnusedPatchesAndPortConfigs();
     const bool isInput = flags.getTag() == AudioIoFlags::Tag::input;
-    const AudioConfig initialConfig = *config;
     // Find / create AudioPortConfigs for the device port and the mix port,
     // then find / create a patch between them, and open a stream on the mix port.
     AudioPortConfig devicePortConfig;
     bool created = false;
-    RETURN_STATUS_IF_ERROR(findOrCreateDevicePortConfig(device, config,
-                    &devicePortConfig, &created));
-    LOG_ALWAYS_FATAL_IF(devicePortConfig.id == 0);
+    RETURN_STATUS_IF_ERROR(findOrCreatePortConfig(device, config,
+                                                  &devicePortConfig, &created));
     if (created) {
         cleanups->add(&Hal2AidlMapper::resetPortConfig, devicePortConfig.id);
     }
-    RETURN_STATUS_IF_ERROR(findOrCreateMixPortConfig(*config, flags, ioHandle, source,
+    RETURN_STATUS_IF_ERROR(findOrCreatePortConfig(*config, flags, ioHandle, source,
                     std::set<int32_t>{devicePortConfig.portId}, mixPortConfig, &created));
     if (created) {
         cleanups->add(&Hal2AidlMapper::resetPortConfig, mixPortConfig->id);
     }
     setConfigFromPortConfig(config, *mixPortConfig);
-    bool retryWithSuggestedConfig = false;   // By default, let the framework to retry.
-    if (mixPortConfig->id == 0 && config->base == AudioConfigBase{}) {
-        // The HAL proposes a default config, can retry here.
-        retryWithSuggestedConfig = true;
-    } else if (isInput && config->base != initialConfig.base) {
-        // If the resulting config is different, we must stop and provide the config to the
-        // framework so that it can retry.
-        mixPortConfig->id = 0;
-    } else if (!isInput && mixPortConfig->id == 0 &&
-                    (initialConfig.base.format.type == AudioFormatType::PCM ||
-                            !isBitPositionFlagSet(flags.get<AudioIoFlags::output>(),
-                                    AudioOutputFlags::DIRECT) ||
-                            isBitPositionFlagSet(flags.get<AudioIoFlags::output>(),
-                                    AudioOutputFlags::COMPRESS_OFFLOAD))) {
-        // The framework does not retry opening non-direct PCM and IEC61937 outputs, need to retry
-        // here (see 'AudioHwDevice::openOutputStream').
-        retryWithSuggestedConfig = true;
-    }
-    if (mixPortConfig->id == 0 && retryWithSuggestedConfig) {
-        ALOGD("%s: retrying to find/create a mix port config using config %s", __func__,
-                config->toString().c_str());
-        RETURN_STATUS_IF_ERROR(findOrCreateMixPortConfig(*config, flags, ioHandle, source,
-                        std::set<int32_t>{devicePortConfig.portId}, mixPortConfig, &created));
-        if (created) {
-            cleanups->add(&Hal2AidlMapper::resetPortConfig, mixPortConfig->id);
-        }
-        setConfigFromPortConfig(config, *mixPortConfig);
-    }
-    if (mixPortConfig->id == 0) {
-        ALOGD("%p %s: returning suggested config for the stream: %s", this, __func__,
-                config->toString().c_str());
-        return OK;
-    }
     if (isInput) {
         RETURN_STATUS_IF_ERROR(findOrCreatePatch(
                         {devicePortConfig.id}, {mixPortConfig->id}, patch, &created));
@@ -742,18 +706,6 @@
     return OK;
 }
 
-status_t Hal2AidlMapper::setPortConfig(
-        const AudioPortConfig& requestedPortConfig, const std::set<int32_t>& destinationPortIds,
-        AudioPortConfig* portConfig, Cleanups* cleanups) {
-    bool created = false;
-    RETURN_STATUS_IF_ERROR(findOrCreatePortConfig(
-                    requestedPortConfig, destinationPortIds, portConfig, &created));
-    if (created && cleanups != nullptr) {
-        cleanups->add(&Hal2AidlMapper::resetPortConfig, portConfig->id);
-    }
-    return OK;
-}
-
 status_t Hal2AidlMapper::releaseAudioPatch(int32_t patchId) {
     return releaseAudioPatches({patchId});
 }
diff --git a/media/libaudiohal/impl/Hal2AidlMapper.h b/media/libaudiohal/impl/Hal2AidlMapper.h
index ee55b22..70a2bd7 100644
--- a/media/libaudiohal/impl/Hal2AidlMapper.h
+++ b/media/libaudiohal/impl/Hal2AidlMapper.h
@@ -36,11 +36,6 @@
 // structures directly. Mapper does the job of translating the "legacy" way of identifying ports
 // and port configs (by device addresses and I/O handles) into AIDL IDs. Once the framework will
 // be updated to provide these IDs directly to libaudiohal, the need for the mapper will cease.
-//
-// Note that unlike DeviceHalInterface, which sometimes allows a method to return an error,
-// but still consider some of the outputs to be valid (for example, in 'open{Input|Output}Stream'),
-// 'Hal2AidlMapper' follows the Binder convention. It means that if a method returns an error,
-// the outputs may not be initialized at all and should not be considered by the caller.
 class Hal2AidlMapper {
   public:
     using Cleanups = Cleanups<Hal2AidlMapper>;
@@ -54,6 +49,27 @@
             const std::vector<::aidl::android::media::audio::common::AudioPortConfig>& sources,
             const std::vector<::aidl::android::media::audio::common::AudioPortConfig>& sinks,
             int32_t* patchId, Cleanups* cleanups);
+    status_t findOrCreatePortConfig(
+            const ::aidl::android::media::audio::common::AudioDevice& device,
+            const ::aidl::android::media::audio::common::AudioConfig* config,
+            ::aidl::android::media::audio::common::AudioPortConfig* portConfig,
+            bool* created);
+    status_t findOrCreatePortConfig(
+            const ::aidl::android::media::audio::common::AudioConfig& config,
+            const std::optional<::aidl::android::media::audio::common::AudioIoFlags>& flags,
+            int32_t ioHandle,
+            ::aidl::android::media::audio::common::AudioSource source,
+            const std::set<int32_t>& destinationPortIds,
+            ::aidl::android::media::audio::common::AudioPortConfig* portConfig, bool* created);
+    status_t findOrCreatePortConfig(
+        const ::aidl::android::media::audio::common::AudioPortConfig& requestedPortConfig,
+        const std::set<int32_t>& destinationPortIds,
+        ::aidl::android::media::audio::common::AudioPortConfig* portConfig, bool* created);
+    status_t findOrCreatePortConfig(
+        const ::aidl::android::media::audio::common::AudioPortConfig& requestedPortConfig,
+        const std::set<int32_t>& destinationPortIds,
+        ::aidl::android::media::audio::common::AudioPortConfig* portConfig,
+        Cleanups* cleanups = nullptr);
     status_t findPortConfig(
             const ::aidl::android::media::audio::common::AudioDevice& device,
             ::aidl::android::media::audio::common::AudioPortConfig* portConfig);
@@ -72,8 +88,6 @@
         return ::aidl::android::convertContainer(mRoutes, routes, converter);
     }
     status_t initialize();
-    // If the resulting 'mixPortConfig->id' is 0, that means the stream was not created,
-    // and 'config' is a suggested config.
     status_t prepareToOpenStream(
         int32_t ioHandle,
         const ::aidl::android::media::audio::common::AudioDevice& device,
@@ -83,11 +97,6 @@
         ::aidl::android::media::audio::common::AudioConfig* config,
         ::aidl::android::media::audio::common::AudioPortConfig* mixPortConfig,
         ::aidl::android::hardware::audio::core::AudioPatch* patch);
-    status_t setPortConfig(
-        const ::aidl::android::media::audio::common::AudioPortConfig& requestedPortConfig,
-        const std::set<int32_t>& destinationPortIds,
-        ::aidl::android::media::audio::common::AudioPortConfig* portConfig,
-        Cleanups* cleanups = nullptr);
     status_t releaseAudioPatch(int32_t patchId);
     void resetUnusedPatchesAndPortConfigs();
     status_t setDevicePortConnectedState(
@@ -119,11 +128,9 @@
             const ::aidl::android::media::audio::common::AudioPort& p);
     bool audioDeviceMatches(const ::aidl::android::media::audio::common::AudioDevice& device,
             const ::aidl::android::media::audio::common::AudioPortConfig& p);
-    // If the 'result->id' is 0, that means, the config was not created/updated,
-    // and the 'result' is a suggestion from the HAL.
     status_t createOrUpdatePortConfig(
             const ::aidl::android::media::audio::common::AudioPortConfig& requestedPortConfig,
-            ::aidl::android::media::audio::common::AudioPortConfig* result, bool *created);
+            PortConfigs::iterator* result, bool *created);
     void eraseConnectedPort(int32_t portId);
     status_t findOrCreatePatch(
         const std::set<int32_t>& sourcePortConfigIds,
@@ -132,24 +139,6 @@
     status_t findOrCreatePatch(
         const ::aidl::android::hardware::audio::core::AudioPatch& requestedPatch,
         ::aidl::android::hardware::audio::core::AudioPatch* patch, bool* created);
-    status_t findOrCreateDevicePortConfig(
-            const ::aidl::android::media::audio::common::AudioDevice& device,
-            const ::aidl::android::media::audio::common::AudioConfig* config,
-            ::aidl::android::media::audio::common::AudioPortConfig* portConfig,
-            bool* created);
-    // If the resulting 'portConfig->id' is 0, that means the config was not created,
-    // and 'portConfig' is a suggested config.
-    status_t findOrCreateMixPortConfig(
-            const ::aidl::android::media::audio::common::AudioConfig& config,
-            const std::optional<::aidl::android::media::audio::common::AudioIoFlags>& flags,
-            int32_t ioHandle,
-            ::aidl::android::media::audio::common::AudioSource source,
-            const std::set<int32_t>& destinationPortIds,
-            ::aidl::android::media::audio::common::AudioPortConfig* portConfig, bool* created);
-    status_t findOrCreatePortConfig(
-        const ::aidl::android::media::audio::common::AudioPortConfig& requestedPortConfig,
-        const std::set<int32_t>& destinationPortIds,
-        ::aidl::android::media::audio::common::AudioPortConfig* portConfig, bool* created);
     Patches::iterator findPatch(const std::set<int32_t>& sourcePortConfigIds,
             const std::set<int32_t>& sinkPortConfigIds);
     Ports::iterator findPort(const ::aidl::android::media::audio::common::AudioDevice& device);