From fb1acdec67f10c789abefbe4e11ea67a1e008dec Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Mon, 12 Dec 2022 18:57:36 +0000 Subject: [PATCH] audio: Add effect attachment to devices and streams Add the following methods: - IModule.{add|remove}DeviceEffect; - IStream.{add|remove}Effect. Bug: 205884982 Test: atest VtsHalAudioCoreTargetTest Change-Id: I4066e2d10a8e08d634010cfe9eb8f832157e725f --- audio/aidl/Android.bp | 1 + .../android/hardware/audio/core/IModule.aidl | 2 + .../hardware/audio/core/IStreamCommon.aidl | 2 + .../android/hardware/audio/core/IModule.aidl | 34 +++++++++- .../hardware/audio/core/IStreamCommon.aidl | 27 ++++++++ audio/aidl/default/Module.cpp | 24 +++++++ audio/aidl/default/Stream.cpp | 22 +++++++ audio/aidl/default/include/core-impl/Module.h | 8 +++ audio/aidl/default/include/core-impl/Stream.h | 26 ++++++++ .../vts/VtsHalAudioCoreModuleTargetTest.cpp | 65 +++++++++++++++++++ 10 files changed, 210 insertions(+), 1 deletion(-) diff --git a/audio/aidl/Android.bp b/audio/aidl/Android.bp index 674b6447f9..09189710c5 100644 --- a/audio/aidl/Android.bp +++ b/audio/aidl/Android.bp @@ -131,6 +131,7 @@ aidl_interface { "android.hardware.common-V2", "android.hardware.common.fmq-V1", "android.hardware.audio.common-V1", + "android.hardware.audio.effect-V1", "android.media.audio.common.types-V2", ], backend: { diff --git a/audio/aidl/aidl_api/android.hardware.audio.core/current/android/hardware/audio/core/IModule.aidl b/audio/aidl/aidl_api/android.hardware.audio.core/current/android/hardware/audio/core/IModule.aidl index ebfa94b5eb..dd2279d554 100644 --- a/audio/aidl/aidl_api/android.hardware.audio.core/current/android/hardware/audio/core/IModule.aidl +++ b/audio/aidl/aidl_api/android.hardware.audio.core/current/android/hardware/audio/core/IModule.aidl @@ -64,6 +64,8 @@ interface IModule { int generateHwAvSyncId(); android.hardware.audio.core.VendorParameter[] getVendorParameters(in @utf8InCpp String[] ids); void setVendorParameters(in android.hardware.audio.core.VendorParameter[] parameters, boolean async); + void addDeviceEffect(int portConfigId, in android.hardware.audio.effect.IEffect effect); + void removeDeviceEffect(int portConfigId, in android.hardware.audio.effect.IEffect effect); @VintfStability parcelable OpenInputStreamArguments { int portConfigId; diff --git a/audio/aidl/aidl_api/android.hardware.audio.core/current/android/hardware/audio/core/IStreamCommon.aidl b/audio/aidl/aidl_api/android.hardware.audio.core/current/android/hardware/audio/core/IStreamCommon.aidl index 8471c79a8e..f0bf100932 100644 --- a/audio/aidl/aidl_api/android.hardware.audio.core/current/android/hardware/audio/core/IStreamCommon.aidl +++ b/audio/aidl/aidl_api/android.hardware.audio.core/current/android/hardware/audio/core/IStreamCommon.aidl @@ -38,4 +38,6 @@ interface IStreamCommon { void updateHwAvSyncId(int hwAvSyncId); android.hardware.audio.core.VendorParameter[] getVendorParameters(in @utf8InCpp String[] ids); void setVendorParameters(in android.hardware.audio.core.VendorParameter[] parameters, boolean async); + void addEffect(in android.hardware.audio.effect.IEffect effect); + void removeEffect(in android.hardware.audio.effect.IEffect effect); } diff --git a/audio/aidl/android/hardware/audio/core/IModule.aidl b/audio/aidl/android/hardware/audio/core/IModule.aidl index 7facc6cbc6..b278ac4137 100644 --- a/audio/aidl/android/hardware/audio/core/IModule.aidl +++ b/audio/aidl/android/hardware/audio/core/IModule.aidl @@ -30,6 +30,7 @@ import android.hardware.audio.core.MicrophoneInfo; import android.hardware.audio.core.ModuleDebug; import android.hardware.audio.core.StreamDescriptor; import android.hardware.audio.core.VendorParameter; +import android.hardware.audio.effect.IEffect; import android.media.audio.common.AudioOffloadInfo; import android.media.audio.common.AudioPort; import android.media.audio.common.AudioPortConfig; @@ -515,7 +516,8 @@ interface IModule { * @throws EX_ILLEGAL_ARGUMENT If the port config can not be found by the ID. * @throws EX_ILLEGAL_STATE In the following cases: * - If the port config has a stream opened on it; - * - If the port config is used by a patch. + * - If the port config is used by a patch; + * - If the port config has an audio effect on it. */ void resetAudioPortConfig(int portConfigId); @@ -728,4 +730,34 @@ interface IModule { * @throws EX_UNSUPPORTED_OPERATION If the module does not support vendor parameters. */ void setVendorParameters(in VendorParameter[] parameters, boolean async); + + /** + * Apply an audio effect to a device port. + * + * The audio effect applies to all audio input or output on the specific + * configuration of the device audio port. The effect is inserted according + * to its insertion preference specified by the 'flags.insert' field of the + * EffectDescriptor. + * + * @param portConfigId The ID of the audio port config. + * @param effect The effect instance. + * @throws EX_ILLEGAL_ARGUMENT If the device port config can not be found by the ID, + * or the effect reference is invalid. + * @throws EX_UNSUPPORTED_OPERATION If the module does not support device port effects. + */ + void addDeviceEffect(int portConfigId, in IEffect effect); + + /** + * Stop applying an audio effect to a device port. + * + * Undo the action of the 'addDeviceEffect' method. + * + * @param portConfigId The ID of the audio port config. + * @param effect The effect instance. + * @throws EX_ILLEGAL_ARGUMENT If the device port config can not be found by the ID, + * or the effect reference is invalid, or the effect is + * not currently applied to the port config. + * @throws EX_UNSUPPORTED_OPERATION If the module does not support device port effects. + */ + void removeDeviceEffect(int portConfigId, in IEffect effect); } diff --git a/audio/aidl/android/hardware/audio/core/IStreamCommon.aidl b/audio/aidl/android/hardware/audio/core/IStreamCommon.aidl index 84f7309eb5..533ef67475 100644 --- a/audio/aidl/android/hardware/audio/core/IStreamCommon.aidl +++ b/audio/aidl/android/hardware/audio/core/IStreamCommon.aidl @@ -17,6 +17,7 @@ package android.hardware.audio.core; import android.hardware.audio.core.VendorParameter; +import android.hardware.audio.effect.IEffect; /** * This interface contains operations that are common to input and output @@ -86,4 +87,30 @@ interface IStreamCommon { * @throws EX_UNSUPPORTED_OPERATION If the stream does not support vendor parameters. */ void setVendorParameters(in VendorParameter[] parameters, boolean async); + + /** + * Apply an audio effect to the stream. + * + * This method is intended for the cases when the effect has an offload + * implementation, since software effects can be applied at the client side. + * + * @param effect The effect instance. + * @throws EX_ILLEGAL_ARGUMENT If the effect reference is invalid. + * @throws EX_ILLEGAL_STATE If the stream is closed. + * @throws EX_UNSUPPORTED_OPERATION If the module does not support audio effects. + */ + void addEffect(in IEffect effect); + + /** + * Stop applying an audio effect to the stream. + * + * Undo the action of the 'addEffect' method. + * + * @param effect The effect instance. + * @throws EX_ILLEGAL_ARGUMENT If the effect reference is invalid, or the effect is + * not currently applied to the stream. + * @throws EX_ILLEGAL_STATE If the stream is closed. + * @throws EX_UNSUPPORTED_OPERATION If the module does not support audio effects. + */ + void removeEffect(in IEffect effect); } diff --git a/audio/aidl/default/Module.cpp b/audio/aidl/default/Module.cpp index d52e328517..4a424bdfd8 100644 --- a/audio/aidl/default/Module.cpp +++ b/audio/aidl/default/Module.cpp @@ -969,4 +969,28 @@ ndk::ScopedAStatus Module::setVendorParameters(const std::vector& in_effect) { + if (in_effect == nullptr) { + LOG(DEBUG) << __func__ << ": port id " << in_portConfigId << ", null effect"; + } else { + LOG(DEBUG) << __func__ << ": port id " << in_portConfigId << ", effect Binder " + << in_effect->asBinder().get(); + } + return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); +} + +ndk::ScopedAStatus Module::removeDeviceEffect( + int32_t in_portConfigId, + const std::shared_ptr<::aidl::android::hardware::audio::effect::IEffect>& in_effect) { + if (in_effect == nullptr) { + LOG(DEBUG) << __func__ << ": port id " << in_portConfigId << ", null effect"; + } else { + LOG(DEBUG) << __func__ << ": port id " << in_portConfigId << ", effect Binder " + << in_effect->asBinder().get(); + } + return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); +} + } // namespace aidl::android::hardware::audio::core diff --git a/audio/aidl/default/Stream.cpp b/audio/aidl/default/Stream.cpp index e984091337..bb123a21b1 100644 --- a/audio/aidl/default/Stream.cpp +++ b/audio/aidl/default/Stream.cpp @@ -540,6 +540,28 @@ ndk::ScopedAStatus StreamCommonImpl::setVendorParameters return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); } +template +ndk::ScopedAStatus StreamCommonImpl::addEffect( + const std::shared_ptr<::aidl::android::hardware::audio::effect::IEffect>& in_effect) { + if (in_effect == nullptr) { + LOG(DEBUG) << __func__ << ": null effect"; + } else { + LOG(DEBUG) << __func__ << ": effect Binder" << in_effect->asBinder().get(); + } + return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); +} + +template +ndk::ScopedAStatus StreamCommonImpl::removeEffect( + const std::shared_ptr<::aidl::android::hardware::audio::effect::IEffect>& in_effect) { + if (in_effect == nullptr) { + LOG(DEBUG) << __func__ << ": null effect"; + } else { + LOG(DEBUG) << __func__ << ": effect Binder" << in_effect->asBinder().get(); + } + return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); +} + template ndk::ScopedAStatus StreamCommonImpl::close() { LOG(DEBUG) << __func__; diff --git a/audio/aidl/default/include/core-impl/Module.h b/audio/aidl/default/include/core-impl/Module.h index 6baaa764b5..aa05d2a924 100644 --- a/audio/aidl/default/include/core-impl/Module.h +++ b/audio/aidl/default/include/core-impl/Module.h @@ -92,6 +92,14 @@ class Module : public BnModule { std::vector* _aidl_return) override; ndk::ScopedAStatus setVendorParameters(const std::vector& in_parameters, bool in_async) override; + ndk::ScopedAStatus addDeviceEffect( + int32_t in_portConfigId, + const std::shared_ptr<::aidl::android::hardware::audio::effect::IEffect>& in_effect) + override; + ndk::ScopedAStatus removeDeviceEffect( + int32_t in_portConfigId, + const std::shared_ptr<::aidl::android::hardware::audio::effect::IEffect>& in_effect) + override; void cleanUpPatch(int32_t patchId); ndk::ScopedAStatus createStreamContext( diff --git a/audio/aidl/default/include/core-impl/Stream.h b/audio/aidl/default/include/core-impl/Stream.h index e8b2c54056..a5d240f3f5 100644 --- a/audio/aidl/default/include/core-impl/Stream.h +++ b/audio/aidl/default/include/core-impl/Stream.h @@ -212,6 +212,12 @@ struct StreamCommonInterface { std::vector* _aidl_return) = 0; virtual ndk::ScopedAStatus setVendorParameters( const std::vector& in_parameters, bool in_async) = 0; + virtual ndk::ScopedAStatus addEffect( + const std::shared_ptr<::aidl::android::hardware::audio::effect::IEffect>& + in_effect) = 0; + virtual ndk::ScopedAStatus removeEffect( + const std::shared_ptr<::aidl::android::hardware::audio::effect::IEffect>& + in_effect) = 0; }; class StreamCommon : public BnStreamCommon { @@ -242,6 +248,20 @@ class StreamCommon : public BnStreamCommon { return delegate != nullptr ? delegate->setVendorParameters(in_parameters, in_async) : ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); } + ndk::ScopedAStatus addEffect( + const std::shared_ptr<::aidl::android::hardware::audio::effect::IEffect>& in_effect) + override { + auto delegate = mDelegate.lock(); + return delegate != nullptr ? delegate->addEffect(in_effect) + : ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); + } + ndk::ScopedAStatus removeEffect( + const std::shared_ptr<::aidl::android::hardware::audio::effect::IEffect>& in_effect) + override { + auto delegate = mDelegate.lock(); + return delegate != nullptr ? delegate->removeEffect(in_effect) + : ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); + } // It is possible that on the client side the proxy for IStreamCommon will outlive // the IStream* instance, and the server side IStream* instance will get destroyed // while this IStreamCommon instance is still alive. @@ -257,6 +277,12 @@ class StreamCommonImpl : public StreamCommonInterface { std::vector* _aidl_return) override; ndk::ScopedAStatus setVendorParameters(const std::vector& in_parameters, bool in_async) override; + ndk::ScopedAStatus addEffect( + const std::shared_ptr<::aidl::android::hardware::audio::effect::IEffect>& in_effect) + override; + ndk::ScopedAStatus removeEffect( + const std::shared_ptr<::aidl::android::hardware::audio::effect::IEffect>& in_effect) + override; ndk::ScopedAStatus getStreamCommon(std::shared_ptr* _aidl_return); ndk::ScopedAStatus init() { diff --git a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp index 50fb981163..4e3f525be5 100644 --- a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp +++ b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp @@ -1746,6 +1746,33 @@ TEST_P(AudioCoreModule, SetVendorParameters) { } } +// See b/262930731. In the absence of offloaded effect implementations, +// currently we can only pass a nullptr, and the HAL module must either reject +// it as an invalid argument, or say that offloaded effects are not supported. +TEST_P(AudioCoreModule, AddRemoveEffectInvalidArguments) { + ndk::ScopedAStatus addEffectStatus = module->addDeviceEffect(-1, nullptr); + ndk::ScopedAStatus removeEffectStatus = module->removeDeviceEffect(-1, nullptr); + const bool isSupported = addEffectStatus.getExceptionCode() != EX_UNSUPPORTED_OPERATION; + if (isSupported) { + EXPECT_EQ(EX_ILLEGAL_ARGUMENT, addEffectStatus.getExceptionCode()); + EXPECT_EQ(EX_ILLEGAL_ARGUMENT, removeEffectStatus.getExceptionCode()); + } else if (EX_UNSUPPORTED_OPERATION != removeEffectStatus.getExceptionCode()) { + GTEST_FAIL() << "addEffect and removeEffect must be either supported or not supported " + << "together"; + } else { + GTEST_SKIP() << "Offloaded effects not supported"; + } + // Test rejection of a nullptr effect with a valid device port Id. + ASSERT_NO_FATAL_FAILURE(SetUpModuleConfig()); + const auto configs = moduleConfig->getPortConfigsForAttachedDevicePorts(); + for (const auto& config : configs) { + WithAudioPortConfig portConfig(config); + ASSERT_NO_FATAL_FAILURE(portConfig.SetUp(module.get())); + EXPECT_STATUS(EX_ILLEGAL_ARGUMENT, module->addDeviceEffect(portConfig.getId(), nullptr)); + EXPECT_STATUS(EX_ILLEGAL_ARGUMENT, module->removeDeviceEffect(portConfig.getId(), nullptr)); + } +} + class AudioCoreTelephony : public AudioCoreModuleBase, public testing::TestWithParam { public: void SetUp() override { @@ -2080,6 +2107,43 @@ class AudioStream : public AudioCoreModule { } } + // See b/262930731. In the absence of offloaded effect implementations, + // currently we can only pass a nullptr, and the HAL module must either reject + // it as an invalid argument, or say that offloaded effects are not supported. + void AddRemoveEffectInvalidArguments() { + const auto ports = + moduleConfig->getMixPorts(IOTraits::is_input, false /*attachedOnly*/); + if (ports.empty()) { + GTEST_SKIP() << "No mix ports"; + } + bool atLeastOneSupports = false; + for (const auto& port : ports) { + const auto portConfig = moduleConfig->getSingleConfigForMixPort(true, port); + if (!portConfig.has_value()) continue; + WithStream stream(portConfig.value()); + ASSERT_NO_FATAL_FAILURE(stream.SetUp(module.get(), kDefaultBufferSizeFrames)); + std::shared_ptr streamCommon; + ASSERT_IS_OK(stream.get()->getStreamCommon(&streamCommon)); + ASSERT_NE(nullptr, streamCommon); + ndk::ScopedAStatus addEffectStatus = streamCommon->addEffect(nullptr); + ndk::ScopedAStatus removeEffectStatus = streamCommon->removeEffect(nullptr); + const bool isSupported = addEffectStatus.getExceptionCode() != EX_UNSUPPORTED_OPERATION; + if (isSupported) { + EXPECT_EQ(EX_ILLEGAL_ARGUMENT, addEffectStatus.getExceptionCode()); + EXPECT_EQ(EX_ILLEGAL_ARGUMENT, removeEffectStatus.getExceptionCode()); + atLeastOneSupports = true; + } else if (EX_UNSUPPORTED_OPERATION != removeEffectStatus.getExceptionCode()) { + ADD_FAILURE() + << "addEffect and removeEffect must be either supported or not supported " + << "together"; + atLeastOneSupports = true; + } + } + if (!atLeastOneSupports) { + GTEST_SKIP() << "Offloaded effects not supported"; + } + } + void OpenTwiceSamePortConfigImpl(const AudioPortConfig& portConfig) { WithStream stream1(portConfig); ASSERT_NO_FATAL_FAILURE(stream1.SetUp(module.get(), kDefaultBufferSizeFrames)); @@ -2157,6 +2221,7 @@ TEST_IN_AND_OUT_STREAM(UpdateHwAvSyncId); TEST_IN_AND_OUT_STREAM(GetVendorParameters); TEST_IN_AND_OUT_STREAM(SetVendorParameters); TEST_IN_AND_OUT_STREAM(HwGainHwVolume); +TEST_IN_AND_OUT_STREAM(AddRemoveEffectInvalidArguments); namespace aidl::android::hardware::audio::core { std::ostream& operator<<(std::ostream& os, const IStreamIn::MicrophoneDirection& md) {