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.
- Check reply size before writing status in EffectHandle::command()
Bug: 32707507
Bug: 32095713
Change-Id: Ia1098cba2cd32cc2d1c9dfdff4adc2388dfed80e
diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp
index 0aef53f..bf82078 100644
--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -1105,7 +1105,7 @@
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) {
@@ -1885,7 +1885,7 @@
}
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++;
@@ -1903,7 +1903,7 @@
pid_t caller = IPCThreadState::self()->getCallingPid();
ALOGV("releasing %d from %d", audioSession, caller);
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--;
@@ -1921,6 +1921,18 @@
ALOGW_IF(caller != getpid_cached, "session id %d not found for pid %d", audioSession, caller);
}
+bool AudioFlinger::isSessionAcquired_l(int 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");
@@ -2253,8 +2265,9 @@
sp<Client> client = registerPid_l(pid);
// create effect on selected output thread
+ bool pinned = (sessionId > AUDIO_SESSION_OUTPUT_MIX) && isSessionAcquired_l(sessionId);
handle = thread->createEffect_l(client, effectClient, priority, sessionId,
- &desc, enabled, &lStatus);
+ &desc, enabled, &lStatus, pinned);
if (handle != 0 && id != NULL) {
*id = handle->id();
}
diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h
index 53e238e..02f0fa0 100644
--- a/services/audioflinger/AudioFlinger.h
+++ b/services/audioflinger/AudioFlinger.h
@@ -470,6 +470,7 @@
bool isNonOffloadableGlobalEffectEnabled_l();
void onNonOffloadableGlobalEffectEnable();
+ bool isSessionAcquired_l(int audioSession);
class AudioHwDevice {
public:
diff --git a/services/audioflinger/AudioPolicyService.cpp b/services/audioflinger/AudioPolicyService.cpp
index 2e48c2d..29cde2a 100644
--- a/services/audioflinger/AudioPolicyService.cpp
+++ b/services/audioflinger/AudioPolicyService.cpp
@@ -383,16 +383,14 @@
return;
}
Mutex::Autolock _l(mLock);
- mpAudioPolicy->release_input(mpAudioPolicy, input);
-
ssize_t index = mInputs.indexOfKey(input);
- if (index < 0) {
- return;
+ if (index >= 0) {
+ InputDesc *inputDesc = mInputs.valueAt(index);
+ setPreProcessorEnabled(inputDesc, false);
+ delete inputDesc;
+ mInputs.removeItemsAt(index);
}
- InputDesc *inputDesc = mInputs.valueAt(index);
- setPreProcessorEnabled(inputDesc, false);
- delete inputDesc;
- mInputs.removeItemsAt(index);
+ mpAudioPolicy->release_input(mpAudioPolicy, input);
}
status_t AudioPolicyService::initStreamVolume(audio_stream_type_t stream,
diff --git a/services/audioflinger/Effects.cpp b/services/audioflinger/Effects.cpp
index 7aa1489..9e06358 100644
--- a/services/audioflinger/Effects.cpp
+++ b/services/audioflinger/Effects.cpp
@@ -57,8 +57,9 @@
const wp<AudioFlinger::EffectChain>& chain,
effect_descriptor_t *desc,
int id,
- int sessionId)
- : mPinned(sessionId > AUDIO_SESSION_OUTPUT_MIX),
+ int 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
@@ -68,7 +69,7 @@
// mDisableWaitCnt is set by process() and updateState() and not used before then
mSuspended(false)
{
- ALOGV("Constructor %p", this);
+ ALOGV("Constructor %p pinned %d", this, pinned);
int lStatus;
// create effect engine from effect factory
@@ -83,6 +84,8 @@
goto Error;
}
+ setOffloaded(thread->type() == ThreadBase::OFFLOAD, thread->id());
+
ALOGV("Constructor success name %s, Interface %p", mDescriptor.name, mEffectInterface);
return;
Error:
@@ -95,9 +98,8 @@
{
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();
}
}
@@ -112,7 +114,7 @@
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
@@ -139,9 +141,14 @@
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++) {
@@ -150,9 +157,10 @@
}
}
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
@@ -179,7 +187,7 @@
// 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;
}
}
@@ -187,19 +195,16 @@
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);
- {
- sp<ThreadBase> thread = mThread.promote();
- if (thread != 0) {
- thread->disconnectEffect(keep, handle, unpinIfLast);
- }
+ Mutex::Autolock _l(mLock);
+ ssize_t numHandles = removeHandle_l(handle);
+ if ((numHandles == 0) && (!mPinned || unpinIfLast)) {
+ AudioSystem::unregisterEffect(mId);
}
- return mHandles.size();
+ return numHandles;
}
void AudioFlinger::EffectModule::updateState() {
@@ -496,6 +501,17 @@
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 ||
@@ -588,7 +604,7 @@
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);
}
}
@@ -641,7 +657,7 @@
}
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);
}
}
@@ -806,8 +822,7 @@
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();
}
@@ -926,7 +941,7 @@
result.append("\t\t\tPid 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->dump(buffer, SIZE);
result.append(buffer);
}
@@ -954,7 +969,7 @@
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);
@@ -980,26 +995,20 @@
AudioFlinger::EffectHandle::~EffectHandle()
{
ALOGV("Destructor %p", this);
-
- if (mEffect == 0) {
- mDestroyed = true;
- return;
- }
- mEffect->lock();
- mDestroyed = true;
- mEffect->unlock();
disconnect(false);
}
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;
@@ -1007,20 +1016,20 @@
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 {
@@ -1030,12 +1039,12 @@
Mutex::Autolock _l(t->mLock);
t->broadcast_l();
}
- if (!mEffect->isOffloadable()) {
+ if (!effect->isOffloadable()) {
if (thread->type() == ThreadBase::OFFLOAD) {
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();
}
}
@@ -1047,27 +1056,29 @@
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) {
PlaybackThread *t = (PlaybackThread *)thread.get();
Mutex::Autolock _l(t->mLock);
@@ -1080,25 +1091,39 @@
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
@@ -1118,26 +1143,52 @@
void *pReplyData)
{
ALOGVV("command(), cmdCode: %d, mHasControl: %d, mEffect: %p",
- cmdCode, mHasControl, (mEffect == 0) ? 0 : mEffect.get());
+ cmdCode, mHasControl, mEffect.unsafe_get());
+ if (cmdCode == EFFECT_CMD_ENABLE) {
+ if (*replySize < sizeof(int)) {
+ android_errorWriteLog(0x534e4554, "32095713");
+ return BAD_VALUE;
+ }
+ *(int *)pReplyData = NO_ERROR;
+ *replySize = sizeof(int);
+ return enable();
+ } else if (cmdCode == EFFECT_CMD_DISABLE) {
+ if (*replySize < sizeof(int)) {
+ android_errorWriteLog(0x534e4554, "32095713");
+ return BAD_VALUE;
+ }
+ *(int *)pReplyData = NO_ERROR;
+ *replySize = sizeof(int);
+ return disable();
+ }
+
+ 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;
}
// handle commands that are not forwarded transparently to effect engine
if (cmdCode == EFFECT_CMD_SET_PARAM_COMMIT) {
+ if (*replySize < sizeof(int)) {
+ android_errorWriteLog(0x534e4554, "32095713");
+ return BAD_VALUE;
+ }
+ *(int *)pReplyData = NO_ERROR;
+ *replySize = sizeof(int);
+
// No need to trylock() here as this function is executed in the binder thread serving a
// particular client process: no risk to block the whole media server process or mixer
// threads if we are stuck here
Mutex::Autolock _l(mCblk->lock);
-
// keep local copy of index in case of client corruption b/32220769
const uint32_t clientIndex = mCblk->clientIndex;
const uint32_t serverIndex = mCblk->serverIndex;
@@ -1171,7 +1222,7 @@
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,
@@ -1200,15 +1251,9 @@
mCblk->serverIndex = 0;
mCblk->clientIndex = 0;
return status;
- } else if (cmdCode == EFFECT_CMD_ENABLE) {
- *(int *)pReplyData = NO_ERROR;
- return enable();
- } else if (cmdCode == EFFECT_CMD_DISABLE) {
- *(int *)pReplyData = NO_ERROR;
- 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)
@@ -1290,7 +1335,6 @@
if (mOwnInBuffer) {
delete mInBuffer;
}
-
}
// getEffectFromDesc_l() must be called with ThreadBase::mLock held
@@ -1396,13 +1440,38 @@
}
}
-// 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,
+ int 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) {
@@ -1512,8 +1581,9 @@
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();
@@ -1528,6 +1598,10 @@
mEffects[i]->state() == EffectModule::STOPPING) {
mEffects[i]->stop();
}
+ if (release) {
+ mEffects[i]->release_l();
+ }
+
if (type == EFFECT_FLAG_TYPE_AUXILIARY) {
delete[] effect->inBuffer();
} else {
@@ -1539,6 +1613,7 @@
mEffects.removeAt(i);
ALOGV("removeEffect_l() effect %p, removed from chain %p at rank %d", effect.get(),
this, i);
+
break;
}
}
@@ -1546,7 +1621,7 @@
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();
@@ -1555,7 +1630,7 @@
}
}
-// 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();
@@ -1564,7 +1639,7 @@
}
}
-// 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();
@@ -1711,7 +1786,7 @@
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 b717857..b0b6618 100644
--- a/services/audioflinger/Effects.h
+++ b/services/audioflinger/Effects.h
@@ -45,7 +45,8 @@
const wp<AudioFlinger::EffectChain>& chain,
effect_descriptor_t *desc,
int id,
- int sessionId);
+ int sessionId,
+ bool pinned);
virtual ~EffectModule();
enum effect_state {
@@ -93,8 +94,9 @@
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; }
@@ -119,6 +121,7 @@
{ return (mDescriptor.flags & EFFECT_FLAG_OFFLOAD_SUPPORTED) != 0; }
status_t setOffloaded(bool offloaded, audio_io_handle_t io);
bool isOffloaded() const;
+ void release_l();
void dump(int fd, const Vector<String16>& args);
@@ -201,12 +204,17 @@
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 dump(char* buffer, size_t size);
@@ -215,7 +223,8 @@
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
@@ -226,8 +235,7 @@
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.
@@ -260,8 +268,15 @@
mLock.unlock();
}
+ status_t createEffect_l(sp<EffectModule>& effect,
+ ThreadBase *thread,
+ effect_descriptor_t *desc,
+ int id,
+ int 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 994c7db..6b2bc5e 100644
--- a/services/audioflinger/Threads.cpp
+++ b/services/audioflinger/Threads.cpp
@@ -739,8 +739,8 @@
int sessionId,
effect_descriptor_t *desc,
int *enabled,
- status_t *status
- )
+ status_t *status,
+ bool pinned)
{
sp<EffectModule> effect;
sp<EffectHandle> handle;
@@ -809,14 +809,7 @@
}
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;
- }
- effect->setOffloaded(mType == OFFLOAD, mId);
-
- lStatus = chain->addEffect_l(effect);
+ lStatus = chain->createEffect_l(effect, this, desc, id, sessionId, pinned);
if (lStatus != NO_ERROR) {
goto Exit;
}
@@ -856,6 +849,32 @@
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) {
+ 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);
@@ -914,9 +933,9 @@
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());
@@ -925,7 +944,7 @@
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 bb92df1..b63c8e6 100644
--- a/services/audioflinger/Threads.h
+++ b/services/audioflinger/Threads.h
@@ -158,7 +158,8 @@
int sessionId,
effect_descriptor_t *desc,
int *enabled,
- status_t *status);
+ status_t *status,
+ bool pinned);
void disconnectEffect(const sp< EffectModule>& effect,
EffectHandle *handle,
bool unpinIfLast);
@@ -198,7 +199,9 @@
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) {}
// returns either EFFECT_SESSION if effects on this audio session exist in one