Audio effects: track CPU and memory use separately

Before this change, CPU and memory usage for an audio effect were
registered and checked against the limit by audio policy manager
upon effect instantiation. Even if an effect was not enabled
it would prevent another effect to be created if the CPU load budget
was exceeded, which was too restrictive.

This change adds a method to register/unregister CPU load only when
an effect is enabled or disabled.
It also adds a mechanism to place all effects on the global output mix
in suspend state (disabled) when an effect is enabled on a specific session.
This will allow applications using session effects to have the priority
over others using global effects.

Also fixes some issues with suspend/restore mechanism:
- avoid taking actions when an effect is disconnected and was not enabled.
- do not remove a session from the suspended sessions list when corresponding
effect chain is destroyed.

Change-Id: I5225278aba1ae13d0d0997bfe26a0c9fb46b17d3

8 files changed, 95 insertions, 27 deletions

diff --git a/include/media/AudioSystem.h b/include/media/AudioSystem.h
index eb22e32..e0d7898 100644
--- a/include/media/AudioSystem.h
+++ b/include/media/AudioSystem.h
@@ -183,6 +183,7 @@ public:
                                     int session,
                                     int id);
     static status_t unregisterEffect(int id);
+    static status_t setEffectEnabled(int id, bool enabled);
 
     static const sp<IAudioPolicyService>& get_audio_policy_service();
 
diff --git a/include/media/IAudioPolicyService.h b/include/media/IAudioPolicyService.h
index ed265e1..9807cbe 100644
--- a/include/media/IAudioPolicyService.h
+++ b/include/media/IAudioPolicyService.h
@@ -84,6 +84,7 @@ public:
                                     int session,
                                     int id) = 0;
     virtual status_t unregisterEffect(int id) = 0;
+    virtual status_t setEffectEnabled(int id, bool enabled) = 0;
     virtual bool     isStreamActive(int stream, uint32_t inPastMs = 0) const = 0;
     virtual status_t queryDefaultPreProcessing(int audioSession,
                                               effect_descriptor_t *descriptors,
diff --git a/media/libmedia/AudioSystem.cpp b/media/libmedia/AudioSystem.cpp
index b26ed71..bb91fa9 100644
--- a/media/libmedia/AudioSystem.cpp
+++ b/media/libmedia/AudioSystem.cpp
@@ -710,6 +710,13 @@ status_t AudioSystem::unregisterEffect(int id)
     return aps->unregisterEffect(id);
 }
 
+status_t AudioSystem::setEffectEnabled(int id, bool enabled)
+{
+    const sp<IAudioPolicyService>& aps = AudioSystem::get_audio_policy_service();
+    if (aps == 0) return PERMISSION_DENIED;
+    return aps->setEffectEnabled(id, enabled);
+}
+
 status_t AudioSystem::isStreamActive(int stream, bool* state, uint32_t inPastMs)
 {
     const sp<IAudioPolicyService>& aps = AudioSystem::get_audio_policy_service();
diff --git a/media/libmedia/IAudioPolicyService.cpp b/media/libmedia/IAudioPolicyService.cpp
index 15f4be0..50b4855 100644
--- a/media/libmedia/IAudioPolicyService.cpp
+++ b/media/libmedia/IAudioPolicyService.cpp
@@ -53,7 +53,8 @@ enum {
     UNREGISTER_EFFECT,
     IS_STREAM_ACTIVE,
     GET_DEVICES_FOR_STREAM,
-    QUERY_DEFAULT_PRE_PROCESSING
+    QUERY_DEFAULT_PRE_PROCESSING,
+    SET_EFFECT_ENABLED
 };
 
 class BpAudioPolicyService : public BpInterface<IAudioPolicyService>
@@ -313,6 +314,16 @@ public:
         return static_cast <status_t> (reply.readInt32());
     }
 
