Clean up EffectHalInterface
Remove `isLocal`, it's a legacy from the HIDL transition times.
Make `effectId` specific to EffectHalHidl, remove usages
outside of libaudiohal.
Bug: 205884982
Test: m
Change-Id: I0486d5fb48d8cfce46aca9152136e7a1a68cf215
diff --git a/media/libaudiohal/impl/DeviceHalHidl.cpp b/media/libaudiohal/impl/DeviceHalHidl.cpp
index cd83171..4066f3c 100644
--- a/media/libaudiohal/impl/DeviceHalHidl.cpp
+++ b/media/libaudiohal/impl/DeviceHalHidl.cpp
@@ -32,6 +32,7 @@
#include <util/CoreUtils.h>
#include "DeviceHalHidl.h"
+#include "EffectHalHidl.h"
#include "ParameterUtils.h"
#include "StreamHalHidl.h"
@@ -501,8 +502,9 @@
audio_port_handle_t device, sp<EffectHalInterface> effect) {
TIME_CHECK();
if (mDevice == 0) return NO_INIT;
+ auto hidlEffect = sp<effect::EffectHalHidl>::cast(effect);
return processReturn("addDeviceEffect", mDevice->addDeviceEffect(
- static_cast<AudioPortHandle>(device), effect->effectId()));
+ static_cast<AudioPortHandle>(device), hidlEffect->effectId()));
}
#else
status_t DeviceHalHidl::addDeviceEffect(
@@ -516,8 +518,9 @@
audio_port_handle_t device, sp<EffectHalInterface> effect) {
TIME_CHECK();
if (mDevice == 0) return NO_INIT;
+ auto hidlEffect = sp<effect::EffectHalHidl>::cast(effect);
return processReturn("removeDeviceEffect", mDevice->removeDeviceEffect(
- static_cast<AudioPortHandle>(device), effect->effectId()));
+ static_cast<AudioPortHandle>(device), hidlEffect->effectId()));
}
#else
status_t DeviceHalHidl::removeDeviceEffect(
diff --git a/media/libaudiohal/impl/EffectConversionHelperHidl.h b/media/libaudiohal/impl/EffectConversionHelperHidl.h
index 4371d12..ed696bf 100644
--- a/media/libaudiohal/impl/EffectConversionHelperHidl.h
+++ b/media/libaudiohal/impl/EffectConversionHelperHidl.h
@@ -19,9 +19,9 @@
#include "ConversionHelperHidl.h"
-#include PATH(android/hardware/audio/effect/FILE_VERSION/types.h)
+#include PATH(android/hardware/audio/effect/COMMON_TYPES_FILE_VERSION/types.h)
-using EffectResult = ::android::hardware::audio::effect::CPP_VERSION::Result;
+using EffectResult = ::android::hardware::audio::effect::COMMON_TYPES_CPP_VERSION::Result;
namespace android {
diff --git a/media/libaudiohal/impl/EffectHalAidl.cpp b/media/libaudiohal/impl/EffectHalAidl.cpp
index 36abfa4..ae4a530 100644
--- a/media/libaudiohal/impl/EffectHalAidl.cpp
+++ b/media/libaudiohal/impl/EffectHalAidl.cpp
@@ -61,12 +61,11 @@
namespace effect {
EffectHalAidl::EffectHalAidl(const std::shared_ptr<IFactory>& factory,
- const std::shared_ptr<IEffect>& effect, uint64_t effectId,
+ const std::shared_ptr<IEffect>& effect,
int32_t sessionId, int32_t ioId, const Descriptor& desc,
bool isProxyEffect)
: mFactory(factory),
mEffect(effect),
- mEffectId(effectId),
mSessionId(sessionId),
mIoId(ioId),
mDesc(desc),
diff --git a/media/libaudiohal/impl/EffectHalAidl.h b/media/libaudiohal/impl/EffectHalAidl.h
index 47049d7..1b7a3d6 100644
--- a/media/libaudiohal/impl/EffectHalAidl.h
+++ b/media/libaudiohal/impl/EffectHalAidl.h
@@ -56,13 +56,8 @@
// Free resources on the remote side.
status_t close() override;
- // Whether it's a local implementation.
- bool isLocal() const override { return false; }
-
status_t dump(int fd) override;
- uint64_t effectId() const override { return mEffectId; }
-
const std::shared_ptr<::aidl::android::hardware::audio::effect::IEffect> getIEffect() const {
return mEffect;
}
@@ -75,7 +70,6 @@
const std::shared_ptr<::aidl::android::hardware::audio::effect::IFactory> mFactory;
const std::shared_ptr<::aidl::android::hardware::audio::effect::IEffect> mEffect;
- const uint64_t mEffectId;
const int32_t mSessionId;
const int32_t mIoId;
const ::aidl::android::hardware::audio::effect::Descriptor mDesc;
@@ -93,7 +87,7 @@
EffectHalAidl(
const std::shared_ptr<::aidl::android::hardware::audio::effect::IFactory>& factory,
const std::shared_ptr<::aidl::android::hardware::audio::effect::IEffect>& effect,
- uint64_t effectId, int32_t sessionId, int32_t ioId,
+ int32_t sessionId, int32_t ioId,
const ::aidl::android::hardware::audio::effect::Descriptor& desc,
bool isProxyEffect);
bool setEffectReverse(bool reverse);
diff --git a/media/libaudiohal/impl/EffectHalHidl.h b/media/libaudiohal/impl/EffectHalHidl.h
index 94dcd7e..dda21ed 100644
--- a/media/libaudiohal/impl/EffectHalHidl.h
+++ b/media/libaudiohal/impl/EffectHalHidl.h
@@ -17,7 +17,7 @@
#ifndef ANDROID_HARDWARE_EFFECT_HAL_HIDL_H
#define ANDROID_HARDWARE_EFFECT_HAL_HIDL_H
-#include PATH(android/hardware/audio/effect/FILE_VERSION/IEffect.h)
+#include PATH(android/hardware/audio/effect/COMMON_TYPES_FILE_VERSION/IEffect.h)
#include <media/audiohal/EffectHalInterface.h>
#include <fmq/EventFlag.h>
#include <fmq/MessageQueue.h>
@@ -31,7 +31,7 @@
namespace android {
namespace effect {
-using namespace ::android::hardware::audio::effect::CPP_VERSION;
+using namespace ::android::hardware::audio::effect::COMMON_TYPES_CPP_VERSION;
class EffectHalHidl : public EffectHalInterface, public EffectConversionHelperHidl
{
@@ -59,12 +59,9 @@
// Free resources on the remote side.
virtual status_t close();
- // Whether it's a local implementation.
- virtual bool isLocal() const { return false; }
-
virtual status_t dump(int fd);
- virtual uint64_t effectId() const { return mEffectId; }
+ uint64_t effectId() const { return mEffectId; }
private:
friend class EffectsFactoryHalHidl;
diff --git a/media/libaudiohal/impl/EffectsFactoryHalAidl.cpp b/media/libaudiohal/impl/EffectsFactoryHalAidl.cpp
index cf19dae..57395fa 100644
--- a/media/libaudiohal/impl/EffectsFactoryHalAidl.cpp
+++ b/media/libaudiohal/impl/EffectsFactoryHalAidl.cpp
@@ -201,20 +201,12 @@
Descriptor desc;
RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(aidlEffect->getDescriptor(&desc)));
- uint64_t effectId;
- {
- std::lock_guard lg(mLock);
- effectId = ++mEffectIdCounter;
- }
-
- *effect =
- sp<EffectHalAidl>::make(mFactory, aidlEffect, effectId, sessionId, ioId, desc, isProxy);
+ *effect = sp<EffectHalAidl>::make(mFactory, aidlEffect, sessionId, ioId, desc, isProxy);
return OK;
}
status_t EffectsFactoryHalAidl::dumpEffects(int fd) {
status_t ret = OK;
- std::lock_guard lg(mLock);
// record the error ret and continue dump as many effects as possible
for (const auto& proxy : mProxyList) {
if (status_t temp = BAD_VALUE; proxy && (temp = proxy->dump(fd, nullptr, 0)) != OK) {
diff --git a/media/libaudiohal/impl/EffectsFactoryHalAidl.h b/media/libaudiohal/impl/EffectsFactoryHalAidl.h
index b900cb4..73089b0 100644
--- a/media/libaudiohal/impl/EffectsFactoryHalAidl.h
+++ b/media/libaudiohal/impl/EffectsFactoryHalAidl.h
@@ -19,7 +19,6 @@
#include <cstddef>
#include <list>
#include <memory>
-#include <mutex>
#include <aidl/android/hardware/audio/effect/IFactory.h>
#include <aidl/android/hardware/audio/effect/Processing.h>
@@ -88,9 +87,6 @@
// list of the EffectProxy instances
std::list<std::shared_ptr<EffectProxy>> mProxyList;
- std::mutex mLock;
- uint64_t mEffectIdCounter GUARDED_BY(mLock) = 0; // Align with HIDL (0 is INVALID_ID)
-
virtual ~EffectsFactoryHalAidl() = default;
status_t getHalDescriptorWithImplUuid(
const ::aidl::android::media::audio::common::AudioUuid& uuid,
diff --git a/media/libaudiohal/impl/StreamHalHidl.cpp b/media/libaudiohal/impl/StreamHalHidl.cpp
index 82062cc..72eadc6 100644
--- a/media/libaudiohal/impl/StreamHalHidl.cpp
+++ b/media/libaudiohal/impl/StreamHalHidl.cpp
@@ -33,6 +33,7 @@
#include <util/CoreUtils.h>
#include "DeviceHalHidl.h"
+#include "EffectHalHidl.h"
#include "ParameterUtils.h"
#include "StreamHalHidl.h"
@@ -144,13 +145,15 @@
status_t StreamHalHidl::addEffect(sp<EffectHalInterface> effect) {
TIME_CHECK();
if (!mStream) return NO_INIT;
- return processReturn("addEffect", mStream->addEffect(effect->effectId()));
+ auto hidlEffect = sp<effect::EffectHalHidl>::cast(effect);
+ return processReturn("addEffect", mStream->addEffect(hidlEffect->effectId()));
}
status_t StreamHalHidl::removeEffect(sp<EffectHalInterface> effect) {
TIME_CHECK();
if (!mStream) return NO_INIT;
- return processReturn("removeEffect", mStream->removeEffect(effect->effectId()));
+ auto hidlEffect = sp<effect::EffectHalHidl>::cast(effect);
+ return processReturn("removeEffect", mStream->removeEffect(hidlEffect->effectId()));
}
status_t StreamHalHidl::standby() {