Merge "audioflinger: fix pre processing effect leak" into lmp-dev

8 files changed, 133 insertions, 37 deletions

diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp
index cee7feb..e200857 100644
--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -418,6 +418,13 @@ status_t AudioFlinger::dump(int fd, const Vector<String16>& args)
             mRecordThreads.valueAt(i)->dump(fd, args);
         }
 
+        // dump orphan effect chains
+        if (mOrphanEffectChains.size() != 0) {
+            write(fd, "  Orphan Effect Chains\n", strlen("  Orphan Effect Chains\n"));
+            for (size_t i = 0; i < mOrphanEffectChains.size(); i++) {
+                mOrphanEffectChains.valueAt(i)->dump(fd, args);
+            }
+        }
         // dump all hardware devs
         for (size_t i = 0; i < mAudioHwDevs.size(); i++) {
             audio_hw_device_t *dev = mAudioHwDevs.valueAt(i)->hwDevice();
@@ -1421,7 +1428,7 @@ sp<IAudioRecord> AudioFlinger::openRecord(
                 *sessionId = lSessionId;
             }
         }
-        ALOGV("openRecord() lSessionId: %d", lSessionId);
+        ALOGV("openRecord() lSessionId: %d input %d", lSessionId, input);
 
         // TODO: the uid should be passed in as a parameter to openRecord
         recordTrack = thread->createRecordTrack_l(client, sampleRate, format, channelMask,
@@ -2027,6 +2034,16 @@ status_t AudioFlinger::closeInput_nonvirtual(audio_io_handle_t input)
         }
 
         ALOGV("closeInput() %d", input);
+        {
+            // If we still have effect chains, it means that a client still holds a handle
+            // on at least one effect. We must keep the chain alive in case a new record
+            // thread is opened for a new capture on the same session
+            Mutex::Autolock _sl(thread->mLock);
+            Vector< sp<EffectChain> > effectChains = thread->getEffectChains_l();
+            for (size_t i = 0; i < effectChains.size(); i++) {
+                putOrphanEffectChain_l(effectChains[i]);
+            }
+        }
         audioConfigChanged(AudioSystem::INPUT_CLOSED, input, NULL);
         mRecordThreads.removeItem(input);
     }
@@ -2456,6 +2473,13 @@ sp<IEffect> AudioFlinger::createEffect(
                 lStatus = BAD_VALUE;
                 goto Exit;
             }
+        } else {
+            // Check if one effect chain was awaiting for an effect to be created on this
+            // session and used it instead of creating a new one.
+            sp<EffectChain> chain = getOrphanEffectChain_l((audio_session_t)sessionId);
+            if (chain != 0) {
+                thread->addEffectChain_l(chain);
+            }
         }
 
         sp<Client> client = registerPid(pid);
@@ -2628,6 +2652,49 @@ void AudioFlinger::onNonOffloadableGlobalEffectEnable()
 
 }
 
