Merge changes from topic "aidl_audio_effect_2"

* changes:
  AIDL effect: Refine some implementation and test logic.
  AIDL effect: Initial IEffect interface implementation and vts test
  AIDL effect: Initial IEffect interface definition

audio/aidl/Android.bp

@@ -148,6 +148,7 @@ aidl_interface {
     vendor_available: true,
     srcs: [
         "android/hardware/audio/effect/Descriptor.aidl",
+        "android/hardware/audio/effect/IEffect.aidl",
         "android/hardware/audio/effect/IFactory.aidl",
     ],
     imports: [
@@ -165,3 +166,19 @@ aidl_interface {
         },
     },
 }
+
+latest_android_hardware_audio_effect = "android.hardware.audio.effect-V1"
+
+cc_defaults {
+    name: "latest_android_hardware_audio_effect_ndk_shared",
+    shared_libs: [
+        latest_android_hardware_audio_effect + "-ndk",
+    ],
+}
+
+cc_defaults {
+    name: "latest_android_hardware_audio_effect_ndk_static",
+    static_libs: [
+        latest_android_hardware_audio_effect + "-ndk",
+    ],
+}

audio/aidl/aidl_api/android.hardware.audio.effect/current/android/hardware/audio/effect/IEffect.aidl

@@ -0,0 +1,40 @@
+/*
+ * Copyright (C) 2022 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+///////////////////////////////////////////////////////////////////////////////
+// THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE.                          //
+///////////////////////////////////////////////////////////////////////////////
+
+// This file is a snapshot of an AIDL file. Do not edit it manually. There are
+// two cases:
+// 1). this is a frozen version file - do not edit this in any case.
+// 2). this is a 'current' file. If you make a backwards compatible change to
+//     the interface (from the latest frozen version), the build system will
+//     prompt you to update this file with `m <name>-update-api`.
+//
+// You must not make a backward incompatible change to any AIDL file built
+// with the aidl_interface module type with versions property set. The module
+// type is used to build AIDL files in a way that they can be used across
+// independently updatable components of the system. If a device is shipped
+// with such a backward incompatible change, it has a high risk of breaking
+// later when a module using the interface is updated, e.g., Mainline modules.
+
+package android.hardware.audio.effect;
+@VintfStability
+interface IEffect {
+  void open();
+  void close();
+  android.hardware.audio.effect.Descriptor getDescriptor();
+}

audio/aidl/aidl_api/android.hardware.audio.effect/current/android/hardware/audio/effect/IFactory.aidl

@@ -35,4 +35,6 @@ package android.hardware.audio.effect;
 @VintfStability
 interface IFactory {
   android.hardware.audio.effect.Descriptor.Identity[] queryEffects(in @nullable android.media.audio.common.AudioUuid type, in @nullable android.media.audio.common.AudioUuid implementation);
+  android.hardware.audio.effect.IEffect createEffect(in android.media.audio.common.AudioUuid implUuid);
+  void destroyEffect(in android.hardware.audio.effect.IEffect handle);
 }

audio/aidl/android/hardware/audio/effect/IEffect.aidl

@@ -0,0 +1,65 @@
+/*
+ * Copyright (C) 2022 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package android.hardware.audio.effect;
+
+import android.hardware.audio.effect.Descriptor;
+
+/**
+ * Effect interfaces definitions to configure and control the effect instance.
+ */
+@VintfStability
+interface IEffect {
+    /**
+     * Open an effect instance, effect should not start processing data before receive START
+     * command. All necessary information should be allocated and instance should transfer to IDLE
+     * state after open() call has been handled successfully.
+     * After open, the effect instance should be able to handle all IEffect interface calls.
+     *
+     * @throws a EX_UNSUPPORTED_OPERATION if device capability/resource is not enough or system
+     *         failure happens.
+     * @note Open an already-opened effect instance should do nothing and should not throw an error.
+     */
+    void open();
+
+    /**
+     * Called by the client to close the effect instance, processing thread should be destroyed and
+     * consume no CPU after close.
+     *
+     * It is recommended to close the effect on the client side as soon as it becomes unused, it's
+     * client responsibility to make sure all parameter/buffer is correct if client wants to reopen
+     * a closed instance.
+     *
+     * Effect instance close interface should always succeed unless:
+     * 1. The effect instance is not in a proper state to be closed, for example it's still in
+     * processing state.
+     * 2. There is system/hardware related failure when close.
+     *
+     * @throws EX_ILLEGAL_STATE if the effect instance is not in a proper state to be closed.
+     * @throws EX_UNSUPPORTED_OPERATION if the effect instance failed to close for any other reason.
+     * @note Close an already-closed effect should do nothing and should not throw an error.
+     */
+    void close();
+
+    /**
+     * Return the @c Descriptor of this effect instance.
+     *
+     * Must be available for the effect instance at anytime and should always succeed.
+     *
+     * @return Descriptor The @c Descriptor of this effect instance.
+     */
+    Descriptor getDescriptor();
+}

