Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f2591bc0 authored by Francois Gaffie's avatar Francois Gaffie Committed by Eric Laurent
Browse files

AudioFlinger: Keep track of music effect thread when primary output is unavailable



Bug: 307216402
Test: make

Audio Policy Manager may temporary run without primary output.
If music effects are instantiated, APM will lose track of music
output (may point to invalid io handle of primary output).
AF will save these effects on first playback thread.
Thus, these music effects become impossible to be moved back to
music playback thread.

This CL fixes this by:
    -using chain parking mecanism in audio flinger when output
hosting session mix is closed
    -fixing session mix creation if APM has temporarily no output:
in this case, AudioFlinger will use the first available output to host
output mix effect
    -enhancing moveEffect function to check in the orphan chains if
the source output is not found
    -enhancing moveEffect function to check among other outputs
when source is not found to find the output mix session.

Signed-off-by: default avatarFrancois Gaffie <francois.gaffie@renault.com>
Change-Id: I7690aab62d6dabc6351e7b283810bdcaf92bb3de
parent 143dc5d2
Loading
Loading
Loading
Loading
+78 −21
Original line number Original line Diff line number Diff line
@@ -3059,6 +3059,25 @@ status_t AudioFlinger::closeOutput_nonvirtual(audio_io_handle_t output)




            mPlaybackThreads.removeItem(output);
            mPlaybackThreads.removeItem(output);
            // Save AUDIO_SESSION_OUTPUT_MIX effect to orphan chains
            // Output Mix Effect session is used to manage Music Effect by AudioPolicy Manager.
            // It exists across all playback threads.
            if (playbackThread->type() == IAfThreadBase::MIXER
                    || playbackThread->type() == IAfThreadBase::OFFLOAD
                    || playbackThread->type() == IAfThreadBase::SPATIALIZER) {
                sp<IAfEffectChain> mixChain;
                {
                    audio_utils::scoped_lock sl(playbackThread->mutex());
                    mixChain = playbackThread->getEffectChain_l(AUDIO_SESSION_OUTPUT_MIX);
                    if (mixChain != nullptr) {
                        ALOGW("%s() output %d moving mix session to orphans", __func__, output);
                        playbackThread->removeEffectChain_l(mixChain);
                    }
                }
                if (mixChain != nullptr) {
                    putOrphanEffectChain_l(mixChain);
                }
            }
            // save all effects to the default thread
            // save all effects to the default thread
            if (mPlaybackThreads.size()) {
            if (mPlaybackThreads.size()) {
                IAfPlaybackThread* const dstThread =
                IAfPlaybackThread* const dstThread =
@@ -4203,7 +4222,8 @@ status_t AudioFlinger::createEffect(const media::CreateEffectRequest& request,
            // before creating the AudioEffect or the io handle must be specified.
            // before creating the AudioEffect or the io handle must be specified.
            //
            //
            // Detect if the effect is created after an AudioRecord is destroyed.
            // Detect if the effect is created after an AudioRecord is destroyed.
            if (getOrphanEffectChain_l(sessionId).get() != nullptr) {
            if ((sessionId != AUDIO_SESSION_OUTPUT_MIX) &&
                    getOrphanEffectChain_l(sessionId).get() != nullptr) {
                ALOGE("%s: effect %s with no specified io handle is denied because the AudioRecord"
                ALOGE("%s: effect %s with no specified io handle is denied because the AudioRecord"
                      " for session %d no longer exists",
                      " for session %d no longer exists",
                      __func__, descOut.name, sessionId);
                      __func__, descOut.name, sessionId);
@@ -4259,7 +4279,8 @@ status_t AudioFlinger::createEffect(const media::CreateEffectRequest& request,
                    goto Exit;
                    goto Exit;
                }
                }
            }
            }
        } else {
        }
        if (thread->type() == IAfThreadBase::RECORD || sessionId == AUDIO_SESSION_OUTPUT_MIX) {
            // Check if one effect chain was awaiting for an effect to be created on this
            // 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.
            // session and used it instead of creating a new one.
            sp<IAfEffectChain> chain = getOrphanEffectChain_l(sessionId);
            sp<IAfEffectChain> chain = getOrphanEffectChain_l(sessionId);
@@ -4363,17 +4384,39 @@ NO_THREAD_SAFETY_ANALYSIS
        }
        }
        return ret;
        return ret;
    }
    }
    IAfPlaybackThread* const srcThread = checkPlaybackThread_l(srcIo);

    if (srcThread == nullptr) {
    IAfPlaybackThread* dstThread = checkPlaybackThread_l(dstIo);
        ALOGW("%s() bad srcIo %d", __func__, srcIo);
        return BAD_VALUE;
    }
    IAfPlaybackThread* const dstThread = checkPlaybackThread_l(dstIo);
    if (dstThread == nullptr) {
    if (dstThread == nullptr) {
        ALOGW("%s() bad dstIo %d", __func__, dstIo);
        ALOGW("%s() bad dstIo %d", __func__, dstIo);
        return BAD_VALUE;
        return BAD_VALUE;
    }
    }


    IAfPlaybackThread* srcThread = checkPlaybackThread_l(srcIo);
    sp<IAfEffectChain> orphanChain = getOrphanEffectChain_l(sessionId);
    if (srcThread == nullptr && orphanChain == nullptr && sessionId == AUDIO_SESSION_OUTPUT_MIX) {
        ALOGW("%s() AUDIO_SESSION_OUTPUT_MIX not found in orphans, checking other mix", __func__);
        for (size_t i = 0; i < mPlaybackThreads.size(); i++) {
            const sp<IAfPlaybackThread> pt = mPlaybackThreads.valueAt(i);
            const uint32_t sessionType = pt->hasAudioSession(AUDIO_SESSION_OUTPUT_MIX);
            if ((pt->type() == IAfThreadBase::MIXER || pt->type() == IAfThreadBase::OFFLOAD) &&
                    ((sessionType & IAfThreadBase::EFFECT_SESSION) != 0)) {
                srcThread = pt.get();
                ALOGW("%s() found srcOutput %d hosting AUDIO_SESSION_OUTPUT_MIX", __func__,
                      pt->id());
                break;
            }
        }
    }
    if (srcThread == nullptr && orphanChain == nullptr) {
        ALOGW("moveEffects() bad srcIo %d", srcIo);
        return BAD_VALUE;
    }
    // dstThread pointer validity has already been checked
    if (orphanChain != nullptr) {
        audio_utils::scoped_lock _ll(dstThread->mutex());
        return moveEffectChain_ll(sessionId, nullptr, dstThread, orphanChain.get());
    }
    // srcThread pointer validity has already been checked
    audio_utils::scoped_lock _ll(dstThread->mutex(), srcThread->mutex());
    audio_utils::scoped_lock _ll(dstThread->mutex(), srcThread->mutex());
    return moveEffectChain_ll(sessionId, srcThread, dstThread);
    return moveEffectChain_ll(sessionId, srcThread, dstThread);
}
}
@@ -4399,12 +4442,17 @@ void AudioFlinger::setEffectSuspended(int effectId,
// moveEffectChain_ll must be called with the AudioFlinger::mutex()
// moveEffectChain_ll must be called with the AudioFlinger::mutex()
// and both srcThread and dstThread mutex()s held
// and both srcThread and dstThread mutex()s held
status_t AudioFlinger::moveEffectChain_ll(audio_session_t sessionId,
status_t AudioFlinger::moveEffectChain_ll(audio_session_t sessionId,
        IAfPlaybackThread* srcThread, IAfPlaybackThread* dstThread)
        IAfPlaybackThread* srcThread, IAfPlaybackThread* dstThread,
        IAfEffectChain* srcChain)
{
{
    ALOGV("%s: session %d from thread %p to thread %p",
    ALOGV("%s: session %d from thread %p to thread %p %s",
            __func__, sessionId, srcThread, dstThread);
            __func__, sessionId, srcThread, dstThread,
            (srcChain != nullptr ? "from specific chain" : ""));
    ALOG_ASSERT((srcThread != nullptr) != (srcChain != nullptr),
                "no source provided for source chain");


    sp<IAfEffectChain> chain = srcThread->getEffectChain_l(sessionId);
    sp<IAfEffectChain> chain =
          srcChain != nullptr ? srcChain : srcThread->getEffectChain_l(sessionId);
    if (chain == 0) {
    if (chain == 0) {
        ALOGW("%s: effect chain for session %d not on source thread %p",
        ALOGW("%s: effect chain for session %d not on source thread %p",
                __func__, sessionId, srcThread);
                __func__, sessionId, srcThread);
@@ -4424,8 +4472,9 @@ status_t AudioFlinger::moveEffectChain_ll(audio_session_t sessionId,
    // otherwise unnecessary as removeEffect_l() will remove the chain when last effect is
    // otherwise unnecessary as removeEffect_l() will remove the chain when last effect is
    // removed.
    // removed.
    // TODO(b/216875016): consider holding the effect chain locks for the duration of the move.
    // TODO(b/216875016): consider holding the effect chain locks for the duration of the move.
    if (srcThread != nullptr) {
        srcThread->removeEffectChain_l(chain);
        srcThread->removeEffectChain_l(chain);

    }
    // transfer all effects one by one so that new effect chain is created on new thread with
    // transfer all effects one by one so that new effect chain is created on new thread with
    // correct buffer sizes and audio parameters and effect engines reconfigured accordingly
    // correct buffer sizes and audio parameters and effect engines reconfigured accordingly
    sp<IAfEffectChain> dstChain;
    sp<IAfEffectChain> dstChain;
@@ -4435,7 +4484,11 @@ status_t AudioFlinger::moveEffectChain_ll(audio_session_t sessionId,
    // process effects one by one.
    // process effects one by one.
    for (sp<IAfEffectModule> effect = chain->getEffectFromId_l(0); effect != nullptr;
    for (sp<IAfEffectModule> effect = chain->getEffectFromId_l(0); effect != nullptr;
            effect = chain->getEffectFromId_l(0)) {
            effect = chain->getEffectFromId_l(0)) {
        if (srcThread != nullptr) {
            srcThread->removeEffect_l(effect);
            srcThread->removeEffect_l(effect);
        } else {
            chain->removeEffect_l(effect);
        }
        removed.add(effect);
        removed.add(effect);
        status = dstThread->addEffect_ll(effect);
        status = dstThread->addEffect_ll(effect);
        if (status != NO_ERROR) {
        if (status != NO_ERROR) {
@@ -4463,7 +4516,7 @@ status_t AudioFlinger::moveEffectChain_ll(audio_session_t sessionId,
        for (const auto& effect : removed) {
        for (const auto& effect : removed) {
            dstThread->removeEffect_l(effect); // Note: Depending on error location, the last
            dstThread->removeEffect_l(effect); // Note: Depending on error location, the last
                                               // effect may not have been placed on dstThread.
                                               // effect may not have been placed on dstThread.
            if (srcThread->addEffect_ll(effect) == NO_ERROR) {
            if (srcThread != nullptr && srcThread->addEffect_ll(effect) == NO_ERROR) {
                ++restored;
                ++restored;
                if (dstChain == nullptr) {
                if (dstChain == nullptr) {
                    dstChain = effect->getCallback()->chain().promote();
                    dstChain = effect->getCallback()->chain().promote();
@@ -4494,15 +4547,19 @@ status_t AudioFlinger::moveEffectChain_ll(audio_session_t sessionId,
        if (errorString.empty()) {
        if (errorString.empty()) {
            errorString = StringPrintf("%s: failed status %d", __func__, status);
            errorString = StringPrintf("%s: failed status %d", __func__, status);
        }
        }
        ALOGW("%s: %s unsuccessful move of session %d from srcThread %p to dstThread %p "
        ALOGW("%s: %s unsuccessful move of session %d from %s %p to dstThread %p "
                "(%zu effects removed from srcThread, %zu effects restored to srcThread, "
                "(%zu effects removed from srcThread, %zu effects restored to srcThread, "
                "%zu effects started)",
                "%zu effects started)",
                __func__, errorString.c_str(), sessionId, srcThread, dstThread,
                __func__, errorString.c_str(), sessionId,
                (srcThread != nullptr ? "srcThread" : "srcChain"),
                (srcThread != nullptr ? (void*) srcThread : (void*) srcChain), dstThread,
                removed.size(), restored, started);
                removed.size(), restored, started);
    } else {
    } else {
        ALOGD("%s: successful move of session %d from srcThread %p to dstThread %p "
        ALOGD("%s: successful move of session %d from %s %p to dstThread %p "
                "(%zu effects moved, %zu effects started)",
                "(%zu effects moved, %zu effects started)",
                __func__, sessionId, srcThread, dstThread, removed.size(), started);
                __func__, sessionId, (srcThread != nullptr ? "srcThread" : "srcChain"),
                (srcThread != nullptr ? (void*) srcThread : (void*) srcChain), dstThread,
                removed.size(), started);
    }
    }
    return status;
    return status;
}
}
+2 −1
Original line number Original line Diff line number Diff line
@@ -373,7 +373,8 @@ private:
            EXCLUDES_AudioFlinger_Mutex;
            EXCLUDES_AudioFlinger_Mutex;


    status_t moveEffectChain_ll(audio_session_t sessionId,
    status_t moveEffectChain_ll(audio_session_t sessionId,
            IAfPlaybackThread* srcThread, IAfPlaybackThread* dstThread) final
            IAfPlaybackThread* srcThread, IAfPlaybackThread* dstThread,
            IAfEffectChain* srcChain = nullptr) final
            REQUIRES(mutex(), audio_utils::ThreadBase_Mutex);
            REQUIRES(mutex(), audio_utils::ThreadBase_Mutex);


    // This is a helper that is called during incoming binder calls.
    // This is a helper that is called during incoming binder calls.
+4 −3
Original line number Original line Diff line number Diff line
@@ -2492,6 +2492,7 @@ size_t EffectChain::removeEffect_l(const sp<IAfEffectModule>& effect,
    size_t size = mEffects.size();
    size_t size = mEffects.size();
    uint32_t type = effect->desc().flags & EFFECT_FLAG_TYPE_MASK;
    uint32_t type = effect->desc().flags & EFFECT_FLAG_TYPE_MASK;


    const bool hasThreadAttached = mEffectCallback->hasThreadAttached();
    for (size_t i = 0; i < size; i++) {
    for (size_t i = 0; i < size; i++) {
        if (effect == mEffects[i]) {
        if (effect == mEffects[i]) {
            // calling stop here will remove pre-processing effect from the audio HAL.
            // calling stop here will remove pre-processing effect from the audio HAL.
@@ -2504,8 +2505,8 @@ size_t EffectChain::removeEffect_l(const sp<IAfEffectModule>& effect,
            if (release) {
            if (release) {
                mEffects[i]->release_l();
                mEffects[i]->release_l();
            }
            }

            // Skip operation when no thread attached (could lead to sigfpe as framecount is 0...)
            if (type != EFFECT_FLAG_TYPE_AUXILIARY) {
            if (hasThreadAttached && type != EFFECT_FLAG_TYPE_AUXILIARY) {
                if (i == size - 1 && i != 0) {
                if (i == size - 1 && i != 0) {
                    mEffects[i - 1]->configure_l();
                    mEffects[i - 1]->configure_l();
                    mEffects[i - 1]->setOutBuffer(mOutBuffer);
                    mEffects[i - 1]->setOutBuffer(mOutBuffer);
@@ -2517,7 +2518,7 @@ size_t EffectChain::removeEffect_l(const sp<IAfEffectModule>& effect,
            // make sure the input buffer configuration for the new first effect in the chain
            // make sure the input buffer configuration for the new first effect in the chain
            // is updated if needed (can switch from HAL channel mask to mixer channel mask)
            // is updated if needed (can switch from HAL channel mask to mixer channel mask)
            if (type != EFFECT_FLAG_TYPE_AUXILIARY // TODO(b/284522658) breaks for aux FX, why?
            if (type != EFFECT_FLAG_TYPE_AUXILIARY // TODO(b/284522658) breaks for aux FX, why?
                    && i == 0 && size > 1) {
                    && hasThreadAttached && i == 0 && size > 1) {
                mEffects[0]->configure_l();
                mEffects[0]->configure_l();
                mEffects[0]->setInBuffer(mInBuffer);
                mEffects[0]->setInBuffer(mInBuffer);
                mEffects[0]->updateAccessMode_l();      // reconfig if needed.
                mEffects[0]->updateAccessMode_l();      // reconfig if needed.
+3 −1
Original line number Original line Diff line number Diff line
@@ -616,7 +616,9 @@ public:
            mThreadType = thread->type();
            mThreadType = thread->type();
            mAfThreadCallback = thread->afThreadCallback();
            mAfThreadCallback = thread->afThreadCallback();
        }
        }

        bool hasThreadAttached() const {
            return thread().promote() != nullptr;
        }
    private:
    private:
        const wp<IAfEffectChain> mChain;
        const wp<IAfEffectChain> mChain;
        mediautils::atomic_wp<IAfThreadBase> mThread;
        mediautils::atomic_wp<IAfThreadBase> mThread;
+2 −1
Original line number Original line Diff line number Diff line
@@ -95,7 +95,8 @@ public:
    virtual bool updateOrphanEffectChains(const sp<IAfEffectModule>& effect)
    virtual bool updateOrphanEffectChains(const sp<IAfEffectModule>& effect)
            EXCLUDES_AudioFlinger_Mutex = 0;
            EXCLUDES_AudioFlinger_Mutex = 0;
    virtual status_t moveEffectChain_ll(audio_session_t sessionId,
    virtual status_t moveEffectChain_ll(audio_session_t sessionId,
            IAfPlaybackThread* srcThread, IAfPlaybackThread* dstThread)
            IAfPlaybackThread* srcThread, IAfPlaybackThread* dstThread,
            IAfEffectChain* srcChain = nullptr)
            REQUIRES(mutex(), audio_utils::ThreadBase_Mutex) = 0;
            REQUIRES(mutex(), audio_utils::ThreadBase_Mutex) = 0;


    virtual void requestLogMerge() = 0;
    virtual void requestLogMerge() = 0;