Merge "AudioEffectTest : Fix the assert condition in TestHapticEffect" into main
diff --git a/media/libaudiohal/impl/EffectsFactoryHalAidl.cpp b/media/libaudiohal/impl/EffectsFactoryHalAidl.cpp
index 740bf60..2753906 100644
--- a/media/libaudiohal/impl/EffectsFactoryHalAidl.cpp
+++ b/media/libaudiohal/impl/EffectsFactoryHalAidl.cpp
@@ -42,6 +42,8 @@
using ::aidl::android::hardware::audio::effect::Descriptor;
using ::aidl::android::hardware::audio::effect::IFactory;
using ::aidl::android::hardware::audio::effect::Processing;
+using ::aidl::android::media::audio::common::AudioDevice;
+using ::aidl::android::media::audio::common::AudioDeviceAddress;
using ::aidl::android::media::audio::common::AudioSource;
using ::aidl::android::media::audio::common::AudioStreamType;
using ::aidl::android::media::audio::common::AudioUuid;
@@ -281,7 +283,8 @@
auto getConfigProcessingWithAidlProcessing =
[&](const auto& aidlProcess, std::vector<effectsConfig::InputStream>& preprocess,
- std::vector<effectsConfig::OutputStream>& postprocess) {
+ std::vector<effectsConfig::OutputStream>& postprocess,
+ std::vector<effectsConfig::DeviceEffects>& deviceprocess) {
if (aidlProcess.type.getTag() == Processing::Type::streamType) {
AudioStreamType aidlType =
aidlProcess.type.template get<Processing::Type::streamType>();
@@ -313,6 +316,25 @@
effectsConfig::InputStream stream = {.type = type.value(),
.effects = std::move(effects)};
preprocess.emplace_back(stream);
+ } else if (aidlProcess.type.getTag() == Processing::Type::device) {
+ AudioDevice aidlDevice =
+ aidlProcess.type.template get<Processing::Type::device>();
+ std::vector<std::shared_ptr<const effectsConfig::Effect>> effects;
+ std::transform(aidlProcess.ids.begin(), aidlProcess.ids.end(),
+ std::back_inserter(effects), getConfigEffectWithDescriptor);
+ audio_devices_t type;
+ char address[AUDIO_DEVICE_MAX_ADDRESS_LEN];
+ status_t status = ::aidl::android::aidl2legacy_AudioDevice_audio_device(
+ aidlDevice, &type, address);
+ if (status != NO_ERROR) {
+ ALOGE("%s device effect has invalid device type / address", __func__);
+ return;
+ }
+ effectsConfig::DeviceEffects device = {
+ {.type = type, .effects = std::move(effects)},
+ .address = address,
+ };
+ deviceprocess.emplace_back(device);
}
};
@@ -320,17 +342,21 @@
[&]() -> std::shared_ptr<const effectsConfig::Processings> {
std::vector<effectsConfig::InputStream> preprocess;
std::vector<effectsConfig::OutputStream> postprocess;
+ std::vector<effectsConfig::DeviceEffects> deviceprocess;
for (const auto& processing : mAidlProcessings) {
- getConfigProcessingWithAidlProcessing(processing, preprocess, postprocess);
+ getConfigProcessingWithAidlProcessing(processing, preprocess, postprocess,
+ deviceprocess);
}
- if (0 == preprocess.size() && 0 == postprocess.size()) {
+ if (0 == preprocess.size() && 0 == postprocess.size() &&
+ 0 == deviceprocess.size()) {
return nullptr;
}
return std::make_shared<const effectsConfig::Processings>(
effectsConfig::Processings({.preprocess = std::move(preprocess),
- .postprocess = std::move(postprocess)}));
+ .postprocess = std::move(postprocess),
+ .deviceprocess = std::move(deviceprocess)}));
}());
return processings;
diff --git a/media/libaudiohal/impl/StreamHalAidl.cpp b/media/libaudiohal/impl/StreamHalAidl.cpp
index fb4b549..94de8ea 100644
--- a/media/libaudiohal/impl/StreamHalAidl.cpp
+++ b/media/libaudiohal/impl/StreamHalAidl.cpp
@@ -796,6 +796,14 @@
}
status_t StreamOutHalAidl::drain(bool earlyNotify) {
+ if (!mStream) return NO_INIT;
+
+ if(const auto state = getState(); state == StreamDescriptor::State::IDLE) {
+ ALOGD("%p %s stream already in IDLE state", this, __func__);
+ if(mContext.isAsynchronous()) onDrainReady();
+ return OK;
+ }
+
return StreamHalAidl::drain(earlyNotify);
}
diff --git a/media/libaudiohal/impl/StreamHalAidl.h b/media/libaudiohal/impl/StreamHalAidl.h
index 9cb2cff..0587640 100644
--- a/media/libaudiohal/impl/StreamHalAidl.h
+++ b/media/libaudiohal/impl/StreamHalAidl.h
@@ -215,6 +215,11 @@
~StreamHalAidl() override;
+ ::aidl::android::hardware::audio::core::StreamDescriptor::State getState() {
+ std::lock_guard l(mLock);
+ return mLastReply.state;
+ }
+
status_t getLatency(uint32_t *latency);
// Always returns non-negative values.
@@ -268,10 +273,6 @@
result.format = config.format;
return result;
}
- ::aidl::android::hardware::audio::core::StreamDescriptor::State getState() {
- std::lock_guard l(mLock);
- return mLastReply.state;
- }
// Note: Since `sendCommand` takes mLock while holding mCommandReplyLock, never call
// it with `mLock` being held.
status_t sendCommand(
diff --git a/media/libstagefright/webm/WebmWriter.cpp b/media/libstagefright/webm/WebmWriter.cpp
index ca862b0..151ce7c 100644
--- a/media/libstagefright/webm/WebmWriter.cpp
+++ b/media/libstagefright/webm/WebmWriter.cpp
@@ -290,7 +290,7 @@
// Max file duration limit is set
if (mMaxFileDurationLimitUs != 0) {
if (bitRate > 0) {
- int64_t size2 = ((mMaxFileDurationLimitUs * bitRate * 6) / 1000 / 8000000);
+ int64_t size2 = ((mMaxFileDurationLimitUs / 1000) * bitRate * 6) / 8000000;
if (mMaxFileSizeLimitBytes != 0 && mIsFileSizeLimitExplicitlyRequested) {
// When both file size and duration limits are set,
// we use the smaller limit of the two.
diff --git a/services/audioflinger/DeviceEffectManager.cpp b/services/audioflinger/DeviceEffectManager.cpp
index feae97e..7cb9329 100644
--- a/services/audioflinger/DeviceEffectManager.cpp
+++ b/services/audioflinger/DeviceEffectManager.cpp
@@ -71,10 +71,15 @@
void DeviceEffectManager::onReleaseAudioPatch(audio_patch_handle_t handle) {
ALOGV("%s", __func__);
+ // Keep a reference on disconnected handle to delay destruction without lock held.
+ std::vector<sp<IAfEffectHandle>> disconnectedHandles{};
audio_utils::lock_guard _l(mutex());
for (auto& effectProxies : mDeviceEffects) {
for (auto& effect : effectProxies.second) {
- effect->onReleasePatch(handle);
+ sp<IAfEffectHandle> disconnectedHandle = effect->onReleasePatch(handle);
+ if (disconnectedHandle != nullptr) {
+ disconnectedHandles.push_back(std::move(disconnectedHandle));
+ }
}
}
}
diff --git a/services/audioflinger/Effects.cpp b/services/audioflinger/Effects.cpp
index 8a29fb2..a6a2cdf 100644
--- a/services/audioflinger/Effects.cpp
+++ b/services/audioflinger/Effects.cpp
@@ -1757,6 +1757,9 @@
const sp<media::IEffectClient>& effectClient,
int32_t priority, bool notifyFramesProcessed)
{
+ if (client == nullptr && effectClient == nullptr) {
+ return sp<InternalEffectHandle>::make(effect, notifyFramesProcessed);
+ }
return sp<EffectHandle>::make(
effect, client, effectClient, priority, notifyFramesProcessed);
}
@@ -1764,12 +1767,14 @@
EffectHandle::EffectHandle(const sp<IAfEffectBase>& effect,
const sp<Client>& client,
const sp<media::IEffectClient>& effectClient,
- int32_t priority, bool notifyFramesProcessed)
- : BnEffect(),
+ int32_t priority, bool notifyFramesProcessed,
+ bool isInternal,
+ audio_utils::MutexOrder mutexOrder)
+ : BnEffect(), mMutex(audio_utils::mutex{mutexOrder}),
mEffect(effect), mEffectClient(media::EffectClientAsyncProxy::makeIfNeeded(effectClient)),
mClient(client), mCblk(nullptr),
mPriority(priority), mHasControl(false), mEnabled(false), mDisconnected(false),
- mNotifyFramesProcessed(notifyFramesProcessed)
+ mNotifyFramesProcessed(notifyFramesProcessed), mIsInternal(isInternal)
{
ALOGV("constructor %p client %p", this, client.get());
setMinSchedulerPolicy(SCHED_NORMAL, ANDROID_PRIORITY_AUDIO);
@@ -1936,7 +1941,7 @@
void EffectHandle::disconnect(bool unpinIfLast)
{
- audio_utils::lock_guard _l(mutex());
+ audio_utils::unique_lock _l(mutex());
ALOGV("disconnect(%s) %p", unpinIfLast ? "true" : "false", this);
if (mDisconnected) {
if (unpinIfLast) {
@@ -1948,11 +1953,19 @@
{
sp<IAfEffectBase> effect = mEffect.promote();
if (effect != 0) {
+ // Unlock e.g. for device effect: may need to acquire AudioFlinger lock
+ // Also Internal Effect Handle would require Proxy lock (and vice versa).
+ if (isInternal()) {
+ _l.unlock();
+ }
if (effect->disconnectHandle(this, unpinIfLast) > 0) {
ALOGW("%s Effect handle %p disconnected after thread destruction",
__func__, this);
}
effect->updatePolicyState();
+ if (isInternal()) {
+ _l.lock();
+ }
}
}
@@ -3541,8 +3554,7 @@
mHalEffect->setDevices({mDevice});
}
}
- *handle = new EffectHandle(mHalEffect, nullptr, nullptr, 0 /*priority*/,
- mNotifyFramesProcessed);
+ *handle = new InternalEffectHandle(mHalEffect, mNotifyFramesProcessed);
status = (*handle)->initCheck();
if (status == OK) {
status = mHalEffect->addHandle((*handle).get());
@@ -3589,15 +3601,16 @@
return status;
}
-void DeviceEffectProxy::onReleasePatch(audio_patch_handle_t patchHandle) {
- sp<IAfEffectHandle> effect;
+sp<IAfEffectHandle> DeviceEffectProxy::onReleasePatch(audio_patch_handle_t patchHandle) {
+ sp<IAfEffectHandle> disconnectedHandle;
{
audio_utils::lock_guard _l(proxyMutex());
if (mEffectHandles.find(patchHandle) != mEffectHandles.end()) {
- effect = mEffectHandles.at(patchHandle);
+ disconnectedHandle = std::move(mEffectHandles.at(patchHandle));
mEffectHandles.erase(patchHandle);
}
}
+ return disconnectedHandle;
}
diff --git a/services/audioflinger/Effects.h b/services/audioflinger/Effects.h
index 5c90176..9ecf89e 100644
--- a/services/audioflinger/Effects.h
+++ b/services/audioflinger/Effects.h
@@ -344,7 +344,8 @@
EffectHandle(const sp<IAfEffectBase>& effect,
const sp<Client>& client,
const sp<media::IEffectClient>& effectClient,
- int32_t priority, bool notifyFramesProcessed);
+ int32_t priority, bool notifyFramesProcessed, bool isInternal = false,
+ audio_utils::MutexOrder mutexOrder = audio_utils::MutexOrder::kEffectHandle_Mutex);
~EffectHandle() override;
status_t onTransact(
uint32_t code, const Parcel& data, Parcel* reply, uint32_t flags) final;
@@ -364,6 +365,11 @@
int32_t* _aidl_return) final;
const sp<Client>& client() const final { return mClient; }
+ /**
+ * Checks if the handle is internal, aka created by AudioFlinger for internal needs (e.g.
+ * device effect HAL handle or device effect thread handle).
+ */
+ virtual bool isInternal() const { return mIsInternal; }
sp<android::media::IEffect> asIEffect() final {
return sp<android::media::IEffect>::fromExisting(this);
@@ -401,15 +407,18 @@
void dumpToBuffer(char* buffer, size_t size) const final;
+protected:
+ // protects IEffect method calls
+ mutable audio_utils::mutex mMutex;
private:
DISALLOW_COPY_AND_ASSIGN(EffectHandle);
- audio_utils::mutex& mutex() const RETURN_CAPABILITY(android::audio_utils::EffectHandle_Mutex) {
+ virtual audio_utils::mutex& mutex() const
+ RETURN_CAPABILITY(android::audio_utils::EffectHandle_Mutex) {
return mMutex;
}
- // protects IEffect method calls
- mutable audio_utils::mutex mMutex{audio_utils::MutexOrder::kEffectHandle_Mutex};
+
const wp<IAfEffectBase> mEffect; // pointer to controlled EffectModule
const sp<media::IEffectClient> mEffectClient; // callback interface for client notifications
/*const*/ sp<Client> mClient; // client for shared memory allocation, see
@@ -425,6 +434,28 @@
bool mDisconnected; // Set to true by disconnect()
const bool mNotifyFramesProcessed; // true if the client callback event
// EVENT_FRAMES_PROCESSED must be generated
+ const bool mIsInternal;
+};
+
+/**
+ * There are 2 types of effects:
+ * -Session Effect: handle is directly called from the client, without AudioFlinger lock.
+ * -Device Effect: a device effect proxy is aggregating a collection of internal effect handles that
+ * controls the same effect added on all audio patches involving the device effect selected port
+ * requested either by a client or by AudioPolicyEffects. These internal effect handles do not have
+ * client. Sequence flow implies a different locking order, hence the lock is specialied.
+ */
+class InternalEffectHandle : public EffectHandle {
+public:
+ InternalEffectHandle(const sp<IAfEffectBase>& effect, bool notifyFramesProcessed) :
+ EffectHandle(effect, /* client= */ nullptr, /* effectClient= */ nullptr,
+ /* priority= */ 0, notifyFramesProcessed, /* isInternal */ true,
+ audio_utils::MutexOrder::kDeviceEffectHandle_Mutex) {}
+
+ virtual audio_utils::mutex& mutex() const
+ RETURN_CAPABILITY(android::audio_utils::DeviceEffectHandle_Mutex) {
+ return mMutex;
+ }
};
// the EffectChain class represents a group of effects associated to one audio session.
@@ -762,7 +793,7 @@
status_t onUpdatePatch(audio_patch_handle_t oldPatchHandle, audio_patch_handle_t newPatchHandle,
const IAfPatchPanel::Patch& patch) final;
- void onReleasePatch(audio_patch_handle_t patchHandle) final;
+ sp<IAfEffectHandle> onReleasePatch(audio_patch_handle_t patchHandle) final;
size_t removeEffect(const sp<IAfEffectModule>& effect) final;
diff --git a/services/audioflinger/IAfEffect.h b/services/audioflinger/IAfEffect.h
index f1edcfe..3452e94 100644
--- a/services/audioflinger/IAfEffect.h
+++ b/services/audioflinger/IAfEffect.h
@@ -400,7 +400,14 @@
virtual status_t onUpdatePatch(audio_patch_handle_t oldPatchHandle,
audio_patch_handle_t newPatchHandle,
const IAfPatchPanel::Patch& patch) = 0;
- virtual void onReleasePatch(audio_patch_handle_t patchHandle) = 0;
+ /**
+ * Checks (and release) of the effect handle is linked with the given release patch handle.
+ *
+ * @param patchHandle handle of the released patch
+ * @return a reference on the effect handle released if any, nullptr otherwise.
+ * It allows to delay the destruction of the handle.
+ */
+ virtual sp<IAfEffectHandle> onReleasePatch(audio_patch_handle_t patchHandle) = 0;
virtual void dump2(int fd, int spaces) const = 0; // TODO(b/291319101) naming?
diff --git a/services/audiopolicy/common/managerdefinitions/include/EffectDescriptor.h b/services/audiopolicy/common/managerdefinitions/include/EffectDescriptor.h
index b92cd70..f7b9b33 100644
--- a/services/audiopolicy/common/managerdefinitions/include/EffectDescriptor.h
+++ b/services/audiopolicy/common/managerdefinitions/include/EffectDescriptor.h
@@ -99,21 +99,20 @@
* @return ioHandle if found, AUDIO_IO_HANDLE_NONE otherwise.
*/
audio_io_handle_t getIoForSession(audio_session_t sessionId,
- const effect_uuid_t *effectType = nullptr) const;
- bool hasOrphansForSession(audio_session_t sessionId) const;
- EffectDescriptorCollection getOrphanEffectsForSession(audio_session_t sessionId) const;
- void dump(String8 *dst, int spaces = 0, bool verbose = true) const;
+ const effect_uuid_t* effectType = nullptr) const;
/**
- * @brief Checks if there is at least one orphan effect with given sessionId and effect type
- * uuid.
+ * @brief Checks if there is at least one orphan effect with given sessionId and optional effect
+ * type uuid.
* @param sessionId Session ID.
- * @param effectType Effect type UUID, the implementation will be same as hasOrphansForSession
- * if null.
+ * @param effectType Optional effect type UUID pointer to effect_uuid_t, nullptr by default.
* @return True if there is an orphan effect for given sessionId and type UUID, false otherwise.
*/
- bool hasOrphanEffectsForSessionAndType(audio_session_t sessionId,
- const effect_uuid_t* effectType) const;
+ bool hasOrphansForSession(audio_session_t sessionId,
+ const effect_uuid_t* effectType = nullptr) const;
+
+ EffectDescriptorCollection getOrphanEffectsForSession(audio_session_t sessionId) const;
+ void dump(String8 *dst, int spaces = 0, bool verbose = true) const;
private:
status_t setEffectEnabled(const sp<EffectDescriptor> &effectDesc, bool enabled);
diff --git a/services/audiopolicy/common/managerdefinitions/src/EffectDescriptor.cpp b/services/audiopolicy/common/managerdefinitions/src/EffectDescriptor.cpp
index 7971b61..090da6c 100644
--- a/services/audiopolicy/common/managerdefinitions/src/EffectDescriptor.cpp
+++ b/services/audiopolicy/common/managerdefinitions/src/EffectDescriptor.cpp
@@ -210,27 +210,13 @@
}
}
-bool EffectDescriptorCollection::hasOrphansForSession(audio_session_t sessionId) const
-{
+bool EffectDescriptorCollection::hasOrphansForSession(audio_session_t sessionId,
+ const effect_uuid_t* effectType) const {
for (size_t i = 0; i < size(); ++i) {
sp<EffectDescriptor> effect = valueAt(i);
- if (effect->mSession == sessionId && effect->mIsOrphan) {
- return true;
- }
- }
- return false;
-}
-
-bool EffectDescriptorCollection::hasOrphanEffectsForSessionAndType(
- audio_session_t sessionId, const effect_uuid_t* effectType) const {
- if (effectType == nullptr) {
- return hasOrphansForSession(sessionId);
- }
-
- for (size_t i = 0; i < size(); ++i) {
- sp<EffectDescriptor> effect = valueAt(i);
- if (effect->mIsOrphan && effect->mSession == sessionId &&
- memcmp(&effect->mDesc.type, effectType, sizeof(effect_uuid_t)) == 0) {
+ if (effect->mSession == sessionId && effect->mIsOrphan &&
+ (effectType == nullptr ||
+ memcmp(&effect->mDesc.type, effectType, sizeof(effect_uuid_t)) == 0)) {
return true;
}
}
diff --git a/services/audiopolicy/managerdefault/AudioPolicyManager.cpp b/services/audiopolicy/managerdefault/AudioPolicyManager.cpp
index aae248b..811ca12 100644
--- a/services/audiopolicy/managerdefault/AudioPolicyManager.cpp
+++ b/services/audiopolicy/managerdefault/AudioPolicyManager.cpp
@@ -2087,8 +2087,7 @@
// matching criteria values in priority order for best matching output so far
std::vector<uint32_t> bestMatchCriteria(8, 0);
- const bool hasOrphanHaptic =
- mEffects.hasOrphanEffectsForSessionAndType(sessionId, FX_IID_HAPTICGENERATOR);
+ const bool hasOrphanHaptic = mEffects.hasOrphansForSession(sessionId, FX_IID_HAPTICGENERATOR);
const uint32_t channelCount = audio_channel_count_from_out_mask(channelMask);
const uint32_t hapticChannelCount = audio_channel_count_from_out_mask(
channelMask & AUDIO_CHANNEL_HAPTIC_ALL);
@@ -6145,7 +6144,7 @@
// means haptic use cases (either the client channelmask includes haptic bits, or created a
// HapticGenerator effect for this session) are not supported.
return clientHapticChannel == 0 &&
- !mEffects.hasOrphanEffectsForSessionAndType(sessionId, FX_IID_HAPTICGENERATOR);
+ !mEffects.hasOrphansForSession(sessionId, FX_IID_HAPTICGENERATOR);
}
}
@@ -6519,7 +6518,7 @@
status_t status = inputDesc->open(nullptr,
availProfileDevices.itemAt(0),
AUDIO_SOURCE_MIC,
- AUDIO_INPUT_FLAG_NONE,
+ (audio_input_flags_t) inProfile->getFlags(),
&input);
if (status != NO_ERROR) {
ALOGW("%s: Cannot open input stream for device %s for profile %s on hw module %s",
@@ -6834,7 +6833,8 @@
desc = new AudioInputDescriptor(profile, mpClientInterface);
audio_io_handle_t input = AUDIO_IO_HANDLE_NONE;
ALOGV("%s opening input for profile %s", __func__, profile->getTagName().c_str());
- status = desc->open(nullptr, device, AUDIO_SOURCE_MIC, AUDIO_INPUT_FLAG_NONE, &input);
+ status = desc->open(nullptr, device, AUDIO_SOURCE_MIC,
+ (audio_input_flags_t) profile->getFlags(), &input);
if (status == NO_ERROR) {
const String8& address = String8(device->address().c_str());