audio/aidl/android/hardware/audio/effect/IFactory.aidl

@@ -17,6 +17,7 @@
 package android.hardware.audio.effect;
 
 import android.hardware.audio.effect.Descriptor;
+import android.hardware.audio.effect.IEffect;
 import android.media.audio.common.AudioUuid;
 
 /**
@@ -42,4 +43,30 @@ interface IFactory {
      */
     Descriptor.Identity[] queryEffects(
             in @nullable AudioUuid type, in @nullable AudioUuid implementation);
+
+    /**
+     * Called by the audio framework to create the effect (identified by the implementation UUID
+     * parameter).
+     *
+     * The effect instance should be able to maintain its own context and parameters after creation.
+     *
+     * @param implUuid UUID for the effect implementation which instance will be created based on.
+     * @return The effect instance handle created.
+     * @throws EX_ILLEGAL_ARGUMENT if the implUuid is not valid.
+     * @throws EX_TRANSACTION_FAILED if device capability/resource is not enough to create the
+     *         effect instance.
+     */
+    IEffect createEffect(in AudioUuid implUuid);
+
+    /**
+     * Called by the framework to destroy the effect and free up all currently allocated resources.
+     * It is recommended to destroy the effect from the client side as soon as it is becomes unused.
+     *
+     * The client must ensure effect instance is closed before destroy.
+     *
+     * @param handle The handle of effect instance to be destroyed.
+     * @throws EX_ILLEGAL_ARGUMENT if the effect handle is not valid.
+     * @throws EX_ILLEGAL_STATE if the effect instance is not in a proper state to be destroyed.
+     */
+    void destroyEffect(in IEffect handle);
 }

audio/aidl/default/Android.bp