+status_t AudioFlinger::putOrphanEffectChain_l(const sp<AudioFlinger::EffectChain>& chain)
+{
+    audio_session_t session = (audio_session_t)chain->sessionId();
+    ssize_t index = mOrphanEffectChains.indexOfKey(session);
+    ALOGV("putOrphanEffectChain_l session %d index %d", session, index);
+    if (index >= 0) {
+        ALOGW("putOrphanEffectChain_l chain for session %d already present", session);
+        return ALREADY_EXISTS;
+    }
+    mOrphanEffectChains.add(session, chain);
+    return NO_ERROR;
+}
+
+sp<AudioFlinger::EffectChain> AudioFlinger::getOrphanEffectChain_l(audio_session_t session)
+{
+    sp<EffectChain> chain;
+    ssize_t index = mOrphanEffectChains.indexOfKey(session);
+    ALOGV("getOrphanEffectChain_l session %d index %d", session, index);
+    if (index >= 0) {
+        chain = mOrphanEffectChains.valueAt(index);
+        mOrphanEffectChains.removeItemsAt(index);
+    }
+    return chain;
+}
+
+bool AudioFlinger::updateOrphanEffectChains(const sp<AudioFlinger::EffectModule>& effect)
+{
+    Mutex::Autolock _l(mLock);
+    audio_session_t session = (audio_session_t)effect->sessionId();
+    ssize_t index = mOrphanEffectChains.indexOfKey(session);
+    ALOGV("updateOrphanEffectChains session %d index %d", session, index);
+    if (index >= 0) {
+        sp<EffectChain> chain = mOrphanEffectChains.valueAt(index);
+        if (chain->removeEffect_l(effect) == 0) {
+            ALOGV("updateOrphanEffectChains removing effect chain at index %d", index);
+            mOrphanEffectChains.removeItemsAt(index);
+        }
+        return true;
+    }
+    return false;
+}
+
+
 struct Entry {
 #define MAX_NAME 32     // %Y%m%d%H%M%S_%d.wav
     char mName[MAX_NAME];
diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h
index 753314f..1003017 100644
--- a/services/audioflinger/AudioFlinger.h
+++ b/services/audioflinger/AudioFlinger.h
@@ -569,6 +569,23 @@ private:
                 bool isNonOffloadableGlobalEffectEnabled_l();
                 void onNonOffloadableGlobalEffectEnable();
 
+                // Store an effect chain to mOrphanEffectChains keyed vector.
+                // Called when a thread exits and effects are still attached to it.
+                // If effects are later created on the same session, they will reuse the same
+                // effect chain and same instances in the effect library.
+                // return ALREADY_EXISTS if a chain with the same session already exists in
+                // mOrphanEffectChains. Note that this should never happen as there is only one
+                // chain for a given session and it is attached to only one thread at a time.
+                status_t        putOrphanEffectChain_l(const sp<EffectChain>& chain);
+                // Get an effect chain for the specified session in mOrphanEffectChains and remove
+                // it if found. Returns 0 if not found (this is the most common case).
+                sp<EffectChain> getOrphanEffectChain_l(audio_session_t session);
+                // Called when the last effect handle on an effect instance is removed. If this
+                // effect belongs to an effect chain in mOrphanEffectChains, the chain is updated
+                // and removed from mOrphanEffectChains if it does not contain any effect.
+                // Return true if the effect was found in mOrphanEffectChains, false otherwise.
+                bool            updateOrphanEffectChains(const sp<EffectModule>& effect);
+
     class AudioHwDevice {
     public:
         enum Flags {
@@ -713,6 +730,9 @@ private:
                 Vector < sp<SyncEvent> > mPendingSyncEvents; // sync events awaiting for a session
                                                              // to be created
 
+                // Effect chains without a valid thread
+                DefaultKeyedVector< audio_session_t , sp<EffectChain> > mOrphanEffectChains;
+
 private:
     sp<Client>  registerPid(pid_t pid);    // always returns non-0
 
diff --git a/services/audioflinger/Effects.cpp b/services/audioflinger/Effects.cpp
index 365f271..15f1f23 100644
--- a/services/audioflinger/Effects.cpp
+++ b/services/audioflinger/Effects.cpp
@@ -68,7 +68,8 @@ AudioFlinger::EffectModule::EffectModule(ThreadBase *thread,
       mStatus(NO_INIT), mState(IDLE),
       // mMaxDisableWaitCnt is set by configure() and not used before then
       // mDisableWaitCnt is set by process() and updateState() and not used before then
-      mSuspended(false)
+      mSuspended(false),
+      mAudioFlinger(thread->mAudioFlinger)
 {
     ALOGV("Constructor %p", this);
     int lStatus;
@@ -197,9 +198,19 @@ size_t AudioFlinger::EffectModule::disconnect(EffectHandle *handle, bool unpinIf
     // destructor before we exit
     sp<EffectModule> keep(this);
     {
-        sp<ThreadBase> thread = mThread.promote();
-        if (thread != 0) {
-            thread->disconnectEffect(keep, handle, unpinIfLast);
+        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);
+            }
         }
     }
     return mHandles.size();
@@ -1911,4 +1922,13 @@ bool AudioFlinger::EffectChain::isNonOffloadableEnabled()
     return false;
 }
 
+void AudioFlinger::EffectChain::setThread(const sp<ThreadBase>& thread)
+{
+    Mutex::Autolock _l(mLock);
+    mThread = thread;
+    for (size_t i = 0; i < mEffects.size(); i++) {
+        mEffects[i]->setThread(thread);
+    }
+}
+
 }; // namespace android
diff --git a/services/audioflinger/Effects.h b/services/audioflinger/Effects.h
index 4170fd4..eaf90e7 100644
--- a/services/audioflinger/Effects.h
+++ b/services/audioflinger/Effects.h
@@ -153,6 +153,7 @@ mutable Mutex               mLock;      // mutex for process, commands and handl
     uint32_t mDisableWaitCnt;       // current process() calls count during disable period.
     bool     mSuspended;            // effect is suspended: temporarily disabled by framework
     bool     mOffloaded;            // effect is currently offloaded to the audio DSP
+    wp<AudioFlinger>    mAudioFlinger;
 };
 
 // The EffectHandle class implements the IEffect interface. It provides resources
@@ -347,6 +348,8 @@ protected:
 
     void clearInputBuffer_l(sp<ThreadBase> thread);
 
+    void setThread(const sp<ThreadBase>& thread);
+
     wp<ThreadBase> mThread;     // parent mixer thread
     Mutex mLock;                // mutex protecting effect list
     Vector< sp<EffectModule> > mEffects; // list of effect modules
diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp
index 97b1753..3d17c89 100644
--- a/services/audioflinger/Threads.cpp
+++ b/services/audioflinger/Threads.cpp
@@ -1147,21 +1147,6 @@ void AudioFlinger::ThreadBase::setMode(audio_mode_t mode)
     }
 }
 