+    virtual status_t setEffectEnabled(int id, bool enabled)
+    {
+        Parcel data, reply;
+        data.writeInterfaceToken(IAudioPolicyService::getInterfaceDescriptor());
+        data.writeInt32(id);
+        data.writeInt32(enabled);
+        remote()->transact(SET_EFFECT_ENABLED, data, &reply);
+        return static_cast <status_t> (reply.readInt32());
+    }
+
     virtual bool isStreamActive(int stream, uint32_t inPastMs) const
     {
         Parcel data, reply;
@@ -577,6 +588,14 @@ status_t BnAudioPolicyService::onTransact(
             return NO_ERROR;
         } break;
 
+        case SET_EFFECT_ENABLED: {
+            CHECK_INTERFACE(IAudioPolicyService, data, reply);
+            int id = data.readInt32();
+            bool enabled = static_cast <bool>(data.readInt32());
+            reply->writeInt32(static_cast <int32_t>(setEffectEnabled(id, enabled)));
+            return NO_ERROR;
+        } break;
+
         case IS_STREAM_ACTIVE: {
             CHECK_INTERFACE(IAudioPolicyService, data, reply);
             int stream = data.readInt32();
diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp
index d6bfda6..95c469d 100644
--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -1232,18 +1232,6 @@ void AudioFlinger::ThreadBase::checkSuspendOnAddEffectChain_l(const sp<EffectCha
     }
 }
 
-void AudioFlinger::ThreadBase::updateSuspendedSessionsOnRemoveEffectChain_l(
-        const sp<EffectChain>& chain)
-{
-    int index = mSuspendedSessions.indexOfKey(chain->sessionId());
-    if (index < 0) {
-        return;
-    }
-    LOGV("updateSuspendedSessionsOnRemoveEffectChain_l() removed suspended session %d",
-         chain->sessionId());
-    mSuspendedSessions.removeItemsAt(index);
-}
-
 void AudioFlinger::ThreadBase::updateSuspendedSessions_l(const effect_uuid_t *type,
                                                          bool suspend,
                                                          int sessionId)
@@ -1311,7 +1299,14 @@ void AudioFlinger::ThreadBase::checkSuspendOnEffectEnabled(const sp<EffectModule
 {
     Mutex::Autolock _l(mLock);
 
-    // TODO: implement PlaybackThread or RecordThread specific behavior here
+    if (mType != RECORD) {
+        // suspend all effects in AUDIO_SESSION_OUTPUT_MIX when enabling any effect on
+        // another session. This gives the priority to well behaved effect control panels
+        // and applications not using global effects.
+        if (sessionId != AUDIO_SESSION_OUTPUT_MIX) {
+            setEffectSuspended_l(NULL, enabled, AUDIO_SESSION_OUTPUT_MIX);
+        }
+    }
 
     sp<EffectChain> chain = getEffectChain_l(sessionId);
     if (chain != 0) {
@@ -5847,7 +5842,6 @@ size_t AudioFlinger::PlaybackThread::removeEffectChain_l(const sp<EffectChain>&
 
     for (size_t i = 0; i < mEffectChains.size(); i++) {
         if (chain == mEffectChains[i]) {
-            updateSuspendedSessionsOnRemoveEffectChain_l(chain);
             mEffectChains.removeAt(i);
             // detach all active tracks from the chain
             for (size_t i = 0 ; i < mActiveTracks.size() ; ++i) {
@@ -5939,7 +5933,6 @@ size_t AudioFlinger::RecordThread::removeEffectChain_l(const sp<EffectChain>& ch
             "removeEffectChain_l() %p invalid chain size %d on thread %p",
             chain.get(), mEffectChains.size(), this);
     if (mEffectChains.size() == 1) {
-        updateSuspendedSessionsOnRemoveEffectChain_l(chain);
         mEffectChains.removeAt(0);
     }
     return 0;
@@ -6393,10 +6386,16 @@ status_t AudioFlinger::EffectModule::command(uint32_t cmdCode,
 
 status_t AudioFlinger::EffectModule::setEnabled(bool enabled)
 {
+
     Mutex::Autolock _l(mLock);
     LOGV("setEnabled %p enabled %d", this, enabled);
 
     if (enabled != isEnabled()) {
+        status_t status = AudioSystem::setEffectEnabled(mId, enabled);
+        if (enabled && status != NO_ERROR) {
+            return status;
+        }
+
         switch (mState) {
         // going from disabled to enabled
         case IDLE:
@@ -6704,6 +6703,10 @@ status_t AudioFlinger::EffectHandle::enable()
     if (!mHasControl) return INVALID_OPERATION;
     if (mEffect == 0) return DEAD_OBJECT;
 
+    if (mEnabled) {
+        return NO_ERROR;
+    }
+
     mEnabled = true;
 
     sp<ThreadBase> thread = mEffect->thread().promote();
@@ -6716,7 +6719,14 @@ status_t AudioFlinger::EffectHandle::enable()
         return NO_ERROR;
     }
 
-    return mEffect->setEnabled(true);
+    status_t status = mEffect->setEnabled(true);
+    if (status != NO_ERROR) {
+        if (thread != 0) {
+            thread->checkSuspendOnEffectEnabled(mEffect, false, mEffect->sessionId());
+        }
+        mEnabled = false;
+    }
+    return status;
 }
 
 status_t AudioFlinger::EffectHandle::disable()
@@ -6725,6 +6735,9 @@ status_t AudioFlinger::EffectHandle::disable()
     if (!mHasControl) return INVALID_OPERATION;
     if (mEffect == 0) return DEAD_OBJECT;
 
+    if (!mEnabled) {
+        return NO_ERROR;
+    }
     mEnabled = false;
 
     if (mEffect->suspended()) {
@@ -6754,9 +6767,11 @@ void AudioFlinger::EffectHandle::disconnect(bool unpiniflast)
     }
     mEffect->disconnect(this, unpiniflast);
 
-    sp<ThreadBase> thread = mEffect->thread().promote();
-    if (thread != 0) {
-        thread->checkSuspendOnEffectEnabled(mEffect, false, mEffect->sessionId());
+    if (mEnabled) {
+        sp<ThreadBase> thread = mEffect->thread().promote();
+        if (thread != 0) {
+            thread->checkSuspendOnEffectEnabled(mEffect, false, mEffect->sessionId());
+        }
     }
 
     // release sp on module => module destructor can be called now
@@ -7367,15 +7382,22 @@ void AudioFlinger::EffectChain::setEffectSuspendedAll_l(bool suspend)
     }
 }
 
+bool AudioFlinger::EffectChain::isEffectEligibleForSuspend(const effect_descriptor_t& desc)
+{
+    // auxiliary effects and visualizer are never suspended on output mix
+    if ((mSessionId == AUDIO_SESSION_OUTPUT_MIX) &&
+        (((desc.flags & EFFECT_FLAG_TYPE_MASK) == EFFECT_FLAG_TYPE_AUXILIARY) ||
+         (memcmp(&desc.type, SL_IID_VISUALIZATION, sizeof(effect_uuid_t)) == 0))) {
+        return false;
+    }
+    return true;
+}
+
 Vector< sp<AudioFlinger::EffectModule> > AudioFlinger::EffectChain::getSuspendEligibleEffects()
 {
     Vector< sp<EffectModule> > effects;
     for (size_t i = 0; i < mEffects.size(); i++) {
-        effect_descriptor_t desc = mEffects[i]->desc();
-        // auxiliary effects and vizualizer are never suspended on output mix
-        if ((mSessionId == AUDIO_SESSION_OUTPUT_MIX) && (
-            ((desc.flags & EFFECT_FLAG_TYPE_MASK) == EFFECT_FLAG_TYPE_AUXILIARY) ||
-             (memcmp(&desc.type, SL_IID_VISUALIZATION, sizeof(effect_uuid_t)) == 0))) {
+        if (!isEffectEligibleForSuspend(mEffects[i]->desc())) {
             continue;
         }
         effects.add(mEffects[i]);
@@ -7405,8 +7427,15 @@ void AudioFlinger::EffectChain::checkSuspendOnEffectEnabled(const sp<EffectModul
             if (index < 0) {
                 return;
             }
+            if (!isEffectEligibleForSuspend(effect->desc())) {
+                return;
+            }
             setEffectSuspended_l(&effect->desc().type, enabled);
             index = mSuspendedEffects.indexOfKey(effect->desc().type.timeLow);
+            if (index < 0) {
+                LOGW("checkSuspendOnEffectEnabled() Fx should be suspended here!");
+                return;
+            }
         }
         LOGV("checkSuspendOnEffectEnabled() enable suspending fx %08x",
              effect->desc().type.timeLow);
diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h
index 3a0aac9..1141f6c 100644
--- a/services/audioflinger/AudioFlinger.h
+++ b/services/audioflinger/AudioFlinger.h
@@ -522,8 +522,6 @@ private:
                                                           int sessionId);
                     // check if some effects must be suspended when an effect chain is added
                     void checkSuspendOnAddEffectChain_l(const sp<EffectChain>& chain);
-                    // updated mSuspendedSessions when an effect chain is removed
-                    void updateSuspendedSessionsOnRemoveEffectChain_l(const sp<EffectChain>& chain);
 
         friend class AudioFlinger;
         friend class Track;
@@ -1320,6 +1318,10 @@ private:
         Vector< sp<EffectModule> > getSuspendEligibleEffects();
         // get an effect module if it is currently enable
         sp<EffectModule> getEffectIfEnabled(const effect_uuid_t *type);
+        // true if the effect whose descriptor is passed can be suspended
+        // OEMs can modify the rules implemented in this method to exclude specific effect
+        // types or implementations from the suspend/restore mechanism.
+        bool isEffectEligibleForSuspend(const effect_descriptor_t& desc);
 
         wp<ThreadBase> mThread;     // parent mixer thread
         Mutex mLock;                // mutex protecting effect list
diff --git a/services/audioflinger/AudioPolicyService.cpp b/services/audioflinger/AudioPolicyService.cpp
index 6d06d83..d747b5a 100644
--- a/services/audioflinger/AudioPolicyService.cpp
+++ b/services/audioflinger/AudioPolicyService.cpp
@@ -488,6 +488,14 @@ status_t AudioPolicyService::unregisterEffect(int id)
     return mpAudioPolicy->unregister_effect(mpAudioPolicy, id);
 }
 
+status_t AudioPolicyService::setEffectEnabled(int id, bool enabled)
+{
+    if (mpAudioPolicy == NULL) {
+        return NO_INIT;
+    }
+    return mpAudioPolicy->set_effect_enabled(mpAudioPolicy, id, enabled);
+}
+
 bool AudioPolicyService::isStreamActive(int stream, uint32_t inPastMs) const
 {
     if (mpAudioPolicy == NULL) {
diff --git a/services/audioflinger/AudioPolicyService.h b/services/audioflinger/AudioPolicyService.h
index 834b794..d898a53 100644
--- a/services/audioflinger/AudioPolicyService.h
+++ b/services/audioflinger/AudioPolicyService.h
@@ -102,6 +102,7 @@ public:
                                     int session,
                                     int id);
     virtual status_t unregisterEffect(int id);
+    virtual status_t setEffectEnabled(int id, bool enabled);
     virtual bool isStreamActive(int stream, uint32_t inPastMs = 0) const;
 
     virtual status_t queryDefaultPreProcessing(int audioSession,