Merge changes from topic "effect_proxy_align" into main

* changes:
  EffectProxy: remove the AUXILIARY flag copy
  Align AIDL effect proxy behavior with HIDL

4 files changed, 27 insertions, 45 deletions

diff --git a/media/libaudiohal/impl/EffectProxy.cpp b/media/libaudiohal/impl/EffectProxy.cpp
index 9aa02e2fc1..fb4658f19d 100644
--- a/media/libaudiohal/impl/EffectProxy.cpp
+++ b/media/libaudiohal/impl/EffectProxy.cpp
@@ -82,8 +82,7 @@ ndk::ScopedAStatus EffectProxy::destroy() {
 ndk::ScopedAStatus EffectProxy::setOffloadParam(const effect_offload_param_t* offload) {
     const auto& itor = std::find_if(mSubEffects.begin(), mSubEffects.end(), [&](const auto& sub) {
         const auto& desc = sub.descriptor;
-        return offload->isOffload ==
-               (desc.common.flags.hwAcceleratorMode == Flags::HardwareAccelerator::TUNNEL);
+        return offload->isOffload == desc.common.flags.offloadIndication;
     });
     if (itor == mSubEffects.end()) {
         ALOGE("%s no %soffload sub-effect found", __func__, offload->isOffload ? "" : "non-");
@@ -93,7 +92,7 @@ ndk::ScopedAStatus EffectProxy::setOffloadParam(const effect_offload_param_t* of
     }
 
     mActiveSubIdx = std::distance(mSubEffects.begin(), itor);
-    ALOGI("%s: active %soffload sub-effect %zu descriptor: %s", __func__,
+    ALOGI("%s: active %soffload sub-effect %zu: %s", __func__,
           offload->isOffload ? "" : "non-", mActiveSubIdx,
           ::android::audio::utils::toString(mSubEffects[mActiveSubIdx].descriptor.common.id.uuid)
                   .c_str());
@@ -163,7 +162,7 @@ ndk::ScopedAStatus EffectProxy::close() {
 
 ndk::ScopedAStatus EffectProxy::getDescriptor(Descriptor* desc) {
     *desc = mSubEffects[mActiveSubIdx].descriptor;
-    desc->common.id.uuid = desc->common.id.proxy.value();
+    desc->common = mDescriptorCommon;
     return ndk::ScopedAStatus::ok();
 }
 
@@ -185,42 +184,35 @@ ndk::ScopedAStatus EffectProxy::buildDescriptor(const AudioUuid& uuid,
     return ndk::ScopedAStatus::ok();
 }
 
+// Sub-effects are required to have identical features, so here we return the SW sub-effect
+// descriptor, with the implementation UUID replaced with proxy UUID, and flags setting respect all
+// sub-effects.
 Descriptor::Common EffectProxy::buildDescriptorCommon(
         const AudioUuid& uuid, const std::vector<Descriptor>& subEffectDescs) {
-    // initial flag values before we know which sub-effect to active (with setOffloadParam)
-    // align to HIDL EffectProxy flags
-    Descriptor::Common common = {.flags = {.type = Flags::Type::INSERT,
-                                           .insert = Flags::Insert::LAST,
-                                           .volume = Flags::Volume::CTRL}};
-
+    Descriptor::Common swCommon;
+    const Flags& firstFlag = subEffectDescs[0].common.flags;
+    bool offloadExist = false;
     for (const auto& desc : subEffectDescs) {
-        if (desc.common.flags.hwAcceleratorMode == Flags::HardwareAccelerator::TUNNEL) {
-            common.flags.hwAcceleratorMode = Flags::HardwareAccelerator::TUNNEL;
-        }
-
-        // set indication if any sub-effect indication was set
-        common.flags.offloadIndication |= desc.common.flags.offloadIndication;
-        common.flags.deviceIndication |= desc.common.flags.deviceIndication;
-        common.flags.audioModeIndication |= desc.common.flags.audioModeIndication;
-        common.flags.audioSourceIndication |= desc.common.flags.audioSourceIndication;
-        // Set to NONE if any sub-effect not supporting any Volume command
-        if (desc.common.flags.volume == Flags::Volume::NONE) {
-            common.flags.volume = Flags::Volume::NONE;
+        if (desc.common.flags.offloadIndication) {
+            offloadExist = true;
+        } else {
+            swCommon = desc.common;
         }
-        // set to AUXILIARY if any sub-effect is of AUXILIARY type
-        if (desc.common.flags.type == Flags::Type::AUXILIARY) {
-            common.flags.type = Flags::Type::AUXILIARY;
+        if (desc.common.flags.audioModeIndication != firstFlag.audioModeIndication ||
+            desc.common.flags.audioSourceIndication != firstFlag.audioSourceIndication ||
+            desc.common.flags.sinkMetadataIndication != firstFlag.sinkMetadataIndication ||
+            desc.common.flags.sourceMetadataIndication != firstFlag.sourceMetadataIndication ||
+            desc.common.flags.deviceIndication != firstFlag.deviceIndication) {
+            ALOGW("Inconsistent flags %s vs %s", desc.common.flags.toString().c_str(),
+                  firstFlag.toString().c_str());
         }
     }
 
-    // copy type UUID from any of sub-effects, all sub-effects should have same type
-    common.id.type = subEffectDescs[0].common.id.type;
+    swCommon.flags.offloadIndication = offloadExist;
     // replace implementation UUID with proxy UUID.
-    common.id.uuid = uuid;
-    common.id.proxy = std::nullopt;
-    common.name = "Proxy";
-    common.implementor = "AOSP";
-    return common;
+    swCommon.id.uuid = uuid;
+    swCommon.id.proxy = std::nullopt;
+    return swCommon;
 }
 
 // Handle with active sub-effect first, only send to other sub-effects when success
diff --git a/media/libaudiohal/impl/EffectsFactoryHalAidl.cpp b/media/libaudiohal/impl/EffectsFactoryHalAidl.cpp
index 3b2f344fe8..64cc7ed3d8 100644
--- a/media/libaudiohal/impl/EffectsFactoryHalAidl.cpp
+++ b/media/libaudiohal/impl/EffectsFactoryHalAidl.cpp
@@ -188,7 +188,6 @@ status_t EffectsFactoryHalAidl::createEffect(const effect_uuid_t* uuid, int32_t
         aidlEffect = ndk::SharedRefBase::make<EffectProxy>(
                 aidlUuid, mProxyUuidDescriptorMap.at(aidlUuid) /* sub-effect descriptor list */,
                 mFactory);
-        mProxyList.emplace_back(std::static_pointer_cast<EffectProxy>(aidlEffect));
     } else {
         RETURN_STATUS_IF_ERROR(
                 statusTFromBinderStatus(mFactory->createEffect(aidlUuid, &aidlEffect)));
@@ -205,15 +204,9 @@ status_t EffectsFactoryHalAidl::createEffect(const effect_uuid_t* uuid, int32_t
 }
 
 status_t EffectsFactoryHalAidl::dumpEffects(int fd) {
-    status_t ret = OK;
-    // record the error ret and continue dump as many effects as possible
-    for (const auto& proxy : mProxyList) {
-        if (status_t temp = BAD_VALUE; proxy && (temp = proxy->dump(fd, nullptr, 0)) != OK) {
-            ret = temp;
-        }
-    }
+    // TODO: b/333803769 improve the effect dump implementation
     RETURN_STATUS_IF_ERROR(mFactory->dump(fd, nullptr, 0));
-    return ret;
+    return OK;
 }
 
 status_t EffectsFactoryHalAidl::allocateBuffer(size_t size, sp<EffectBufferHalInterface>* buffer) {
diff --git a/media/libaudiohal/impl/EffectsFactoryHalAidl.h b/media/libaudiohal/impl/EffectsFactoryHalAidl.h
index 73089b0088..3b8628c8fd 100644
--- a/media/libaudiohal/impl/EffectsFactoryHalAidl.h
+++ b/media/libaudiohal/impl/EffectsFactoryHalAidl.h
@@ -84,9 +84,6 @@ class EffectsFactoryHalAidl final : public EffectsFactoryHalInterface {
     // Query result of pre and post processing from effect factory
     const std::vector<Processing> mAidlProcessings;
 
-    // list of the EffectProxy instances
-    std::list<std::shared_ptr<EffectProxy>> mProxyList;
-
     virtual ~EffectsFactoryHalAidl() = default;
     status_t getHalDescriptorWithImplUuid(
             const ::aidl::android::media::audio::common::AudioUuid& uuid,
diff --git a/media/libeffects/visualizer/aidl/Visualizer.cpp b/media/libeffects/visualizer/aidl/Visualizer.cpp
index 9c2b71ef13..9b493d43a8 100644
--- a/media/libeffects/visualizer/aidl/Visualizer.cpp
+++ b/media/libeffects/visualizer/aidl/Visualizer.cpp
@@ -72,7 +72,7 @@ const Descriptor VisualizerImpl::kDescriptor = {
                           .uuid = getEffectImplUuidVisualizer(),
                           .proxy = std::nullopt},
                    .flags = {.type = Flags::Type::INSERT,
-                             .insert = Flags::Insert::LAST,
+                             .insert = Flags::Insert::FIRST,
                              .volume = Flags::Volume::NONE},
                    .name = VisualizerImpl::kEffectName,
                    .implementor = "The Android Open Source Project"},