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

Commit 86b46a20 authored by Ray Essick's avatar Ray Essick
Browse files

Expand nuplayer mutex for mediametrics management 

refactor some mutex for how nuplayer sets up mediametrics data.
expanded locking to eliminate a couple race conditions.

Bug: 151644303
Bug: 151643722
Test: poc attached to bugs
Merged-In: I75f29a6254c5eab5d4f524ee7a7ef59f93a0b405
Merged-In: Ia2e68ef616e249a6e8746b9068f22bd208a0ffc8
Change-Id: I1e9bbcd67a1510f70fad66e8ef77f529008e248a
parent 07ff5a7e

media/libmediaplayerservice/nuplayer/NuPlayerDriver.cpp

+54 −23
Original line number Diff line number Diff line
@@ -115,6 +115,7 @@ NuPlayerDriver::~NuPlayerDriver() { 
    updateMetrics("destructor");

    logMetrics("destructor");



    Mutex::Autolock autoLock(mMetricsLock);

    if (mAnalyticsItem != NULL) {

        delete mAnalyticsItem;

        mAnalyticsItem = NULL;
@@ -128,6 +129,8 @@ status_t NuPlayerDriver::initCheck() { 
status_t NuPlayerDriver::setUID(uid_t uid) {

    mPlayer->setUID(uid);

    mClientUid = uid;



    Mutex::Autolock autoLock(mMetricsLock);

    if (mAnalyticsItem) {

        mAnalyticsItem->setUid(mClientUid);

    }
@@ -541,10 +544,47 @@ void NuPlayerDriver::updateMetrics(const char *where) { 
    }

    ALOGV("updateMetrics(%p) from %s at state %d", this, where, mState);



    // gather the final stats for this record

    // gather the final track statistics for this record

    Vector<sp<AMessage>> trackStats;

    mPlayer->getStats(&trackStats);



    // getDuration() uses mLock

    int duration_ms = -1;

    getDuration(&duration_ms);



    mPlayer->updateInternalTimers();



    int64_t playingTimeUs;

    int64_t rebufferingTimeUs;

    int32_t rebufferingEvents;

    bool rebufferingAtExit;

    {

        Mutex::Autolock autoLock(mLock);



        playingTimeUs = mPlayingTimeUs;

        rebufferingTimeUs = mRebufferingTimeUs;

        rebufferingEvents = mRebufferingEvents;

        rebufferingAtExit = mRebufferingAtExit;

    }



    // finish the rest of the gathering holding mLock;

    // some of the fields we read are updated under mLock.

    // we also avoid any races within mMetricsItem machinery

    Mutex::Autolock autoLock(mMetricsLock);



    mAnalyticsItem->setInt64(kPlayerDuration, duration_ms);



    // these update under mLock, we'll read them under mLock

    mAnalyticsItem->setInt64(kPlayerPlaying, (playingTimeUs+500)/1000 );



    if (rebufferingEvents != 0) {

        mAnalyticsItem->setInt64(kPlayerRebuffering, (rebufferingTimeUs+500)/1000 );

        mAnalyticsItem->setInt32(kPlayerRebufferingCount, rebufferingEvents);

        mAnalyticsItem->setInt32(kPlayerRebufferingAtExit, rebufferingAtExit);

    }



    mAnalyticsItem->setCString(kPlayerDataSourceType, mPlayer->getDataSourceType());



    if (trackStats.size() > 0) {

        for (size_t i = 0; i < trackStats.size(); ++i) {

            const sp<AMessage> &stats = trackStats.itemAt(i);
@@ -586,24 +626,6 @@ void NuPlayerDriver::updateMetrics(const char *where) { 
        }

    }



    // always provide duration and playing time, even if they have 0/unknown values.



    // getDuration() uses mLock for mutex -- careful where we use it.

    int duration_ms = -1;

    getDuration(&duration_ms);

    mAnalyticsItem->setInt64(kPlayerDuration, duration_ms);



    mPlayer->updateInternalTimers();



    mAnalyticsItem->setInt64(kPlayerPlaying, (mPlayingTimeUs+500)/1000 );



    if (mRebufferingEvents != 0) {

        mAnalyticsItem->setInt64(kPlayerRebuffering, (mRebufferingTimeUs+500)/1000 );

        mAnalyticsItem->setInt32(kPlayerRebufferingCount, mRebufferingEvents);

        mAnalyticsItem->setInt32(kPlayerRebufferingAtExit, mRebufferingAtExit);

    }



    mAnalyticsItem->setCString(kPlayerDataSourceType, mPlayer->getDataSourceType());

}
@@ -613,6 +635,9 @@ void NuPlayerDriver::logMetrics(const char *where) { 
    }

    ALOGV("logMetrics(%p) from %s at state %d", this, where, mState);



    // make sure that the stats are stable while we're writing them.

    Mutex::Autolock autoLock(mMetricsLock);



    if (mAnalyticsItem == NULL || mAnalyticsItem->isEnabled() == false) {

        return;

    }
@@ -775,6 +800,9 @@ status_t NuPlayerDriver::getParameter(int key, Parcel *reply) { 
        // mtrX -- a play on 'metrics' (not matrix)

        // gather current info all together, parcel it, and send it back

        updateMetrics("api");



        // make sure that the stats are static while we're writing to the parcel

        Mutex::Autolock autoLock(mMetricsLock);

        mAnalyticsItem->writeToParcel(reply);

        return OK;

    }
@@ -1000,12 +1028,15 @@ void NuPlayerDriver::notifyListener_l( 
            // ext1 is our primary 'error type' value. Only add ext2 when non-zero.

            // [test against msg is due to fall through from previous switch value]

            if (msg == MEDIA_ERROR) {

                Mutex::Autolock autoLock(mMetricsLock);

                if (mAnalyticsItem != NULL) {

                    mAnalyticsItem->setInt32(kPlayerError, ext1);

                    if (ext2 != 0) {

                        mAnalyticsItem->setInt32(kPlayerErrorCode, ext2);

                    }

                    mAnalyticsItem->setCString(kPlayerErrorState, stateString(mState).c_str());

                }

            }

            mAtEOS = true;

            break;

        }

media/libmediaplayerservice/nuplayer/NuPlayerDriver.h

+1 −0
Original line number Diff line number Diff line
@@ -142,6 +142,7 @@ private: 
    uint32_t mPlayerFlags;



    MediaAnalyticsItem *mAnalyticsItem;

    mutable Mutex mMetricsLock;

    uid_t mClientUid;



    bool mAtEOS;