-void AudioFlinger::ThreadBase::disconnectEffect(const sp<EffectModule>& effect,
-                                                    EffectHandle *handle,
-                                                    bool unpinIfLast) {
-
-    Mutex::Autolock _l(mLock);
-    ALOGV("disconnectEffect() %p effect %p", this, effect.get());
-    // delete the effect module if removing last handle on it
-    if (effect->removeHandle(handle) == 0) {
-        if (!effect->isPinned() || unpinIfLast) {
-            removeEffect_l(effect);
-            AudioSystem::unregisterEffect(effect->id());
-        }
-    }
-}
-
 void AudioFlinger::ThreadBase::getAudioPortConfig(struct audio_port_config *config)
 {
     config->type = AUDIO_PORT_TYPE_MIX;
@@ -2278,7 +2263,7 @@ status_t AudioFlinger::PlaybackThread::addEffectChain_l(const sp<EffectChain>& c
             }
         }
     }
-
+    chain->setThread(this);
     chain->setInBuffer(buffer, ownsBuffer);
     chain->setOutBuffer(reinterpret_cast<int16_t*>(mEffectBufferEnabled
             ? mEffectBuffer : mSinkBuffer));
@@ -6188,10 +6173,11 @@ status_t AudioFlinger::RecordThread::addEffectChain_l(const sp<EffectChain>& cha
 {
     // only one chain per input thread
     if (mEffectChains.size() != 0) {
+        ALOGW("addEffectChain_l() already one chain %p on thread %p", chain.get(), this);
         return INVALID_OPERATION;
     }
     ALOGV("addEffectChain_l() %p on thread %p", chain.get(), this);
-
+    chain->setThread(this);
     chain->setInBuffer(NULL);
     chain->setOutBuffer(NULL);
 
diff --git a/services/audioflinger/Threads.h b/services/audioflinger/Threads.h
index 648502b..fd025b5 100644
--- a/services/audioflinger/Threads.h
+++ b/services/audioflinger/Threads.h
@@ -283,9 +283,6 @@ public:
                                     effect_descriptor_t *desc,
                                     int *enabled,
                                     status_t *status /*non-NULL*/);
-                void disconnectEffect(const sp< EffectModule>& effect,
-                                      EffectHandle *handle,
-                                      bool unpinIfLast);
 
                 // return values for hasAudioSession (bit field)
                 enum effect_state {
diff --git a/services/audioflinger/TrackBase.h b/services/audioflinger/TrackBase.h
index 864daa5..98bf96e 100644
--- a/services/audioflinger/TrackBase.h
+++ b/services/audioflinger/TrackBase.h
@@ -166,6 +166,7 @@ protected:
     sp<NBAIO_Source>    mTeeSource;
     bool                mTerminated;
     track_type          mType;      // must be one of TYPE_DEFAULT, TYPE_OUTPUT, TYPE_PATCH ...
+    audio_io_handle_t   mThreadIoHandle; // I/O handle of the thread the track is attached to
 };
 
 // PatchProxyBufferProvider interface is implemented by PatchTrack and PatchRecord.
diff --git a/services/audioflinger/Tracks.cpp b/services/audioflinger/Tracks.cpp
index 6cbab04..c0a75b9 100644
--- a/services/audioflinger/Tracks.cpp
+++ b/services/audioflinger/Tracks.cpp
@@ -96,7 +96,8 @@ AudioFlinger::ThreadBase::TrackBase::TrackBase(
         mServerProxy(NULL),
         mId(android_atomic_inc(&nextTrackId)),
         mTerminated(false),
-        mType(type)
+        mType(type),
+        mThreadIoHandle(thread->id())
 {
     // if the caller is us, trust the specified uid
     if (IPCThreadState::self()->getCallingPid() != getpid_cached || clientUid == -1) {
@@ -482,14 +483,15 @@ void AudioFlinger::PlaybackThread::Track::destroy()
     // this Track with its member mTrack.
     sp<Track> keep(this);
     { // scope for mLock
+        bool wasActive = false;
         sp<ThreadBase> thread = mThread.promote();
         if (thread != 0) {
             Mutex::Autolock _l(thread->mLock);
             PlaybackThread *playbackThread = (PlaybackThread *)thread.get();
-            bool wasActive = playbackThread->destroyTrack_l(this);
-            if (isExternalTrack() && !wasActive) {
-                AudioSystem::releaseOutput(thread->id());
-            }
+            wasActive = playbackThread->destroyTrack_l(this);
+        }
+        if (isExternalTrack() && !wasActive) {
+            AudioSystem::releaseOutput(mThreadIoHandle);
         }
     }
 }
@@ -2050,7 +2052,7 @@ void AudioFlinger::RecordThread::RecordTrack::stop()
     if (thread != 0) {
         RecordThread *recordThread = (RecordThread *)thread.get();
         if (recordThread->stop(this) && isExternalTrack()) {
-            AudioSystem::stopInput(recordThread->id(), (audio_session_t)mSessionId);
+            AudioSystem::stopInput(mThreadIoHandle, (audio_session_t)mSessionId);
         }
     }
 }
@@ -2060,14 +2062,14 @@ void AudioFlinger::RecordThread::RecordTrack::destroy()
     // see comments at AudioFlinger::PlaybackThread::Track::destroy()
     sp<RecordTrack> keep(this);
     {
+        if (isExternalTrack()) {
+            if (mState == ACTIVE || mState == RESUMING) {
+                AudioSystem::stopInput(mThreadIoHandle, (audio_session_t)mSessionId);
+            }
+            AudioSystem::releaseInput(mThreadIoHandle, (audio_session_t)mSessionId);
+        }
         sp<ThreadBase> thread = mThread.promote();
         if (thread != 0) {
-            if (isExternalTrack()) {
-                if (mState == ACTIVE || mState == RESUMING) {
-                    AudioSystem::stopInput(thread->id(), (audio_session_t)mSessionId);
-                }
-                AudioSystem::releaseInput(thread->id(), (audio_session_t)mSessionId);
-            }
             Mutex::Autolock _l(thread->mLock);
             RecordThread *recordThread = (RecordThread *) thread.get();
             recordThread->destroyTrack_l(this);