From c74b93fdf3ddb6fdab7878edab4d304fb14917f9 Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Tue, 2 Aug 2011 13:33:41 -0700 Subject: [PATCH] Keep effects sessions active when the caller dies. Don't remove effects until the session they are in goes away or all AudioEffects have been explicitly released. This allows the control panel process to die without stopping the effects. Change-Id: I4496e5df080230ca1af149dec95c1309ab8ea888 --- include/media/AudioSystem.h | 2 + include/media/IAudioFlinger.h | 3 + media/libmedia/AudioRecord.cpp | 3 + media/libmedia/AudioSystem.cpp | 14 ++ media/libmedia/AudioTrack.cpp | 2 + media/libmedia/IAudioFlinger.cpp | 32 ++++- media/libmedia/mediaplayer.cpp | 8 +- services/audioflinger/AudioFlinger.cpp | 189 ++++++++++++++++++++++--- services/audioflinger/AudioFlinger.h | 31 +++- 9 files changed, 254 insertions(+), 30 deletions(-) diff --git a/include/media/AudioSystem.h b/include/media/AudioSystem.h index f20e2343d3c96..eb22e323e4da6 100644 --- a/include/media/AudioSystem.h +++ b/include/media/AudioSystem.h @@ -108,6 +108,8 @@ public: static unsigned int getInputFramesLost(audio_io_handle_t ioHandle); static int newAudioSessionId(); + static void acquireAudioSessionId(int audioSession); + static void releaseAudioSessionId(int audioSession); // types of io configuration change events received with ioConfigChanged() enum io_config_event { diff --git a/include/media/IAudioFlinger.h b/include/media/IAudioFlinger.h index 4037c464a072c..9e3cb7f3c3830 100644 --- a/include/media/IAudioFlinger.h +++ b/include/media/IAudioFlinger.h @@ -139,6 +139,9 @@ public: virtual int newAudioSessionId() = 0; + virtual void acquireAudioSessionId(int audioSession) = 0; + virtual void releaseAudioSessionId(int audioSession) = 0; + virtual status_t queryNumberEffects(uint32_t *numEffects) = 0; virtual status_t queryEffect(uint32_t index, effect_descriptor_t *pDescriptor) = 0; diff --git a/media/libmedia/AudioRecord.cpp b/media/libmedia/AudioRecord.cpp index 16554c265b5e4..e5062ab9d2e7c 100644 --- a/media/libmedia/AudioRecord.cpp +++ b/media/libmedia/AudioRecord.cpp @@ -114,6 +114,7 @@ AudioRecord::~AudioRecord() } mAudioRecord.clear(); IPCThreadState::self()->flushCommands(); + AudioSystem::releaseAudioSessionId(mSessionId); } } @@ -233,6 +234,7 @@ status_t AudioRecord::set( mInputSource = (uint8_t)inputSource; mFlags = flags; mInput = input; + AudioSystem::acquireAudioSessionId(mSessionId); return NO_ERROR; } @@ -465,6 +467,7 @@ status_t AudioRecord::openRecord_l( ((uint16_t)flags) << 16, &mSessionId, &status); + if (record == 0) { LOGE("AudioFlinger could not create record track, status: %d", status); return status; diff --git a/media/libmedia/AudioSystem.cpp b/media/libmedia/AudioSystem.cpp index 5009957abc3ba..b26ed717d8d16 100644 --- a/media/libmedia/AudioSystem.cpp +++ b/media/libmedia/AudioSystem.cpp @@ -356,6 +356,20 @@ int AudioSystem::newAudioSessionId() { return af->newAudioSessionId(); } +void AudioSystem::acquireAudioSessionId(int audioSession) { + const sp& af = AudioSystem::get_audio_flinger(); + if (af != 0) { + af->acquireAudioSessionId(audioSession); + } +} + +void AudioSystem::releaseAudioSessionId(int audioSession) { + const sp& af = AudioSystem::get_audio_flinger(); + if (af != 0) { + af->releaseAudioSessionId(audioSession); + } +} + // --------------------------------------------------------------------------- void AudioSystem::AudioFlingerClient::binderDied(const wp& who) { diff --git a/media/libmedia/AudioTrack.cpp b/media/libmedia/AudioTrack.cpp index 31eb97a968a75..3949c3936902a 100644 --- a/media/libmedia/AudioTrack.cpp +++ b/media/libmedia/AudioTrack.cpp @@ -134,6 +134,7 @@ AudioTrack::~AudioTrack() } mAudioTrack.clear(); IPCThreadState::self()->flushCommands(); + AudioSystem::releaseAudioSessionId(mSessionId); } } @@ -259,6 +260,7 @@ status_t AudioTrack::set( mNewPosition = 0; mUpdatePeriod = 0; mFlags = flags; + AudioSystem::acquireAudioSessionId(mSessionId); return NO_ERROR; } diff --git a/media/libmedia/IAudioFlinger.cpp b/media/libmedia/IAudioFlinger.cpp index 4a1296209ab5f..d58834b81684d 100644 --- a/media/libmedia/IAudioFlinger.cpp +++ b/media/libmedia/IAudioFlinger.cpp @@ -1,4 +1,4 @@ -/* //device/extlibs/pv/android/IAudioflinger.cpp +/* ** ** Copyright 2007, The Android Open Source Project ** @@ -63,6 +63,8 @@ enum { GET_RENDER_POSITION, GET_INPUT_FRAMES_LOST, NEW_AUDIO_SESSION_ID, + ACQUIRE_AUDIO_SESSION_ID, + RELEASE_AUDIO_SESSION_ID, QUERY_NUM_EFFECTS, QUERY_EFFECT, GET_EFFECT_DESCRIPTOR, @@ -526,6 +528,22 @@ public: return id; } + virtual void acquireAudioSessionId(int audioSession) + { + Parcel data, reply; + data.writeInterfaceToken(IAudioFlinger::getInterfaceDescriptor()); + data.writeInt32(audioSession); + remote()->transact(ACQUIRE_AUDIO_SESSION_ID, data, &reply); + } + + virtual void releaseAudioSessionId(int audioSession) + { + Parcel data, reply; + data.writeInterfaceToken(IAudioFlinger::getInterfaceDescriptor()); + data.writeInt32(audioSession); + remote()->transact(RELEASE_AUDIO_SESSION_ID, data, &reply); + } + virtual status_t queryNumberEffects(uint32_t *numEffects) { Parcel data, reply; @@ -919,6 +937,18 @@ status_t BnAudioFlinger::onTransact( reply->writeInt32(newAudioSessionId()); return NO_ERROR; } break; + case ACQUIRE_AUDIO_SESSION_ID: { + CHECK_INTERFACE(IAudioFlinger, data, reply); + int audioSession = data.readInt32(); + acquireAudioSessionId(audioSession); + return NO_ERROR; + } break; + case RELEASE_AUDIO_SESSION_ID: { + CHECK_INTERFACE(IAudioFlinger, data, reply); + int audioSession = data.readInt32(); + releaseAudioSessionId(audioSession); + return NO_ERROR; + } break; case QUERY_NUM_EFFECTS: { CHECK_INTERFACE(IAudioFlinger, data, reply); uint32_t numEffects; diff --git a/media/libmedia/mediaplayer.cpp b/media/libmedia/mediaplayer.cpp index 3dd9249d4ec48..67a66a289f1cf 100644 --- a/media/libmedia/mediaplayer.cpp +++ b/media/libmedia/mediaplayer.cpp @@ -61,12 +61,14 @@ MediaPlayer::MediaPlayer() mVideoWidth = mVideoHeight = 0; mLockThreadId = 0; mAudioSessionId = AudioSystem::newAudioSessionId(); + AudioSystem::acquireAudioSessionId(mAudioSessionId); mSendLevel = 0; } MediaPlayer::~MediaPlayer() { LOGV("destructor"); + AudioSystem::releaseAudioSessionId(mAudioSessionId); disconnect(); IPCThreadState::self()->flushCommands(); } @@ -618,7 +620,11 @@ status_t MediaPlayer::setAudioSessionId(int sessionId) if (sessionId < 0) { return BAD_VALUE; } - mAudioSessionId = sessionId; + if (sessionId != mAudioSessionId) { + AudioSystem::releaseAudioSessionId(mAudioSessionId); + AudioSystem::acquireAudioSessionId(sessionId); + mAudioSessionId = sessionId; + } return NO_ERROR; } diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp index e201b17344d94..d6bfda6f24134 100644 --- a/services/audioflinger/AudioFlinger.cpp +++ b/services/audioflinger/AudioFlinger.cpp @@ -264,6 +264,14 @@ status_t AudioFlinger::dumpClients(int fd, const Vector& args) } } } + + result.append("Global session refs:\n"); + result.append(" session pid cnt\n"); + for (size_t i = 0; i < mAudioSessionRefs.size(); i++) { + AudioSessionRef *r = mAudioSessionRefs[i]; + snprintf(buffer, SIZE, " %7d %3d %3d\n", r->sessionid, r->pid, r->cnt); + result.append(buffer); + } write(fd, result.string(), result.size()); return NO_ERROR; } @@ -892,6 +900,25 @@ void AudioFlinger::removeNotificationClient(pid_t pid) LOGV("removeNotificationClient() %p, pid %d", client.get(), pid); mNotificationClients.removeItem(pid); } + + LOGV("%d died, releasing its sessions", pid); + int num = mAudioSessionRefs.size(); + bool removed = false; + for (int i = 0; i< num; i++) { + AudioSessionRef *ref = mAudioSessionRefs.itemAt(i); + LOGV(" pid %d @ %d", ref->pid, i); + if (ref->pid == pid) { + LOGV(" removing entry for pid %d session %d", pid, ref->sessionid); + mAudioSessionRefs.removeAt(i); + delete ref; + removed = true; + i--; + num--; + } + } + if (removed) { + purgeStaleEffects_l(); + } } // audioConfigChanged_l() must be called with AudioFlinger::mLock held @@ -5042,6 +5069,109 @@ int AudioFlinger::newAudioSessionId() return nextUniqueId(); } +void AudioFlinger::acquireAudioSessionId(int audioSession) +{ + Mutex::Autolock _l(mLock); + int caller = IPCThreadState::self()->getCallingPid(); + LOGV("acquiring %d from %d", audioSession, caller); + int num = mAudioSessionRefs.size(); + for (int i = 0; i< num; i++) { + AudioSessionRef *ref = mAudioSessionRefs.editItemAt(i); + if (ref->sessionid == audioSession && ref->pid == caller) { + ref->cnt++; + LOGV(" incremented refcount to %d", ref->cnt); + return; + } + } + AudioSessionRef *ref = new AudioSessionRef(); + ref->sessionid = audioSession; + ref->pid = caller; + ref->cnt = 1; + mAudioSessionRefs.push(ref); + LOGV(" added new entry for %d", ref->sessionid); +} + +void AudioFlinger::releaseAudioSessionId(int audioSession) +{ + Mutex::Autolock _l(mLock); + int caller = IPCThreadState::self()->getCallingPid(); + LOGV("releasing %d from %d", audioSession, caller); + int num = mAudioSessionRefs.size(); + for (int i = 0; i< num; i++) { + AudioSessionRef *ref = mAudioSessionRefs.itemAt(i); + if (ref->sessionid == audioSession && ref->pid == caller) { + ref->cnt--; + LOGV(" decremented refcount to %d", ref->cnt); + if (ref->cnt == 0) { + mAudioSessionRefs.removeAt(i); + delete ref; + purgeStaleEffects_l(); + } + return; + } + } + LOGW("session id %d not found for pid %d", audioSession, caller); +} + +void AudioFlinger::purgeStaleEffects_l() { + + LOGV("purging stale effects"); + + Vector< sp > chains; + + for (size_t i = 0; i < mPlaybackThreads.size(); i++) { + sp t = mPlaybackThreads.valueAt(i); + for (size_t j = 0; j < t->mEffectChains.size(); j++) { + sp ec = t->mEffectChains[j]; + chains.push(ec); + } + } + for (size_t i = 0; i < mRecordThreads.size(); i++) { + sp t = mRecordThreads.valueAt(i); + for (size_t j = 0; j < t->mEffectChains.size(); j++) { + sp ec = t->mEffectChains[j]; + chains.push(ec); + } + } + + for (size_t i = 0; i < chains.size(); i++) { + sp ec = chains[i]; + int sessionid = ec->sessionId(); + sp t = ec->mThread.promote(); + if (t == 0) { + continue; + } + size_t numsessionrefs = mAudioSessionRefs.size(); + bool found = false; + for (size_t k = 0; k < numsessionrefs; k++) { + AudioSessionRef *ref = mAudioSessionRefs.itemAt(k); + if (ref->sessionid == sessionid) { + LOGV(" session %d still exists for %d with %d refs", + sessionid, ref->pid, ref->cnt); + found = true; + break; + } + } + if (!found) { + // remove all effects from the chain + while (ec->mEffects.size()) { + sp effect = ec->mEffects[0]; + effect->unPin(); + Mutex::Autolock _l (t->mLock); + t->removeEffect_l(effect); + for (size_t j = 0; j < effect->mHandles.size(); j++) { + sp handle = effect->mHandles[j].promote(); + if (handle != 0) { + handle->mEffect.clear(); + } + } + AudioSystem::unregisterEffect(effect->id()); + } + } + } + return; +} + // checkPlaybackThread_l() must be called with AudioFlinger::mLock held AudioFlinger::PlaybackThread *AudioFlinger::checkPlaybackThread_l(int output) const { @@ -5199,6 +5329,7 @@ sp AudioFlinger::createEffect(pid_t pid, } uint32_t numEffects = 0; effect_descriptor_t d; + d.flags = 0; // prevent compiler warning bool found = false; lStatus = EffectQueryNumberEffects(&numEffects); @@ -5302,7 +5433,7 @@ sp AudioFlinger::createEffect(pid_t pid, mClients.add(pid, client); } - // create effect on selected output trhead + // create effect on selected output thread handle = thread->createEffect_l(client, effectClient, priority, sessionId, &desc, enabled, &lStatus); if (handle != 0 && id != NULL) { @@ -5344,7 +5475,7 @@ status_t AudioFlinger::moveEffects(int sessionId, int srcOutput, int dstOutput) return NO_ERROR; } -// moveEffectChain_l mustbe called with both srcThread and dstThread mLocks held +// moveEffectChain_l must be called with both srcThread and dstThread mLocks held status_t AudioFlinger::moveEffectChain_l(int sessionId, AudioFlinger::PlaybackThread *srcThread, AudioFlinger::PlaybackThread *dstThread, @@ -5370,7 +5501,7 @@ status_t AudioFlinger::moveEffectChain_l(int sessionId, // correct buffer sizes and audio parameters and effect engines reconfigured accordingly int dstOutput = dstThread->id(); sp dstChain; - uint32_t strategy; + uint32_t strategy = 0; // prevent compiler warning sp effect = chain->getEffectFromId_l(0); while (effect != 0) { srcThread->removeEffect_l(effect); @@ -5632,14 +5763,17 @@ void AudioFlinger::ThreadBase::setMode(uint32_t mode) } void AudioFlinger::ThreadBase::disconnectEffect(const sp& effect, - const wp& handle) { + const wp& handle, + bool unpiniflast) { Mutex::Autolock _l(mLock); LOGV("disconnectEffect() %p effect %p", this, effect.get()); // delete the effect module if removing last handle on it if (effect->removeHandle(handle) == 0) { - removeEffect_l(effect); - AudioSystem::unregisterEffect(effect->id()); + if (!effect->isPinned() || unpiniflast) { + removeEffect_l(effect); + AudioSystem::unregisterEffect(effect->id()); + } } } @@ -5847,6 +5981,9 @@ AudioFlinger::EffectModule::EffectModule(const wp& wThread, goto Error; } + if (mSessionId > AUDIO_SESSION_OUTPUT_MIX) { + mPinned = true; + } LOGV("Constructor success name %s, Interface %p", mDescriptor.name, mEffectInterface); return; Error: @@ -5938,7 +6075,7 @@ size_t AudioFlinger::EffectModule::removeHandle(const wp& handle) // Prevent calls to process() and other functions on effect interface from now on. // The effect engine will be released by the destructor when the last strong reference on // this object is released which can happen after next process is called. - if (size == 0) { + if (size == 0 && !mPinned) { mState = DESTROYED; } @@ -5955,9 +6092,7 @@ sp AudioFlinger::EffectModule::controlHandle() return handle; } - - -void AudioFlinger::EffectModule::disconnect(const wp& handle) +void AudioFlinger::EffectModule::disconnect(const wp& handle, bool unpiniflast) { LOGV("disconnect() %p handle %p ", this, handle.unsafe_get()); // keep a strong reference on this EffectModule to avoid calling the @@ -5966,7 +6101,7 @@ void AudioFlinger::EffectModule::disconnect(const wp& handle) { sp thread = mThread.promote(); if (thread != 0) { - thread->disconnectEffect(keep, handle); + thread->disconnectEffect(keep, handle, unpiniflast); } } } @@ -6533,11 +6668,14 @@ AudioFlinger::EffectHandle::EffectHandle(const sp& effect, const sp& effectClient, int32_t priority) : BnEffect(), - mEffect(effect), mEffectClient(effectClient), mClient(client), + mEffect(effect), mEffectClient(effectClient), mClient(client), mCblk(NULL), mPriority(priority), mHasControl(false), mEnabled(false) { LOGV("constructor %p", this); + if (client == 0) { + return; + } int bufOffset = ((sizeof(effect_param_cblk_t) - 1) / sizeof(int) + 1) * sizeof(int); mCblkMemory = client->heap()->allocate(EFFECT_PARAM_BUFFER_SIZE + bufOffset); if (mCblkMemory != 0) { @@ -6556,7 +6694,7 @@ AudioFlinger::EffectHandle::EffectHandle(const sp& effect, AudioFlinger::EffectHandle::~EffectHandle() { LOGV("Destructor %p", this); - disconnect(); + disconnect(false); LOGV("Destructor DONE %p", this); } @@ -6605,12 +6743,16 @@ status_t AudioFlinger::EffectHandle::disable() void AudioFlinger::EffectHandle::disconnect() { - LOGV("disconnect %p", this); + disconnect(true); +} + +void AudioFlinger::EffectHandle::disconnect(bool unpiniflast) +{ + LOGV("disconnect(%s)", unpiniflast ? "true" : "false"); if (mEffect == 0) { return; } - - mEffect->disconnect(this); + mEffect->disconnect(this, unpiniflast); sp thread = mEffect->thread().promote(); if (thread != 0) { @@ -6619,11 +6761,11 @@ void AudioFlinger::EffectHandle::disconnect() // release sp on module => module destructor can be called now mEffect.clear(); - if (mCblk) { - mCblk->~effect_param_cblk_t(); // destroy our shared-structure. - } - mCblkMemory.clear(); // and free the shared memory if (mClient != 0) { + if (mCblk) { + mCblk->~effect_param_cblk_t(); // destroy our shared-structure. + } + mCblkMemory.clear(); // and free the shared memory Mutex::Autolock _l(mClient->audioFlinger()->mLock); mClient.clear(); } @@ -6643,6 +6785,7 @@ status_t AudioFlinger::EffectHandle::command(uint32_t cmdCode, 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) { @@ -6749,15 +6892,15 @@ status_t AudioFlinger::EffectHandle::onTransact( void AudioFlinger::EffectHandle::dump(char* buffer, size_t size) { - bool locked = tryLock(mCblk->lock); + bool locked = mCblk ? tryLock(mCblk->lock) : false; snprintf(buffer, size, "\t\t\t%05d %05d %01u %01u %05u %05u\n", (mClient == NULL) ? getpid() : mClient->pid(), mPriority, mHasControl, !locked, - mCblk->clientIndex, - mCblk->serverIndex + mCblk ? mCblk->clientIndex : 0, + mCblk ? mCblk->serverIndex : 0 ); if (locked) { diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h index 4fa70a2f9964b..3a0aac94eb23d 100644 --- a/services/audioflinger/AudioFlinger.h +++ b/services/audioflinger/AudioFlinger.h @@ -149,6 +149,10 @@ public: virtual int newAudioSessionId(); + virtual void acquireAudioSessionId(int audioSession); + + virtual void releaseAudioSessionId(int audioSession); + virtual status_t queryNumberEffects(uint32_t *numEffects); virtual status_t queryEffect(uint32_t index, effect_descriptor_t *descriptor); @@ -215,6 +219,7 @@ private: status_t initCheck() const; virtual void onFirstRef(); audio_hw_device_t* findSuitableHwDev_l(uint32_t devices); + void purgeStaleEffects_l(); // Internal dump utilites. status_t dumpPermissionDenial(int fd, const Vector& args); @@ -436,7 +441,8 @@ private: int *enabled, status_t *status); void disconnectEffect(const sp< EffectModule>& effect, - const wp& handle); + const wp& handle, + bool unpiniflast); // return values for hasAudioSession (bit field) enum effect_state { @@ -519,6 +525,7 @@ private: // updated mSuspendedSessions when an effect chain is removed void updateSuspendedSessionsOnRemoveEffectChain_l(const sp& chain); + friend class AudioFlinger; friend class Track; friend class TrackBase; friend class PlaybackThread; @@ -607,7 +614,6 @@ private: protected: friend class ThreadBase; - friend class AudioFlinger; friend class TrackHandle; friend class PlaybackThread; friend class MixerThread; @@ -1100,7 +1106,7 @@ private: wp& thread() { return mThread; } status_t addHandle(sp& handle); - void disconnect(const wp& handle); + void disconnect(const wp& handle, bool unpiniflast); size_t removeHandle (const wp& handle); effect_descriptor_t& desc() { return mDescriptor; } @@ -1115,9 +1121,15 @@ private: sp controlHandle(); + bool isPinned() { return mPinned; } + void unPin() { mPinned = false; } + status_t dump(int fd, const Vector& args); protected: + friend class EffectHandle; + friend class AudioFlinger; + bool mPinned; // Maximum time allocated to effect engines to complete the turn off sequence static const uint32_t MAX_DISABLE_TIME_MS = 10000; @@ -1169,6 +1181,7 @@ private: uint32_t *replySize, void *pReplyData); virtual void disconnect(); + virtual void disconnect(bool unpiniflast); virtual sp getCblk() const; virtual status_t onTransact(uint32_t code, const Parcel& data, Parcel* reply, uint32_t flags); @@ -1196,7 +1209,8 @@ private: void dump(char* buffer, size_t size); protected: - + friend class AudioFlinger; + friend class EffectModule; EffectHandle(const EffectHandle&); EffectHandle& operator =(const EffectHandle&); @@ -1288,7 +1302,7 @@ private: status_t dump(int fd, const Vector& args); protected: - + friend class AudioFlinger; EffectChain(const EffectChain&); EffectChain& operator =(const EffectChain&); @@ -1344,6 +1358,12 @@ private: hwDev(dev), stream(in) {} }; + struct AudioSessionRef { + int sessionid; + pid_t pid; + int cnt; + }; + friend class RecordThread; friend class PlaybackThread; @@ -1369,6 +1389,7 @@ private: uint32_t mMode; bool mBtNrec; + Vector mAudioSessionRefs; };