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

Commit fe574c32 authored by Shalaj Jain's avatar Shalaj Jain
Browse files

stagefright: extendedstats: Synchronize logging mechanism 

Add a lock to all public functions of extendedstats to prevent
clients from modifying the same resource at the same time. For example
two clients can call getLogEntry and without locks one will end
up traversing the vector while the other is modifying it, leading
to a crash.

Change-Id: I908e0ebcc01026c258554214338f43c5d1226eb7
parent 24a7316b

include/media/stagefright/ExtendedStats.h

+8 −4
Original line number Diff line number Diff line
@@ -125,9 +125,9 @@ public: 


    ~ExtendedStats();

    void log(LogType type, const char* key, statsDataType value, bool condition = true);

    sp<LogEntry> getLogEntry(const char *key, LogType type);

    virtual void dump(const char* key = NULL) const;

    virtual void reset(const char* key) const;

    virtual void dump(const char* key = NULL);

    virtual void reset(const char* key);

    virtual void clear();



    static int64_t getSystemTime() {

        struct timeval tv;
@@ -159,14 +159,17 @@ public: 
        mWindowSize = windowSize;

    }



private:

    sp<LogEntry> getLogEntry(const char *key, LogType type);



protected:

    KeyedVector<AString, sp<LogEntry> > mLogEntry;

    Mutex mLock;



    ExtendedStats(const ExtendedStats&) {}

    AString mName;

    pid_t mTid;



    Mutex mLock;

    int32_t mWindowSize;

};
@@ -228,6 +231,7 @@ protected: 


    sp<ExtendedStats> mProfileTimes;

    int32_t mFrameRate;

    Mutex mLock;



    /* helper functions */

    void resetConsecutiveFramesDropped();

media/libstagefright/ExtendedStats.cpp

+23 −9
Original line number Diff line number Diff line
@@ -42,14 +42,13 @@ namespace android { 


/* constructors and destructors */

ExtendedStats::ExtendedStats(const char *id, pid_t tid) {

    mLogEntry.clear();

    clear();

    mName.setTo(id);

    mTid = tid;

    mWindowSize = kMaxWindowSize;

}



ExtendedStats::~ExtendedStats() {

    mLogEntry.clear();

    clear();

}



ExtendedStats::LogEntry::LogEntry()
@@ -223,8 +222,6 @@ sp<ExtendedStats::LogEntry> ExtendedStats::getLogEntry(const char *key, 


    /* if this entry doesn't exist, add it in the log and return it */

    if (idx < 0) {

        /* keep write access to the KeyedVector thread-safe */

        Mutex::Autolock autoLock(mLock);

        sp<LogEntry> logEntry = createLogEntry(type, mWindowSize);

        mLogEntry.add(key, logEntry);

        return logEntry;
@@ -234,15 +231,18 @@ sp<ExtendedStats::LogEntry> ExtendedStats::getLogEntry(const char *key, 
}



void ExtendedStats::log(LogType type, const char* key, statsDataType value, bool condition) {



    Mutex::Autolock lock(mLock);

    if ( !condition || !key)

        return;



    getLogEntry(key, type)->insert(value);

}



void ExtendedStats::dump(const char* key) const {

void ExtendedStats::dump(const char* key) {

    // If no key is provided, print all

    // TBD: print label and sentinels

    Mutex::Autolock lock(mLock);

    if (key) {

        ssize_t idx = mLogEntry.indexOfKey(key);

        if (idx >= 0) {
@@ -258,7 +258,8 @@ void ExtendedStats::dump(const char* key) const { 
    }

}



void ExtendedStats::reset(const char* key) const {

void ExtendedStats::reset(const char* key) {

    Mutex::Autolock lock(mLock);

    if (key) {

        ssize_t idx = mLogEntry.indexOfKey(key);

        if (idx >= 0) {
@@ -267,6 +268,14 @@ void ExtendedStats::reset(const char* key) const { 
    }

}



void ExtendedStats::clear() {

    Mutex::Autolock lock(mLock);

    mLogEntry.clear();

    mTid = -1;

    mWindowSize = kMaxWindowSize;

    mName = "";

}



ExtendedStats::AutoProfile::AutoProfile(

        const char* name, sp<MediaExtendedStats> mediaExtendedStats,

        bool condition, bool profileOnce)
@@ -315,6 +324,7 @@ MediaExtendedStats::MediaExtendedStats(const char* name, pid_t tid) { 


    mName = name;

    mTid = tid;

    mProfileTimes = new ExtendedStats(mName.c_str(), mTid);



    reset();

}
@@ -330,6 +340,8 @@ void MediaExtendedStats::resetConsecutiveFramesDropped() { 
/** MediaExtendedStats methods **/



void MediaExtendedStats::reset() {

    Mutex::Autolock lock(mLock);



    mCurrentConsecutiveFramesDropped = 0;

    mMaxConsecutiveFramesDropped = 0;

    mNumChainedDrops = 0;
@@ -340,7 +352,7 @@ void MediaExtendedStats::reset() { 
    mHeightDimensions.clear();



    mFrameRate = 30;

    mProfileTimes = new ExtendedStats(mName.c_str(), mTid);

    mProfileTimes->clear();

}
@@ -350,6 +362,7 @@ void MediaExtendedStats::logFrameDropped() { 
}



void MediaExtendedStats::logDimensions(int32_t width, int32_t height) {

    Mutex::Autolock lock(mLock);

    if (mWidthDimensions.empty() || mWidthDimensions.top() != width ||

        mHeightDimensions.empty() || mHeightDimensions.top() != height) {

        mWidthDimensions.push(width);
@@ -362,6 +375,7 @@ void MediaExtendedStats::logBitRate(int64_t frameSize, int64_t timestamp) { 
}



MediaExtendedStats::~MediaExtendedStats() {

    mProfileTimes = NULL;

}



/***************************** PlayerExtendedStats ************************/