From db7c079f284f6e91266f6653ae0ec198b1c5006e Mon Sep 17 00:00:00 2001 From: Eric Laurent Date: Wed, 10 Aug 2011 10:37:50 -0700 Subject: 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 --- include/media/AudioSystem.h | 1 + include/media/IAudioPolicyService.h | 1 + media/libmedia/AudioSystem.cpp | 7 +++ media/libmedia/IAudioPolicyService.cpp | 21 +++++++- services/audioflinger/AudioFlinger.cpp | 77 +++++++++++++++++++--------- services/audioflinger/AudioFlinger.h | 6 ++- services/audioflinger/AudioPolicyService.cpp | 8 +++ services/audioflinger/AudioPolicyService.h | 1 + 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& 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& 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& 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 @@ -313,6 +314,16 @@ public: return static_cast (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 (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 (data.readInt32()); + reply->writeInt32(static_cast (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& 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 chain = getEffectChain_l(sessionId); if (chain != 0) { @@ -5847,7 +5842,6 @@ size_t AudioFlinger::PlaybackThread::removeEffectChain_l(const sp& 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& 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 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 thread = mEffect->thread().promote(); - if (thread != 0) { - thread->checkSuspendOnEffectEnabled(mEffect, false, mEffect->sessionId()); + if (mEnabled) { + sp 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::EffectChain::getSuspendEligibleEffects() { Vector< sp > 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 spdesc())) { + 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& chain); - // updated mSuspendedSessions when an effect chain is removed - void updateSuspendedSessionsOnRemoveEffectChain_l(const sp& chain); friend class AudioFlinger; friend class Track; @@ -1320,6 +1318,10 @@ private: Vector< sp > getSuspendEligibleEffects(); // get an effect module if it is currently enable sp 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 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, -- cgit v1.1