diff --git a/audio/aidl/android/hardware/audio/effect/IEffect.aidl b/audio/aidl/android/hardware/audio/effect/IEffect.aidl index 44e916b7bf..d7a9501569 100644 --- a/audio/aidl/android/hardware/audio/effect/IEffect.aidl +++ b/audio/aidl/android/hardware/audio/effect/IEffect.aidl @@ -31,24 +31,26 @@ interface IEffect { * * @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 not result in throw error. + * @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, instance context will be kept after - * close, but processing thread should be destroyed and consume no CPU. 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. + * Called by the client to close the effect instance, processing thread should be destroyed and + * consume no CPU after close. * - * Effect instance close interface should always success unless: + * 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 not result in throw error. + * @note Close an already-closed effect should do nothing and should not throw an error. */ void close(); diff --git a/audio/aidl/default/EffectFactory.cpp b/audio/aidl/default/EffectFactory.cpp index ea9d470e9c..a9848fd018 100644 --- a/audio/aidl/default/EffectFactory.cpp +++ b/audio/aidl/default/EffectFactory.cpp @@ -29,23 +29,9 @@ Factory::Factory() { // TODO: implement this with xml parser on audio_effect.xml, and filter with optional // parameters. Descriptor::Identity id; - id.type = {static_cast(0x0bed4300), - 0xddd6, - 0x11db, - 0x8f34, - {0x00, 0x02, 0xa5, 0xd5, 0xc5, 0x1b}}; - id.uuid = EqualizerUUID; + id.type = EqualizerTypeUUID; + id.uuid = EqualizerSwImplUUID; mIdentityList.push_back(id); - // TODO: Add visualizer with default implementation later -#if 0 - id.type = {static_cast(0xd3467faa), - 0xacc7, - 0x4d34, - 0xacaf, - {0x00, 0x02, 0xa5, 0xd5, 0xc5, 0x1b}}; - id.uuid = VisualizerUUID; - mIdentityList.push_back(id); -#endif } ndk::ScopedAStatus Factory::queryEffects(const std::optional& in_type, @@ -63,7 +49,7 @@ 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 == EqualizerUUID) { + if (in_impl_uuid == EqualizerSwImplUUID) { *_aidl_return = ndk::SharedRefBase::make(); } else { LOG(ERROR) << __func__ << ": UUID " diff --git a/audio/aidl/default/EffectMain.cpp b/audio/aidl/default/EffectMain.cpp index b30f2e7c12..3219dd66a4 100644 --- a/audio/aidl/default/EffectMain.cpp +++ b/audio/aidl/default/EffectMain.cpp @@ -23,7 +23,7 @@ int main() { // This is a debug implementation, always enable debug logging. android::base::SetMinimumLogSeverity(::android::base::DEBUG); - ABinderProcess_setThreadPoolMaxThreadCount(1); + ABinderProcess_setThreadPoolMaxThreadCount(0); auto effectFactory = ndk::SharedRefBase::make(); diff --git a/audio/aidl/default/equalizer/Equalizer.cpp b/audio/aidl/default/equalizer/Equalizer.cpp index dae3ab7737..8b157faf0f 100644 --- a/audio/aidl/default/equalizer/Equalizer.cpp +++ b/audio/aidl/default/equalizer/Equalizer.cpp @@ -21,15 +21,6 @@ namespace aidl::android::hardware::audio::effect { -Equalizer::Equalizer() { - // Implementation UUID - mDesc.common.id.uuid = {static_cast(0xce772f20), - 0x847d, - 0x11df, - 0xbb17, - {0x00, 0x02, 0xa5, 0xd5, 0xc5, 0x1b}}; -} - ndk::ScopedAStatus Equalizer::open() { LOG(DEBUG) << __func__; return ndk::ScopedAStatus::ok(); diff --git a/audio/aidl/default/include/equalizer-impl/Equalizer.h b/audio/aidl/default/include/equalizer-impl/Equalizer.h index 44b1d6deff..ea16cb9fac 100644 --- a/audio/aidl/default/include/equalizer-impl/Equalizer.h +++ b/audio/aidl/default/include/equalizer-impl/Equalizer.h @@ -21,23 +21,31 @@ namespace aidl::android::hardware::audio::effect { -// Equalizer implementation UUID. -static const ::aidl::android::media::audio::common::AudioUuid EqualizerUUID = { +// Equalizer type UUID. +static const ::aidl::android::media::audio::common::AudioUuid EqualizerTypeUUID = { static_cast(0x0bed4300), 0xddd6, 0x11db, 0x8f34, {0x00, 0x02, 0xa5, 0xd5, 0xc5, 0x1b}}; +// Equalizer implementation UUID. +static const ::aidl::android::media::audio::common::AudioUuid EqualizerSwImplUUID = { + static_cast(0x0bed4300), + 0x847d, + 0x11df, + 0xbb17, + {0x00, 0x02, 0xa5, 0xd5, 0xc5, 0x1b}}; + class Equalizer : public BnEffect { public: - Equalizer(); + 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 = EqualizerUUID}; + Descriptor mDesc = {.common = {.id = {.type = EqualizerTypeUUID, .uuid = EqualizerSwImplUUID}}}; }; } // namespace aidl::android::hardware::audio::effect diff --git a/audio/aidl/vts/VtsHalAudioEffectTargetTest.cpp b/audio/aidl/vts/VtsHalAudioEffectTargetTest.cpp index 9b100b1959..8b5eb13cdb 100644 --- a/audio/aidl/vts/VtsHalAudioEffectTargetTest.cpp +++ b/audio/aidl/vts/VtsHalAudioEffectTargetTest.cpp @@ -33,6 +33,7 @@ #include #include "AudioHalBinderServiceUtil.h" +#include "TestUtils.h" using namespace android; @@ -45,7 +46,7 @@ using aidl::android::media::audio::common::AudioUuid; class EffectFactoryHelper { public: - EffectFactoryHelper(const std::string& name) : mServiceName(name) {} + explicit EffectFactoryHelper(const std::string& name) : mServiceName(name) {} void ConnectToFactoryService() { mEffectFactory = IFactory::fromBinder(binderUtil.connectToService(mServiceName)); @@ -60,27 +61,22 @@ class EffectFactoryHelper { void QueryAllEffects() { EXPECT_NE(mEffectFactory, nullptr); - ScopedAStatus status = - mEffectFactory->queryEffects(std::nullopt, std::nullopt, &mCompleteIds); - EXPECT_EQ(status.getExceptionCode(), EX_NONE); + EXPECT_IS_OK(mEffectFactory->queryEffects(std::nullopt, std::nullopt, &mCompleteIds)); } void QueryEffects(const std::optional& in_type, const std::optional& in_instance, std::vector* _aidl_return) { EXPECT_NE(mEffectFactory, nullptr); - ScopedAStatus status = mEffectFactory->queryEffects(in_type, in_instance, _aidl_return); - EXPECT_EQ(status.getExceptionCode(), EX_NONE); + EXPECT_IS_OK(mEffectFactory->queryEffects(in_type, in_instance, _aidl_return)); mIds = *_aidl_return; } void CreateEffects() { EXPECT_NE(mEffectFactory, nullptr); - ScopedAStatus status; for (const auto& id : mIds) { std::shared_ptr effect; - status = mEffectFactory->createEffect(id.uuid, &effect); - EXPECT_EQ(status.getExceptionCode(), EX_NONE) << id.toString(); + EXPECT_IS_OK(mEffectFactory->createEffect(id.uuid, &effect)); EXPECT_NE(effect, nullptr) << id.toString(); mEffectIdMap[effect] = id; } @@ -88,10 +84,8 @@ class EffectFactoryHelper { void DestroyEffects() { EXPECT_NE(mEffectFactory, nullptr); - ScopedAStatus status; for (const auto& it : mEffectIdMap) { - status = mEffectFactory->destroyEffect(it.first); - EXPECT_EQ(status.getExceptionCode(), EX_NONE) << it.second.toString(); + EXPECT_IS_OK(mEffectFactory->destroyEffect(it.first)); } mEffectIdMap.clear(); } @@ -143,7 +137,7 @@ TEST_P(EffectFactoryTest, CanBeRestarted) { TEST_P(EffectFactoryTest, QueriedDescriptorList) { std::vector descriptors; mFactory.QueryEffects(std::nullopt, std::nullopt, &descriptors); - EXPECT_NE(static_cast(descriptors.size()), 0); + EXPECT_NE(descriptors.size(), 0UL); } TEST_P(EffectFactoryTest, DescriptorUUIDNotNull) { @@ -159,52 +153,52 @@ TEST_P(EffectFactoryTest, DescriptorUUIDNotNull) { TEST_P(EffectFactoryTest, QueriedDescriptorNotExistType) { std::vector descriptors; mFactory.QueryEffects(nullUuid, std::nullopt, &descriptors); - EXPECT_EQ(static_cast(descriptors.size()), 0); + EXPECT_EQ(descriptors.size(), 0UL); } TEST_P(EffectFactoryTest, QueriedDescriptorNotExistInstance) { std::vector descriptors; mFactory.QueryEffects(std::nullopt, nullUuid, &descriptors); - EXPECT_EQ(static_cast(descriptors.size()), 0); + EXPECT_EQ(descriptors.size(), 0UL); } TEST_P(EffectFactoryTest, CreateAndDestroyRepeat) { std::vector descriptors; mFactory.QueryEffects(std::nullopt, std::nullopt, &descriptors); - int numIds = static_cast(mFactory.GetEffectIds().size()); - EXPECT_NE(numIds, 0); + auto numIds = mFactory.GetEffectIds().size(); + EXPECT_NE(numIds, 0UL); - EXPECT_EQ(static_cast(mFactory.GetEffectMap().size()), 0); + EXPECT_EQ(mFactory.GetEffectMap().size(), 0UL); mFactory.CreateEffects(); - EXPECT_EQ(static_cast(mFactory.GetEffectMap().size()), numIds); + EXPECT_EQ(mFactory.GetEffectMap().size(), numIds); mFactory.DestroyEffects(); - EXPECT_EQ(static_cast(mFactory.GetEffectMap().size()), 0); + EXPECT_EQ(mFactory.GetEffectMap().size(), 0UL); // Create and destroy again mFactory.CreateEffects(); - EXPECT_EQ(static_cast(mFactory.GetEffectMap().size()), numIds); + EXPECT_EQ(mFactory.GetEffectMap().size(), numIds); mFactory.DestroyEffects(); - EXPECT_EQ(static_cast(mFactory.GetEffectMap().size()), 0); + EXPECT_EQ(mFactory.GetEffectMap().size(), 0UL); } TEST_P(EffectFactoryTest, CreateMultipleInstanceOfSameEffect) { std::vector descriptors; mFactory.QueryEffects(std::nullopt, std::nullopt, &descriptors); - int numIds = static_cast(mFactory.GetEffectIds().size()); - EXPECT_NE(numIds, 0); + auto numIds = mFactory.GetEffectIds().size(); + EXPECT_NE(numIds, 0UL); - EXPECT_EQ(static_cast(mFactory.GetEffectMap().size()), 0); + EXPECT_EQ(mFactory.GetEffectMap().size(), 0UL); mFactory.CreateEffects(); - EXPECT_EQ(static_cast(mFactory.GetEffectMap().size()), numIds); + EXPECT_EQ(mFactory.GetEffectMap().size(), numIds); // Create effect instances of same implementation mFactory.CreateEffects(); - EXPECT_EQ(static_cast(mFactory.GetEffectMap().size()), 2 * numIds); + EXPECT_EQ(mFactory.GetEffectMap().size(), 2 * numIds); mFactory.CreateEffects(); - EXPECT_EQ(static_cast(mFactory.GetEffectMap().size()), 3 * numIds); + EXPECT_EQ(mFactory.GetEffectMap().size(), 3 * numIds); mFactory.DestroyEffects(); - EXPECT_EQ(static_cast(mFactory.GetEffectMap().size()), 0); + EXPECT_EQ(mFactory.GetEffectMap().size(), 0UL); } INSTANTIATE_TEST_SUITE_P(EffectFactoryTest, EffectFactoryTest, @@ -226,26 +220,19 @@ class AudioEffect : public testing::TestWithParam { } void OpenEffects() { - auto open = [](const std::shared_ptr& effect) { - ScopedAStatus status = effect->open(); - EXPECT_EQ(status.getExceptionCode(), EX_NONE); - }; + auto open = [](const std::shared_ptr& effect) { EXPECT_IS_OK(effect->open()); }; EXPECT_NO_FATAL_FAILURE(ForEachEffect(open)); } void CloseEffects() { - auto close = [](const std::shared_ptr& effect) { - ScopedAStatus status = effect->close(); - EXPECT_EQ(status.getExceptionCode(), EX_NONE); - }; + auto close = [](const std::shared_ptr& effect) { EXPECT_IS_OK(effect->close()); }; EXPECT_NO_FATAL_FAILURE(ForEachEffect(close)); } void GetEffectDescriptors() { auto get = [](const std::shared_ptr& effect) { Descriptor desc; - ScopedAStatus status = effect->getDescriptor(&desc); - EXPECT_EQ(status.getExceptionCode(), EX_NONE); + EXPECT_IS_OK(effect->getDescriptor(&desc)); }; EXPECT_NO_FATAL_FAILURE(ForEachEffect(get)); } @@ -253,7 +240,6 @@ class AudioEffect : public testing::TestWithParam { template void ForEachEffect(Functor functor) { auto effectMap = mFactory.GetEffectMap(); - ScopedAStatus status; for (const auto& it : effectMap) { SCOPED_TRACE(it.second.toString()); functor(it.first); @@ -299,10 +285,9 @@ TEST_P(AudioEffect, DescriptorIdExistAndUnique) { auto checker = [&](const std::shared_ptr& effect) { Descriptor desc; std::vector idList; - ScopedAStatus status = effect->getDescriptor(&desc); - EXPECT_EQ(status.getExceptionCode(), EX_NONE); + EXPECT_IS_OK(effect->getDescriptor(&desc)); mFactory.QueryEffects(desc.common.id.type, desc.common.id.uuid, &idList); - EXPECT_EQ(static_cast(idList.size()), 1); + EXPECT_EQ(idList.size(), 1UL); }; EXPECT_NO_FATAL_FAILURE(ForEachEffect(checker)); @@ -313,7 +298,7 @@ TEST_P(AudioEffect, DescriptorIdExistAndUnique) { auto vec = mFactory.GetCompleteEffectIdList(); std::unordered_set idSet(0, stringHash); for (auto it : vec) { - EXPECT_EQ(static_cast(idSet.count(it)), 0); + EXPECT_EQ(idSet.count(it), 0UL); idSet.insert(it); } }