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

Commit 7434e816 authored by jiabin's avatar jiabin
Browse files

Accessing tee patches with holding the thread lock. 

There is race that the tee patches can be updated from thread loop and
the tee patches can also be accessed when the client has some
operations. In that case, it will be safe to access tee patches with
holding the thread lock.

Bug: 286576245
Test: atest AudioPlaybackCaptureTest
Change-Id: Ia704350ed5b3c914448ee609da33b867e3aa7626
parent 1579be6d

services/audioflinger/AudioFlinger.cpp

+1 −1
Original line number Diff line number Diff line
@@ -4000,7 +4000,7 @@ void AudioFlinger::updateSecondaryOutputsForTrack_l( 
        patchTrack->setPeerProxy(patchRecord, true /* holdReference */);

        patchRecord->setPeerProxy(patchTrack, false /* holdReference */);

    }

    track->setTeePatchesToUpdate(std::move(teePatches));

    track->setTeePatchesToUpdate_l(std::move(teePatches));

}



sp<AudioFlinger::SyncEvent> AudioFlinger::createSyncEvent(AudioSystem::sync_event_t type,

services/audioflinger/PlaybackTracks.h

+4 −3
Original line number Diff line number Diff line
@@ -187,8 +187,8 @@ public: 
            sp<os::ExternalVibration> getExternalVibration() const { return mExternalVibration; }



            // This function should be called with holding thread lock.

            void    updateTeePatches();

            void    setTeePatchesToUpdate(TeePatches teePatchesToUpdate);

            void    updateTeePatches_l();

            void    setTeePatchesToUpdate_l(TeePatches teePatchesToUpdate);



    void tallyUnderrunFrames(size_t frames) override {

       if (isOut()) { // we expect this from output tracks only
@@ -335,8 +335,9 @@ protected: 


private:

    void                interceptBuffer(const AudioBufferProvider::Buffer& buffer);

    // Must hold thread lock to access tee patches

    template <class F>

    void                forEachTeePatchTrack(F f) {

    void                forEachTeePatchTrack_l(F f) {

        for (auto& tp : mTeePatches) { f(tp.patchTrack); }

    };

services/audioflinger/Threads.cpp

+1 −1
Original line number Diff line number Diff line
@@ -4100,7 +4100,7 @@ NO_THREAD_SAFETY_ANALYSIS // manual locking of AudioFlinger 
            setHalLatencyMode_l();



            for (const auto &track : mActiveTracks ) {

                track->updateTeePatches();

                track->updateTeePatches_l();

            }



            // signal actual start of output stream when the render position reported by the kernel

services/audioflinger/Tracks.cpp

+14 −12
Original line number Diff line number Diff line
@@ -779,12 +779,12 @@ void AudioFlinger::PlaybackThread::Track::destroy() 
            Mutex::Autolock _l(thread->mLock);

            PlaybackThread *playbackThread = (PlaybackThread *)thread.get();

            wasActive = playbackThread->destroyTrack_l(this);

            forEachTeePatchTrack_l([](const auto& patchTrack) { patchTrack->destroy(); });

        }

        if (isExternalTrack() && !wasActive) {

            AudioSystem::releaseOutput(mPortId);

        }

    }

    forEachTeePatchTrack([](auto patchTrack) { patchTrack->destroy(); });

}



void AudioFlinger::PlaybackThread::Track::appendDumpHeader(String8& result)
@@ -1157,12 +1157,13 @@ status_t AudioFlinger::PlaybackThread::Track::start(AudioSystem::sync_event_t ev 
            buffer.mFrameCount = 1;

            (void) mAudioTrackServerProxy->obtainBuffer(&buffer, true /*ackFlush*/);

        }

        if (status == NO_ERROR) {

            forEachTeePatchTrack_l([](const auto& patchTrack) { patchTrack->start(); });

        }

    } else {

        status = BAD_VALUE;

    }

    if (status == NO_ERROR) {

        forEachTeePatchTrack([](auto patchTrack) { patchTrack->start(); });



        // send format to AudioManager for playback activity monitoring

        sp<IAudioManager> audioManager = thread->mAudioFlinger->getOrCreateAudioManager();

        if (audioManager && mPortId != AUDIO_PORT_HANDLE_NONE) {
@@ -1212,8 +1213,8 @@ void AudioFlinger::PlaybackThread::Track::stop() 
            ALOGV("%s(%d): not stopping/stopped => stopping/stopped on thread %d",

                    __func__, mId, (int)mThreadIoHandle);

        }

        forEachTeePatchTrack_l([](const auto& patchTrack) { patchTrack->stop(); });

    }

    forEachTeePatchTrack([](auto patchTrack) { patchTrack->stop(); });

}



void AudioFlinger::PlaybackThread::Track::pause()
@@ -1248,9 +1249,9 @@ void AudioFlinger::PlaybackThread::Track::pause() 
        default:

            break;

        }

    }

        // Pausing the TeePatch to avoid a glitch on underrun, at the cost of buffered audio loss.

    forEachTeePatchTrack([](auto patchTrack) { patchTrack->pause(); });

        forEachTeePatchTrack_l([](const auto& patchTrack) { patchTrack->pause(); });

    }

}



void AudioFlinger::PlaybackThread::Track::flush()
@@ -1311,9 +1312,10 @@ void AudioFlinger::PlaybackThread::Track::flush() 
        // before mixer thread can run. This is important when offloading

        // because the hardware buffer could hold a large amount of audio

        playbackThread->broadcast_l();

        // Flush the Tee to avoid on resume playing old data and glitching on the transition to

        // new data

        forEachTeePatchTrack_l([](const auto& patchTrack) { patchTrack->flush(); });

    }

    // Flush the Tee to avoid on resume playing old data and glitching on the transition to new data

    forEachTeePatchTrack([](auto patchTrack) { patchTrack->flush(); });

}



// must be called with thread lock held
@@ -1491,19 +1493,19 @@ void AudioFlinger::PlaybackThread::Track::copyMetadataTo(MetadataInserter& backI 
    *backInserter++ = metadata;

}



void AudioFlinger::PlaybackThread::Track::updateTeePatches() {

void AudioFlinger::PlaybackThread::Track::updateTeePatches_l() {

    if (mTeePatchesToUpdate.has_value()) {

        forEachTeePatchTrack([](auto patchTrack) { patchTrack->destroy(); });

        forEachTeePatchTrack_l([](const auto& patchTrack) { patchTrack->destroy(); });

        mTeePatches = mTeePatchesToUpdate.value();

        if (mState == TrackBase::ACTIVE || mState == TrackBase::RESUMING ||

                mState == TrackBase::STOPPING_1) {

            forEachTeePatchTrack([](auto patchTrack) { patchTrack->start(); });

            forEachTeePatchTrack_l([](const auto& patchTrack) { patchTrack->start(); });

        }

        mTeePatchesToUpdate.reset();

    }

}



void AudioFlinger::PlaybackThread::Track::setTeePatchesToUpdate(TeePatches teePatchesToUpdate) {

void AudioFlinger::PlaybackThread::Track::setTeePatchesToUpdate_l(TeePatches teePatchesToUpdate) {

    ALOGW_IF(mTeePatchesToUpdate.has_value(),

             "%s, existing tee patches to update will be ignored", __func__);

    mTeePatchesToUpdate = std::move(teePatchesToUpdate);