@@ -68,6 +68,7 @@ cc_defaults {
         "libbase",
         "libbinder_ndk",
         "android.hardware.audio.effect-V1-ndk",
+        "libequalizer",
     ],
     cflags: [
         "-Wall",
@@ -80,7 +81,10 @@ cc_defaults {
 cc_library_static {
     name: "libaudioeffectserviceexampleimpl",
     defaults: ["aidlaudioeffectservice_defaults"],
-    export_include_dirs: ["include"],
+    export_include_dirs: [
+        "include",
+        "include/equalizer-impl/",
+    ],
     srcs: [
         "EffectFactory.cpp",
     ],

audio/aidl/default/EffectFactory.cpp

@@ -29,19 +29,8 @@ Factory::Factory() {
     // TODO: implement this with xml parser on audio_effect.xml, and filter with optional
     // parameters.
     Descriptor::Identity id;
-    id.type = {static_cast<int32_t>(0x0bed4300),
-               0xddd6,
-               0x11db,
-               0x8f34,
-               {0x00, 0x02, 0xa5, 0xd5, 0xc5, 0x1b}};
-    id.uuid = EqualizerUUID;
-    mIdentityList.push_back(id);
-    id.type = {static_cast<int32_t>(0xd3467faa),
-               0xacc7,
-               0x4d34,
-               0xacaf,
-               {0x00, 0x02, 0xa5, 0xd5, 0xc5, 0x1b}};
-    id.uuid = VisualizerUUID;
+    id.type = EqualizerTypeUUID;
+    id.uuid = EqualizerSwImplUUID;
     mIdentityList.push_back(id);
 }
 
@@ -56,4 +45,28 @@ ndk::ScopedAStatus Factory::queryEffects(const std::optional<AudioUuid>& in_type
     return ndk::ScopedAStatus::ok();
 }
 
+ndk::ScopedAStatus Factory::createEffect(
+        const AudioUuid& in_impl_uuid,
+        std::shared_ptr<::aidl::android::hardware::audio::effect::IEffect>* _aidl_return) {
+    LOG(DEBUG) << __func__ << ": UUID " << in_impl_uuid.toString();
+    if (in_impl_uuid == EqualizerSwImplUUID) {
+        *_aidl_return = ndk::SharedRefBase::make<Equalizer>();
+    } else {
+        LOG(ERROR) << __func__ << ": UUID "
+                   << " not supported";
+        return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT);
+    }
+    return ndk::ScopedAStatus::ok();
+}
+
+ndk::ScopedAStatus Factory::destroyEffect(
+        const std::shared_ptr<::aidl::android::hardware::audio::effect::IEffect>& in_handle) {
+    if (in_handle) {
+        // TODO: b/245393900 need check the instance state with IEffect.getState before destroy.
+        return ndk::ScopedAStatus::ok();
+    } else {
+        return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT);
+    }
+}
+
 }  // namespace aidl::android::hardware::audio::effect

audio/aidl/default/EffectMain.cpp

