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

Commit f191963f authored by Ray Essick's avatar Ray Essick Committed by android-build-team Robot
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
Change-Id: Ia2e68ef616e249a6e8746b9068f22bd208a0ffc8
(cherry picked from commit e5767553)
parent 7df13bfc

media/libmediaplayerservice/nuplayer/NuPlayerDriver.cpp

+58 −28
Original line number Diff line number Diff line
@@ -117,6 +117,7 @@ NuPlayerDriver::~NuPlayerDriver() { 
    updateMetrics("destructor");

    logMetrics("destructor");



    Mutex::Autolock autoLock(mMetricsLock);

    if (mAnalyticsItem != NULL) {

        delete mAnalyticsItem;

        mAnalyticsItem = NULL;
@@ -130,6 +131,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);

    }
@@ -543,10 +546,46 @@ 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);

    mAnalyticsItem->setInt64(kPlayerDuration, 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 mAnalyticsItem machinery

    Mutex::Autolock autoLock(mMetricsLock);



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



    if (mRebufferingEvents != 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);
@@ -591,26 +630,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());

}
@@ -620,6 +639,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;

    }
@@ -778,11 +800,16 @@ status_t NuPlayerDriver::setParameter( 


status_t NuPlayerDriver::getParameter(int key, Parcel *reply) {



    if (key == FOURCC('m','t','r','X') && mAnalyticsItem != NULL) {

    if (key == FOURCC('m','t','r','X')) {

        // mtrX -- a play on 'metrics' (not matrix)

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

        updateMetrics("api");



        // ensure mAnalyticsItem stability while writing to parcel

        Mutex::Autolock autoLock(mMetricsLock);

        if (mAnalyticsItem != NULL) {

            mAnalyticsItem->writeToParcel(reply);

        }

        return OK;

    }
@@ -1006,13 +1033,16 @@ void NuPlayerDriver::notifyListener_l( 
            // when we have an error, add it to the analytics for this playback.

            // 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 && mAnalyticsItem != NULL) {

            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;