AIDL Effect: Update locking in EffectImpl
Avoid locking for all EffectContext calls.
Locking inside context seems not necessary for now as all binder calls
already serialized.
Add BassBoost AIDL placeholder implementation.
Bug: 261646550
Test: atest VtsHalAudioEffectTargetTest
Change-Id: Iaa41f013d5756801553e20b995aab5ddc845cf32
diff --git a/audio/aidl/default/include/effect-impl/EffectContext.h b/audio/aidl/default/include/effect-impl/EffectContext.h
index f608e12..95645d5 100644
--- a/audio/aidl/default/include/effect-impl/EffectContext.h
+++ b/audio/aidl/default/include/effect-impl/EffectContext.h
@@ -16,16 +16,13 @@
#pragma once
#include <Utils.h>
-#include <android-base/logging.h>
-#include <utils/Log.h>
-#include <cstddef>
-#include <cstdint>
#include <memory>
-#include <utility>
#include <vector>
-#include <aidl/android/hardware/audio/effect/BnEffect.h>
+#include <android-base/logging.h>
#include <fmq/AidlMessageQueue.h>
+
+#include <aidl/android/hardware/audio/effect/BnEffect.h>
#include "EffectTypes.h"
namespace aidl::android::hardware::audio::effect {
@@ -74,13 +71,10 @@
std::shared_ptr<DataMQ> getOutputDataFmq() { return mOutputMQ; }
float* getWorkBuffer() { return static_cast<float*>(mWorkBuffer.data()); }
- // TODO: update with actual available size
- size_t availableToRead() { return mWorkBuffer.capacity(); }
- size_t availableToWrite() { return mWorkBuffer.capacity(); }
// reset buffer status by abandon all data and status in FMQ
void resetBuffer() {
- auto buffer = getWorkBuffer();
+ auto buffer = static_cast<float*>(mWorkBuffer.data());
std::vector<IEffect::Status> status(mStatusMQ->availableToRead());
mInputMQ->read(buffer, mInputMQ->availableToRead());
mOutputMQ->read(buffer, mOutputMQ->availableToRead());
@@ -89,9 +83,9 @@
void dupeFmq(IEffect::OpenEffectReturn* effectRet) {
if (effectRet) {
- effectRet->statusMQ = getStatusFmq()->dupeDesc();
- effectRet->inputDataMQ = getInputDataFmq()->dupeDesc();
- effectRet->outputDataMQ = getOutputDataFmq()->dupeDesc();
+ effectRet->statusMQ = mStatusMQ->dupeDesc();
+ effectRet->inputDataMQ = mInputMQ->dupeDesc();
+ effectRet->outputDataMQ = mOutputMQ->dupeDesc();
}
}
size_t getInputFrameSize() { return mInputFrameSize; }
@@ -138,7 +132,8 @@
protected:
// common parameters
int mSessionId = INVALID_AUDIO_SESSION_ID;
- size_t mInputFrameSize, mOutputFrameSize;
+ size_t mInputFrameSize;
+ size_t mOutputFrameSize;
Parameter::Common mCommon;
aidl::android::media::audio::common::AudioDeviceDescription mOutputDevice;
aidl::android::media::audio::common::AudioMode mMode;
diff --git a/audio/aidl/default/include/effect-impl/EffectImpl.h b/audio/aidl/default/include/effect-impl/EffectImpl.h
index d9825da..f5e2aec 100644
--- a/audio/aidl/default/include/effect-impl/EffectImpl.h
+++ b/audio/aidl/default/include/effect-impl/EffectImpl.h
@@ -15,45 +15,35 @@
*/
#pragma once
-#include <aidl/android/hardware/audio/effect/BnEffect.h>
-#include <fmq/AidlMessageQueue.h>
#include <cstdlib>
#include <memory>
-#include <mutex>
+#include <aidl/android/hardware/audio/effect/BnEffect.h>
+#include <fmq/AidlMessageQueue.h>
+
+#include "EffectContext.h"
+#include "EffectThread.h"
#include "EffectTypes.h"
#include "effect-impl/EffectContext.h"
+#include "effect-impl/EffectThread.h"
#include "effect-impl/EffectTypes.h"
-#include "effect-impl/EffectWorker.h"
namespace aidl::android::hardware::audio::effect {
-class EffectImpl : public BnEffect, public EffectWorker {
+class EffectImpl : public BnEffect, public EffectThread {
public:
EffectImpl() = default;
virtual ~EffectImpl() = default;
- /**
- * Each effect implementation CAN override these methods if necessary
- * If you would like implement IEffect::open completely, override EffectImpl::open(), if you
- * want to keep most of EffectImpl logic but have a little customize, try override openImpl().
- * openImpl() will be called at the beginning of EffectImpl::open() without lock protection.
- *
- * Same for closeImpl().
- */
virtual ndk::ScopedAStatus open(const Parameter::Common& common,
const std::optional<Parameter::Specific>& specific,
OpenEffectReturn* ret) override;
virtual ndk::ScopedAStatus close() override;
virtual ndk::ScopedAStatus command(CommandId id) override;
- virtual ndk::ScopedAStatus commandStart() { return ndk::ScopedAStatus::ok(); }
- virtual ndk::ScopedAStatus commandStop() { return ndk::ScopedAStatus::ok(); }
- virtual ndk::ScopedAStatus commandReset() { return ndk::ScopedAStatus::ok(); }
virtual ndk::ScopedAStatus getState(State* state) override;
virtual ndk::ScopedAStatus setParameter(const Parameter& param) override;
virtual ndk::ScopedAStatus getParameter(const Parameter::Id& id, Parameter* param) override;
- virtual IEffect::Status effectProcessImpl(float* in, float* out, int process) override;
virtual ndk::ScopedAStatus setParameterCommon(const Parameter& param);
virtual ndk::ScopedAStatus getParameterCommon(const Parameter::Tag& tag, Parameter* param);
@@ -63,21 +53,32 @@
virtual ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) = 0;
virtual ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id,
Parameter::Specific* specific) = 0;
+
+ virtual std::string getEffectName() = 0;
+ virtual IEffect::Status effectProcessImpl(float* in, float* out, int samples) override;
+
+ /**
+ * 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.
+ */
virtual std::shared_ptr<EffectContext> createContext(const Parameter::Common& common) = 0;
+ virtual std::shared_ptr<EffectContext> getContext() = 0;
virtual RetCode releaseContext() = 0;
protected:
- /*
- * Lock is required if effectProcessImpl (which is running in an independent thread) needs to
- * access state and parameters.
- */
- std::mutex mMutex;
- State mState GUARDED_BY(mMutex) = State::INIT;
+ State mState = State::INIT;
IEffect::Status status(binder_status_t status, size_t consumed, size_t produced);
void cleanUp();
- private:
- std::shared_ptr<EffectContext> mContext GUARDED_BY(mMutex);
+ /**
+ * Optional CommandId handling methods for effects to override.
+ * For CommandId::START, EffectImpl call commandImpl before starting the EffectThread
+ * processing.
+ * For CommandId::STOP and CommandId::RESET, EffectImpl call commandImpl after stop the
+ * EffectThread processing.
+ */
+ virtual ndk::ScopedAStatus commandImpl(CommandId id);
};
} // 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 09a0000..4b6cecd 100644
--- a/audio/aidl/default/include/effect-impl/EffectThread.h
+++ b/audio/aidl/default/include/effect-impl/EffectThread.h
@@ -22,6 +22,7 @@
#include <android-base/thread_annotations.h>
#include <system/thread_defs.h>
+#include "effect-impl/EffectContext.h"
#include "effect-impl/EffectTypes.h"
namespace aidl::android::hardware::audio::effect {
@@ -33,7 +34,7 @@
virtual ~EffectThread();
// called by effect implementation.
- RetCode createThread(const std::string& name,
+ RetCode createThread(std::shared_ptr<EffectContext> context, const std::string& name,
const int priority = ANDROID_PRIORITY_URGENT_AUDIO);
RetCode destroyThread();
RetCode startThread();
@@ -42,15 +43,43 @@
// Will call process() in a loop if the thread is running.
void threadLoop();
- // User of EffectThread must implement the effect processing logic in this method.
- virtual void process() = 0;
- const int MAX_TASK_COMM_LEN = 15;
+ /**
+ * @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.
+ *
+ * @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;
+
+ /**
+ * The default EffectThread::process() implementation doesn't need to lock. It will only
+ * access the FMQ and mWorkBuffer in EffectContext, since they will only be changed in
+ * EffectImpl IEffect::open() (in this case EffectThread just created and not running yet) and
+ * IEffect::command(CommandId::RESET) (in this case EffectThread already stopped).
+ *
+ * process() call effectProcessImpl for effect processing, and because effectProcessImpl is
+ * implemented by effects, process() must not hold lock before call into effectProcessImpl to
+ * avoid deadlock.
+ */
+ virtual void process();
private:
- std::mutex mMutex;
+ const int kMaxTaskNameLen = 15;
+ std::mutex mThreadMutex;
std::condition_variable mCv;
- bool mExit GUARDED_BY(mMutex) = false;
- bool mStop GUARDED_BY(mMutex) = true;
+ bool mExit GUARDED_BY(mThreadMutex) = false;
+ bool mStop GUARDED_BY(mThreadMutex) = true;
+ std::shared_ptr<EffectContext> mThreadContext GUARDED_BY(mThreadMutex);
std::thread mThread;
int mPriority;
std::string mName;
diff --git a/audio/aidl/default/include/effect-impl/EffectWorker.h b/audio/aidl/default/include/effect-impl/EffectWorker.h
index 6a78eab..b456817 100644
--- a/audio/aidl/default/include/effect-impl/EffectWorker.h
+++ b/audio/aidl/default/include/effect-impl/EffectWorker.h
@@ -63,7 +63,7 @@
// must implement by each effect implementation
// TODO: consider if this interface need adjustment to handle in-place processing
- virtual IEffect::Status effectProcessImpl(float* in, float* out, int processSamples) = 0;
+ virtual IEffect::Status effectProcessImpl(float* in, float* out, int samples) = 0;
private:
// make sure the context only set once.