@@ -23,10 +23,11 @@
 int main() {
     // This is a debug implementation, always enable debug logging.
     android::base::SetMinimumLogSeverity(::android::base::DEBUG);
-    ABinderProcess_setThreadPoolMaxThreadCount(16);
+    ABinderProcess_setThreadPoolMaxThreadCount(0);
 
     auto effectFactory =
             ndk::SharedRefBase::make<aidl::android::hardware::audio::effect::Factory>();
+
     std::string serviceName = std::string() + effectFactory->descriptor + "/default";
     binder_status_t status =
             AServiceManager_addService(effectFactory->asBinder().get(), serviceName.c_str());

audio/aidl/default/android.hardware.audio.effect.service-aidl.example.rc

@@ -2,7 +2,7 @@ service vendor.audio-effect-hal-aidl /vendor/bin/hw/android.hardware.audio.effec
     class hal
     user audioserver
     # media gid needed for /dev/fm (radio) and for /data/misc/media (tee)
-    group audio camera drmrpc inet media mediadrm net_bt net_bt_admin net_bw_acct wakelock context_hub
+    group audio media
     capabilities BLOCK_SUSPEND
     ioprio rt 4
     task_profiles ProcessCapacityHigh HighPerformance

audio/aidl/default/equalizer/Android.bp

@@ -0,0 +1,45 @@
+/*
+ * Copyright (C) 2022 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package {
+    // See: http://go/android-license-faq
+    // A large-scale-change added 'default_applicable_licenses' to import
+    // all of the 'license_kinds' from "hardware_interfaces_license"
+    // to get the below license kinds:
+    //   SPDX-license-identifier-Apache-2.0
+    default_applicable_licenses: ["hardware_interfaces_license"],
+}
+
+cc_library_shared {
+    name: "libequalizer",
+    vendor: true,
+    shared_libs: [
+        "libbase",
+        "libbinder_ndk",
+        "libstagefright_foundation",
+    ],
+    defaults: [
+        "latest_android_media_audio_common_types_ndk_shared",
+        "latest_android_hardware_audio_effect_ndk_shared",
+    ],
+    include_dirs: ["hardware/interfaces/audio/aidl/default/include/equalizer-impl"],
+    srcs: [
+        "Equalizer.cpp",
+    ],
+    visibility: [
+        "//hardware/interfaces/audio/aidl/default",
+    ],
+}

audio/aidl/default/equalizer/Equalizer.cpp

@@ -0,0 +1,40 @@
+/*
+ * Copyright (C) 2022 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#define LOG_TAG "AHAL_Equalizer"
+#include <android-base/logging.h>
+
+#include "Equalizer.h"
+
+namespace aidl::android::hardware::audio::effect {
+
+ndk::ScopedAStatus Equalizer::open() {
+    LOG(DEBUG) << __func__;
+    return ndk::ScopedAStatus::ok();
+}
+
+ndk::ScopedAStatus Equalizer::close() {
+    LOG(DEBUG) << __func__;
+    return ndk::ScopedAStatus::ok();
+}
+
+ndk::ScopedAStatus Equalizer::getDescriptor(Descriptor* _aidl_return) {
+    LOG(DEBUG) << __func__ << "descriptor " << mDesc.toString();
+    *_aidl_return = mDesc;
+    return ndk::ScopedAStatus::ok();
+}
+
+}  // namespace aidl::android::hardware::audio::effect

audio/aidl/default/include/effectFactory-impl/EffectFactory.h

@@ -40,6 +40,28 @@ class Factory : public BnFactory {
             const std::optional<::aidl::android::media::audio::common::AudioUuid>& in_instance,
             std::vector<Descriptor::Identity>* out_descriptor) override;
 
+    /**
+     * @brief Create an effect instance for a certain implementation (identified by UUID).
+     *
+     * @param in_impl_uuid Effect implementation UUID.
+     * @param _aidl_return A pointer to created effect instance.
+     * @return ndk::ScopedAStatus
+     */
+    ndk::ScopedAStatus createEffect(
+            const ::aidl::android::media::audio::common::AudioUuid& in_impl_uuid,
+            std::shared_ptr<::aidl::android::hardware::audio::effect::IEffect>* _aidl_return)
+            override;
+
+    /**
+     * @brief Destroy an effect instance.
+     *
+     * @param in_handle Effect instance handle.
+     * @return ndk::ScopedAStatus
+     */
+    ndk::ScopedAStatus destroyEffect(
+            const std::shared_ptr<::aidl::android::hardware::audio::effect::IEffect>& in_handle)
+            override;
+
   private:
     // List of effect descriptors supported by the devices.
     std::vector<Descriptor::Identity> mIdentityList;

audio/aidl/default/include/equalizer-impl/Equalizer.h

@@ -16,16 +16,36 @@
 
 #pragma once
 
+#include <aidl/android/hardware/audio/effect/BnEffect.h>
 #include <cstdlib>
 
 namespace aidl::android::hardware::audio::effect {
 
+// Equalizer type UUID.
+static const ::aidl::android::media::audio::common::AudioUuid EqualizerTypeUUID = {
+        static_cast<int32_t>(0x0bed4300),
+        0xddd6,
+        0x11db,
+        0x8f34,
+        {0x00, 0x02, 0xa5, 0xd5, 0xc5, 0x1b}};
+
 // Equalizer implementation UUID.
-static const ::aidl::android::media::audio::common::AudioUuid EqualizerUUID = {
-        static_cast<int32_t>(0xce772f20),
+static const ::aidl::android::media::audio::common::AudioUuid EqualizerSwImplUUID = {
+        static_cast<int32_t>(0x0bed4300),
         0x847d,
         0x11df,
         0xbb17,
         {0x00, 0x02, 0xa5, 0xd5, 0xc5, 0x1b}};
 
-}  // namespace aidl::android::hardware::audio::effect
+class Equalizer : public BnEffect {
+  public:
+    Equalizer() = default;
+    ndk::ScopedAStatus open() override;
+    ndk::ScopedAStatus close() override;
+    ndk::ScopedAStatus getDescriptor(Descriptor* _aidl_return) override;
+
+  private:
+    // Effect descriptor.
+    Descriptor mDesc = {.common = {.id = {.type = EqualizerTypeUUID, .uuid = EqualizerSwImplUUID}}};
+};
+}  // namespace aidl::android::hardware::audio::effect

audio/aidl/vts/VtsHalAudioEffectTargetTest.cpp

@@ -14,7 +14,11 @@
  * limitations under the License.
  */
 
+#include <memory>
 #include <string>
+#include <unordered_map>
+#include <unordered_set>
+#include <vector>
 
 #define LOG_TAG "VtsHalAudioEffect"
 
@@ -29,43 +33,88 @@
 #include <aidl/android/hardware/audio/effect/IFactory.h>
 
 #include "AudioHalBinderServiceUtil.h"
+#include "TestUtils.h"
 
 using namespace android;
 
 using ndk::ScopedAStatus;
 
 using aidl::android::hardware::audio::effect::Descriptor;
+using aidl::android::hardware::audio::effect::IEffect;
 using aidl::android::hardware::audio::effect::IFactory;
 using aidl::android::media::audio::common::AudioUuid;
 
-namespace ndk {
-std::ostream& operator<<(std::ostream& str, const ScopedAStatus& status) {
-    str << status.getDescription();
-    return str;
-}
-}  // namespace ndk
-
-class EffectFactory : public testing::TestWithParam<std::string> {
+class EffectFactoryHelper {
   public:
-    void SetUp() override { ASSERT_NO_FATAL_FAILURE(ConnectToService()); }
+    explicit EffectFactoryHelper(const std::string& name) : mServiceName(name) {}
 
-    void TearDown() override {}
-
-    void ConnectToService() {
-        serviceName = GetParam();
-        factory = IFactory::fromBinder(binderUtil.connectToService(serviceName));
-        ASSERT_NE(factory, nullptr);
+    void ConnectToFactoryService() {
+        mEffectFactory = IFactory::fromBinder(binderUtil.connectToService(mServiceName));
+        ASSERT_NE(mEffectFactory, nullptr);
     }
 
-    void RestartService() {
-        ASSERT_NE(factory, nullptr);
-        factory = IFactory::fromBinder(binderUtil.restartService());
-        ASSERT_NE(factory, nullptr);
+    void RestartFactoryService() {
+        ASSERT_NE(mEffectFactory, nullptr);
+        mEffectFactory = IFactory::fromBinder(binderUtil.restartService());
+        ASSERT_NE(mEffectFactory, nullptr);
     }
 
-    std::shared_ptr<IFactory> factory;
-    std::string serviceName;
+    void QueryAllEffects() {
+        EXPECT_NE(mEffectFactory, nullptr);
+        EXPECT_IS_OK(mEffectFactory->queryEffects(std::nullopt, std::nullopt, &mCompleteIds));
+    }
+
+    void QueryEffects(const std::optional<AudioUuid>& in_type,
+                      const std::optional<AudioUuid>& in_instance,
+                      std::vector<Descriptor::Identity>* _aidl_return) {
+        EXPECT_NE(mEffectFactory, nullptr);
+        EXPECT_IS_OK(mEffectFactory->queryEffects(in_type, in_instance, _aidl_return));
+        mIds = *_aidl_return;
+    }
+
+    void CreateEffects() {
+        EXPECT_NE(mEffectFactory, nullptr);
+        for (const auto& id : mIds) {
+            std::shared_ptr<IEffect> effect;
+            EXPECT_IS_OK(mEffectFactory->createEffect(id.uuid, &effect));
+            EXPECT_NE(effect, nullptr) << id.toString();
+            mEffectIdMap[effect] = id;
+        }
+    }
+
+    void DestroyEffects() {
+        EXPECT_NE(mEffectFactory, nullptr);
+        for (const auto& it : mEffectIdMap) {
+            EXPECT_IS_OK(mEffectFactory->destroyEffect(it.first));
+        }
+        mEffectIdMap.clear();
+    }
+
+    std::shared_ptr<IFactory> GetFactory() { return mEffectFactory; }
+    const std::vector<Descriptor::Identity>& GetEffectIds() { return mIds; }
+    const std::vector<Descriptor::Identity>& GetCompleteEffectIdList() { return mCompleteIds; }
+    const std::unordered_map<std::shared_ptr<IEffect>, Descriptor::Identity>& GetEffectMap() {
+        return mEffectIdMap;
+    }
+
+  private:
+    std::shared_ptr<IFactory> mEffectFactory;
+    std::string mServiceName;
     AudioHalBinderServiceUtil binderUtil;
+    std::vector<Descriptor::Identity> mIds;
+    std::vector<Descriptor::Identity> mCompleteIds;
+    std::unordered_map<std::shared_ptr<IEffect>, Descriptor::Identity> mEffectIdMap;
+};
+
+/// Effect factory testing.
+class EffectFactoryTest : public testing::TestWithParam<std::string> {
+  public:
+    void SetUp() override { ASSERT_NO_FATAL_FAILURE(mFactory.ConnectToFactoryService()); }
+
+    void TearDown() override { mFactory.DestroyEffects(); }
+
+    EffectFactoryHelper mFactory = EffectFactoryHelper(GetParam());
+
     // TODO: these UUID can get from config file
     // ec7178ec-e5e1-4432-a3f4-4657e6795210
     const AudioUuid nullUuid = {static_cast<int32_t>(0xec7178ec),
@@ -77,25 +126,23 @@ class EffectFactory : public testing::TestWithParam<std::string> {
             static_cast<int32_t>(0x0), 0x0, 0x0, 0x0, {0x0, 0x0, 0x0, 0x0, 0x0, 0x0}};
 };
 
-TEST_P(EffectFactory, SetupAndTearDown) {
+TEST_P(EffectFactoryTest, SetupAndTearDown) {
     // Intentionally empty test body.
 }
 
-TEST_P(EffectFactory, CanBeRestarted) {
-    ASSERT_NO_FATAL_FAILURE(RestartService());
+TEST_P(EffectFactoryTest, CanBeRestarted) {
+    ASSERT_NO_FATAL_FAILURE(mFactory.RestartFactoryService());
 }
 
-TEST_P(EffectFactory, QueriedDescriptorList) {
+TEST_P(EffectFactoryTest, QueriedDescriptorList) {
     std::vector<Descriptor::Identity> descriptors;
-    ScopedAStatus status = factory->queryEffects(std::nullopt, std::nullopt, &descriptors);
-    EXPECT_EQ(EX_NONE, status.getExceptionCode());
-    EXPECT_NE(static_cast<int>(descriptors.size()), 0);
+    mFactory.QueryEffects(std::nullopt, std::nullopt, &descriptors);
+    EXPECT_NE(descriptors.size(), 0UL);
 }
 
-TEST_P(EffectFactory, DescriptorUUIDNotNull) {
+TEST_P(EffectFactoryTest, DescriptorUUIDNotNull) {
     std::vector<Descriptor::Identity> descriptors;
-    ScopedAStatus status = factory->queryEffects(std::nullopt, std::nullopt, &descriptors);
-    EXPECT_EQ(EX_NONE, status.getExceptionCode());
+    mFactory.QueryEffects(std::nullopt, std::nullopt, &descriptors);
     // TODO: Factory eventually need to return the full list of MUST supported AOSP effects.
     for (auto& desc : descriptors) {
         EXPECT_NE(desc.type, zeroUuid);
@@ -103,28 +150,167 @@ TEST_P(EffectFactory, DescriptorUUIDNotNull) {
     }
 }
 
-TEST_P(EffectFactory, QueriedDescriptorNotExistType) {
+TEST_P(EffectFactoryTest, QueriedDescriptorNotExistType) {
     std::vector<Descriptor::Identity> descriptors;
-    ScopedAStatus status = factory->queryEffects(nullUuid, std::nullopt, &descriptors);
-    EXPECT_EQ(EX_NONE, status.getExceptionCode());
-    EXPECT_EQ(static_cast<int>(descriptors.size()), 0);
+    mFactory.QueryEffects(nullUuid, std::nullopt, &descriptors);
+    EXPECT_EQ(descriptors.size(), 0UL);
 }
 
-TEST_P(EffectFactory, QueriedDescriptorNotExistInstance) {
+TEST_P(EffectFactoryTest, QueriedDescriptorNotExistInstance) {
     std::vector<Descriptor::Identity> descriptors;
-    ScopedAStatus status = factory->queryEffects(std::nullopt, nullUuid, &descriptors);
-    EXPECT_EQ(EX_NONE, status.getExceptionCode());
-    EXPECT_EQ(static_cast<int>(descriptors.size()), 0);
+    mFactory.QueryEffects(std::nullopt, nullUuid, &descriptors);
+    EXPECT_EQ(descriptors.size(), 0UL);
 }
 
-INSTANTIATE_TEST_SUITE_P(EffectFactoryTest, EffectFactory,
+TEST_P(EffectFactoryTest, CreateAndDestroyRepeat) {
+    std::vector<Descriptor::Identity> descriptors;
+    mFactory.QueryEffects(std::nullopt, std::nullopt, &descriptors);
+    auto numIds = mFactory.GetEffectIds().size();
+    EXPECT_NE(numIds, 0UL);
+
+    EXPECT_EQ(mFactory.GetEffectMap().size(), 0UL);
+    mFactory.CreateEffects();
+    EXPECT_EQ(mFactory.GetEffectMap().size(), numIds);
+    mFactory.DestroyEffects();
+    EXPECT_EQ(mFactory.GetEffectMap().size(), 0UL);
+
+    // Create and destroy again
+    mFactory.CreateEffects();
+    EXPECT_EQ(mFactory.GetEffectMap().size(), numIds);
+    mFactory.DestroyEffects();
+    EXPECT_EQ(mFactory.GetEffectMap().size(), 0UL);
+}
+
+TEST_P(EffectFactoryTest, CreateMultipleInstanceOfSameEffect) {
+    std::vector<Descriptor::Identity> descriptors;
+    mFactory.QueryEffects(std::nullopt, std::nullopt, &descriptors);
+    auto numIds = mFactory.GetEffectIds().size();
+    EXPECT_NE(numIds, 0UL);
+
+    EXPECT_EQ(mFactory.GetEffectMap().size(), 0UL);
+    mFactory.CreateEffects();
+    EXPECT_EQ(mFactory.GetEffectMap().size(), numIds);
+    // Create effect instances of same implementation
+    mFactory.CreateEffects();
+    EXPECT_EQ(mFactory.GetEffectMap().size(), 2 * numIds);
+
+    mFactory.CreateEffects();
+    EXPECT_EQ(mFactory.GetEffectMap().size(), 3 * numIds);
+
+    mFactory.DestroyEffects();
+    EXPECT_EQ(mFactory.GetEffectMap().size(), 0UL);
+}
+
+INSTANTIATE_TEST_SUITE_P(EffectFactoryTest, EffectFactoryTest,
                          testing::ValuesIn(android::getAidlHalInstanceNames(IFactory::descriptor)),
                          android::PrintInstanceNameToString);
-GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(EffectFactory);
+GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(EffectFactoryTest);
+
+/// Effect testing.
+class AudioEffect : public testing::TestWithParam<std::string> {
+  public:
+    void SetUp() override {
+        ASSERT_NO_FATAL_FAILURE(mFactory.ConnectToFactoryService());
+        ASSERT_NO_FATAL_FAILURE(mFactory.CreateEffects());
+    }
+
+    void TearDown() override {
+        CloseEffects();
+        ASSERT_NO_FATAL_FAILURE(mFactory.DestroyEffects());
+    }
+
+    void OpenEffects() {
+        auto open = [](const std::shared_ptr<IEffect>& effect) { EXPECT_IS_OK(effect->open()); };
+        EXPECT_NO_FATAL_FAILURE(ForEachEffect(open));
+    }
+
+    void CloseEffects() {
+        auto close = [](const std::shared_ptr<IEffect>& effect) { EXPECT_IS_OK(effect->close()); };
+        EXPECT_NO_FATAL_FAILURE(ForEachEffect(close));
+    }
+
+    void GetEffectDescriptors() {
+        auto get = [](const std::shared_ptr<IEffect>& effect) {
+            Descriptor desc;
+            EXPECT_IS_OK(effect->getDescriptor(&desc));
+        };
+        EXPECT_NO_FATAL_FAILURE(ForEachEffect(get));
+    }
+
+    template <typename Functor>
+    void ForEachEffect(Functor functor) {
+        auto effectMap = mFactory.GetEffectMap();
+        for (const auto& it : effectMap) {
+            SCOPED_TRACE(it.second.toString());
+            functor(it.first);
+        }
+    }
+
+    EffectFactoryHelper mFactory = EffectFactoryHelper(GetParam());
+};
+
+TEST_P(AudioEffect, OpenEffectTest) {
+    EXPECT_NO_FATAL_FAILURE(OpenEffects());
+}
+
+TEST_P(AudioEffect, OpenAndCloseEffect) {
+    EXPECT_NO_FATAL_FAILURE(OpenEffects());
+    EXPECT_NO_FATAL_FAILURE(CloseEffects());
+}
+
+TEST_P(AudioEffect, CloseUnopenedEffectTest) {
+    EXPECT_NO_FATAL_FAILURE(CloseEffects());
+}
+
+TEST_P(AudioEffect, DoubleOpenCloseEffects) {
+    EXPECT_NO_FATAL_FAILURE(OpenEffects());
+    EXPECT_NO_FATAL_FAILURE(CloseEffects());
+    EXPECT_NO_FATAL_FAILURE(OpenEffects());
+    EXPECT_NO_FATAL_FAILURE(CloseEffects());
+
+    EXPECT_NO_FATAL_FAILURE(OpenEffects());
+    EXPECT_NO_FATAL_FAILURE(OpenEffects());
+    EXPECT_NO_FATAL_FAILURE(CloseEffects());
+
+    EXPECT_NO_FATAL_FAILURE(OpenEffects());
+    EXPECT_NO_FATAL_FAILURE(CloseEffects());
+    EXPECT_NO_FATAL_FAILURE(CloseEffects());
+}
+
+TEST_P(AudioEffect, GetDescriptors) {
+    EXPECT_NO_FATAL_FAILURE(GetEffectDescriptors());
+}
+
+TEST_P(AudioEffect, DescriptorIdExistAndUnique) {
+    auto checker = [&](const std::shared_ptr<IEffect>& effect) {
+        Descriptor desc;
+        std::vector<Descriptor::Identity> idList;
+        EXPECT_IS_OK(effect->getDescriptor(&desc));
+        mFactory.QueryEffects(desc.common.id.type, desc.common.id.uuid, &idList);
+        EXPECT_EQ(idList.size(), 1UL);
+    };
+    EXPECT_NO_FATAL_FAILURE(ForEachEffect(checker));
+
+    // Check unique with a set
+    auto stringHash = [](const Descriptor::Identity& id) {
+        return std::hash<std::string>()(id.toString());
+    };
+    auto vec = mFactory.GetCompleteEffectIdList();
+    std::unordered_set<Descriptor::Identity, decltype(stringHash)> idSet(0, stringHash);
+    for (auto it : vec) {
+        EXPECT_EQ(idSet.count(it), 0UL);
+        idSet.insert(it);
+    }
+}
+
+INSTANTIATE_TEST_SUITE_P(AudioEffectTest, AudioEffect,
+                         testing::ValuesIn(android::getAidlHalInstanceNames(IFactory::descriptor)),
+                         android::PrintInstanceNameToString);
+GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(AudioEffect);
 
 int main(int argc, char** argv) {
     ::testing::InitGoogleTest(&argc, argv);
     ABinderProcess_setThreadPoolMaxThreadCount(1);
     ABinderProcess_startThreadPool();
     return RUN_ALL_TESTS();
-}
+}

compatibility_matrices/compatibility_matrix.current.xml

@@ -16,7 +16,7 @@
             <instance>default</instance>
         </interface>
     </hal>
-    <hal format="hidl" optional="false">
+    <hal format="hidl" optional="true">
         <name>android.hardware.audio.effect</name>
         <version>6.0</version>
         <version>7.0</version>
@@ -37,7 +37,7 @@
             <instance>default</instance>
         </interface>
     </hal>
-    <hal format="aidl" optional="false">
+    <hal format="aidl" optional="true">
         <name>android.hardware.audio.effect</name>
         <version>1</version>
         <interface>