stagefright: use handler instead of handler-id in AMessage
This avoids locking gLooperRoster mutex on post() and deliver().
Bug: 19607784
Change-Id: If6d9d7884dbb08fc390983bda896d223803476ba
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 AHandler : public RefBase {
AHandler()
: mID(0),
+ mVerboseStats(false),
mMessageCounter(0) {
}
@@ -36,23 +37,40 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 handlerID = mNextHandlerID++;
mHandlers.add(handlerID, info);
- handler->setID(handlerID);
+ handler->setID(handlerID, looper);
return handlerID;
}
@@ -68,7 +68,7 @@
sp<AHandler> handler = info.mHandler.promote();
if (handler != NULL) {
- handler->setID(0);
+ handler->setID(0, NULL);
}
mHandlers.removeItemsAt(index);
@@ -100,96 +100,35 @@
}
}
-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;
+ handler->clear();
+ looper->clear();
+ return;
}
- sp<ALooper> looper = mHandlers.valueAt(index).mLooper.promote();
-
- if (looper == NULL) {
- mHandlers.removeItemsAt(index);
- return NULL;
- }
-
- 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
}
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 @@
// 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) {