diff options
author | Lajos Molnar <lajos@google.com> | 2015-03-04 17:00:10 -0800 |
---|---|---|
committer | Lajos Molnar <lajos@google.com> | 2015-03-05 17:49:50 -0800 |
commit | 5804a76ac5f9f3c311f1bbbcc5ebdc8f8568ae14 (patch) | |
tree | 159660930b1c7f6300ea3054e47ce46719083a16 | |
parent | c10e7f121d346b433c32d1c0d791c4b547cbe60e (diff) | |
download | frameworks_av-5804a76ac5f9f3c311f1bbbcc5ebdc8f8568ae14.zip frameworks_av-5804a76ac5f9f3c311f1bbbcc5ebdc8f8568ae14.tar.gz frameworks_av-5804a76ac5f9f3c311f1bbbcc5ebdc8f8568ae14.tar.bz2 |
stagefright: use handler instead of handler-id in AMessage
This avoids locking gLooperRoster mutex on post() and deliver().
Bug: 19607784
Change-Id: If6d9d7884dbb08fc390983bda896d223803476ba
-rw-r--r-- | include/media/stagefright/foundation/AHandler.h | 24 | ||||
-rw-r--r-- | include/media/stagefright/foundation/ALooper.h | 8 | ||||
-rw-r--r-- | include/media/stagefright/foundation/ALooperRoster.h | 6 | ||||
-rw-r--r-- | include/media/stagefright/foundation/AMessage.h | 18 | ||||
-rw-r--r-- | media/libstagefright/foundation/AHandler.cpp | 18 | ||||
-rw-r--r-- | media/libstagefright/foundation/ALooper.cpp | 2 | ||||
-rw-r--r-- | media/libstagefright/foundation/ALooperRoster.cpp | 100 | ||||
-rw-r--r-- | media/libstagefright/foundation/AMessage.cpp | 55 |
8 files changed, 126 insertions, 105 deletions
diff --git a/include/media/stagefright/foundation/AHandler.h b/include/media/stagefright/foundation/AHandler.h index 41ade77..fe02a86 100644 --- a/include/media/stagefright/foundation/AHandler.h +++ b/include/media/stagefright/foundation/AHandler.h @@ -29,6 +29,7 @@ struct AMessage; struct AHandler : public RefBase { AHandler() : mID(0), + mVerboseStats(false), mMessageCounter(0) { } @@ -36,23 +37,40 @@ struct AHandler : public RefBase { return mID; } - sp<ALooper> looper(); + sp<ALooper> looper() const { + return mLooper.promote(); + } + + wp<ALooper> getLooper() const { + return mLooper; + } + + wp<AHandler> getHandler() const { + // allow getting a weak reference to a const handler + return const_cast<AHandler *>(this); + } protected: virtual void onMessageReceived(const sp<AMessage> &msg) = 0; private: - friend struct ALooperRoster; + friend struct AMessage; // deliverMessage() + friend struct ALooperRoster; // setID() ALooper::handler_id mID; + wp<ALooper> mLooper; - void setID(ALooper::handler_id id) { + inline void setID(ALooper::handler_id id, wp<ALooper> looper) { mID = id; + mLooper = looper; } + bool mVerboseStats; uint32_t mMessageCounter; KeyedVector<uint32_t, uint32_t> mMessages; + void deliverMessage(const sp<AMessage> &msg); + DISALLOW_EVIL_CONSTRUCTORS(AHandler); }; diff --git a/include/media/stagefright/foundation/ALooper.h b/include/media/stagefright/foundation/ALooper.h index 70e0c5e..150cdba 100644 --- a/include/media/stagefright/foundation/ALooper.h +++ b/include/media/stagefright/foundation/ALooper.h @@ -53,11 +53,15 @@ struct ALooper : public RefBase { static int64_t GetNowUs(); + const char *getName() const { + return mName.c_str(); + } + protected: virtual ~ALooper(); private: - friend struct ALooperRoster; + friend struct AMessage; // post() struct Event { int64_t mWhenUs; @@ -81,6 +85,6 @@ private: DISALLOW_EVIL_CONSTRUCTORS(ALooper); }; -} // namespace android +} // namespace android #endif // A_LOOPER_H_ diff --git a/include/media/stagefright/foundation/ALooperRoster.h b/include/media/stagefright/foundation/ALooperRoster.h index a0be8eb..63d52d9 100644 --- a/include/media/stagefright/foundation/ALooperRoster.h +++ b/include/media/stagefright/foundation/ALooperRoster.h @@ -33,15 +33,13 @@ struct ALooperRoster { void unregisterHandler(ALooper::handler_id handlerID); void unregisterStaleHandlers(); - status_t postMessage(const sp<AMessage> &msg, int64_t delayUs = 0); - void deliverMessage(const sp<AMessage> &msg); - status_t postAndAwaitResponse( const sp<AMessage> &msg, sp<AMessage> *response); void postReply(uint32_t replyID, const sp<AMessage> &reply); - sp<ALooper> findLooper(ALooper::handler_id handlerID); + void getHandlerAndLooper( + ALooper::handler_id handlerID, wp<AHandler> *handler, wp<ALooper> *looper); void dump(int fd, const Vector<String16>& args); diff --git a/include/media/stagefright/foundation/AMessage.h b/include/media/stagefright/foundation/AMessage.h index a9e235b..beaefdd 100644 --- a/include/media/stagefright/foundation/AMessage.h +++ b/include/media/stagefright/foundation/AMessage.h @@ -26,11 +26,14 @@ namespace android { struct ABuffer; +struct AHandler; struct AString; struct Parcel; struct AMessage : public RefBase { - AMessage(uint32_t what = 0, ALooper::handler_id target = 0); + AMessage(); + AMessage(uint32_t what, ALooper::handler_id target = 0); + AMessage(uint32_t what, const sp<const AHandler> &handler); static sp<AMessage> FromParcel(const Parcel &parcel); void writeToParcel(Parcel *parcel) const; @@ -39,7 +42,7 @@ struct AMessage : public RefBase { uint32_t what() const; void setTarget(ALooper::handler_id target); - ALooper::handler_id target() const; + void setTarget(const sp<const AHandler> &handler); void clear(); @@ -76,7 +79,7 @@ struct AMessage : public RefBase { const char *name, int32_t *left, int32_t *top, int32_t *right, int32_t *bottom) const; - void post(int64_t delayUs = 0); + status_t post(int64_t delayUs = 0); // Posts the message to its target and waits for a response (or error) // before returning. @@ -117,9 +120,16 @@ protected: virtual ~AMessage(); private: + friend struct ALooper; // deliver() + uint32_t mWhat; + + // used only for debugging ALooper::handler_id mTarget; + wp<AHandler> mHandler; + wp<ALooper> mLooper; + struct Rect { int32_t mLeft, mTop, mRight, mBottom; }; @@ -157,6 +167,8 @@ private: size_t findItemIndex(const char *name, size_t len) const; + void deliver(); + DISALLOW_EVIL_CONSTRUCTORS(AMessage); }; diff --git a/media/libstagefright/foundation/AHandler.cpp b/media/libstagefright/foundation/AHandler.cpp index bd5f7e9..7dbbe54 100644 --- a/media/libstagefright/foundation/AHandler.cpp +++ b/media/libstagefright/foundation/AHandler.cpp @@ -19,15 +19,23 @@ #include <utils/Log.h> #include <media/stagefright/foundation/AHandler.h> - -#include <media/stagefright/foundation/ALooperRoster.h> +#include <media/stagefright/foundation/AMessage.h> namespace android { -sp<ALooper> AHandler::looper() { - extern ALooperRoster gLooperRoster; +void AHandler::deliverMessage(const sp<AMessage> &msg) { + onMessageReceived(msg); + mMessageCounter++; - return gLooperRoster.findLooper(id()); + if (mVerboseStats) { + uint32_t what = msg->what(); + ssize_t idx = mMessages.indexOfKey(what); + if (idx < 0) { + mMessages.add(what, 1); + } else { + mMessages.editValueAt(idx)++; + } + } } } // namespace android diff --git a/media/libstagefright/foundation/ALooper.cpp b/media/libstagefright/foundation/ALooper.cpp index 88b1c92..617d32b 100644 --- a/media/libstagefright/foundation/ALooper.cpp +++ b/media/libstagefright/foundation/ALooper.cpp @@ -210,7 +210,7 @@ bool ALooper::loop() { mEventQueue.erase(mEventQueue.begin()); } - gLooperRoster.deliverMessage(event.mMessage); + event.mMessage->deliver(); // NOTE: It's important to note that at this point our "ALooper" object // may no longer exist (its final reference may have gone away while diff --git a/media/libstagefright/foundation/ALooperRoster.cpp b/media/libstagefright/foundation/ALooperRoster.cpp index 2d57aee..3484579 100644 --- a/media/libstagefright/foundation/ALooperRoster.cpp +++ b/media/libstagefright/foundation/ALooperRoster.cpp @@ -49,7 +49,7 @@ ALooper::handler_id ALooperRoster::registerHandler( ALooper::handler_id handlerID = mNextHandlerID++; mHandlers.add(handlerID, info); - handler->setID(handlerID); + handler->setID(handlerID, looper); return handlerID; } @@ -68,7 +68,7 @@ void ALooperRoster::unregisterHandler(ALooper::handler_id handlerID) { sp<AHandler> handler = info.mHandler.promote(); if (handler != NULL) { - handler->setID(0); + handler->setID(0, NULL); } mHandlers.removeItemsAt(index); @@ -100,96 +100,35 @@ void ALooperRoster::unregisterStaleHandlers() { } } -status_t ALooperRoster::postMessage( - const sp<AMessage> &msg, int64_t delayUs) { - - sp<ALooper> looper = findLooper(msg->target()); - - if (looper == NULL) { - return -ENOENT; - } - looper->post(msg, delayUs); - return OK; -} - -void ALooperRoster::deliverMessage(const sp<AMessage> &msg) { - sp<AHandler> handler; - - { - Mutex::Autolock autoLock(mLock); - - ssize_t index = mHandlers.indexOfKey(msg->target()); - - if (index < 0) { - ALOGW("failed to deliver message. Target handler not registered."); - return; - } - - const HandlerInfo &info = mHandlers.valueAt(index); - handler = info.mHandler.promote(); - - if (handler == NULL) { - ALOGW("failed to deliver message. " - "Target handler %d registered, but object gone.", - msg->target()); - - mHandlers.removeItemsAt(index); - return; - } - } - - handler->onMessageReceived(msg); - handler->mMessageCounter++; - - if (verboseStats) { - uint32_t what = msg->what(); - ssize_t idx = handler->mMessages.indexOfKey(what); - if (idx < 0) { - handler->mMessages.add(what, 1); - } else { - handler->mMessages.editValueAt(idx)++; - } - } -} - -sp<ALooper> ALooperRoster::findLooper(ALooper::handler_id handlerID) { +void ALooperRoster::getHandlerAndLooper( + ALooper::handler_id handlerID, wp<AHandler> *handler, wp<ALooper> *looper) { Mutex::Autolock autoLock(mLock); ssize_t index = mHandlers.indexOfKey(handlerID); if (index < 0) { - return NULL; - } - - sp<ALooper> looper = mHandlers.valueAt(index).mLooper.promote(); - - if (looper == NULL) { - mHandlers.removeItemsAt(index); - return NULL; + handler->clear(); + looper->clear(); + return; } - return looper; + *handler = mHandlers.valueAt(index).mHandler; + *looper = mHandlers.valueAt(index).mLooper; } status_t ALooperRoster::postAndAwaitResponse( const sp<AMessage> &msg, sp<AMessage> *response) { - sp<ALooper> looper = findLooper(msg->target()); - - if (looper == NULL) { - ALOGW("failed to post message. " - "Target handler %d still registered, but object gone.", - msg->target()); - response->clear(); - return -ENOENT; - } - Mutex::Autolock autoLock(mLock); uint32_t replyID = mNextReplyID++; msg->setInt32("replyID", replyID); - looper->post(msg, 0 /* delayUs */); + status_t err = msg->post(0 /* delayUs */); + if (err != OK) { + response->clear(); + return err; + } ssize_t index; while ((index = mReplies.indexOfKey(replyID)) < 0) { @@ -225,7 +164,7 @@ static void makeFourCC(uint32_t fourcc, char *s) { void ALooperRoster::dump(int fd, const Vector<String16>& args) { bool clear = false; bool oldVerbose = verboseStats; - for (size_t i = 0;i < args.size(); i++) { + for (size_t i = 0; i < args.size(); i++) { if (args[i] == String16("-c")) { clear = true; } else if (args[i] == String16("-von")) { @@ -241,22 +180,23 @@ void ALooperRoster::dump(int fd, const Vector<String16>& args) { Mutex::Autolock autoLock(mLock); size_t n = mHandlers.size(); - s.appendFormat(" %zd registered handlers:\n", n); + s.appendFormat(" %zu registered handlers:\n", n); for (size_t i = 0; i < n; i++) { - s.appendFormat(" %zd: ", i); + s.appendFormat(" %d: ", mHandlers.keyAt(i)); HandlerInfo &info = mHandlers.editValueAt(i); sp<ALooper> looper = info.mLooper.promote(); if (looper != NULL) { - s.append(looper->mName.c_str()); + s.append(looper->getName()); sp<AHandler> handler = info.mHandler.promote(); if (handler != NULL) { + handler->mVerboseStats = verboseStats; s.appendFormat(": %u messages processed", handler->mMessageCounter); if (verboseStats) { for (size_t j = 0; j < handler->mMessages.size(); j++) { char fourcc[15]; makeFourCC(handler->mMessages.keyAt(j), fourcc); - s.appendFormat("\n %s: %d", + s.appendFormat("\n %s: %u", fourcc, handler->mMessages.valueAt(j)); } diff --git a/media/libstagefright/foundation/AMessage.cpp b/media/libstagefright/foundation/AMessage.cpp index 1f46bc9..fbc321f 100644 --- a/media/libstagefright/foundation/AMessage.cpp +++ b/media/libstagefright/foundation/AMessage.cpp @@ -27,6 +27,7 @@ #include "ABuffer.h" #include "ADebug.h" #include "ALooperRoster.h" +#include "AHandler.h" #include "AString.h" #include <binder/Parcel.h> @@ -36,10 +37,23 @@ namespace android { extern ALooperRoster gLooperRoster; +AMessage::AMessage(void) + : mWhat(0), + mTarget(0), + mNumItems(0) { +} + AMessage::AMessage(uint32_t what, ALooper::handler_id target) : mWhat(what), - mTarget(target), + mTarget(0), + mNumItems(0) { + setTarget(target); +} + +AMessage::AMessage(uint32_t what, const sp<const AHandler> &handler) + : mWhat(what), mNumItems(0) { + setTarget(handler); } AMessage::~AMessage() { @@ -56,10 +70,19 @@ uint32_t AMessage::what() const { void AMessage::setTarget(ALooper::handler_id handlerID) { mTarget = handlerID; + gLooperRoster.getHandlerAndLooper(handlerID, &mHandler, &mLooper); } -ALooper::handler_id AMessage::target() const { - return mTarget; +void AMessage::setTarget(const sp<const AHandler> &handler) { + if (handler == NULL) { + mTarget = 0; + mHandler.clear(); + mLooper.clear(); + } else { + mTarget = handler->id(); + mHandler = handler->getHandler(); + mLooper = handler->getLooper(); + } } void AMessage::clear() { @@ -322,8 +345,25 @@ bool AMessage::findRect( return true; } -void AMessage::post(int64_t delayUs) { - gLooperRoster.postMessage(this, delayUs); +void AMessage::deliver() { + sp<AHandler> handler = mHandler.promote(); + if (handler == NULL) { + ALOGW("failed to deliver message as target handler %d is gone.", mTarget); + return; + } + + handler->deliverMessage(this); +} + +status_t AMessage::post(int64_t delayUs) { + sp<ALooper> looper = mLooper.promote(); + if (looper == NULL) { + ALOGW("failed to post message as target looper for handler %d is gone.", mTarget); + return -ENOENT; + } + + looper->post(this, delayUs); + return OK; } status_t AMessage::postAndAwaitResponse(sp<AMessage> *response) { @@ -348,7 +388,7 @@ bool AMessage::senderAwaitsResponse(uint32_t *replyID) const { } sp<AMessage> AMessage::dup() const { - sp<AMessage> msg = new AMessage(mWhat, mTarget); + sp<AMessage> msg = new AMessage(mWhat, mHandler.promote()); msg->mNumItems = mNumItems; #ifdef DUMP_STATS @@ -532,7 +572,8 @@ AString AMessage::debugString(int32_t indent) const { // static sp<AMessage> AMessage::FromParcel(const Parcel &parcel) { int32_t what = parcel.readInt32(); - sp<AMessage> msg = new AMessage(what); + sp<AMessage> msg = new AMessage(); + msg->setWhat(what); msg->mNumItems = static_cast<size_t>(parcel.readInt32()); for (size_t i = 0; i < msg->mNumItems; ++i) { |