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

Commit 372e8f2b authored by Ray Essick's avatar Ray Essick
Browse files

Improve concurrency access for updateMetrics() 

nuplayer's updateMetrics() referenced an unprotected shared stats buffer.
It's a small buffer, so we now make a copy during updateMetrics()
[at a point where we are mutexed] instead of putting a mutex on the
underlying frequently used construct.

Ensure that nuplayer2 has the same protections.

Bug: 123256408
Test: race condition
parent 1342d074

media/libmediaplayer2/nuplayer2/NuPlayer2.cpp

+7 −0
Original line number Diff line number Diff line
@@ -1289,6 +1289,7 @@ void NuPlayer2::onMessageReceived(const sp<AMessage> &msg) { 
            } else if (what == DecoderBase::kWhatShutdownCompleted) {

                ALOGV("%s shutdown completed", audio ? "audio" : "video");

                if (audio) {

                    Mutex::Autolock autoLock(mDecoderLock);

                    mAudioDecoder.clear();

                    mAudioDecoderError = false;

                    ++mAudioDecoderGeneration;
@@ -1296,6 +1297,7 @@ void NuPlayer2::onMessageReceived(const sp<AMessage> &msg) { 
                    CHECK_EQ((int)mFlushingAudio, (int)SHUTTING_DOWN_DECODER);

                    mFlushingAudio = SHUT_DOWN;

                } else {

                    Mutex::Autolock autoLock(mDecoderLock);

                    mVideoDecoder.clear();

                    mVideoDecoderError = false;

                    ++mVideoDecoderGeneration;
@@ -1967,6 +1969,7 @@ void NuPlayer2::restartAudio( 
        int64_t currentPositionUs, bool forceNonOffload, bool needsToCreateAudioDecoder) {

    if (mAudioDecoder != NULL) {

        mAudioDecoder->pause();

        Mutex::Autolock autoLock(mDecoderLock);

        mAudioDecoder.clear();

        mAudioDecoderError = false;

        ++mAudioDecoderGeneration;
@@ -2085,6 +2088,8 @@ status_t NuPlayer2::instantiateDecoder( 
        }

    }



    Mutex::Autolock autoLock(mDecoderLock);



    if (audio) {

        sp<AMessage> notify = new AMessage(kWhatAudioNotify, this);

        ++mAudioDecoderGeneration;
@@ -2395,6 +2400,8 @@ void NuPlayer2::getStats(Vector<sp<AMessage> > *mTrackStats) { 
    CHECK(mTrackStats != NULL);



    mTrackStats->clear();



    Mutex::Autolock autoLock(mDecoderLock);

    if (mVideoDecoder != NULL) {

        mTrackStats->push_back(mVideoDecoder->getStats());

    }

media/libmediaplayer2/nuplayer2/NuPlayer2.h

+1 −0
Original line number Diff line number Diff line
@@ -197,6 +197,7 @@ private: 
    sp<DecoderBase> mVideoDecoder;

    bool mOffloadAudio;

    sp<DecoderBase> mAudioDecoder;

    Mutex mDecoderLock;  // guard |mAudioDecoder| and |mVideoDecoder|.

    sp<CCDecoder> mCCDecoder;

    sp<Renderer> mRenderer;

    sp<ALooper> mRendererLooper;

media/libmediaplayer2/nuplayer2/NuPlayer2Decoder.cpp

+4 −1
Original line number Diff line number Diff line
@@ -109,7 +109,10 @@ sp<AMessage> NuPlayer2::Decoder::getStats() const { 
    mStats->setInt64("frames-dropped-output", mNumOutputFramesDropped);

    mStats->setFloat("frame-rate-total", mFrameRateTotal);



    return mStats;

    // i'm mutexed right now.

    // make our own copy, so we aren't victim to any later changes.

    sp<AMessage> copiedStats = mStats->dup();

    return copiedStats;

}



status_t NuPlayer2::Decoder::setVideoSurface(const sp<ANativeWindowWrapper> &nww) {

media/libmediaplayerservice/nuplayer/NuPlayerDecoder.cpp

+6 −1
Original line number Diff line number Diff line
@@ -107,11 +107,16 @@ NuPlayer::Decoder::~Decoder() { 
}



sp<AMessage> NuPlayer::Decoder::getStats() const {



    mStats->setInt64("frames-total", mNumFramesTotal);

    mStats->setInt64("frames-dropped-input", mNumInputFramesDropped);

    mStats->setInt64("frames-dropped-output", mNumOutputFramesDropped);

    mStats->setFloat("frame-rate-total", mFrameRateTotal);

    return mStats;



    // i'm mutexed right now.

    // make our own copy, so we aren't victim to any later changes.

    sp<AMessage> copiedStats = mStats->dup();

    return copiedStats;

}



status_t NuPlayer::Decoder::setVideoSurface(const sp<Surface> &surface) {