Make a separate MediaPlayer2Base::NotifyCallback
This avoids the collision with notify_callback_f.
Also, this CL should fix the potential use-after-free in notify().
Test: Builds properly
Bug: 70546581
Change-Id: Ia4919522d00c94591d6f14a27cb6465a8c4efaeb
diff --git a/media/libmedia/MediaPlayer2Factory.cpp b/media/libmedia/MediaPlayer2Factory.cpp
index ca7b96f..d6aab70 100644
--- a/media/libmedia/MediaPlayer2Factory.cpp
+++ b/media/libmedia/MediaPlayer2Factory.cpp
@@ -126,8 +126,8 @@
sp<MediaPlayer2Base> MediaPlayer2Factory::createPlayer(
player2_type playerType,
- void* cookie,
- notify_callback_f notifyFunc,
+ const wp<MediaPlayer2Engine> &client,
+ MediaPlayer2Base::NotifyCallback notifyFunc,
pid_t pid) {
sp<MediaPlayer2Base> p;
IFactory* factory;
@@ -152,7 +152,7 @@
init_result = p->initCheck();
if (init_result == NO_ERROR) {
- p->setNotifyCallback(cookie, notifyFunc);
+ p->setNotifyCallback(client, notifyFunc);
} else {
ALOGE("Failed to create player object of type %d, initCheck failed"
" (res = %d)", playerType, init_result);
diff --git a/media/libmedia/MediaPlayer2Factory.h b/media/libmedia/MediaPlayer2Factory.h
index 5813b2e..799b5f3 100644
--- a/media/libmedia/MediaPlayer2Factory.h
+++ b/media/libmedia/MediaPlayer2Factory.h
@@ -65,8 +65,8 @@
const sp<DataSource> &source);
static sp<MediaPlayer2Base> createPlayer(player2_type playerType,
- void* cookie,
- notify_callback_f notifyFunc,
+ const wp<MediaPlayer2Engine> &client,
+ MediaPlayer2Base::NotifyCallback notifyFunc,
pid_t pid);
static void registerBuiltinFactories();
diff --git a/media/libmedia/MediaPlayer2Manager.cpp b/media/libmedia/MediaPlayer2Manager.cpp
index 9b7f98a..637214e 100644
--- a/media/libmedia/MediaPlayer2Manager.cpp
+++ b/media/libmedia/MediaPlayer2Manager.cpp
@@ -1258,12 +1258,13 @@
}
void MediaPlayer2Manager::Client::notify(
- void* cookie, int msg, int ext1, int ext2, const Parcel *obj)
+ const wp<MediaPlayer2Engine> &listener, int msg, int ext1, int ext2, const Parcel *obj)
{
- Client* client = static_cast<Client*>(cookie);
- if (client == NULL) {
+ sp<MediaPlayer2Engine> spListener = listener.promote();
+ if (spListener == NULL) {
return;
}
+ Client* client = static_cast<Client*>(spListener.get());
sp<MediaPlayer2EngineClient> c;
sp<Client> nextClient;
@@ -1311,7 +1312,7 @@
}
if (c != NULL) {
- ALOGV("[%d] notify (%p, %d, %d, %d)", client->mConnId, cookie, msg, ext1, ext2);
+ ALOGV("[%d] notify (%p, %d, %d, %d)", client->mConnId, spListener.get(), msg, ext1, ext2);
c->notify(msg, ext1, ext2, obj);
}
}
@@ -1405,7 +1406,9 @@
#if CALLBACK_ANTAGONIZER
const int Antagonizer::interval = 10000; // 10 msecs
-Antagonizer::Antagonizer(notify_callback_f cb, void* client) :
+Antagonizer::Antagonizer(
+ MediaPlayer2Manager::NotifyCallback cb,
+ const wp<MediaPlayer2Engine> &client) :
mExit(false), mActive(false), mClient(client), mCb(cb)
{
createThread(callbackThread, this);
diff --git a/media/libmedia/MediaPlayer2Manager.h b/media/libmedia/MediaPlayer2Manager.h
index b346f40..8f0c491 100644
--- a/media/libmedia/MediaPlayer2Manager.h
+++ b/media/libmedia/MediaPlayer2Manager.h
@@ -46,20 +46,22 @@
#if CALLBACK_ANTAGONIZER
class Antagonizer {
public:
- Antagonizer(notify_callback_f cb, void* client);
+ Antagonizer(
+ MediaPlayer2Base::NotifyCallback cb,
+ const wp<MediaPlayer2Engine> &client);
void start() { mActive = true; }
void stop() { mActive = false; }
void kill();
private:
static const int interval;
Antagonizer();
- static int callbackThread(void* cookie);
- Mutex mLock;
- Condition mCondition;
- bool mExit;
- bool mActive;
- void* mClient;
- notify_callback_f mCb;
+ static int callbackThread(void *cookie);
+ Mutex mLock;
+ Condition mCondition;
+ bool mExit;
+ bool mActive;
+ wp<MediaPlayer2Engine> mClient;
+ MediaPlayer2Base::NotifyCallback mCb;
};
#endif
@@ -301,7 +303,7 @@
status_t setDataSource_post(const sp<MediaPlayer2Base>& p,
status_t status);
- static void notify(void* cookie, int msg,
+ static void notify(const wp<MediaPlayer2Engine> &listener, int msg,
int ext1, int ext2, const Parcel *obj);
pid_t pid() const { return mPid; }
diff --git a/media/libmedia/include/media/MediaPlayer2Interface.h b/media/libmedia/include/media/MediaPlayer2Interface.h
index bd1146d..0dd487f 100644
--- a/media/libmedia/include/media/MediaPlayer2Interface.h
+++ b/media/libmedia/include/media/MediaPlayer2Interface.h
@@ -67,14 +67,14 @@
// duration below which we do not allow deep audio buffering
#define AUDIO_SINK_MIN_DEEP_BUFFER_DURATION_US 5000000
-// callback mechanism for passing messages to MediaPlayer2 object
-typedef void (*notify_callback_f)(void* cookie,
- int msg, int ext1, int ext2, const Parcel *obj);
-
// abstract base class - use MediaPlayer2Interface
class MediaPlayer2Base : public RefBase
{
public:
+ // callback mechanism for passing messages to MediaPlayer2 object
+ typedef void (*NotifyCallback)(const wp<MediaPlayer2Engine> &listener,
+ int msg, int ext1, int ext2, const Parcel *obj);
+
// AudioSink: abstraction layer for audio output
class AudioSink : public RefBase {
public:
@@ -158,7 +158,7 @@
virtual status_t enableAudioDeviceCallback(bool enabled);
};
- MediaPlayer2Base() : mCookie(0), mNotify(0) {}
+ MediaPlayer2Base() : mClient(0), mNotify(0) {}
virtual ~MediaPlayer2Base() {}
virtual status_t initCheck() = 0;
virtual bool hardwareOutput() = 0;
@@ -272,22 +272,22 @@
};
void setNotifyCallback(
- void* cookie, notify_callback_f notifyFunc) {
+ const wp<MediaPlayer2Engine> &client, NotifyCallback notifyFunc) {
Mutex::Autolock autoLock(mNotifyLock);
- mCookie = cookie; mNotify = notifyFunc;
+ mClient = client; mNotify = notifyFunc;
}
void sendEvent(int msg, int ext1=0, int ext2=0,
const Parcel *obj=NULL) {
- notify_callback_f notifyCB;
- void* cookie;
+ NotifyCallback notifyCB;
+ wp<MediaPlayer2Engine> client;
{
Mutex::Autolock autoLock(mNotifyLock);
notifyCB = mNotify;
- cookie = mCookie;
+ client = mClient;
}
- if (notifyCB) notifyCB(cookie, msg, ext1, ext2, obj);
+ if (notifyCB) notifyCB(client, msg, ext1, ext2, obj);
}
virtual status_t dump(int /* fd */, const Vector<String16>& /* args */) const {
@@ -305,9 +305,9 @@
private:
friend class MediaPlayer2Manager;
- Mutex mNotifyLock;
- void* mCookie;
- notify_callback_f mNotify;
+ Mutex mNotifyLock;
+ wp<MediaPlayer2Engine> mClient;
+ NotifyCallback mNotify;
};
// Implement this class for media players that use the AudioFlinger software mixer