summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorEric Laurent <elaurent@google.com>2016-12-01 15:28:29 -0800
committerSean McCreary <mccreary@mcwest.org>2017-03-22 12:36:15 -0600
commitdac1444e4926f94d8d9ac6b6a098ac101ce4a7be (patch)
treef5ce31efebcd13cd9e32dced444b7d2b63d3e727
parent0574c56e88e96d33c923a8f54364ac0bf3dc5a91 (diff)
downloadframeworks_av-dac1444e4926f94d8d9ac6b6a098ac101ce4a7be.zip
frameworks_av-dac1444e4926f94d8d9ac6b6a098ac101ce4a7be.tar.gz
frameworks_av-dac1444e4926f94d8d9ac6b6a098ac101ce4a7be.tar.bz2
DO NOT MERGE - improve audio effect framwework thread safety
- Reorganize handle effect creation code to make sure the effect engine is created with both thread and effect chain mutex held. - Reorganize handle disconnect code to make sure the effect engine is released with both thread and effect chain mutex held. - Protect IEffect interface methods in EffectHande with a Mutex. - Only pin effect if the session was acquired first. - Do not use strong pointer to EffectModule in EffectHandles: only the EffectChain has a single strong reference to the EffectModule. Bug: 32707507 CVE-2017-0479 CVE-2017-0480 CVE-2017-0499 Change-Id: Ia1098cba2cd32cc2d1c9dfdff4adc2388dfed80e (cherry picked from commit b378b73dd7480b584340b8028802c9ca2d625123) (cherry picked from commit 22e26d8ee73488c58ba3e7928e5da155151abfd0 with backport by <sultanxda@gmail.com>)
-rw-r--r--services/audioflinger/AudioFlinger.cpp23
-rw-r--r--services/audioflinger/AudioFlinger.h1
-rw-r--r--services/audioflinger/Effects.cpp224
-rw-r--r--services/audioflinger/Effects.h37
-rw-r--r--services/audioflinger/Threads.cpp51
-rw-r--r--services/audioflinger/Threads.h7
-rw-r--r--services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp5
7 files changed, 225 insertions, 123 deletions
diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp
index 23215dd..9d435e9 100644
--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -1346,7 +1346,7 @@ void AudioFlinger::removeNotificationClient(pid_t pid)
ALOGV("%d died, releasing its sessions", pid);
size_t num = mAudioSessionRefs.size();
bool removed = false;
- for (size_t i = 0; i< num; ) {
+ for (size_t i = 0; i < num; ) {
AudioSessionRef *ref = mAudioSessionRefs.itemAt(i);
ALOGV(" pid %d @ %d", ref->mPid, i);
if (ref->mPid == pid) {
@@ -2343,7 +2343,7 @@ void AudioFlinger::acquireAudioSessionId(int audioSession, pid_t pid)
}
size_t num = mAudioSessionRefs.size();
- for (size_t i = 0; i< num; i++) {
+ for (size_t i = 0; i < num; i++) {
AudioSessionRef *ref = mAudioSessionRefs.editItemAt(i);
if (ref->mSessionid == audioSession && ref->mPid == caller) {
ref->mCnt++;
@@ -2364,7 +2364,7 @@ void AudioFlinger::releaseAudioSessionId(int audioSession, pid_t pid)
caller = pid;
}
size_t num = mAudioSessionRefs.size();
- for (size_t i = 0; i< num; i++) {
+ for (size_t i = 0; i < num; i++) {
AudioSessionRef *ref = mAudioSessionRefs.itemAt(i);
if (ref->mSessionid == audioSession && ref->mPid == caller) {
ref->mCnt--;
@@ -2382,6 +2382,18 @@ void AudioFlinger::releaseAudioSessionId(int audioSession, pid_t pid)
ALOGW_IF(caller != getpid_cached, "session id %d not found for pid %d", audioSession, caller);
}
+bool AudioFlinger::isSessionAcquired_l(audio_session_t audioSession)
+{
+ size_t num = mAudioSessionRefs.size();
+ for (size_t i = 0; i < num; i++) {
+ AudioSessionRef *ref = mAudioSessionRefs.itemAt(i);
+ if (ref->mSessionid == audioSession) {
+ return true;
+ }
+ }
+ return false;
+}
+
void AudioFlinger::purgeStaleEffects_l() {
ALOGV("purging stale effects");
@@ -2723,8 +2735,9 @@ sp<IEffect> AudioFlinger::createEffect(
sp<Client> client = registerPid(pid);
// create effect on selected output thread
+ bool pinned = (sessionId > AUDIO_SESSION_OUTPUT_MIX) && isSessionAcquired_l((audio_session_t)sessionId);
handle = thread->createEffect_l(client, effectClient, priority, sessionId,
- &desc, enabled, &lStatus);
+ &desc, enabled, &lStatus, pinned);
if (handle != 0 && id != NULL) {
*id = handle->id();
}
@@ -2924,7 +2937,7 @@ bool AudioFlinger::updateOrphanEffectChains(const sp<AudioFlinger::EffectModule>
ALOGV("updateOrphanEffectChains session %d index %d", session, index);
if (index >= 0) {
sp<EffectChain> chain = mOrphanEffectChains.valueAt(index);
- if (chain->removeEffect_l(effect) == 0) {
+ if (chain->removeEffect_l(effect, true) == 0) {
ALOGV("updateOrphanEffectChains removing effect chain at index %d", index);
mOrphanEffectChains.removeItemsAt(index);
}
diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h
index 08fa70d..3e9a088 100644
--- a/services/audioflinger/AudioFlinger.h
+++ b/services/audioflinger/AudioFlinger.h
@@ -593,6 +593,7 @@ private:
void removeNotificationClient(pid_t pid);
bool isNonOffloadableGlobalEffectEnabled_l();
void onNonOffloadableGlobalEffectEnable();
+ bool isSessionAcquired_l(audio_session_t audioSession);
// Store an effect chain to mOrphanEffectChains keyed vector.
// Called when a thread exits and effects are still attached to it.
diff --git a/services/audioflinger/Effects.cpp b/services/audioflinger/Effects.cpp
index b9fe741..fb9e157 100644
--- a/services/audioflinger/Effects.cpp
+++ b/services/audioflinger/Effects.cpp
@@ -59,8 +59,9 @@ AudioFlinger::EffectModule::EffectModule(ThreadBase *thread,
const wp<AudioFlinger::EffectChain>& chain,
effect_descriptor_t *desc,
int id,
- int sessionId)
- : mPinned(sessionId > AUDIO_SESSION_OUTPUT_MIX),
+ audio_session_t sessionId,
+ bool pinned)
+ : mPinned(pinned),
mThread(thread), mChain(chain), mId(id), mSessionId(sessionId),
mDescriptor(*desc),
// mConfig is set by configure() and not used before then
@@ -71,7 +72,7 @@ AudioFlinger::EffectModule::EffectModule(ThreadBase *thread,
mSuspended(false),
mAudioFlinger(thread->mAudioFlinger)
{
- ALOGV("Constructor %p", this);
+ ALOGV("Constructor %p pinned %d", this, pinned);
int lStatus;
// create effect engine from effect factory
@@ -86,6 +87,8 @@ AudioFlinger::EffectModule::EffectModule(ThreadBase *thread,
goto Error;
}
+ setOffloaded(thread->type() == ThreadBase::OFFLOAD, thread->id());
+
ALOGV("Constructor success name %s, Interface %p", mDescriptor.name, mEffectInterface);
return;
Error:
@@ -98,9 +101,8 @@ AudioFlinger::EffectModule::~EffectModule()
{
ALOGV("Destructor %p", this);
if (mEffectInterface != NULL) {
- remove_effect_from_hal_l();
- // release effect engine
- EffectRelease(mEffectInterface);
+ ALOGW("EffectModule %p destructor called with unreleased interface", this);
+ release_l();
}
}
@@ -115,7 +117,7 @@ status_t AudioFlinger::EffectModule::addHandle(EffectHandle *handle)
size_t i;
for (i = 0; i < size; i++) {
EffectHandle *h = mHandles[i];
- if (h == NULL || h->destroyed_l()) {
+ if (h == NULL || h->disconnected()) {
continue;
}
// first non destroyed handle is considered in control
@@ -143,9 +145,14 @@ status_t AudioFlinger::EffectModule::addHandle(EffectHandle *handle)
return status;
}
-size_t AudioFlinger::EffectModule::removeHandle(EffectHandle *handle)
+ssize_t AudioFlinger::EffectModule::removeHandle(EffectHandle *handle)
{
Mutex::Autolock _l(mLock);
+ return removeHandle_l(handle);
+}
+
+ssize_t AudioFlinger::EffectModule::removeHandle_l(EffectHandle *handle)
+{
size_t size = mHandles.size();
size_t i;
for (i = 0; i < size; i++) {
@@ -154,9 +161,10 @@ size_t AudioFlinger::EffectModule::removeHandle(EffectHandle *handle)
}
}
if (i == size) {
- return size;
+ ALOGW("%s %p handle not found %p", __FUNCTION__, this, handle);
+ return BAD_VALUE;
}
- ALOGV("removeHandle() %p removed handle %p in position %d", this, handle, i);
+ ALOGV("removeHandle_l() %p removed handle %p in position %zu", this, handle, i);
mHandles.removeAt(i);
// if removed from first place, move effect control from this handle to next in line
@@ -183,7 +191,7 @@ AudioFlinger::EffectHandle *AudioFlinger::EffectModule::controlHandle_l()
// the first valid handle in the list has control over the module
for (size_t i = 0; i < mHandles.size(); i++) {
EffectHandle *h = mHandles[i];
- if (h != NULL && !h->destroyed_l()) {
+ if (h != NULL && !h->disconnected()) {
return h;
}
}
@@ -191,29 +199,22 @@ AudioFlinger::EffectHandle *AudioFlinger::EffectModule::controlHandle_l()
return NULL;
}
-size_t AudioFlinger::EffectModule::disconnect(EffectHandle *handle, bool unpinIfLast)
+// unsafe method called when the effect parent thread has been destroyed
+ssize_t AudioFlinger::EffectModule::disconnectHandle(EffectHandle *handle, bool unpinIfLast)
{
ALOGV("disconnect() %p handle %p", this, handle);
- // keep a strong reference on this EffectModule to avoid calling the
- // destructor before we exit
- sp<EffectModule> keep(this);
- {
- if (removeHandle(handle) == 0) {
- if (!isPinned() || unpinIfLast) {
- sp<ThreadBase> thread = mThread.promote();
- if (thread != 0) {
- Mutex::Autolock _l(thread->mLock);
- thread->removeEffect_l(this);
- }
- sp<AudioFlinger> af = mAudioFlinger.promote();
- if (af != 0) {
- af->updateOrphanEffectChains(this);
- }
- AudioSystem::unregisterEffect(mId);
- }
+ Mutex::Autolock _l(mLock);
+ ssize_t numHandles = removeHandle_l(handle);
+ if ((numHandles == 0) && (!mPinned || unpinIfLast)) {
+ AudioSystem::unregisterEffect(mId);
+ sp<AudioFlinger> af = mAudioFlinger.promote();
+ if (af != 0) {
+ mLock.unlock();
+ af->updateOrphanEffectChains(this);
+ mLock.lock();
}
}
- return mHandles.size();
+ return numHandles;
}
void AudioFlinger::EffectModule::updateState() {
@@ -528,6 +529,17 @@ status_t AudioFlinger::EffectModule::stop_l()
return status;
}
+// must be called with EffectChain::mLock held
+void AudioFlinger::EffectModule::release_l()
+{
+ if (mEffectInterface != NULL) {
+ remove_effect_from_hal_l();
+ // release effect engine
+ EffectRelease(mEffectInterface);
+ mEffectInterface = NULL;
+ }
+}
+
status_t AudioFlinger::EffectModule::remove_effect_from_hal_l()
{
if ((mDescriptor.flags & EFFECT_FLAG_TYPE_MASK) == EFFECT_FLAG_TYPE_PRE_PROC ||
@@ -620,7 +632,7 @@ status_t AudioFlinger::EffectModule::command(uint32_t cmdCode,
uint32_t size = (replySize == NULL) ? 0 : *replySize;
for (size_t i = 1; i < mHandles.size(); i++) {
EffectHandle *h = mHandles[i];
- if (h != NULL && !h->destroyed_l()) {
+ if (h != NULL && !h->disconnected()) {
h->commandExecuted(cmdCode, cmdSize, pCmdData, size, pReplyData);
}
}
@@ -673,7 +685,7 @@ status_t AudioFlinger::EffectModule::setEnabled_l(bool enabled)
}
for (size_t i = 1; i < mHandles.size(); i++) {
EffectHandle *h = mHandles[i];
- if (h != NULL && !h->destroyed_l()) {
+ if (h != NULL && !h->disconnected()) {
h->setEnabled(enabled);
}
}
@@ -838,8 +850,7 @@ bool AudioFlinger::EffectModule::purgeHandles()
Mutex::Autolock _l(mLock);
for (size_t i = 0; i < mHandles.size(); i++) {
EffectHandle *handle = mHandles[i];
- if (handle != NULL && !handle->destroyed_l()) {
- handle->effect().clear();
+ if (handle != NULL && !handle->disconnected()) {
if (handle->hasControl()) {
enabled = handle->enabled();
}
@@ -1066,7 +1077,7 @@ void AudioFlinger::EffectModule::dump(int fd, const Vector<String16>& args __unu
result.append("\t\t\t Pid Priority Ctrl Locked client server\n");
for (size_t i = 0; i < mHandles.size(); ++i) {
EffectHandle *handle = mHandles[i];
- if (handle != NULL && !handle->destroyed_l()) {
+ if (handle != NULL && !handle->disconnected()) {
handle->dumpToBuffer(buffer, SIZE);
result.append(buffer);
}
@@ -1092,7 +1103,7 @@ AudioFlinger::EffectHandle::EffectHandle(const sp<EffectModule>& effect,
int32_t priority)
: BnEffect(),
mEffect(effect), mEffectClient(effectClient), mClient(client), mCblk(NULL),
- mPriority(priority), mHasControl(false), mEnabled(false), mDestroyed(false)
+ mPriority(priority), mHasControl(false), mEnabled(false), mDisconnected(false)
{
ALOGV("constructor %p", this);
@@ -1115,14 +1126,6 @@ AudioFlinger::EffectHandle::EffectHandle(const sp<EffectModule>& effect,
AudioFlinger::EffectHandle::~EffectHandle()
{
ALOGV("Destructor %p", this);
-
- if (mEffect == 0) {
- mDestroyed = true;
- return;
- }
- mEffect->lock();
- mDestroyed = true;
- mEffect->unlock();
disconnect(false);
}
@@ -1133,13 +1136,15 @@ status_t AudioFlinger::EffectHandle::initCheck()
status_t AudioFlinger::EffectHandle::enable()
{
+ AutoMutex _l(mLock);
ALOGV("enable %p", this);
+ sp<EffectModule> effect = mEffect.promote();
+ if (effect == 0 || mDisconnected) {
+ return DEAD_OBJECT;
+ }
if (!mHasControl) {
return INVALID_OPERATION;
}
- if (mEffect == 0) {
- return DEAD_OBJECT;
- }
if (mEnabled) {
return NO_ERROR;
@@ -1147,20 +1152,20 @@ status_t AudioFlinger::EffectHandle::enable()
mEnabled = true;
- sp<ThreadBase> thread = mEffect->thread().promote();
+ sp<ThreadBase> thread = effect->thread().promote();
if (thread != 0) {
- thread->checkSuspendOnEffectEnabled(mEffect, true, mEffect->sessionId());
+ thread->checkSuspendOnEffectEnabled(effect, true, effect->sessionId());
}
// checkSuspendOnEffectEnabled() can suspend this same effect when enabled
- if (mEffect->suspended()) {
+ if (effect->suspended()) {
return NO_ERROR;
}
- status_t status = mEffect->setEnabled(true);
+ status_t status = effect->setEnabled(true);
if (status != NO_ERROR) {
if (thread != 0) {
- thread->checkSuspendOnEffectEnabled(mEffect, false, mEffect->sessionId());
+ thread->checkSuspendOnEffectEnabled(effect, false, effect->sessionId());
}
mEnabled = false;
} else {
@@ -1171,13 +1176,13 @@ status_t AudioFlinger::EffectHandle::enable()
Mutex::Autolock _l(t->mLock);
t->broadcast_l();
}
- if (!mEffect->isOffloadable()) {
+ if (!effect->isOffloadable()) {
if (thread->type() == ThreadBase::OFFLOAD ||
(thread->type() == ThreadBase::DIRECT && thread->mIsDirectPcm)) {
PlaybackThread *t = (PlaybackThread *)thread.get();
t->invalidateTracks(AUDIO_STREAM_MUSIC);
}
- if (mEffect->sessionId() == AUDIO_SESSION_OUTPUT_MIX) {
+ if (effect->sessionId() == AUDIO_SESSION_OUTPUT_MIX) {
thread->mAudioFlinger->onNonOffloadableGlobalEffectEnable();
}
}
@@ -1189,27 +1194,29 @@ status_t AudioFlinger::EffectHandle::enable()
status_t AudioFlinger::EffectHandle::disable()
{
ALOGV("disable %p", this);
+ AutoMutex _l(mLock);
+ sp<EffectModule> effect = mEffect.promote();
+ if (effect == 0 || mDisconnected) {
+ return DEAD_OBJECT;
+ }
if (!mHasControl) {
return INVALID_OPERATION;
}
- if (mEffect == 0) {
- return DEAD_OBJECT;
- }
if (!mEnabled) {
return NO_ERROR;
}
mEnabled = false;
- if (mEffect->suspended()) {
+ if (effect->suspended()) {
return NO_ERROR;
}
- status_t status = mEffect->setEnabled(false);
+ status_t status = effect->setEnabled(false);
- sp<ThreadBase> thread = mEffect->thread().promote();
+ sp<ThreadBase> thread = effect->thread().promote();
if (thread != 0) {
- thread->checkSuspendOnEffectEnabled(mEffect, false, mEffect->sessionId());
+ thread->checkSuspendOnEffectEnabled(effect, false, effect->sessionId());
if ((thread->type() == ThreadBase::OFFLOAD) ||
(thread->type() == ThreadBase::DIRECT && thread->mIsDirectPcm)){
PlaybackThread *t = (PlaybackThread *)thread.get();
@@ -1223,25 +1230,39 @@ status_t AudioFlinger::EffectHandle::disable()
void AudioFlinger::EffectHandle::disconnect()
{
+ ALOGV("%s %p", __FUNCTION__, this);
disconnect(true);
}
void AudioFlinger::EffectHandle::disconnect(bool unpinIfLast)
{
- ALOGV("disconnect(%s)", unpinIfLast ? "true" : "false");
- if (mEffect == 0) {
+ AutoMutex _l(mLock);
+ ALOGV("disconnect(%s) %p", unpinIfLast ? "true" : "false", this);
+ if (mDisconnected) {
+ if (unpinIfLast) {
+ android_errorWriteLog(0x534e4554, "32707507");
+ }
return;
}
- // restore suspended effects if the disconnected handle was enabled and the last one.
- if ((mEffect->disconnect(this, unpinIfLast) == 0) && mEnabled) {
- sp<ThreadBase> thread = mEffect->thread().promote();
- if (thread != 0) {
- thread->checkSuspendOnEffectEnabled(mEffect, false, mEffect->sessionId());
+ mDisconnected = true;
+ sp<ThreadBase> thread;
+ {
+ sp<EffectModule> effect = mEffect.promote();
+ if (effect != 0) {
+ thread = effect->thread().promote();
+ }
+ }
+ if (thread != 0) {
+ thread->disconnectEffectHandle(this, unpinIfLast);
+ } else {
+ ALOGW("%s Effect handle %p disconnected after thread destruction", __FUNCTION__, this);
+ // try to cleanup as much as we can
+ sp<EffectModule> effect = mEffect.promote();
+ if (effect != 0) {
+ effect->disconnectHandle(this, unpinIfLast);
}
}
- // release sp on module => module destructor can be called now
- mEffect.clear();
if (mClient != 0) {
if (mCblk != NULL) {
// unlike ~TrackBase(), mCblk is never a local new, so don't delete
@@ -1261,15 +1282,17 @@ status_t AudioFlinger::EffectHandle::command(uint32_t cmdCode,
void *pReplyData)
{
ALOGVV("command(), cmdCode: %d, mHasControl: %d, mEffect: %p",
- cmdCode, mHasControl, (mEffect == 0) ? 0 : mEffect.get());
+ cmdCode, mHasControl, mEffect.unsafe_get());
+ AutoMutex _l(mLock);
+ sp<EffectModule> effect = mEffect.promote();
+ if (effect == 0 || mDisconnected) {
+ return DEAD_OBJECT;
+ }
// only get parameter command is permitted for applications not controlling the effect
if (!mHasControl && cmdCode != EFFECT_CMD_GET_PARAM) {
return INVALID_OPERATION;
}
- if (mEffect == 0) {
- return DEAD_OBJECT;
- }
if (mClient == 0) {
return INVALID_OPERATION;
}
@@ -1314,7 +1337,7 @@ status_t AudioFlinger::EffectHandle::command(uint32_t cmdCode,
int reply = 0;
uint32_t rsize = sizeof(reply);
- status_t ret = mEffect->command(EFFECT_CMD_SET_PARAM,
+ status_t ret = effect->command(EFFECT_CMD_SET_PARAM,
size,
param,
&rsize,
@@ -1351,7 +1374,7 @@ status_t AudioFlinger::EffectHandle::command(uint32_t cmdCode,
return disable();
}
- return mEffect->command(cmdCode, cmdSize, pCmdData, replySize, pReplyData);
+ return effect->command(cmdCode, cmdSize, pCmdData, replySize, pReplyData);
}
void AudioFlinger::EffectHandle::setControl(bool hasControl, bool signal, bool enabled)
@@ -1433,7 +1456,6 @@ AudioFlinger::EffectChain::~EffectChain()
if (mOwnInBuffer) {
delete mInBuffer;
}
-
}
// getEffectFromDesc_l() must be called with ThreadBase::mLock held
@@ -1547,13 +1569,38 @@ void AudioFlinger::EffectChain::process_l()
}
}
-// addEffect_l() must be called with PlaybackThread::mLock held
+// createEffect_l() must be called with ThreadBase::mLock held
+status_t AudioFlinger::EffectChain::createEffect_l(sp<EffectModule>& effect,
+ ThreadBase *thread,
+ effect_descriptor_t *desc,
+ int id,
+ audio_session_t sessionId,
+ bool pinned)
+{
+ Mutex::Autolock _l(mLock);
+ effect = new EffectModule(thread, this, desc, id, sessionId, pinned);
+ status_t lStatus = effect->status();
+ if (lStatus == NO_ERROR) {
+ lStatus = addEffect_ll(effect);
+ }
+ if (lStatus != NO_ERROR) {
+ effect.clear();
+ }
+ return lStatus;
+}
+
+// addEffect_l() must be called with ThreadBase::mLock held
status_t AudioFlinger::EffectChain::addEffect_l(const sp<EffectModule>& effect)
{
+ Mutex::Autolock _l(mLock);
+ return addEffect_ll(effect);
+}
+// addEffect_l() must be called with ThreadBase::mLock and EffectChain::mLock held
+status_t AudioFlinger::EffectChain::addEffect_ll(const sp<EffectModule>& effect)
+{
effect_descriptor_t desc = effect->desc();
uint32_t insertPref = desc.flags & EFFECT_FLAG_INSERT_MASK;
- Mutex::Autolock _l(mLock);
effect->setChain(this);
sp<ThreadBase> thread = mThread.promote();
if (thread == 0) {
@@ -1663,8 +1710,9 @@ status_t AudioFlinger::EffectChain::addEffect_l(const sp<EffectModule>& effect)
return NO_ERROR;
}
-// removeEffect_l() must be called with PlaybackThread::mLock held
-size_t AudioFlinger::EffectChain::removeEffect_l(const sp<EffectModule>& effect)
+// removeEffect_l() must be called with ThreadBase::mLock held
+size_t AudioFlinger::EffectChain::removeEffect_l(const sp<EffectModule>& effect,
+ bool release)
{
Mutex::Autolock _l(mLock);
size_t size = mEffects.size();
@@ -1679,6 +1727,10 @@ size_t AudioFlinger::EffectChain::removeEffect_l(const sp<EffectModule>& effect)
mEffects[i]->state() == EffectModule::STOPPING) {
mEffects[i]->stop();
}
+ if (release) {
+ mEffects[i]->release_l();
+ }
+
if (type == EFFECT_FLAG_TYPE_AUXILIARY) {
delete[] effect->inBuffer();
} else {
@@ -1697,7 +1749,7 @@ size_t AudioFlinger::EffectChain::removeEffect_l(const sp<EffectModule>& effect)
return mEffects.size();
}
-// setDevice_l() must be called with PlaybackThread::mLock held
+// setDevice_l() must be called with ThreadBase::mLock held
void AudioFlinger::EffectChain::setDevice_l(audio_devices_t device)
{
size_t size = mEffects.size();
@@ -1706,7 +1758,7 @@ void AudioFlinger::EffectChain::setDevice_l(audio_devices_t device)
}
}
-// setMode_l() must be called with PlaybackThread::mLock held
+// setMode_l() must be called with ThreadBase::mLock held
void AudioFlinger::EffectChain::setMode_l(audio_mode_t mode)
{
size_t size = mEffects.size();
@@ -1715,7 +1767,7 @@ void AudioFlinger::EffectChain::setMode_l(audio_mode_t mode)
}
}
-// setAudioSource_l() must be called with PlaybackThread::mLock held
+// setAudioSource_l() must be called with ThreadBase::mLock held
void AudioFlinger::EffectChain::setAudioSource_l(audio_source_t source)
{
size_t size = mEffects.size();
@@ -1724,7 +1776,7 @@ void AudioFlinger::EffectChain::setAudioSource_l(audio_source_t source)
}
}
-// setVolume_l() must be called with PlaybackThread::mLock held
+// setVolume_l() must be called with ThreadBase::mLock held
bool AudioFlinger::EffectChain::setVolume_l(uint32_t *left, uint32_t *right)
{
uint32_t newLeft = *left;
@@ -1876,7 +1928,7 @@ void AudioFlinger::EffectChain::setEffectSuspended_l(
effect->setSuspended(false);
effect->lock();
EffectHandle *handle = effect->controlHandle_l();
- if (handle != NULL && !handle->destroyed_l()) {
+ if (handle != NULL && !handle->disconnected()) {
effect->setEnabled_l(handle->enabled());
}
effect->unlock();
diff --git a/services/audioflinger/Effects.h b/services/audioflinger/Effects.h
index 6f93f81..4d53c0d 100644
--- a/services/audioflinger/Effects.h
+++ b/services/audioflinger/Effects.h
@@ -45,7 +45,8 @@ public:
const wp<AudioFlinger::EffectChain>& chain,
effect_descriptor_t *desc,
int id,
- int sessionId);
+ audio_session_t sessionId,
+ bool pinned);
virtual ~EffectModule();
enum effect_state {
@@ -93,8 +94,9 @@ public:
const wp<ThreadBase>& thread() { return mThread; }
status_t addHandle(EffectHandle *handle);
- size_t disconnect(EffectHandle *handle, bool unpinIfLast);
- size_t removeHandle(EffectHandle *handle);
+ ssize_t disconnectHandle(EffectHandle *handle, bool unpinIfLast);
+ ssize_t removeHandle(EffectHandle *handle);
+ ssize_t removeHandle_l(EffectHandle *handle);
const effect_descriptor_t& desc() const { return mDescriptor; }
wp<EffectChain>& chain() { return mChain; }
@@ -120,6 +122,7 @@ public:
status_t setOffloaded(bool offloaded, audio_io_handle_t io);
bool isOffloaded() const;
void addEffectToHal_l();
+ void release_l();
void dump(int fd, const Vector<String16>& args);
@@ -204,12 +207,17 @@ public:
bool enabled() const { return mEnabled; }
// Getters
- int id() const { return mEffect->id(); }
+ wp<EffectModule> effect() const { return mEffect; }
+ int id() const {
+ sp<EffectModule> effect = mEffect.promote();
+ if (effect == 0) {
+ return 0;
+ }
+ return effect->id();
+ }
int priority() const { return mPriority; }
bool hasControl() const { return mHasControl; }
- sp<EffectModule> effect() const { return mEffect; }
- // destroyed_l() must be called with the associated EffectModule mLock held
- bool destroyed_l() const { return mDestroyed; }
+ bool disconnected() const { return mDisconnected; }
void dumpToBuffer(char* buffer, size_t size);
@@ -218,7 +226,8 @@ protected:
EffectHandle(const EffectHandle&);
EffectHandle& operator =(const EffectHandle&);
- sp<EffectModule> mEffect; // pointer to controlled EffectModule
+ Mutex mLock; // protects IEffect method calls
+ wp<EffectModule> mEffect; // pointer to controlled EffectModule
sp<IEffectClient> mEffectClient; // callback interface for client notifications
/*const*/ sp<Client> mClient; // client for shared memory allocation, see disconnect()
sp<IMemory> mCblkMemory; // shared memory for control block
@@ -229,8 +238,7 @@ protected:
bool mHasControl; // true if this handle is controlling the effect
bool mEnabled; // cached enable state: needed when the effect is
// restored after being suspended
- bool mDestroyed; // Set to true by destructor. Access with EffectModule
- // mLock held
+ bool mDisconnected; // Set to true by disconnect()
};
// the EffectChain class represents a group of effects associated to one audio session.
@@ -263,8 +271,15 @@ public:
mLock.unlock();
}
+ status_t createEffect_l(sp<EffectModule>& effect,
+ ThreadBase *thread,
+ effect_descriptor_t *desc,
+ int id,
+ audio_session_t sessionId,
+ bool pinned);
status_t addEffect_l(const sp<EffectModule>& handle);
- size_t removeEffect_l(const sp<EffectModule>& handle);
+ status_t addEffect_ll(const sp<EffectModule>& handle);
+ size_t removeEffect_l(const sp<EffectModule>& handle, bool release = false);
int sessionId() const { return mSessionId; }
void setSessionId(int sessionId) { mSessionId = sessionId; }
diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp
index e5e8bdb..d7c9c47 100644
--- a/services/audioflinger/Threads.cpp
+++ b/services/audioflinger/Threads.cpp
@@ -1159,7 +1159,8 @@ sp<AudioFlinger::EffectHandle> AudioFlinger::ThreadBase::createEffect_l(
int sessionId,
effect_descriptor_t *desc,
int *enabled,
- status_t *status)
+ status_t *status,
+ bool pinned)
{
sp<EffectModule> effect;
sp<EffectHandle> handle;
@@ -1253,20 +1254,7 @@ sp<AudioFlinger::EffectHandle> AudioFlinger::ThreadBase::createEffect_l(
}
effectRegistered = true;
// create a new effect module if none present in the chain
- effect = new EffectModule(this, chain, desc, id, sessionId);
- lStatus = effect->status();
- if (lStatus != NO_ERROR) {
- goto Exit;
- }
-
- bool setVal = false;
- if (mType == OFFLOAD || (mType == DIRECT && mIsDirectPcm)) {
- setVal = true;
- }
-
- effect->setOffloaded(setVal, mId);
-
- lStatus = chain->addEffect_l(effect);
+ lStatus = chain->createEffect_l(effect, this, desc, id, (audio_session_t)sessionId, pinned);
if (lStatus != NO_ERROR) {
goto Exit;
}
@@ -1307,6 +1295,33 @@ Exit:
return handle;
}
+void AudioFlinger::ThreadBase::disconnectEffectHandle(EffectHandle *handle,
+ bool unpinIfLast)
+{
+ bool remove = false;
+ sp<EffectModule> effect;
+ {
+ Mutex::Autolock _l(mLock);
+
+ effect = handle->effect().promote();
+ if (effect == 0) {
+ return;
+ }
+ // restore suspended effects if the disconnected handle was enabled and the last one.
+ remove = (effect->removeHandle(handle) == 0) && (!effect->isPinned() || unpinIfLast);
+ if (remove) {
+ removeEffect_l(effect, true);
+ }
+ }
+ if (remove) {
+ mAudioFlinger->updateOrphanEffectChains(effect);
+ AudioSystem::unregisterEffect(effect->id());
+ if (handle->enabled()) {
+ checkSuspendOnEffectEnabled(effect, false, effect->sessionId());
+ }
+ }
+}
+
sp<AudioFlinger::EffectModule> AudioFlinger::ThreadBase::getEffect(int sessionId, int effectId)
{
Mutex::Autolock _l(mLock);
@@ -1371,9 +1386,9 @@ status_t AudioFlinger::ThreadBase::addEffect_l(const sp<EffectModule>& effect)
return NO_ERROR;
}
-void AudioFlinger::ThreadBase::removeEffect_l(const sp<EffectModule>& effect) {
+void AudioFlinger::ThreadBase::removeEffect_l(const sp<EffectModule>& effect, bool release) {
- ALOGV("removeEffect_l() %p effect %p", this, effect.get());
+ ALOGV("%s %p effect %p", __FUNCTION__, this, effect.get());
effect_descriptor_t desc = effect->desc();
if ((desc.flags & EFFECT_FLAG_TYPE_MASK) == EFFECT_FLAG_TYPE_AUXILIARY) {
detachAuxEffect_l(effect->id());
@@ -1382,7 +1397,7 @@ void AudioFlinger::ThreadBase::removeEffect_l(const sp<EffectModule>& effect) {
sp<EffectChain> chain = effect->chain().promote();
if (chain != 0) {
// remove effect chain if removing last effect
- if (chain->removeEffect_l(effect) == 0) {
+ if (chain->removeEffect_l(effect, release) == 0) {
removeEffectChain_l(chain);
}
} else {
diff --git a/services/audioflinger/Threads.h b/services/audioflinger/Threads.h
index 8fab1e4..4009c18 100644
--- a/services/audioflinger/Threads.h
+++ b/services/audioflinger/Threads.h
@@ -309,7 +309,8 @@ public:
int sessionId,
effect_descriptor_t *desc,
int *enabled,
- status_t *status /*non-NULL*/);
+ status_t *status /*non-NULL*/,
+ bool pinned);
// return values for hasAudioSession (bit field)
enum effect_state {
@@ -346,7 +347,9 @@ public:
status_t addEffect_l(const sp< EffectModule>& effect);
// remove and effect module. Also removes the effect chain is this was the last
// effect
- void removeEffect_l(const sp< EffectModule>& effect);
+ void removeEffect_l(const sp< EffectModule>& effect, bool release = false);
+ // disconnect an effect handle from module and destroy module if last handle
+ void disconnectEffectHandle(EffectHandle *handle, bool unpinIfLast);
// detach all tracks connected to an auxiliary effect
virtual void detachAuxEffect_l(int effectId __unused) {}
// returns either EFFECT_SESSION if effects on this audio session exist in one
diff --git a/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp b/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp
index b23c35e..45b3bb0 100644
--- a/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp
+++ b/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp
@@ -417,7 +417,6 @@ void AudioPolicyService::releaseInput(audio_io_handle_t input,
sp<AudioPolicyEffects>audioPolicyEffects;
{
Mutex::Autolock _l(mLock);
- mAudioPolicyManager->releaseInput(input, session);
audioPolicyEffects = mAudioPolicyEffects;
}
if (audioPolicyEffects != 0) {
@@ -427,6 +426,10 @@ void AudioPolicyService::releaseInput(audio_io_handle_t input,
ALOGW("Failed to release effects on input %d", input);
}
}
+ {
+ Mutex::Autolock _l(mLock);
+ mAudioPolicyManager->releaseInput(input, session);
+ }
}
status_t AudioPolicyService::initStreamVolume(audio_stream_type_t stream,