From 5b15e005a9f8d457d436ec3a508077c031bd5598 Mon Sep 17 00:00:00 2001 From: Shunkai Yao Date: Fri, 5 Jan 2024 02:33:47 +0000 Subject: [PATCH 1/2] Effect AIDL: add IEffect.reopen to update the effect instances data FMQ The effect instance may choose to reallocate the input data message queue under specific conditions. For example, when the input format changes, requiring an update to the data message queue allocated during the open time. In such cases, the effect instance can destroy the existing data message queue, when the audio framework see a valid status MQ and invalid data MQ, it call reopen to get the new data message queue. Bug: 302036943 Test: m android.hardware.audio.effect-update-api, m Change-Id: Ia245b154176f64bc3cc6e6049bca4f9c68ad482d Merged-In: Ia245b154176f64bc3cc6e6049bca4f9c68ad482d --- .../hardware/audio/effect/IEffect.aidl | 1 + .../hardware/audio/effect/IEffect.aidl | 37 +++++++++++++++++-- .../android/hardware/audio/effect/state.gv | 6 ++- audio/aidl/default/EffectImpl.cpp | 10 +++++ .../default/include/effect-impl/EffectImpl.h | 1 + 5 files changed, 49 insertions(+), 6 deletions(-) diff --git a/audio/aidl/aidl_api/android.hardware.audio.effect/current/android/hardware/audio/effect/IEffect.aidl b/audio/aidl/aidl_api/android.hardware.audio.effect/current/android/hardware/audio/effect/IEffect.aidl index 8c196e7abf..a1b00be41f 100644 --- a/audio/aidl/aidl_api/android.hardware.audio.effect/current/android/hardware/audio/effect/IEffect.aidl +++ b/audio/aidl/aidl_api/android.hardware.audio.effect/current/android/hardware/audio/effect/IEffect.aidl @@ -41,6 +41,7 @@ interface IEffect { android.hardware.audio.effect.State getState(); void setParameter(in android.hardware.audio.effect.Parameter param); android.hardware.audio.effect.Parameter getParameter(in android.hardware.audio.effect.Parameter.Id paramId); + android.hardware.audio.effect.IEffect.OpenEffectReturn reopen(); @FixedSize @VintfStability parcelable Status { int status; diff --git a/audio/aidl/android/hardware/audio/effect/IEffect.aidl b/audio/aidl/android/hardware/audio/effect/IEffect.aidl index 6097f34ead..266adecc5d 100644 --- a/audio/aidl/android/hardware/audio/effect/IEffect.aidl +++ b/audio/aidl/android/hardware/audio/effect/IEffect.aidl @@ -54,7 +54,16 @@ interface IEffect { } /** - * Return data structure of IEffect.open() interface. + * Return data structure of the IEffect.open() and IEffect.reopen() method. + * + * Contains Fast Message Queues (FMQs) for effect processing status and input/output data. + * The status FMQ remains valid and unchanged after opening, while input/output data FMQs can be + * modified with changes in input/output AudioConfig. If reallocation of data FMQ is necessary, + * the effect instance must release the existing data FMQs to signal the need to the audio + * framework. + * When the audio framework encounters a valid status FMQ and invalid input/output data FMQs, + * it must invoke the IEffect.reopen() method to request the effect instance to reallocate + * the FMQ and return to the audio framework. */ @VintfStability parcelable OpenEffectReturn { @@ -97,7 +106,7 @@ interface IEffect { * 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: + * Effect instance close method should always succeed unless: * 1. The effect instance is not in a proper state to be closed, for example it's still in * State::PROCESSING state. * 2. There is system/hardware related failure when close. @@ -154,8 +163,8 @@ interface IEffect { /** * Get a parameter from the effect instance with parameter ID. * - * This interface must return the current parameter of the effect instance, if no parameter - * has been set by client yet, the default value must be returned. + * This method must return the current parameter of the effect instance, if no parameter has + * been set by client yet, the default value must be returned. * * Must be available for the effect instance after open(). * @@ -165,4 +174,24 @@ interface IEffect { * @throws EX_ILLEGAL_ARGUMENT if the effect instance receive unsupported parameter tag. */ Parameter getParameter(in Parameter.Id paramId); + + /** + * Reopen the effect instance, keeping all previous parameters unchanged, and reallocate only + * the Fast Message Queues (FMQs) allocated during the open time as needed. + * + * When the audio framework encounters a valid status FMQ and invalid data FMQ(s), it calls + * this method to reopen the effect and request the latest data FMQ. + * Upon receiving this call, if the effect instance's data FMQ(s) is invalid, it must reallocate + * the necessary data FMQ(s) and return them to the audio framework. All previous parameters and + * states keep unchanged. + * Once the audio framework successfully completes the call, it must validate the returned FMQs, + * deprecate all old FMQs, and exclusively use the FMQs returned from this method. + * + * @return The reallocated data FMQ and the original status FMQ. + * + * @throws EX_ILLEGAL_STATE if the effect instance is not in a proper state to reallocate FMQ. + * This may occur if the effect instance has already been closed or if there is no need to + * update any data FMQ. + */ + OpenEffectReturn reopen(); } diff --git a/audio/aidl/android/hardware/audio/effect/state.gv b/audio/aidl/android/hardware/audio/effect/state.gv index ce369ba5c8..22c70c8d8f 100644 --- a/audio/aidl/android/hardware/audio/effect/state.gv +++ b/audio/aidl/android/hardware/audio/effect/state.gv @@ -31,6 +31,8 @@ digraph effect_state_machine { IDLE -> INIT[label = "IEffect.close()"]; INIT -> INIT[label = "IEffect.getState\nIEffect.getDescriptor"]; - IDLE -> IDLE[label = "IEffect.getParameter\nIEffect.setParameter\nIEffect.getDescriptor\nIEffect.command(RESET)"]; - PROCESSING -> PROCESSING[label = "IEffect.getParameter\nIEffect.setParameter\nIEffect.getDescriptor"]; + IDLE -> IDLE[label = "IEffect.getParameter\nIEffect.setParameter\nIEffect.getDescriptor\nIEffect.command(RESET)\nIEffect.reopen"]; + PROCESSING + -> PROCESSING + [label = "IEffect.getParameter\nIEffect.setParameter\nIEffect.getDescriptor\nIEffect.reopen"]; } diff --git a/audio/aidl/default/EffectImpl.cpp b/audio/aidl/default/EffectImpl.cpp index c81c731c43..3c12f83e0f 100644 --- a/audio/aidl/default/EffectImpl.cpp +++ b/audio/aidl/default/EffectImpl.cpp @@ -60,6 +60,16 @@ ndk::ScopedAStatus EffectImpl::open(const Parameter::Common& common, return ndk::ScopedAStatus::ok(); } +ndk::ScopedAStatus EffectImpl::reopen(OpenEffectReturn* ret) { + RETURN_IF(mState == State::INIT, EX_ILLEGAL_STATE, "alreadyClosed"); + + // TODO: b/302036943 add reopen implementation + auto context = getContext(); + RETURN_IF(!context, EX_NULL_POINTER, "nullContext"); + context->dupeFmq(ret); + return ndk::ScopedAStatus::ok(); +} + ndk::ScopedAStatus EffectImpl::close() { RETURN_OK_IF(mState == State::INIT); RETURN_IF(mState == State::PROCESSING, EX_ILLEGAL_STATE, "closeAtProcessing"); diff --git a/audio/aidl/default/include/effect-impl/EffectImpl.h b/audio/aidl/default/include/effect-impl/EffectImpl.h index e7d081f6fc..242a268912 100644 --- a/audio/aidl/default/include/effect-impl/EffectImpl.h +++ b/audio/aidl/default/include/effect-impl/EffectImpl.h @@ -43,6 +43,7 @@ class EffectImpl : public BnEffect, public EffectThread { OpenEffectReturn* ret) override; virtual ndk::ScopedAStatus close() override; virtual ndk::ScopedAStatus command(CommandId id) override; + virtual ndk::ScopedAStatus reopen(OpenEffectReturn* ret) override; virtual ndk::ScopedAStatus getState(State* state) override; virtual ndk::ScopedAStatus setParameter(const Parameter& param) override; From 65c7c7051d1f22dfde4520170af304807abf5293 Mon Sep 17 00:00:00 2001 From: Shunkai Yao Date: Tue, 9 Jan 2024 20:50:53 +0000 Subject: [PATCH 2/2] Effect AIDL: implement IEffect.reopen - add IEffect.reopen implementation - now data MQs can update at runtime, sync EffectContext access - add clang thread annotation Bug: 302036943 Test: atest VtsHalAudioEffectTargetTest Test: build and test audio effect on Pixel Change-Id: I3e9fdc2d5eb50b8c1377e0da75573f0eba7ea3f1 Merged-In: I3e9fdc2d5eb50b8c1377e0da75573f0eba7ea3f1 --- audio/aidl/default/Android.bp | 1 + audio/aidl/default/EffectContext.cpp | 227 ++++++++++++++++++ audio/aidl/default/EffectImpl.cpp | 157 +++++++++--- audio/aidl/default/EffectThread.cpp | 55 +---- .../AcousticEchoCancelerSw.cpp | 4 - .../AcousticEchoCancelerSw.h | 18 +- .../AutomaticGainControlV1Sw.cpp | 4 - .../AutomaticGainControlV1Sw.h | 21 +- .../AutomaticGainControlV2Sw.cpp | 4 - .../AutomaticGainControlV2Sw.h | 21 +- audio/aidl/default/bassboost/BassBoostSw.cpp | 4 - audio/aidl/default/bassboost/BassBoostSw.h | 20 +- audio/aidl/default/downmix/DownmixSw.cpp | 4 - audio/aidl/default/downmix/DownmixSw.h | 21 +- .../DynamicsProcessingSw.cpp | 7 +- .../dynamicProcessing/DynamicsProcessingSw.h | 21 +- audio/aidl/default/envReverb/EnvReverbSw.cpp | 4 - audio/aidl/default/envReverb/EnvReverbSw.h | 18 +- audio/aidl/default/equalizer/EqualizerSw.cpp | 4 - audio/aidl/default/equalizer/EqualizerSw.h | 18 +- .../default/extension/ExtensionEffect.cpp | 4 - .../aidl/default/extension/ExtensionEffect.h | 18 +- .../hapticGenerator/HapticGeneratorSw.cpp | 4 - .../hapticGenerator/HapticGeneratorSw.h | 21 +- .../include/effect-impl/EffectContext.h | 137 ++++------- .../default/include/effect-impl/EffectImpl.h | 50 +++- .../include/effect-impl/EffectThread.h | 35 +-- .../default/include/effect-impl/EffectTypes.h | 5 +- .../loudnessEnhancer/LoudnessEnhancerSw.cpp | 4 - .../loudnessEnhancer/LoudnessEnhancerSw.h | 21 +- .../noiseSuppression/NoiseSuppressionSw.cpp | 4 - .../noiseSuppression/NoiseSuppressionSw.h | 21 +- .../default/presetReverb/PresetReverbSw.cpp | 4 - .../default/presetReverb/PresetReverbSw.h | 20 +- .../default/spatializer/SpatializerSw.cpp | 4 - .../aidl/default/spatializer/SpatializerSw.h | 18 +- .../default/virtualizer/VirtualizerSw.cpp | 4 - .../aidl/default/virtualizer/VirtualizerSw.h | 19 +- .../aidl/default/visualizer/VisualizerSw.cpp | 4 - audio/aidl/default/visualizer/VisualizerSw.h | 20 +- audio/aidl/default/volume/VolumeSw.cpp | 4 - audio/aidl/default/volume/VolumeSw.h | 21 +- audio/aidl/vts/EffectHelper.h | 11 + .../aidl/vts/VtsHalAudioEffectTargetTest.cpp | 96 ++++++++ 44 files changed, 735 insertions(+), 447 deletions(-) create mode 100644 audio/aidl/default/EffectContext.cpp diff --git a/audio/aidl/default/Android.bp b/audio/aidl/default/Android.bp index 6d0b95ef12..20682d6384 100644 --- a/audio/aidl/default/Android.bp +++ b/audio/aidl/default/Android.bp @@ -226,6 +226,7 @@ cc_defaults { filegroup { name: "effectCommonFile", srcs: [ + "EffectContext.cpp", "EffectThread.cpp", "EffectImpl.cpp", ], diff --git a/audio/aidl/default/EffectContext.cpp b/audio/aidl/default/EffectContext.cpp new file mode 100644 index 0000000000..2e1291822a --- /dev/null +++ b/audio/aidl/default/EffectContext.cpp @@ -0,0 +1,227 @@ +/* + * Copyright (C) 2024 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. + */ + +#include +#define LOG_TAG "AHAL_EffectContext" +#include "effect-impl/EffectContext.h" +#include "include/effect-impl/EffectTypes.h" + +using aidl::android::hardware::audio::common::getChannelCount; +using aidl::android::hardware::audio::common::getFrameSizeInBytes; +using aidl::android::hardware::audio::effect::IEffect; +using aidl::android::media::audio::common::PcmType; +using ::android::hardware::EventFlag; + +namespace aidl::android::hardware::audio::effect { + +EffectContext::EffectContext(size_t statusDepth, const Parameter::Common& common) { + LOG_ALWAYS_FATAL_IF(RetCode::SUCCESS != setCommon(common), "illegalCommonParameter"); + + // in/outBuffer size in float (FMQ data format defined for DataMQ) + size_t inBufferSizeInFloat = common.input.frameCount * mInputFrameSize / sizeof(float); + size_t outBufferSizeInFloat = common.output.frameCount * mOutputFrameSize / sizeof(float); + + // only status FMQ use the EventFlag + mStatusMQ = std::make_shared(statusDepth, true /*configureEventFlagWord*/); + mInputMQ = std::make_shared(inBufferSizeInFloat); + mOutputMQ = std::make_shared(outBufferSizeInFloat); + + if (!mStatusMQ->isValid() || !mInputMQ->isValid() || !mOutputMQ->isValid()) { + LOG(ERROR) << __func__ << " created invalid FMQ"; + } + + ::android::status_t status = + EventFlag::createEventFlag(mStatusMQ->getEventFlagWord(), &mEfGroup); + LOG_ALWAYS_FATAL_IF(status != ::android::OK || !mEfGroup, " create EventFlagGroup failed "); + mWorkBuffer.reserve(std::max(inBufferSizeInFloat, outBufferSizeInFloat)); +} + +// reset buffer status by abandon input data in FMQ +void EffectContext::resetBuffer() { + auto buffer = static_cast(mWorkBuffer.data()); + std::vector status(mStatusMQ->availableToRead()); + if (mInputMQ) { + mInputMQ->read(buffer, mInputMQ->availableToRead()); + } +} + +void EffectContext::dupeFmqWithReopen(IEffect::OpenEffectReturn* effectRet) { + if (!mInputMQ) { + mInputMQ = std::make_shared(mCommon.input.frameCount * mInputFrameSize / + sizeof(float)); + } + if (!mOutputMQ) { + mOutputMQ = std::make_shared(mCommon.output.frameCount * mOutputFrameSize / + sizeof(float)); + } + dupeFmq(effectRet); +} + +void EffectContext::dupeFmq(IEffect::OpenEffectReturn* effectRet) { + if (effectRet) { + effectRet->statusMQ = mStatusMQ->dupeDesc(); + effectRet->inputDataMQ = mInputMQ->dupeDesc(); + effectRet->outputDataMQ = mOutputMQ->dupeDesc(); + } +} + +float* EffectContext::getWorkBuffer() { + return static_cast(mWorkBuffer.data()); +} + +std::shared_ptr EffectContext::getStatusFmq() const { + return mStatusMQ; +} + +std::shared_ptr EffectContext::getInputDataFmq() const { + return mInputMQ; +} + +std::shared_ptr EffectContext::getOutputDataFmq() const { + return mOutputMQ; +} + +size_t EffectContext::getInputFrameSize() const { + return mInputFrameSize; +} + +size_t EffectContext::getOutputFrameSize() const { + return mOutputFrameSize; +} + +int EffectContext::getSessionId() const { + return mCommon.session; +} + +int EffectContext::getIoHandle() const { + return mCommon.ioHandle; +} + +RetCode EffectContext::setOutputDevice( + const std::vector& device) { + mOutputDevice = device; + return RetCode::SUCCESS; +} + +std::vector +EffectContext::getOutputDevice() { + return mOutputDevice; +} + +RetCode EffectContext::setAudioMode(const aidl::android::media::audio::common::AudioMode& mode) { + mMode = mode; + return RetCode::SUCCESS; +} +aidl::android::media::audio::common::AudioMode EffectContext::getAudioMode() { + return mMode; +} + +RetCode EffectContext::setAudioSource( + const aidl::android::media::audio::common::AudioSource& source) { + mSource = source; + return RetCode::SUCCESS; +} + +aidl::android::media::audio::common::AudioSource EffectContext::getAudioSource() { + return mSource; +} + +RetCode EffectContext::setVolumeStereo(const Parameter::VolumeStereo& volumeStereo) { + mVolumeStereo = volumeStereo; + return RetCode::SUCCESS; +} + +Parameter::VolumeStereo EffectContext::getVolumeStereo() { + return mVolumeStereo; +} + +RetCode EffectContext::setCommon(const Parameter::Common& common) { + LOG(VERBOSE) << __func__ << common.toString(); + auto& input = common.input; + auto& output = common.output; + + if (input.base.format.pcm != aidl::android::media::audio::common::PcmType::FLOAT_32_BIT || + output.base.format.pcm != aidl::android::media::audio::common::PcmType::FLOAT_32_BIT) { + LOG(ERROR) << __func__ << " illegal IO, input " + << ::android::internal::ToString(input.base.format) << ", output " + << ::android::internal::ToString(output.base.format); + return RetCode::ERROR_ILLEGAL_PARAMETER; + } + + if (auto ret = updateIOFrameSize(common); ret != RetCode::SUCCESS) { + return ret; + } + + mInputChannelCount = getChannelCount(input.base.channelMask); + mOutputChannelCount = getChannelCount(output.base.channelMask); + if (mInputChannelCount == 0 || mOutputChannelCount == 0) { + LOG(ERROR) << __func__ << " illegal channel count input " << mInputChannelCount + << ", output " << mOutputChannelCount; + return RetCode::ERROR_ILLEGAL_PARAMETER; + } + + mCommon = common; + return RetCode::SUCCESS; +} + +Parameter::Common EffectContext::getCommon() { + LOG(VERBOSE) << __func__ << mCommon.toString(); + return mCommon; +} + +EventFlag* EffectContext::getStatusEventFlag() { + return mEfGroup; +} + +RetCode EffectContext::updateIOFrameSize(const Parameter::Common& common) { + const auto iFrameSize = ::aidl::android::hardware::audio::common::getFrameSizeInBytes( + common.input.base.format, common.input.base.channelMask); + const auto oFrameSize = ::aidl::android::hardware::audio::common::getFrameSizeInBytes( + common.output.base.format, common.output.base.channelMask); + + bool needUpdateMq = false; + if (mInputMQ && + (mInputFrameSize != iFrameSize || mCommon.input.frameCount != common.input.frameCount)) { + mInputMQ.reset(); + needUpdateMq = true; + } + if (mOutputMQ && + (mOutputFrameSize != oFrameSize || mCommon.output.frameCount != common.output.frameCount)) { + mOutputMQ.reset(); + needUpdateMq = true; + } + mInputFrameSize = iFrameSize; + mOutputFrameSize = oFrameSize; + if (needUpdateMq) { + return notifyDataMqUpdate(); + } + return RetCode::SUCCESS; +} + +RetCode EffectContext::notifyDataMqUpdate() { + if (!mEfGroup) { + LOG(ERROR) << __func__ << ": invalid EventFlag group"; + return RetCode::ERROR_EVENT_FLAG_ERROR; + } + + if (const auto ret = mEfGroup->wake(kEventFlagDataMqUpdate); ret != ::android::OK) { + LOG(ERROR) << __func__ << ": wake failure with ret " << ret; + return RetCode::ERROR_EVENT_FLAG_ERROR; + } + LOG(DEBUG) << __func__ << " : signal client for reopen"; + return RetCode::SUCCESS; +} +} // namespace aidl::android::hardware::audio::effect diff --git a/audio/aidl/default/EffectImpl.cpp b/audio/aidl/default/EffectImpl.cpp index 3c12f83e0f..b76269aa52 100644 --- a/audio/aidl/default/EffectImpl.cpp +++ b/audio/aidl/default/EffectImpl.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ +#include #define LOG_TAG "AHAL_EffectImpl" #include "effect-impl/EffectImpl.h" #include "effect-impl/EffectTypes.h" @@ -22,6 +23,7 @@ using aidl::android::hardware::audio::effect::IEffect; using aidl::android::hardware::audio::effect::State; using aidl::android::media::audio::common::PcmType; +using ::android::hardware::EventFlag; extern "C" binder_exception_t destroyEffect(const std::shared_ptr& instanceSp) { State state; @@ -45,50 +47,62 @@ ndk::ScopedAStatus EffectImpl::open(const Parameter::Common& common, RETURN_IF(common.input.base.format.pcm != common.output.base.format.pcm || common.input.base.format.pcm != PcmType::FLOAT_32_BIT, EX_ILLEGAL_ARGUMENT, "dataMustBe32BitsFloat"); + std::lock_guard lg(mImplMutex); RETURN_OK_IF(mState != State::INIT); - auto context = createContext(common); - RETURN_IF(!context, EX_NULL_POINTER, "createContextFailed"); + mImplContext = createContext(common); + RETURN_IF(!mImplContext, EX_NULL_POINTER, "nullContext"); + mEventFlag = mImplContext->getStatusEventFlag(); if (specific.has_value()) { RETURN_IF_ASTATUS_NOT_OK(setParameterSpecific(specific.value()), "setSpecParamErr"); } mState = State::IDLE; - context->dupeFmq(ret); - RETURN_IF(createThread(context, getEffectName()) != RetCode::SUCCESS, EX_UNSUPPORTED_OPERATION, + mImplContext->dupeFmq(ret); + RETURN_IF(createThread(getEffectName()) != RetCode::SUCCESS, EX_UNSUPPORTED_OPERATION, "FailedToCreateWorker"); return ndk::ScopedAStatus::ok(); } ndk::ScopedAStatus EffectImpl::reopen(OpenEffectReturn* ret) { + std::lock_guard lg(mImplMutex); RETURN_IF(mState == State::INIT, EX_ILLEGAL_STATE, "alreadyClosed"); // TODO: b/302036943 add reopen implementation - auto context = getContext(); - RETURN_IF(!context, EX_NULL_POINTER, "nullContext"); - context->dupeFmq(ret); + RETURN_IF(!mImplContext, EX_NULL_POINTER, "nullContext"); + mImplContext->dupeFmqWithReopen(ret); return ndk::ScopedAStatus::ok(); } ndk::ScopedAStatus EffectImpl::close() { - RETURN_OK_IF(mState == State::INIT); - RETURN_IF(mState == State::PROCESSING, EX_ILLEGAL_STATE, "closeAtProcessing"); + { + std::lock_guard lg(mImplMutex); + RETURN_OK_IF(mState == State::INIT); + RETURN_IF(mState == State::PROCESSING, EX_ILLEGAL_STATE, "closeAtProcessing"); + mState = State::INIT; + } + RETURN_IF(notifyEventFlag(kEventFlagNotEmpty) != RetCode::SUCCESS, EX_ILLEGAL_STATE, + "notifyEventFlagFailed"); // stop the worker thread, ignore the return code RETURN_IF(destroyThread() != RetCode::SUCCESS, EX_UNSUPPORTED_OPERATION, "FailedToDestroyWorker"); - mState = State::INIT; - RETURN_IF(releaseContext() != RetCode::SUCCESS, EX_UNSUPPORTED_OPERATION, - "FailedToCreateWorker"); + + { + std::lock_guard lg(mImplMutex); + releaseContext(); + mImplContext.reset(); + } LOG(DEBUG) << getEffectName() << __func__; return ndk::ScopedAStatus::ok(); } ndk::ScopedAStatus EffectImpl::setParameter(const Parameter& param) { + std::lock_guard lg(mImplMutex); LOG(VERBOSE) << getEffectName() << __func__ << " with: " << param.toString(); - const auto tag = param.getTag(); + const auto& tag = param.getTag(); switch (tag) { case Parameter::common: case Parameter::deviceDescription: @@ -110,8 +124,8 @@ ndk::ScopedAStatus EffectImpl::setParameter(const Parameter& param) { } ndk::ScopedAStatus EffectImpl::getParameter(const Parameter::Id& id, Parameter* param) { - auto tag = id.getTag(); - switch (tag) { + std::lock_guard lg(mImplMutex); + switch (id.getTag()) { case Parameter::Id::commonTag: { RETURN_IF_ASTATUS_NOT_OK(getParameterCommon(id.get(), param), "CommonParamNotSupported"); @@ -131,30 +145,30 @@ ndk::ScopedAStatus EffectImpl::getParameter(const Parameter::Id& id, Parameter* } ndk::ScopedAStatus EffectImpl::setParameterCommon(const Parameter& param) { - auto context = getContext(); - RETURN_IF(!context, EX_NULL_POINTER, "nullContext"); + RETURN_IF(!mImplContext, EX_NULL_POINTER, "nullContext"); - auto tag = param.getTag(); + const auto& tag = param.getTag(); switch (tag) { case Parameter::common: - RETURN_IF(context->setCommon(param.get()) != RetCode::SUCCESS, + RETURN_IF(mImplContext->setCommon(param.get()) != RetCode::SUCCESS, EX_ILLEGAL_ARGUMENT, "setCommFailed"); break; case Parameter::deviceDescription: - RETURN_IF(context->setOutputDevice(param.get()) != + RETURN_IF(mImplContext->setOutputDevice(param.get()) != RetCode::SUCCESS, EX_ILLEGAL_ARGUMENT, "setDeviceFailed"); break; case Parameter::mode: - RETURN_IF(context->setAudioMode(param.get()) != RetCode::SUCCESS, + RETURN_IF(mImplContext->setAudioMode(param.get()) != RetCode::SUCCESS, EX_ILLEGAL_ARGUMENT, "setModeFailed"); break; case Parameter::source: - RETURN_IF(context->setAudioSource(param.get()) != RetCode::SUCCESS, + RETURN_IF(mImplContext->setAudioSource(param.get()) != + RetCode::SUCCESS, EX_ILLEGAL_ARGUMENT, "setSourceFailed"); break; case Parameter::volumeStereo: - RETURN_IF(context->setVolumeStereo(param.get()) != + RETURN_IF(mImplContext->setVolumeStereo(param.get()) != RetCode::SUCCESS, EX_ILLEGAL_ARGUMENT, "setVolumeStereoFailed"); break; @@ -169,28 +183,27 @@ ndk::ScopedAStatus EffectImpl::setParameterCommon(const Parameter& param) { } ndk::ScopedAStatus EffectImpl::getParameterCommon(const Parameter::Tag& tag, Parameter* param) { - auto context = getContext(); - RETURN_IF(!context, EX_NULL_POINTER, "nullContext"); + RETURN_IF(!mImplContext, EX_NULL_POINTER, "nullContext"); switch (tag) { case Parameter::common: { - param->set(context->getCommon()); + param->set(mImplContext->getCommon()); break; } case Parameter::deviceDescription: { - param->set(context->getOutputDevice()); + param->set(mImplContext->getOutputDevice()); break; } case Parameter::mode: { - param->set(context->getAudioMode()); + param->set(mImplContext->getAudioMode()); break; } case Parameter::source: { - param->set(context->getAudioSource()); + param->set(mImplContext->getAudioSource()); break; } case Parameter::volumeStereo: { - param->set(context->getVolumeStereo()); + param->set(mImplContext->getVolumeStereo()); break; } default: { @@ -202,30 +215,34 @@ ndk::ScopedAStatus EffectImpl::getParameterCommon(const Parameter::Tag& tag, Par return ndk::ScopedAStatus::ok(); } -ndk::ScopedAStatus EffectImpl::getState(State* state) { +ndk::ScopedAStatus EffectImpl::getState(State* state) NO_THREAD_SAFETY_ANALYSIS { *state = mState; return ndk::ScopedAStatus::ok(); } ndk::ScopedAStatus EffectImpl::command(CommandId command) { - RETURN_IF(mState == State::INIT, EX_ILLEGAL_STATE, "CommandStateError"); + std::lock_guard lg(mImplMutex); + RETURN_IF(mState == State::INIT, EX_ILLEGAL_STATE, "instanceNotOpen"); LOG(DEBUG) << getEffectName() << __func__ << ": receive command: " << toString(command) << " at state " << toString(mState); switch (command) { case CommandId::START: - RETURN_IF(mState == State::INIT, EX_ILLEGAL_STATE, "instanceNotOpen"); RETURN_OK_IF(mState == State::PROCESSING); RETURN_IF_ASTATUS_NOT_OK(commandImpl(command), "commandImplFailed"); - startThread(); mState = State::PROCESSING; + RETURN_IF(notifyEventFlag(kEventFlagNotEmpty) != RetCode::SUCCESS, EX_ILLEGAL_STATE, + "notifyEventFlagFailed"); + startThread(); break; case CommandId::STOP: case CommandId::RESET: RETURN_OK_IF(mState == State::IDLE); + mState = State::IDLE; + RETURN_IF(notifyEventFlag(kEventFlagNotEmpty) != RetCode::SUCCESS, EX_ILLEGAL_STATE, + "notifyEventFlagFailed"); stopThread(); RETURN_IF_ASTATUS_NOT_OK(commandImpl(command), "commandImplFailed"); - mState = State::IDLE; break; default: LOG(ERROR) << getEffectName() << __func__ << " instance still processing"; @@ -237,19 +254,41 @@ ndk::ScopedAStatus EffectImpl::command(CommandId command) { } ndk::ScopedAStatus EffectImpl::commandImpl(CommandId command) { - auto context = getContext(); - RETURN_IF(!context, EX_NULL_POINTER, "nullContext"); + RETURN_IF(!mImplContext, EX_NULL_POINTER, "nullContext"); if (command == CommandId::RESET) { - context->resetBuffer(); + mImplContext->resetBuffer(); } return ndk::ScopedAStatus::ok(); } +std::shared_ptr EffectImpl::createContext(const Parameter::Common& common) { + return std::make_shared(1 /* statusMqDepth */, common); +} + +RetCode EffectImpl::releaseContext() { + if (mImplContext) { + mImplContext.reset(); + } + return RetCode::SUCCESS; +} + void EffectImpl::cleanUp() { command(CommandId::STOP); close(); } +RetCode EffectImpl::notifyEventFlag(uint32_t flag) { + if (!mEventFlag) { + LOG(ERROR) << getEffectName() << __func__ << ": StatusEventFlag invalid"; + return RetCode::ERROR_EVENT_FLAG_ERROR; + } + if (const auto ret = mEventFlag->wake(flag); ret != ::android::OK) { + LOG(ERROR) << getEffectName() << __func__ << ": wake failure with ret " << ret; + return RetCode::ERROR_EVENT_FLAG_ERROR; + } + return RetCode::SUCCESS; +} + IEffect::Status EffectImpl::status(binder_status_t status, size_t consumed, size_t produced) { IEffect::Status ret; ret.status = status; @@ -258,6 +297,48 @@ IEffect::Status EffectImpl::status(binder_status_t status, size_t consumed, size return ret; } +void EffectImpl::process() { + /** + * wait for the EventFlag without lock, it's ok because the mEfGroup pointer will not change + * in the life cycle of workerThread (threadLoop). + */ + uint32_t efState = 0; + if (!mEventFlag || + ::android::OK != mEventFlag->wait(kEventFlagNotEmpty, &efState, 0 /* no timeout */, + true /* retry */) || + !(efState & kEventFlagNotEmpty)) { + LOG(ERROR) << getEffectName() << __func__ << ": StatusEventFlag - " << mEventFlag + << " efState - " << std::hex << efState; + return; + } + + { + std::lock_guard lg(mImplMutex); + if (mState != State::PROCESSING) { + LOG(DEBUG) << getEffectName() << " skip process in state: " << toString(mState); + return; + } + RETURN_VALUE_IF(!mImplContext, void(), "nullContext"); + auto statusMQ = mImplContext->getStatusFmq(); + auto inputMQ = mImplContext->getInputDataFmq(); + auto outputMQ = mImplContext->getOutputDataFmq(); + auto buffer = mImplContext->getWorkBuffer(); + if (!inputMQ || !outputMQ) { + return; + } + + auto processSamples = inputMQ->availableToRead(); + if (processSamples) { + inputMQ->read(buffer, processSamples); + IEffect::Status status = effectProcessImpl(buffer, buffer, processSamples); + outputMQ->write(buffer, status.fmqProduced); + statusMQ->writeBlocking(&status, 1); + LOG(VERBOSE) << getEffectName() << __func__ << ": done processing, effect consumed " + << status.fmqConsumed << " produced " << status.fmqProduced; + } + } +} + // A placeholder processing implementation to copy samples from input to output IEffect::Status EffectImpl::effectProcessImpl(float* in, float* out, int samples) { for (int i = 0; i < samples; i++) { diff --git a/audio/aidl/default/EffectThread.cpp b/audio/aidl/default/EffectThread.cpp index 47ba9f44cb..fdd48034e8 100644 --- a/audio/aidl/default/EffectThread.cpp +++ b/audio/aidl/default/EffectThread.cpp @@ -25,8 +25,6 @@ #include "effect-impl/EffectThread.h" #include "effect-impl/EffectTypes.h" -using ::android::hardware::EventFlag; - namespace aidl::android::hardware::audio::effect { EffectThread::EffectThread() { @@ -38,31 +36,18 @@ EffectThread::~EffectThread() { LOG(DEBUG) << __func__ << " done"; } -RetCode EffectThread::createThread(std::shared_ptr context, const std::string& name, - int priority) { +RetCode EffectThread::createThread(const std::string& name, int priority) { if (mThread.joinable()) { LOG(WARNING) << mName << __func__ << " thread already created, no-op"; return RetCode::SUCCESS; } + mName = name; mPriority = priority; { std::lock_guard lg(mThreadMutex); mStop = true; mExit = false; - mThreadContext = std::move(context); - auto statusMQ = mThreadContext->getStatusFmq(); - EventFlag* efGroup = nullptr; - ::android::status_t status = - EventFlag::createEventFlag(statusMQ->getEventFlagWord(), &efGroup); - if (status != ::android::OK || !efGroup) { - LOG(ERROR) << mName << __func__ << " create EventFlagGroup failed " << status - << " efGroup " << efGroup; - return RetCode::ERROR_THREAD; - } - mEfGroup.reset(efGroup); - // kickoff and wait for commands (CommandId::START/STOP) or IEffect.close from client - mEfGroup->wake(kEventFlagNotEmpty); } mThread = std::thread(&EffectThread::threadLoop, this); @@ -75,16 +60,12 @@ RetCode EffectThread::destroyThread() { std::lock_guard lg(mThreadMutex); mStop = mExit = true; } - mCv.notify_one(); + mCv.notify_one(); if (mThread.joinable()) { mThread.join(); } - { - std::lock_guard lg(mThreadMutex); - mThreadContext.reset(); - } LOG(DEBUG) << mName << __func__; return RetCode::SUCCESS; } @@ -96,7 +77,6 @@ RetCode EffectThread::startThread() { mCv.notify_one(); } - mEfGroup->wake(kEventFlagNotEmpty); LOG(DEBUG) << mName << __func__; return RetCode::SUCCESS; } @@ -108,7 +88,6 @@ RetCode EffectThread::stopThread() { mCv.notify_one(); } - mEfGroup->wake(kEventFlagNotEmpty); LOG(DEBUG) << mName << __func__; return RetCode::SUCCESS; } @@ -117,13 +96,6 @@ void EffectThread::threadLoop() { pthread_setname_np(pthread_self(), mName.substr(0, kMaxTaskNameLen - 1).c_str()); setpriority(PRIO_PROCESS, 0, mPriority); while (true) { - /** - * wait for the EventFlag without lock, it's ok because the mEfGroup pointer will not change - * in the life cycle of workerThread (threadLoop). - */ - uint32_t efState = 0; - mEfGroup->wait(kEventFlagNotEmpty, &efState); - { std::unique_lock l(mThreadMutex); ::android::base::ScopedLockAssertion lock_assertion(mThreadMutex); @@ -132,27 +104,8 @@ void EffectThread::threadLoop() { LOG(INFO) << __func__ << " EXIT!"; return; } - process_l(); } - } -} - -void EffectThread::process_l() { - RETURN_VALUE_IF(!mThreadContext, void(), "nullContext"); - - auto statusMQ = mThreadContext->getStatusFmq(); - auto inputMQ = mThreadContext->getInputDataFmq(); - auto outputMQ = mThreadContext->getOutputDataFmq(); - auto buffer = mThreadContext->getWorkBuffer(); - - auto processSamples = inputMQ->availableToRead(); - if (processSamples) { - inputMQ->read(buffer, processSamples); - IEffect::Status status = effectProcessImpl(buffer, buffer, processSamples); - outputMQ->write(buffer, status.fmqProduced); - statusMQ->writeBlocking(&status, 1); - LOG(VERBOSE) << mName << __func__ << ": done processing, effect consumed " - << status.fmqConsumed << " produced " << status.fmqProduced; + process(); } } diff --git a/audio/aidl/default/acousticEchoCanceler/AcousticEchoCancelerSw.cpp b/audio/aidl/default/acousticEchoCanceler/AcousticEchoCancelerSw.cpp index 5e18f1b12f..be0927c457 100644 --- a/audio/aidl/default/acousticEchoCanceler/AcousticEchoCancelerSw.cpp +++ b/audio/aidl/default/acousticEchoCanceler/AcousticEchoCancelerSw.cpp @@ -168,10 +168,6 @@ std::shared_ptr AcousticEchoCancelerSw::createContext( return mContext; } -std::shared_ptr AcousticEchoCancelerSw::getContext() { - return mContext; -} - RetCode AcousticEchoCancelerSw::releaseContext() { if (mContext) { mContext.reset(); diff --git a/audio/aidl/default/acousticEchoCanceler/AcousticEchoCancelerSw.h b/audio/aidl/default/acousticEchoCanceler/AcousticEchoCancelerSw.h index 73cf42b17a..95738f8dd1 100644 --- a/audio/aidl/default/acousticEchoCanceler/AcousticEchoCancelerSw.h +++ b/audio/aidl/default/acousticEchoCanceler/AcousticEchoCancelerSw.h @@ -52,21 +52,23 @@ class AcousticEchoCancelerSw final : public EffectImpl { } ndk::ScopedAStatus getDescriptor(Descriptor* _aidl_return) override; - ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) override; - ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, - Parameter::Specific* specific) override; + ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) + REQUIRES(mImplMutex) override; + ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, Parameter::Specific* specific) + REQUIRES(mImplMutex) override; - std::shared_ptr createContext(const Parameter::Common& common) override; - std::shared_ptr getContext() override; - RetCode releaseContext() override; + std::shared_ptr createContext(const Parameter::Common& common) + REQUIRES(mImplMutex) override; + RetCode releaseContext() REQUIRES(mImplMutex) override; std::string getEffectName() override { return kEffectName; }; IEffect::Status effectProcessImpl(float* in, float* out, int samples) override; private: static const std::vector kRanges; - std::shared_ptr mContext; + std::shared_ptr mContext GUARDED_BY(mImplMutex); ndk::ScopedAStatus getParameterAcousticEchoCanceler(const AcousticEchoCanceler::Tag& tag, - Parameter::Specific* specific); + Parameter::Specific* specific) + REQUIRES(mImplMutex); }; } // namespace aidl::android::hardware::audio::effect diff --git a/audio/aidl/default/automaticGainControlV1/AutomaticGainControlV1Sw.cpp b/audio/aidl/default/automaticGainControlV1/AutomaticGainControlV1Sw.cpp index ce10ae1674..d865b7e95a 100644 --- a/audio/aidl/default/automaticGainControlV1/AutomaticGainControlV1Sw.cpp +++ b/audio/aidl/default/automaticGainControlV1/AutomaticGainControlV1Sw.cpp @@ -177,10 +177,6 @@ std::shared_ptr AutomaticGainControlV1Sw::createContext( return mContext; } -std::shared_ptr AutomaticGainControlV1Sw::getContext() { - return mContext; -} - RetCode AutomaticGainControlV1Sw::releaseContext() { if (mContext) { mContext.reset(); diff --git a/audio/aidl/default/automaticGainControlV1/AutomaticGainControlV1Sw.h b/audio/aidl/default/automaticGainControlV1/AutomaticGainControlV1Sw.h index 7d2a69f9d5..76b91aef31 100644 --- a/audio/aidl/default/automaticGainControlV1/AutomaticGainControlV1Sw.h +++ b/audio/aidl/default/automaticGainControlV1/AutomaticGainControlV1Sw.h @@ -53,21 +53,24 @@ class AutomaticGainControlV1Sw final : public EffectImpl { } ndk::ScopedAStatus getDescriptor(Descriptor* _aidl_return) override; - ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) override; - ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, - Parameter::Specific* specific) override; + ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) + REQUIRES(mImplMutex) override; + ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, Parameter::Specific* specific) + REQUIRES(mImplMutex) override; - std::shared_ptr createContext(const Parameter::Common& common) override; - std::shared_ptr getContext() override; - RetCode releaseContext() override; + std::shared_ptr createContext(const Parameter::Common& common) + REQUIRES(mImplMutex) override; + RetCode releaseContext() REQUIRES(mImplMutex) override; std::string getEffectName() override { return kEffectName; }; - IEffect::Status effectProcessImpl(float* in, float* out, int samples) override; + IEffect::Status effectProcessImpl(float* in, float* out, int samples) + REQUIRES(mImplMutex) override; private: static const std::vector kRanges; - std::shared_ptr mContext; + std::shared_ptr mContext GUARDED_BY(mImplMutex); ndk::ScopedAStatus getParameterAutomaticGainControlV1(const AutomaticGainControlV1::Tag& tag, - Parameter::Specific* specific); + Parameter::Specific* specific) + REQUIRES(mImplMutex); }; } // namespace aidl::android::hardware::audio::effect diff --git a/audio/aidl/default/automaticGainControlV2/AutomaticGainControlV2Sw.cpp b/audio/aidl/default/automaticGainControlV2/AutomaticGainControlV2Sw.cpp index 1e336ac012..3ff6e38cdd 100644 --- a/audio/aidl/default/automaticGainControlV2/AutomaticGainControlV2Sw.cpp +++ b/audio/aidl/default/automaticGainControlV2/AutomaticGainControlV2Sw.cpp @@ -180,10 +180,6 @@ std::shared_ptr AutomaticGainControlV2Sw::createContext( return mContext; } -std::shared_ptr AutomaticGainControlV2Sw::getContext() { - return mContext; -} - RetCode AutomaticGainControlV2Sw::releaseContext() { if (mContext) { mContext.reset(); diff --git a/audio/aidl/default/automaticGainControlV2/AutomaticGainControlV2Sw.h b/audio/aidl/default/automaticGainControlV2/AutomaticGainControlV2Sw.h index 9aa60eab60..863d470f90 100644 --- a/audio/aidl/default/automaticGainControlV2/AutomaticGainControlV2Sw.h +++ b/audio/aidl/default/automaticGainControlV2/AutomaticGainControlV2Sw.h @@ -59,21 +59,24 @@ class AutomaticGainControlV2Sw final : public EffectImpl { } ndk::ScopedAStatus getDescriptor(Descriptor* _aidl_return) override; - ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) override; - ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, - Parameter::Specific* specific) override; + ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) + REQUIRES(mImplMutex) override; + ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, Parameter::Specific* specific) + REQUIRES(mImplMutex) override; - std::shared_ptr createContext(const Parameter::Common& common) override; - std::shared_ptr getContext() override; - RetCode releaseContext() override; + std::shared_ptr createContext(const Parameter::Common& common) + REQUIRES(mImplMutex) override; + RetCode releaseContext() REQUIRES(mImplMutex) override; std::string getEffectName() override { return kEffectName; }; - IEffect::Status effectProcessImpl(float* in, float* out, int samples) override; + IEffect::Status effectProcessImpl(float* in, float* out, int samples) + REQUIRES(mImplMutex) override; private: static const std::vector kRanges; - std::shared_ptr mContext; + std::shared_ptr mContext GUARDED_BY(mImplMutex); ndk::ScopedAStatus getParameterAutomaticGainControlV2(const AutomaticGainControlV2::Tag& tag, - Parameter::Specific* specific); + Parameter::Specific* specific) + REQUIRES(mImplMutex); }; } // namespace aidl::android::hardware::audio::effect diff --git a/audio/aidl/default/bassboost/BassBoostSw.cpp b/audio/aidl/default/bassboost/BassBoostSw.cpp index 6072d896d9..60adc3014d 100644 --- a/audio/aidl/default/bassboost/BassBoostSw.cpp +++ b/audio/aidl/default/bassboost/BassBoostSw.cpp @@ -151,10 +151,6 @@ std::shared_ptr BassBoostSw::createContext(const Parameter::Commo return mContext; } -std::shared_ptr BassBoostSw::getContext() { - return mContext; -} - RetCode BassBoostSw::releaseContext() { if (mContext) { mContext.reset(); diff --git a/audio/aidl/default/bassboost/BassBoostSw.h b/audio/aidl/default/bassboost/BassBoostSw.h index 11324727a7..901e4551b8 100644 --- a/audio/aidl/default/bassboost/BassBoostSw.h +++ b/audio/aidl/default/bassboost/BassBoostSw.h @@ -51,21 +51,23 @@ class BassBoostSw final : public EffectImpl { } ndk::ScopedAStatus getDescriptor(Descriptor* _aidl_return) override; - ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) override; - ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, - Parameter::Specific* specific) override; + ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) + REQUIRES(mImplMutex) override; + ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, Parameter::Specific* specific) + REQUIRES(mImplMutex) override; - std::shared_ptr createContext(const Parameter::Common& common) override; - std::shared_ptr getContext() override; - RetCode releaseContext() override; + std::shared_ptr createContext(const Parameter::Common& common) + REQUIRES(mImplMutex) override; + RetCode releaseContext() REQUIRES(mImplMutex) override; std::string getEffectName() override { return kEffectName; }; - IEffect::Status effectProcessImpl(float* in, float* out, int samples) override; + IEffect::Status effectProcessImpl(float* in, float* out, int samples) + REQUIRES(mImplMutex) override; private: static const std::vector kRanges; - std::shared_ptr mContext; + std::shared_ptr mContext GUARDED_BY(mImplMutex); ndk::ScopedAStatus getParameterBassBoost(const BassBoost::Tag& tag, - Parameter::Specific* specific); + Parameter::Specific* specific) REQUIRES(mImplMutex); }; } // namespace aidl::android::hardware::audio::effect diff --git a/audio/aidl/default/downmix/DownmixSw.cpp b/audio/aidl/default/downmix/DownmixSw.cpp index ce5fe20817..19ab2e8f6a 100644 --- a/audio/aidl/default/downmix/DownmixSw.cpp +++ b/audio/aidl/default/downmix/DownmixSw.cpp @@ -144,10 +144,6 @@ std::shared_ptr DownmixSw::createContext(const Parameter::Common& return mContext; } -std::shared_ptr DownmixSw::getContext() { - return mContext; -} - RetCode DownmixSw::releaseContext() { if (mContext) { mContext.reset(); diff --git a/audio/aidl/default/downmix/DownmixSw.h b/audio/aidl/default/downmix/DownmixSw.h index 3f8a09b16c..1a9f0f01b0 100644 --- a/audio/aidl/default/downmix/DownmixSw.h +++ b/audio/aidl/default/downmix/DownmixSw.h @@ -55,20 +55,23 @@ class DownmixSw final : public EffectImpl { } ndk::ScopedAStatus getDescriptor(Descriptor* _aidl_return) override; - ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) override; - ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, - Parameter::Specific* specific) override; + ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) + REQUIRES(mImplMutex) override; + ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, Parameter::Specific* specific) + REQUIRES(mImplMutex) override; - std::shared_ptr createContext(const Parameter::Common& common) override; - std::shared_ptr getContext() override; - RetCode releaseContext() override; + std::shared_ptr createContext(const Parameter::Common& common) + REQUIRES(mImplMutex) override; + RetCode releaseContext() REQUIRES(mImplMutex) override; std::string getEffectName() override { return kEffectName; }; - IEffect::Status effectProcessImpl(float* in, float* out, int sample) override; + IEffect::Status effectProcessImpl(float* in, float* out, int sample) + REQUIRES(mImplMutex) override; private: - std::shared_ptr mContext; + std::shared_ptr mContext GUARDED_BY(mImplMutex); - ndk::ScopedAStatus getParameterDownmix(const Downmix::Tag& tag, Parameter::Specific* specific); + ndk::ScopedAStatus getParameterDownmix(const Downmix::Tag& tag, Parameter::Specific* specific) + REQUIRES(mImplMutex); }; } // namespace aidl::android::hardware::audio::effect diff --git a/audio/aidl/default/dynamicProcessing/DynamicsProcessingSw.cpp b/audio/aidl/default/dynamicProcessing/DynamicsProcessingSw.cpp index e8f90b216b..36face148b 100644 --- a/audio/aidl/default/dynamicProcessing/DynamicsProcessingSw.cpp +++ b/audio/aidl/default/dynamicProcessing/DynamicsProcessingSw.cpp @@ -260,10 +260,6 @@ std::shared_ptr DynamicsProcessingSw::createContext( return mContext; } -std::shared_ptr DynamicsProcessingSw::getContext() { - return mContext; -} - RetCode DynamicsProcessingSw::releaseContext() { if (mContext) { mContext.reset(); @@ -282,6 +278,9 @@ IEffect::Status DynamicsProcessingSw::effectProcessImpl(float* in, float* out, i } RetCode DynamicsProcessingSwContext::setCommon(const Parameter::Common& common) { + if (auto ret = updateIOFrameSize(common); ret != RetCode::SUCCESS) { + return ret; + } mCommon = common; mChannelCount = ::aidl::android::hardware::audio::common::getChannelCount( common.input.base.channelMask); diff --git a/audio/aidl/default/dynamicProcessing/DynamicsProcessingSw.h b/audio/aidl/default/dynamicProcessing/DynamicsProcessingSw.h index 641cf71f68..98edca088c 100644 --- a/audio/aidl/default/dynamicProcessing/DynamicsProcessingSw.h +++ b/audio/aidl/default/dynamicProcessing/DynamicsProcessingSw.h @@ -113,15 +113,17 @@ class DynamicsProcessingSw final : public EffectImpl { } ndk::ScopedAStatus getDescriptor(Descriptor* _aidl_return) override; - ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) override; - ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, - Parameter::Specific* specific) override; + ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) + REQUIRES(mImplMutex) override; + ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, Parameter::Specific* specific) + REQUIRES(mImplMutex) override; - std::shared_ptr createContext(const Parameter::Common& common) override; - std::shared_ptr getContext() override; - RetCode releaseContext() override; + std::shared_ptr createContext(const Parameter::Common& common) + REQUIRES(mImplMutex) override; + RetCode releaseContext() REQUIRES(mImplMutex) override; - IEffect::Status effectProcessImpl(float* in, float* out, int samples) override; + IEffect::Status effectProcessImpl(float* in, float* out, int samples) + REQUIRES(mImplMutex) override; std::string getEffectName() override { return kEffectName; }; private: @@ -130,9 +132,10 @@ class DynamicsProcessingSw final : public EffectImpl { static const Range::DynamicsProcessingRange kPreEqBandRange; static const Range::DynamicsProcessingRange kPostEqBandRange; static const std::vector kRanges; - std::shared_ptr mContext; + std::shared_ptr mContext GUARDED_BY(mImplMutex); ndk::ScopedAStatus getParameterDynamicsProcessing(const DynamicsProcessing::Tag& tag, - Parameter::Specific* specific); + Parameter::Specific* specific) + REQUIRES(mImplMutex); }; // DynamicsProcessingSw diff --git a/audio/aidl/default/envReverb/EnvReverbSw.cpp b/audio/aidl/default/envReverb/EnvReverbSw.cpp index 73975c6996..7937a6a247 100644 --- a/audio/aidl/default/envReverb/EnvReverbSw.cpp +++ b/audio/aidl/default/envReverb/EnvReverbSw.cpp @@ -267,10 +267,6 @@ std::shared_ptr EnvReverbSw::createContext(const Parameter::Commo return mContext; } -std::shared_ptr EnvReverbSw::getContext() { - return mContext; -} - RetCode EnvReverbSw::releaseContext() { if (mContext) { mContext.reset(); diff --git a/audio/aidl/default/envReverb/EnvReverbSw.h b/audio/aidl/default/envReverb/EnvReverbSw.h index 5e31e2f8b2..367462b7da 100644 --- a/audio/aidl/default/envReverb/EnvReverbSw.h +++ b/audio/aidl/default/envReverb/EnvReverbSw.h @@ -100,21 +100,23 @@ class EnvReverbSw final : public EffectImpl { } ndk::ScopedAStatus getDescriptor(Descriptor* _aidl_return) override; - ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) override; - ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, - Parameter::Specific* specific) override; + ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) + REQUIRES(mImplMutex) override; + ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, Parameter::Specific* specific) + REQUIRES(mImplMutex) override; - std::shared_ptr createContext(const Parameter::Common& common) override; - std::shared_ptr getContext() override; - RetCode releaseContext() override; + std::shared_ptr createContext(const Parameter::Common& common) + REQUIRES(mImplMutex) override; + RetCode releaseContext() REQUIRES(mImplMutex) override; IEffect::Status effectProcessImpl(float* in, float* out, int samples) override; std::string getEffectName() override { return kEffectName; } private: static const std::vector kRanges; - std::shared_ptr mContext; + std::shared_ptr mContext GUARDED_BY(mImplMutex); ndk::ScopedAStatus getParameterEnvironmentalReverb(const EnvironmentalReverb::Tag& tag, - Parameter::Specific* specific); + Parameter::Specific* specific) + REQUIRES(mImplMutex); }; } // namespace aidl::android::hardware::audio::effect diff --git a/audio/aidl/default/equalizer/EqualizerSw.cpp b/audio/aidl/default/equalizer/EqualizerSw.cpp index b2add3113c..640b3ba95e 100644 --- a/audio/aidl/default/equalizer/EqualizerSw.cpp +++ b/audio/aidl/default/equalizer/EqualizerSw.cpp @@ -198,10 +198,6 @@ std::shared_ptr EqualizerSw::createContext(const Parameter::Commo return mContext; } -std::shared_ptr EqualizerSw::getContext() { - return mContext; -} - RetCode EqualizerSw::releaseContext() { if (mContext) { mContext.reset(); diff --git a/audio/aidl/default/equalizer/EqualizerSw.h b/audio/aidl/default/equalizer/EqualizerSw.h index 56af3b5821..caaa129501 100644 --- a/audio/aidl/default/equalizer/EqualizerSw.h +++ b/audio/aidl/default/equalizer/EqualizerSw.h @@ -97,15 +97,17 @@ class EqualizerSw final : public EffectImpl { } ndk::ScopedAStatus getDescriptor(Descriptor* _aidl_return) override; - ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) override; - ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, - Parameter::Specific* specific) override; + ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) + REQUIRES(mImplMutex) override; + ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, Parameter::Specific* specific) + REQUIRES(mImplMutex) override; - std::shared_ptr createContext(const Parameter::Common& common) override; - std::shared_ptr getContext() override; - RetCode releaseContext() override; + std::shared_ptr createContext(const Parameter::Common& common) + REQUIRES(mImplMutex) override; + RetCode releaseContext() REQUIRES(mImplMutex) override; - IEffect::Status effectProcessImpl(float* in, float* out, int samples) override; + IEffect::Status effectProcessImpl(float* in, float* out, int samples) + REQUIRES(mImplMutex) override; std::string getEffectName() override { return kEffectName; } private: @@ -113,7 +115,7 @@ class EqualizerSw final : public EffectImpl { static const std::vector kPresets; static const std::vector kRanges; ndk::ScopedAStatus getParameterEqualizer(const Equalizer::Tag& tag, - Parameter::Specific* specific); + Parameter::Specific* specific) REQUIRES(mImplMutex); std::shared_ptr mContext; }; diff --git a/audio/aidl/default/extension/ExtensionEffect.cpp b/audio/aidl/default/extension/ExtensionEffect.cpp index 4a4d71b687..11916c88d2 100644 --- a/audio/aidl/default/extension/ExtensionEffect.cpp +++ b/audio/aidl/default/extension/ExtensionEffect.cpp @@ -123,10 +123,6 @@ std::shared_ptr ExtensionEffect::createContext(const Parameter::C return mContext; } -std::shared_ptr ExtensionEffect::getContext() { - return mContext; -} - RetCode ExtensionEffect::releaseContext() { if (mContext) { mContext.reset(); diff --git a/audio/aidl/default/extension/ExtensionEffect.h b/audio/aidl/default/extension/ExtensionEffect.h index e7a068ba46..b56086068c 100644 --- a/audio/aidl/default/extension/ExtensionEffect.h +++ b/audio/aidl/default/extension/ExtensionEffect.h @@ -54,18 +54,20 @@ class ExtensionEffect final : public EffectImpl { } ndk::ScopedAStatus getDescriptor(Descriptor* _aidl_return) override; - ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) override; - ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, - Parameter::Specific* specific) override; + ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) + REQUIRES(mImplMutex) override; + ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, Parameter::Specific* specific) + REQUIRES(mImplMutex) override; - std::shared_ptr createContext(const Parameter::Common& common) override; - std::shared_ptr getContext() override; - RetCode releaseContext() override; + std::shared_ptr createContext(const Parameter::Common& common) + REQUIRES(mImplMutex) override; + RetCode releaseContext() REQUIRES(mImplMutex) override; std::string getEffectName() override { return kEffectName; }; - IEffect::Status effectProcessImpl(float* in, float* out, int samples) override; + IEffect::Status effectProcessImpl(float* in, float* out, int samples) + REQUIRES(mImplMutex) override; private: - std::shared_ptr mContext; + std::shared_ptr mContext GUARDED_BY(mImplMutex); }; } // namespace aidl::android::hardware::audio::effect diff --git a/audio/aidl/default/hapticGenerator/HapticGeneratorSw.cpp b/audio/aidl/default/hapticGenerator/HapticGeneratorSw.cpp index 27cdac8086..7469ab930e 100644 --- a/audio/aidl/default/hapticGenerator/HapticGeneratorSw.cpp +++ b/audio/aidl/default/hapticGenerator/HapticGeneratorSw.cpp @@ -158,10 +158,6 @@ std::shared_ptr HapticGeneratorSw::createContext(const Parameter: return mContext; } -std::shared_ptr HapticGeneratorSw::getContext() { - return mContext; -} - RetCode HapticGeneratorSw::releaseContext() { if (mContext) { mContext.reset(); diff --git a/audio/aidl/default/hapticGenerator/HapticGeneratorSw.h b/audio/aidl/default/hapticGenerator/HapticGeneratorSw.h index 3bbe41ad29..47f3848796 100644 --- a/audio/aidl/default/hapticGenerator/HapticGeneratorSw.h +++ b/audio/aidl/default/hapticGenerator/HapticGeneratorSw.h @@ -67,21 +67,24 @@ class HapticGeneratorSw final : public EffectImpl { } ndk::ScopedAStatus getDescriptor(Descriptor* _aidl_return) override; - ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) override; - ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, - Parameter::Specific* specific) override; + ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) + REQUIRES(mImplMutex) override; + ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, Parameter::Specific* specific) + REQUIRES(mImplMutex) override; - std::shared_ptr createContext(const Parameter::Common& common) override; - std::shared_ptr getContext() override; - RetCode releaseContext() override; + std::shared_ptr createContext(const Parameter::Common& common) + REQUIRES(mImplMutex) override; + RetCode releaseContext() REQUIRES(mImplMutex) override; - IEffect::Status effectProcessImpl(float* in, float* out, int samples) override; + IEffect::Status effectProcessImpl(float* in, float* out, int samples) + REQUIRES(mImplMutex) override; std::string getEffectName() override { return kEffectName; } private: - std::shared_ptr mContext; + std::shared_ptr mContext GUARDED_BY(mImplMutex); ndk::ScopedAStatus getParameterHapticGenerator(const HapticGenerator::Tag& tag, - Parameter::Specific* specific); + Parameter::Specific* specific) + REQUIRES(mImplMutex); }; } // namespace aidl::android::hardware::audio::effect diff --git a/audio/aidl/default/include/effect-impl/EffectContext.h b/audio/aidl/default/include/effect-impl/EffectContext.h index 89d0c7cf3c..24f3b5dcf2 100644 --- a/audio/aidl/default/include/effect-impl/EffectContext.h +++ b/audio/aidl/default/include/effect-impl/EffectContext.h @@ -21,6 +21,7 @@ #include #include #include +#include #include #include "EffectTypes.h" @@ -36,127 +37,73 @@ class EffectContext { float, ::aidl::android::hardware::common::fmq::SynchronizedReadWrite> DataMQ; - EffectContext(size_t statusDepth, const Parameter::Common& common) { - auto& input = common.input; - auto& output = common.output; - - LOG_ALWAYS_FATAL_IF( - input.base.format.pcm != aidl::android::media::audio::common::PcmType::FLOAT_32_BIT, - "inputFormatNotFloat"); - LOG_ALWAYS_FATAL_IF(output.base.format.pcm != - aidl::android::media::audio::common::PcmType::FLOAT_32_BIT, - "outputFormatNotFloat"); - - size_t inputChannelCount = - ::aidl::android::hardware::audio::common::getChannelCount(input.base.channelMask); - LOG_ALWAYS_FATAL_IF(inputChannelCount == 0, "inputChannelCountNotValid"); - size_t outputChannelCount = - ::aidl::android::hardware::audio::common::getChannelCount(output.base.channelMask); - LOG_ALWAYS_FATAL_IF(outputChannelCount == 0, "outputChannelCountNotValid"); - - mInputFrameSize = ::aidl::android::hardware::audio::common::getFrameSizeInBytes( - input.base.format, input.base.channelMask); - mOutputFrameSize = ::aidl::android::hardware::audio::common::getFrameSizeInBytes( - output.base.format, output.base.channelMask); - // in/outBuffer size in float (FMQ data format defined for DataMQ) - size_t inBufferSizeInFloat = input.frameCount * mInputFrameSize / sizeof(float); - size_t outBufferSizeInFloat = output.frameCount * mOutputFrameSize / sizeof(float); - - // only status FMQ use the EventFlag - mStatusMQ = std::make_shared(statusDepth, true /*configureEventFlagWord*/); - mInputMQ = std::make_shared(inBufferSizeInFloat); - mOutputMQ = std::make_shared(outBufferSizeInFloat); - - if (!mStatusMQ->isValid() || !mInputMQ->isValid() || !mOutputMQ->isValid()) { - LOG(ERROR) << __func__ << " created invalid FMQ"; + EffectContext(size_t statusDepth, const Parameter::Common& common); + virtual ~EffectContext() { + if (mEfGroup) { + ::android::hardware::EventFlag::deleteEventFlag(&mEfGroup); } - mWorkBuffer.reserve(std::max(inBufferSizeInFloat, outBufferSizeInFloat)); - mCommon = common; } - virtual ~EffectContext() {} - std::shared_ptr getStatusFmq() { return mStatusMQ; } - std::shared_ptr getInputDataFmq() { return mInputMQ; } - std::shared_ptr getOutputDataFmq() { return mOutputMQ; } + std::shared_ptr getStatusFmq() const; + std::shared_ptr getInputDataFmq() const; + std::shared_ptr getOutputDataFmq() const; - float* getWorkBuffer() { return static_cast(mWorkBuffer.data()); } + float* getWorkBuffer(); // reset buffer status by abandon input data in FMQ - void resetBuffer() { - auto buffer = static_cast(mWorkBuffer.data()); - std::vector status(mStatusMQ->availableToRead()); - mInputMQ->read(buffer, mInputMQ->availableToRead()); - } + void resetBuffer(); + void dupeFmq(IEffect::OpenEffectReturn* effectRet); + size_t getInputFrameSize() const; + size_t getOutputFrameSize() const; + int getSessionId() const; + int getIoHandle() const; - void dupeFmq(IEffect::OpenEffectReturn* effectRet) { - if (effectRet) { - effectRet->statusMQ = mStatusMQ->dupeDesc(); - effectRet->inputDataMQ = mInputMQ->dupeDesc(); - effectRet->outputDataMQ = mOutputMQ->dupeDesc(); - } - } - size_t getInputFrameSize() { return mInputFrameSize; } - size_t getOutputFrameSize() { return mOutputFrameSize; } - int getSessionId() { return mCommon.session; } - int getIoHandle() { return mCommon.ioHandle; } + virtual void dupeFmqWithReopen(IEffect::OpenEffectReturn* effectRet); virtual RetCode setOutputDevice( - const std::vector& - device) { - mOutputDevice = device; - return RetCode::SUCCESS; - } + const std::vector& device); virtual std::vector - getOutputDevice() { - return mOutputDevice; - } + getOutputDevice(); - virtual RetCode setAudioMode(const aidl::android::media::audio::common::AudioMode& mode) { - mMode = mode; - return RetCode::SUCCESS; - } - virtual aidl::android::media::audio::common::AudioMode getAudioMode() { return mMode; } + virtual RetCode setAudioMode(const aidl::android::media::audio::common::AudioMode& mode); + virtual aidl::android::media::audio::common::AudioMode getAudioMode(); - virtual RetCode setAudioSource(const aidl::android::media::audio::common::AudioSource& source) { - mSource = source; - return RetCode::SUCCESS; - } - virtual aidl::android::media::audio::common::AudioSource getAudioSource() { return mSource; } + virtual RetCode setAudioSource(const aidl::android::media::audio::common::AudioSource& source); + virtual aidl::android::media::audio::common::AudioSource getAudioSource(); - virtual RetCode setVolumeStereo(const Parameter::VolumeStereo& volumeStereo) { - mVolumeStereo = volumeStereo; - return RetCode::SUCCESS; - } - virtual Parameter::VolumeStereo getVolumeStereo() { return mVolumeStereo; } + virtual RetCode setVolumeStereo(const Parameter::VolumeStereo& volumeStereo); + virtual Parameter::VolumeStereo getVolumeStereo(); - virtual RetCode setCommon(const Parameter::Common& common) { - mCommon = common; - LOG(VERBOSE) << __func__ << mCommon.toString(); - return RetCode::SUCCESS; - } - virtual Parameter::Common getCommon() { - LOG(VERBOSE) << __func__ << mCommon.toString(); - return mCommon; - } + virtual RetCode setCommon(const Parameter::Common& common); + virtual Parameter::Common getCommon(); + + virtual ::android::hardware::EventFlag* getStatusEventFlag(); protected: - // common parameters size_t mInputFrameSize; size_t mOutputFrameSize; - Parameter::Common mCommon; - std::vector mOutputDevice; - aidl::android::media::audio::common::AudioMode mMode; - aidl::android::media::audio::common::AudioSource mSource; - Parameter::VolumeStereo mVolumeStereo; + size_t mInputChannelCount; + size_t mOutputChannelCount; + Parameter::Common mCommon = {}; + std::vector mOutputDevice = {}; + aidl::android::media::audio::common::AudioMode mMode = + aidl::android::media::audio::common::AudioMode::SYS_RESERVED_INVALID; + aidl::android::media::audio::common::AudioSource mSource = + aidl::android::media::audio::common::AudioSource::SYS_RESERVED_INVALID; + Parameter::VolumeStereo mVolumeStereo = {}; + RetCode updateIOFrameSize(const Parameter::Common& common); + RetCode notifyDataMqUpdate(); private: // fmq and buffers std::shared_ptr mStatusMQ; std::shared_ptr mInputMQ; std::shared_ptr mOutputMQ; - // TODO handle effect process input and output + // std::shared_ptr mRet; // work buffer set by effect instances, the access and update are in same thread std::vector mWorkBuffer; + + ::android::hardware::EventFlag* mEfGroup; }; } // namespace aidl::android::hardware::audio::effect diff --git a/audio/aidl/default/include/effect-impl/EffectImpl.h b/audio/aidl/default/include/effect-impl/EffectImpl.h index 242a268912..21f6502385 100644 --- a/audio/aidl/default/include/effect-impl/EffectImpl.h +++ b/audio/aidl/default/include/effect-impl/EffectImpl.h @@ -49,33 +49,54 @@ class EffectImpl : public BnEffect, public EffectThread { virtual ndk::ScopedAStatus setParameter(const Parameter& param) override; virtual ndk::ScopedAStatus getParameter(const Parameter::Id& id, Parameter* param) override; - virtual ndk::ScopedAStatus setParameterCommon(const Parameter& param); - virtual ndk::ScopedAStatus getParameterCommon(const Parameter::Tag& tag, Parameter* param); + virtual ndk::ScopedAStatus setParameterCommon(const Parameter& param) REQUIRES(mImplMutex); + virtual ndk::ScopedAStatus getParameterCommon(const Parameter::Tag& tag, Parameter* param) + REQUIRES(mImplMutex); /* Methods MUST be implemented by each effect instances */ virtual ndk::ScopedAStatus getDescriptor(Descriptor* desc) = 0; - virtual ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) = 0; + virtual ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) + REQUIRES(mImplMutex) = 0; virtual ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, - Parameter::Specific* specific) = 0; + Parameter::Specific* specific) + REQUIRES(mImplMutex) = 0; virtual std::string getEffectName() = 0; - virtual IEffect::Status effectProcessImpl(float* in, float* out, int samples) override; + virtual std::shared_ptr createContext(const Parameter::Common& common) + REQUIRES(mImplMutex); + virtual RetCode releaseContext() REQUIRES(mImplMutex) = 0; /** - * Effect context methods must be implemented by each effect. - * Each effect can derive from EffectContext and define its own context, but must upcast to - * EffectContext for EffectImpl to use. + * @brief effectProcessImpl is running in worker thread which created in EffectThread. + * + * EffectThread will make sure effectProcessImpl only be called after startThread() successful + * and before stopThread() successful. + * + * effectProcessImpl implementation must not call any EffectThread interface, otherwise it will + * cause deadlock. + * + * @param in address of input float buffer. + * @param out address of output float buffer. + * @param samples number of samples to process. + * @return IEffect::Status */ - virtual std::shared_ptr createContext(const Parameter::Common& common) = 0; - virtual std::shared_ptr getContext() = 0; - virtual RetCode releaseContext() = 0; + virtual IEffect::Status effectProcessImpl(float* in, float* out, int samples) = 0; + + /** + * process() get data from data MQs, and call effectProcessImpl() for effect data processing. + * Its important for the implementation to use mImplMutex for context synchronization. + */ + void process() override; protected: - State mState = State::INIT; + State mState GUARDED_BY(mImplMutex) = State::INIT; IEffect::Status status(binder_status_t status, size_t consumed, size_t produced); void cleanUp(); + std::mutex mImplMutex; + std::shared_ptr mImplContext GUARDED_BY(mImplMutex); + /** * Optional CommandId handling methods for effects to override. * For CommandId::START, EffectImpl call commandImpl before starting the EffectThread @@ -83,6 +104,9 @@ class EffectImpl : public BnEffect, public EffectThread { * For CommandId::STOP and CommandId::RESET, EffectImpl call commandImpl after stop the * EffectThread processing. */ - virtual ndk::ScopedAStatus commandImpl(CommandId id); + virtual ndk::ScopedAStatus commandImpl(CommandId id) REQUIRES(mImplMutex); + + RetCode notifyEventFlag(uint32_t flag); + ::android::hardware::EventFlag* mEventFlag; }; } // namespace aidl::android::hardware::audio::effect diff --git a/audio/aidl/default/include/effect-impl/EffectThread.h b/audio/aidl/default/include/effect-impl/EffectThread.h index ae51ef70ab..3dbb0e6798 100644 --- a/audio/aidl/default/include/effect-impl/EffectThread.h +++ b/audio/aidl/default/include/effect-impl/EffectThread.h @@ -36,8 +36,7 @@ class EffectThread { virtual ~EffectThread(); // called by effect implementation. - RetCode createThread(std::shared_ptr context, const std::string& name, - int priority = ANDROID_PRIORITY_URGENT_AUDIO); + RetCode createThread(const std::string& name, int priority = ANDROID_PRIORITY_URGENT_AUDIO); RetCode destroyThread(); RetCode startThread(); RetCode stopThread(); @@ -45,33 +44,12 @@ class EffectThread { // Will call process() in a loop if the thread is running. void threadLoop(); - /** - * @brief effectProcessImpl is running in worker thread which created in EffectThread. - * - * Effect implementation should think about concurrency in the implementation if necessary. - * Parameter setting usually implemented in context (derived from EffectContext), and some - * parameter maybe used in the processing, then effect implementation should consider using a - * mutex to protect these parameter. - * - * EffectThread will make sure effectProcessImpl only be called after startThread() successful - * and before stopThread() successful. - * - * effectProcessImpl implementation must not call any EffectThread interface, otherwise it will - * cause deadlock. - * - * @param in address of input float buffer. - * @param out address of output float buffer. - * @param samples number of samples to process. - * @return IEffect::Status - */ - virtual IEffect::Status effectProcessImpl(float* in, float* out, int samples) = 0; - /** * process() call effectProcessImpl() for effect data processing, it is necessary for the * processing to be called under Effect thread mutex mThreadMutex, to avoid the effect state * change before/during data processing, and keep the thread and effect state consistent. */ - virtual void process_l() REQUIRES(mThreadMutex); + virtual void process() = 0; private: static constexpr int kMaxTaskNameLen = 15; @@ -80,16 +58,7 @@ class EffectThread { std::condition_variable mCv; bool mStop GUARDED_BY(mThreadMutex) = true; bool mExit GUARDED_BY(mThreadMutex) = false; - std::shared_ptr mThreadContext GUARDED_BY(mThreadMutex); - struct EventFlagDeleter { - void operator()(::android::hardware::EventFlag* flag) const { - if (flag) { - ::android::hardware::EventFlag::deleteEventFlag(&flag); - } - } - }; - std::unique_ptr<::android::hardware::EventFlag, EventFlagDeleter> mEfGroup; std::thread mThread; int mPriority; std::string mName; diff --git a/audio/aidl/default/include/effect-impl/EffectTypes.h b/audio/aidl/default/include/effect-impl/EffectTypes.h index 4bda7bea21..9740d6ee9c 100644 --- a/audio/aidl/default/include/effect-impl/EffectTypes.h +++ b/audio/aidl/default/include/effect-impl/EffectTypes.h @@ -46,7 +46,8 @@ enum class RetCode { ERROR_NULL_POINTER, /* NULL pointer */ ERROR_ALIGNMENT_ERROR, /* Memory alignment error */ ERROR_BLOCK_SIZE_EXCEED, /* Maximum block size exceeded */ - ERROR_EFFECT_LIB_ERROR + ERROR_EFFECT_LIB_ERROR, /* Effect implementation library error */ + ERROR_EVENT_FLAG_ERROR /* Error with effect event flags */ }; static const int INVALID_AUDIO_SESSION_ID = -1; @@ -67,6 +68,8 @@ inline std::ostream& operator<<(std::ostream& out, const RetCode& code) { return out << "ERROR_BLOCK_SIZE_EXCEED"; case RetCode::ERROR_EFFECT_LIB_ERROR: return out << "ERROR_EFFECT_LIB_ERROR"; + case RetCode::ERROR_EVENT_FLAG_ERROR: + return out << "ERROR_EVENT_FLAG_ERROR"; } return out << "EnumError: " << code; diff --git a/audio/aidl/default/loudnessEnhancer/LoudnessEnhancerSw.cpp b/audio/aidl/default/loudnessEnhancer/LoudnessEnhancerSw.cpp index 7954316654..1e70716a96 100644 --- a/audio/aidl/default/loudnessEnhancer/LoudnessEnhancerSw.cpp +++ b/audio/aidl/default/loudnessEnhancer/LoudnessEnhancerSw.cpp @@ -147,10 +147,6 @@ std::shared_ptr LoudnessEnhancerSw::createContext(const Parameter return mContext; } -std::shared_ptr LoudnessEnhancerSw::getContext() { - return mContext; -} - RetCode LoudnessEnhancerSw::releaseContext() { if (mContext) { mContext.reset(); diff --git a/audio/aidl/default/loudnessEnhancer/LoudnessEnhancerSw.h b/audio/aidl/default/loudnessEnhancer/LoudnessEnhancerSw.h index 25824f20d5..cf71a5f03f 100644 --- a/audio/aidl/default/loudnessEnhancer/LoudnessEnhancerSw.h +++ b/audio/aidl/default/loudnessEnhancer/LoudnessEnhancerSw.h @@ -54,20 +54,23 @@ class LoudnessEnhancerSw final : public EffectImpl { } ndk::ScopedAStatus getDescriptor(Descriptor* _aidl_return) override; - ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) override; - ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, - Parameter::Specific* specific) override; + ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) + REQUIRES(mImplMutex) override; + ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, Parameter::Specific* specific) + REQUIRES(mImplMutex) override; - std::shared_ptr createContext(const Parameter::Common& common) override; - std::shared_ptr getContext() override; - RetCode releaseContext() override; + std::shared_ptr createContext(const Parameter::Common& common) + REQUIRES(mImplMutex) override; + RetCode releaseContext() REQUIRES(mImplMutex) override; - IEffect::Status effectProcessImpl(float* in, float* out, int samples) override; + IEffect::Status effectProcessImpl(float* in, float* out, int samples) + REQUIRES(mImplMutex) override; std::string getEffectName() override { return kEffectName; } private: - std::shared_ptr mContext; + std::shared_ptr mContext GUARDED_BY(mImplMutex); ndk::ScopedAStatus getParameterLoudnessEnhancer(const LoudnessEnhancer::Tag& tag, - Parameter::Specific* specific); + Parameter::Specific* specific) + REQUIRES(mImplMutex); }; } // namespace aidl::android::hardware::audio::effect diff --git a/audio/aidl/default/noiseSuppression/NoiseSuppressionSw.cpp b/audio/aidl/default/noiseSuppression/NoiseSuppressionSw.cpp index a3208df865..d304416dff 100644 --- a/audio/aidl/default/noiseSuppression/NoiseSuppressionSw.cpp +++ b/audio/aidl/default/noiseSuppression/NoiseSuppressionSw.cpp @@ -155,10 +155,6 @@ std::shared_ptr NoiseSuppressionSw::createContext(const Parameter return mContext; } -std::shared_ptr NoiseSuppressionSw::getContext() { - return mContext; -} - RetCode NoiseSuppressionSw::releaseContext() { if (mContext) { mContext.reset(); diff --git a/audio/aidl/default/noiseSuppression/NoiseSuppressionSw.h b/audio/aidl/default/noiseSuppression/NoiseSuppressionSw.h index fc1e028808..acef8ee135 100644 --- a/audio/aidl/default/noiseSuppression/NoiseSuppressionSw.h +++ b/audio/aidl/default/noiseSuppression/NoiseSuppressionSw.h @@ -55,20 +55,23 @@ class NoiseSuppressionSw final : public EffectImpl { } ndk::ScopedAStatus getDescriptor(Descriptor* _aidl_return) override; - ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) override; - ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, - Parameter::Specific* specific) override; + ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) + REQUIRES(mImplMutex) override; + ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, Parameter::Specific* specific) + REQUIRES(mImplMutex) override; - std::shared_ptr createContext(const Parameter::Common& common) override; - std::shared_ptr getContext() override; - RetCode releaseContext() override; + std::shared_ptr createContext(const Parameter::Common& common) + REQUIRES(mImplMutex) override; + RetCode releaseContext() REQUIRES(mImplMutex) override; std::string getEffectName() override { return kEffectName; }; - IEffect::Status effectProcessImpl(float* in, float* out, int samples) override; + IEffect::Status effectProcessImpl(float* in, float* out, int samples) + REQUIRES(mImplMutex) override; private: - std::shared_ptr mContext; + std::shared_ptr mContext GUARDED_BY(mImplMutex); ndk::ScopedAStatus getParameterNoiseSuppression(const NoiseSuppression::Tag& tag, - Parameter::Specific* specific); + Parameter::Specific* specific) + REQUIRES(mImplMutex); }; } // namespace aidl::android::hardware::audio::effect diff --git a/audio/aidl/default/presetReverb/PresetReverbSw.cpp b/audio/aidl/default/presetReverb/PresetReverbSw.cpp index 3f02eb7382..2ac201038d 100644 --- a/audio/aidl/default/presetReverb/PresetReverbSw.cpp +++ b/audio/aidl/default/presetReverb/PresetReverbSw.cpp @@ -161,10 +161,6 @@ std::shared_ptr PresetReverbSw::createContext(const Parameter::Co return mContext; } -std::shared_ptr PresetReverbSw::getContext() { - return mContext; -} - RetCode PresetReverbSw::releaseContext() { if (mContext) { mContext.reset(); diff --git a/audio/aidl/default/presetReverb/PresetReverbSw.h b/audio/aidl/default/presetReverb/PresetReverbSw.h index 9ceee7cca9..61fc88c079 100644 --- a/audio/aidl/default/presetReverb/PresetReverbSw.h +++ b/audio/aidl/default/presetReverb/PresetReverbSw.h @@ -56,21 +56,23 @@ class PresetReverbSw final : public EffectImpl { } ndk::ScopedAStatus getDescriptor(Descriptor* _aidl_return) override; - ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) override; - ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, - Parameter::Specific* specific) override; + ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) + REQUIRES(mImplMutex) override; + ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, Parameter::Specific* specific) + REQUIRES(mImplMutex) override; - std::shared_ptr createContext(const Parameter::Common& common) override; - std::shared_ptr getContext() override; - RetCode releaseContext() override; + std::shared_ptr createContext(const Parameter::Common& common) + REQUIRES(mImplMutex) override; + RetCode releaseContext() REQUIRES(mImplMutex) override; - IEffect::Status effectProcessImpl(float* in, float* out, int samples) override; + IEffect::Status effectProcessImpl(float* in, float* out, int samples) + REQUIRES(mImplMutex) override; std::string getEffectName() override { return kEffectName; } private: - std::shared_ptr mContext; + std::shared_ptr mContext GUARDED_BY(mImplMutex); ndk::ScopedAStatus getParameterPresetReverb(const PresetReverb::Tag& tag, - Parameter::Specific* specific); + Parameter::Specific* specific) REQUIRES(mImplMutex); }; } // namespace aidl::android::hardware::audio::effect diff --git a/audio/aidl/default/spatializer/SpatializerSw.cpp b/audio/aidl/default/spatializer/SpatializerSw.cpp index 434ed5ac38..6d3c4bd16e 100644 --- a/audio/aidl/default/spatializer/SpatializerSw.cpp +++ b/audio/aidl/default/spatializer/SpatializerSw.cpp @@ -141,10 +141,6 @@ std::shared_ptr SpatializerSw::createContext(const Parameter::Com return mContext; } -std::shared_ptr SpatializerSw::getContext() { - return mContext; -} - RetCode SpatializerSw::releaseContext() { if (mContext) { mContext.reset(); diff --git a/audio/aidl/default/spatializer/SpatializerSw.h b/audio/aidl/default/spatializer/SpatializerSw.h index b205704cb1..b321e83b9a 100644 --- a/audio/aidl/default/spatializer/SpatializerSw.h +++ b/audio/aidl/default/spatializer/SpatializerSw.h @@ -50,19 +50,21 @@ class SpatializerSw final : public EffectImpl { ~SpatializerSw(); ndk::ScopedAStatus getDescriptor(Descriptor* _aidl_return) override; - ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) override; - ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, - Parameter::Specific* specific) override; + ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) + REQUIRES(mImplMutex) override; + ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, Parameter::Specific* specific) + REQUIRES(mImplMutex) override; - std::shared_ptr createContext(const Parameter::Common& common) override; - std::shared_ptr getContext() override; - RetCode releaseContext() override; + std::shared_ptr createContext(const Parameter::Common& common) + REQUIRES(mImplMutex) override; + RetCode releaseContext() REQUIRES(mImplMutex) override; std::string getEffectName() override { return kEffectName; }; - IEffect::Status effectProcessImpl(float* in, float* out, int samples) override; + IEffect::Status effectProcessImpl(float* in, float* out, int samples) + REQUIRES(mImplMutex) override; private: static const std::vector kRanges; - std::shared_ptr mContext = nullptr; + std::shared_ptr mContext GUARDED_BY(mImplMutex) = nullptr; }; } // namespace aidl::android::hardware::audio::effect diff --git a/audio/aidl/default/virtualizer/VirtualizerSw.cpp b/audio/aidl/default/virtualizer/VirtualizerSw.cpp index 0e8435ef82..091b0b72c4 100644 --- a/audio/aidl/default/virtualizer/VirtualizerSw.cpp +++ b/audio/aidl/default/virtualizer/VirtualizerSw.cpp @@ -203,10 +203,6 @@ std::shared_ptr VirtualizerSw::createContext(const Parameter::Com return mContext; } -std::shared_ptr VirtualizerSw::getContext() { - return mContext; -} - RetCode VirtualizerSw::releaseContext() { if (mContext) { mContext.reset(); diff --git a/audio/aidl/default/virtualizer/VirtualizerSw.h b/audio/aidl/default/virtualizer/VirtualizerSw.h index 5e114d9abd..9287838083 100644 --- a/audio/aidl/default/virtualizer/VirtualizerSw.h +++ b/audio/aidl/default/virtualizer/VirtualizerSw.h @@ -59,24 +59,25 @@ class VirtualizerSw final : public EffectImpl { } ndk::ScopedAStatus getDescriptor(Descriptor* _aidl_return) override; - ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) override; - ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, - Parameter::Specific* specific) override; + ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) + REQUIRES(mImplMutex) override; + ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, Parameter::Specific* specific) + REQUIRES(mImplMutex) override; - std::shared_ptr createContext(const Parameter::Common& common) override; - std::shared_ptr getContext() override; - RetCode releaseContext() override; + std::shared_ptr createContext(const Parameter::Common& common) + REQUIRES(mImplMutex) override; + RetCode releaseContext() REQUIRES(mImplMutex) override; IEffect::Status effectProcessImpl(float* in, float* out, int samples) override; std::string getEffectName() override { return kEffectName; } private: static const std::vector kRanges; - std::shared_ptr mContext; + std::shared_ptr mContext GUARDED_BY(mImplMutex); ndk::ScopedAStatus getParameterVirtualizer(const Virtualizer::Tag& tag, - Parameter::Specific* specific); + Parameter::Specific* specific) REQUIRES(mImplMutex); ndk::ScopedAStatus getSpeakerAngles(const Virtualizer::SpeakerAnglesPayload payload, - Parameter::Specific* specific); + Parameter::Specific* specific) REQUIRES(mImplMutex); }; } // namespace aidl::android::hardware::audio::effect diff --git a/audio/aidl/default/visualizer/VisualizerSw.cpp b/audio/aidl/default/visualizer/VisualizerSw.cpp index 285c102b6b..54f7f1c52e 100644 --- a/audio/aidl/default/visualizer/VisualizerSw.cpp +++ b/audio/aidl/default/visualizer/VisualizerSw.cpp @@ -190,10 +190,6 @@ std::shared_ptr VisualizerSw::createContext(const Parameter::Comm return mContext; } -std::shared_ptr VisualizerSw::getContext() { - return mContext; -} - RetCode VisualizerSw::releaseContext() { if (mContext) { mContext.reset(); diff --git a/audio/aidl/default/visualizer/VisualizerSw.h b/audio/aidl/default/visualizer/VisualizerSw.h index 995774e197..4b87b04298 100644 --- a/audio/aidl/default/visualizer/VisualizerSw.h +++ b/audio/aidl/default/visualizer/VisualizerSw.h @@ -72,21 +72,23 @@ class VisualizerSw final : public EffectImpl { } ndk::ScopedAStatus getDescriptor(Descriptor* _aidl_return) override; - ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) override; - ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, - Parameter::Specific* specific) override; + ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) + REQUIRES(mImplMutex) override; + ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, Parameter::Specific* specific) + REQUIRES(mImplMutex) override; - std::shared_ptr createContext(const Parameter::Common& common) override; - std::shared_ptr getContext() override; - RetCode releaseContext() override; + std::shared_ptr createContext(const Parameter::Common& common) + REQUIRES(mImplMutex) override; + RetCode releaseContext() REQUIRES(mImplMutex) override; - IEffect::Status effectProcessImpl(float* in, float* out, int samples) override; + IEffect::Status effectProcessImpl(float* in, float* out, int samples) + REQUIRES(mImplMutex) override; std::string getEffectName() override { return kEffectName; } private: static const std::vector kRanges; - std::shared_ptr mContext; + std::shared_ptr mContext GUARDED_BY(mImplMutex); ndk::ScopedAStatus getParameterVisualizer(const Visualizer::Tag& tag, - Parameter::Specific* specific); + Parameter::Specific* specific) REQUIRES(mImplMutex); }; } // namespace aidl::android::hardware::audio::effect diff --git a/audio/aidl/default/volume/VolumeSw.cpp b/audio/aidl/default/volume/VolumeSw.cpp index 890258457f..dd019f6c93 100644 --- a/audio/aidl/default/volume/VolumeSw.cpp +++ b/audio/aidl/default/volume/VolumeSw.cpp @@ -160,10 +160,6 @@ std::shared_ptr VolumeSw::createContext(const Parameter::Common& return mContext; } -std::shared_ptr VolumeSw::getContext() { - return mContext; -} - RetCode VolumeSw::releaseContext() { if (mContext) { mContext.reset(); diff --git a/audio/aidl/default/volume/VolumeSw.h b/audio/aidl/default/volume/VolumeSw.h index 1432b2be0d..3fc0d97320 100644 --- a/audio/aidl/default/volume/VolumeSw.h +++ b/audio/aidl/default/volume/VolumeSw.h @@ -57,21 +57,24 @@ class VolumeSw final : public EffectImpl { } ndk::ScopedAStatus getDescriptor(Descriptor* _aidl_return) override; - ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) override; - ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, - Parameter::Specific* specific) override; + ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) + REQUIRES(mImplMutex) override; + ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id, Parameter::Specific* specific) + REQUIRES(mImplMutex) override; - std::shared_ptr createContext(const Parameter::Common& common) override; - std::shared_ptr getContext() override; - RetCode releaseContext() override; + std::shared_ptr createContext(const Parameter::Common& common) + REQUIRES(mImplMutex) override; + RetCode releaseContext() REQUIRES(mImplMutex) override; - IEffect::Status effectProcessImpl(float* in, float* out, int samples) override; + IEffect::Status effectProcessImpl(float* in, float* out, int samples) + REQUIRES(mImplMutex) override; std::string getEffectName() override { return kEffectName; } private: static const std::vector kRanges; - std::shared_ptr mContext; + std::shared_ptr mContext GUARDED_BY(mImplMutex); - ndk::ScopedAStatus getParameterVolume(const Volume::Tag& tag, Parameter::Specific* specific); + ndk::ScopedAStatus getParameterVolume(const Volume::Tag& tag, Parameter::Specific* specific) + REQUIRES(mImplMutex); }; } // namespace aidl::android::hardware::audio::effect diff --git a/audio/aidl/vts/EffectHelper.h b/audio/aidl/vts/EffectHelper.h index 4a5c537dd5..9fa7f9f74a 100644 --- a/audio/aidl/vts/EffectHelper.h +++ b/audio/aidl/vts/EffectHelper.h @@ -43,6 +43,7 @@ using namespace android; using aidl::android::hardware::audio::effect::CommandId; using aidl::android::hardware::audio::effect::Descriptor; using aidl::android::hardware::audio::effect::IEffect; +using aidl::android::hardware::audio::effect::kEventFlagDataMqUpdate; using aidl::android::hardware::audio::effect::kEventFlagNotEmpty; using aidl::android::hardware::audio::effect::Parameter; using aidl::android::hardware::audio::effect::Range; @@ -191,6 +192,16 @@ class EffectHelper { ASSERT_TRUE(dataMq->read(buffer.data(), expectFloats)); } } + static void expectDataMqUpdateEventFlag(std::unique_ptr& statusMq) { + EventFlag* efGroup; + ASSERT_EQ(::android::OK, + EventFlag::createEventFlag(statusMq->getEventFlagWord(), &efGroup)); + ASSERT_NE(nullptr, efGroup); + uint32_t efState = 0; + EXPECT_EQ(::android::OK, efGroup->wait(kEventFlagDataMqUpdate, &efState, 1'000'000 /*1ms*/, + true /* retry */)); + EXPECT_TRUE(efState & kEventFlagDataMqUpdate); + } static Parameter::Common createParamCommon( int session = 0, int ioHandle = -1, int iSampleRate = 48000, int oSampleRate = 48000, long iFrameCount = 0x100, long oFrameCount = 0x100, diff --git a/audio/aidl/vts/VtsHalAudioEffectTargetTest.cpp b/audio/aidl/vts/VtsHalAudioEffectTargetTest.cpp index 418fedbcb3..01cdd816f5 100644 --- a/audio/aidl/vts/VtsHalAudioEffectTargetTest.cpp +++ b/audio/aidl/vts/VtsHalAudioEffectTargetTest.cpp @@ -609,6 +609,49 @@ TEST_P(AudioEffectTest, SetAndGetParameterVolume) { ASSERT_NO_FATAL_FAILURE(destroy(mFactory, mEffect)); } +// Verify Parameters kept after reset. +TEST_P(AudioEffectTest, SetCommonParameterAndReopen) { + ASSERT_NO_FATAL_FAILURE(create(mFactory, mEffect, mDescriptor)); + + Parameter::Common common = EffectHelper::createParamCommon( + 0 /* session */, 1 /* ioHandle */, 44100 /* iSampleRate */, 44100 /* oSampleRate */, + kInputFrameCount /* iFrameCount */, kOutputFrameCount /* oFrameCount */); + IEffect::OpenEffectReturn ret; + ASSERT_NO_FATAL_FAILURE(open(mEffect, common, std::nullopt /* specific */, &ret, EX_NONE)); + auto statusMQ = std::make_unique(ret.statusMQ); + ASSERT_TRUE(statusMQ->isValid()); + auto inputMQ = std::make_unique(ret.inputDataMQ); + ASSERT_TRUE(inputMQ->isValid()); + auto outputMQ = std::make_unique(ret.outputDataMQ); + ASSERT_TRUE(outputMQ->isValid()); + + Parameter::Id id = Parameter::Id::make(Parameter::common); + common.input.frameCount++; + ASSERT_NO_FATAL_FAILURE(setAndGetParameter(id, Parameter::make(common))); + ASSERT_TRUE(statusMQ->isValid()); + expectDataMqUpdateEventFlag(statusMQ); + EXPECT_IS_OK(mEffect->reopen(&ret)); + inputMQ = std::make_unique(ret.inputDataMQ); + outputMQ = std::make_unique(ret.outputDataMQ); + ASSERT_TRUE(statusMQ->isValid()); + ASSERT_TRUE(inputMQ->isValid()); + ASSERT_TRUE(outputMQ->isValid()); + + common.output.frameCount++; + ASSERT_NO_FATAL_FAILURE(setAndGetParameter(id, Parameter::make(common))); + ASSERT_TRUE(statusMQ->isValid()); + expectDataMqUpdateEventFlag(statusMQ); + EXPECT_IS_OK(mEffect->reopen(&ret)); + inputMQ = std::make_unique(ret.inputDataMQ); + outputMQ = std::make_unique(ret.outputDataMQ); + ASSERT_TRUE(statusMQ->isValid()); + ASSERT_TRUE(inputMQ->isValid()); + ASSERT_TRUE(outputMQ->isValid()); + + ASSERT_NO_FATAL_FAILURE(close(mEffect)); + ASSERT_NO_FATAL_FAILURE(destroy(mFactory, mEffect)); +} + /// Data processing test // Send data to effects and expect it to be consumed by checking statusMQ. // Effects exposing bypass flags or operating in offload mode will be skipped. @@ -684,6 +727,59 @@ TEST_P(AudioEffectDataPathTest, ConsumeDataAfterRestart) { ASSERT_NO_FATAL_FAILURE(destroy(mFactory, mEffect)); } +// Send data to effects and expect it to be consumed after effect reopen (IO AudioConfig change). +// Effects exposing bypass flags or operating in offload mode will be skipped. +TEST_P(AudioEffectDataPathTest, ConsumeDataAfterReopen) { + ASSERT_NO_FATAL_FAILURE(create(mFactory, mEffect, mDescriptor)); + + Parameter::Common common = EffectHelper::createParamCommon( + 0 /* session */, 1 /* ioHandle */, 44100 /* iSampleRate */, 44100 /* oSampleRate */, + kInputFrameCount /* iFrameCount */, kOutputFrameCount /* oFrameCount */); + IEffect::OpenEffectReturn ret; + ASSERT_NO_FATAL_FAILURE(open(mEffect, common, std::nullopt /* specific */, &ret, EX_NONE)); + auto statusMQ = std::make_unique(ret.statusMQ); + ASSERT_TRUE(statusMQ->isValid()); + auto inputMQ = std::make_unique(ret.inputDataMQ); + ASSERT_TRUE(inputMQ->isValid()); + auto outputMQ = std::make_unique(ret.outputDataMQ); + ASSERT_TRUE(outputMQ->isValid()); + + std::vector buffer; + ASSERT_NO_FATAL_FAILURE(command(mEffect, CommandId::START)); + ASSERT_NO_FATAL_FAILURE(expectState(mEffect, State::PROCESSING)); + EXPECT_NO_FATAL_FAILURE(EffectHelper::allocateInputData(common, inputMQ, buffer)); + EXPECT_NO_FATAL_FAILURE(EffectHelper::writeToFmq(statusMQ, inputMQ, buffer)); + EXPECT_NO_FATAL_FAILURE( + EffectHelper::readFromFmq(statusMQ, 1, outputMQ, buffer.size(), buffer)); + + // set a new common parameter with different IO frameCount, reopen + Parameter::Id id = Parameter::Id::make(Parameter::common); + common.input.frameCount += 4; + common.output.frameCount += 4; + ASSERT_NO_FATAL_FAILURE(setAndGetParameter(id, Parameter::make(common))); + ASSERT_TRUE(statusMQ->isValid()); + expectDataMqUpdateEventFlag(statusMQ); + EXPECT_IS_OK(mEffect->reopen(&ret)); + inputMQ = std::make_unique(ret.inputDataMQ); + outputMQ = std::make_unique(ret.outputDataMQ); + ASSERT_TRUE(statusMQ->isValid()); + ASSERT_TRUE(inputMQ->isValid()); + ASSERT_TRUE(outputMQ->isValid()); + + // verify data consume again + EXPECT_NO_FATAL_FAILURE(EffectHelper::allocateInputData(common, inputMQ, buffer)); + EXPECT_NO_FATAL_FAILURE(EffectHelper::writeToFmq(statusMQ, inputMQ, buffer)); + EXPECT_NO_FATAL_FAILURE( + EffectHelper::readFromFmq(statusMQ, 1, outputMQ, buffer.size(), buffer)); + + ASSERT_NO_FATAL_FAILURE(command(mEffect, CommandId::STOP)); + ASSERT_NO_FATAL_FAILURE(expectState(mEffect, State::IDLE)); + EXPECT_NO_FATAL_FAILURE(EffectHelper::readFromFmq(statusMQ, 0, outputMQ, 0, buffer)); + + ASSERT_NO_FATAL_FAILURE(close(mEffect)); + ASSERT_NO_FATAL_FAILURE(destroy(mFactory, mEffect)); +} + // Send data to IDLE effects and expect it to be consumed after effect start. // Effects exposing bypass flags or operating in offload mode will be skipped. TEST_P(AudioEffectDataPathTest, SendDataAtIdleAndConsumeDataInProcessing) {