From 082e4f75a383f957a6ed9186ca0692b694e1ce45 Mon Sep 17 00:00:00 2001 From: Pawin Vongmasa Date: Sun, 17 Dec 2017 02:31:18 -0800 Subject: [PATCH] Refactor MediaPlayerBase's notify Test: make cts -j123 && cts-tradefed run cts-dev -m \ CtsMediaTestCases --compatibility:module-arg \ CtsMediaTestCases:include-annotation:\ android.platform.test.annotations.RequiresDevice Bug: 70546581 Change-Id: Ia3a8eb99c2faf6935c63800ba08f65970cede48e --- .../MediaPlayerFactory.cpp | 5 +- .../MediaPlayerFactory.h | 3 +- .../MediaPlayerService.cpp | 39 +++++------ .../MediaPlayerService.h | 68 +++++++++++-------- .../include/MediaPlayerInterface.h | 30 ++++---- 5 files changed, 76 insertions(+), 69 deletions(-) diff --git a/media/libmediaplayerservice/MediaPlayerFactory.cpp b/media/libmediaplayerservice/MediaPlayerFactory.cpp index 88064d9250..54d6014d12 100644 --- a/media/libmediaplayerservice/MediaPlayerFactory.cpp +++ b/media/libmediaplayerservice/MediaPlayerFactory.cpp @@ -126,8 +126,7 @@ player_type MediaPlayerFactory::getPlayerType(const sp& client, sp MediaPlayerFactory::createPlayer( player_type playerType, - const wp &client, - notify_callback_f notifyFunc, + const sp &listener, pid_t pid) { sp p; IFactory* factory; @@ -152,7 +151,7 @@ sp MediaPlayerFactory::createPlayer( init_result = p->initCheck(); if (init_result == NO_ERROR) { - p->setNotifyCallback(client, notifyFunc); + p->setNotifyCallback(listener); } else { ALOGE("Failed to create player object of type %d, initCheck failed" " (res = %d)", playerType, init_result); diff --git a/media/libmediaplayerservice/MediaPlayerFactory.h b/media/libmediaplayerservice/MediaPlayerFactory.h index 6aedeb2ebc..e88700cf58 100644 --- a/media/libmediaplayerservice/MediaPlayerFactory.h +++ b/media/libmediaplayerservice/MediaPlayerFactory.h @@ -65,8 +65,7 @@ class MediaPlayerFactory { const sp &source); static sp createPlayer(player_type playerType, - const wp &client, - notify_callback_f notifyFunc, + const sp &listener, pid_t pid); static void registerBuiltinFactories(); diff --git a/media/libmediaplayerservice/MediaPlayerService.cpp b/media/libmediaplayerservice/MediaPlayerService.cpp index a963424fd4..cb6404c5c0 100644 --- a/media/libmediaplayerservice/MediaPlayerService.cpp +++ b/media/libmediaplayerservice/MediaPlayerService.cpp @@ -590,10 +590,11 @@ MediaPlayerService::Client::Client( mUid = uid; mRetransmitEndpointValid = false; mAudioAttributes = NULL; + mListener = new Listener(this); #if CALLBACK_ANTAGONIZER ALOGD("create Antagonizer"); - mAntagonizer = new Antagonizer(notify, this); + mAntagonizer = new Antagonizer(mListener); #endif } @@ -627,7 +628,7 @@ void MediaPlayerService::Client::disconnect() // and reset the player. We assume the player will serialize // access to itself if necessary. if (p != 0) { - p->setNotifyCallback(0, 0); + p->setNotifyCallback(0); #if CALLBACK_ANTAGONIZER ALOGD("kill Antagonizer"); mAntagonizer->kill(); @@ -652,7 +653,7 @@ sp MediaPlayerService::Client::createPlayer(player_type playerT p.clear(); } if (p == NULL) { - p = MediaPlayerFactory::createPlayer(playerType, this, notify, mPid); + p = MediaPlayerFactory::createPlayer(playerType, mListener, mPid); } if (p != NULL) { @@ -1430,25 +1431,19 @@ status_t MediaPlayerService::Client::getRetransmitEndpoint( } void MediaPlayerService::Client::notify( - const wp &listener, int msg, int ext1, int ext2, const Parcel *obj) + int msg, int ext1, int ext2, const Parcel *obj) { - sp spListener = listener.promote(); - if (spListener == NULL) { - return; - } - Client* client = static_cast(spListener.get()); - sp c; sp nextClient; status_t errStartNext = NO_ERROR; { - Mutex::Autolock l(client->mLock); - c = client->mClient; - if (msg == MEDIA_PLAYBACK_COMPLETE && client->mNextClient != NULL) { - nextClient = client->mNextClient; + Mutex::Autolock l(mLock); + c = mClient; + if (msg == MEDIA_PLAYBACK_COMPLETE && mNextClient != NULL) { + nextClient = mNextClient; - if (client->mAudioOutput != NULL) - client->mAudioOutput->switchToNextOutput(); + if (mAudioOutput != NULL) + mAudioOutput->switchToNextOutput(); errStartNext = nextClient->start(); } @@ -1474,17 +1469,17 @@ void MediaPlayerService::Client::notify( MEDIA_INFO_METADATA_UPDATE == ext1) { const media::Metadata::Type metadata_type = ext2; - if(client->shouldDropMetadata(metadata_type)) { + if(shouldDropMetadata(metadata_type)) { return; } // Update the list of metadata that have changed. getMetadata // also access mMetadataUpdated and clears it. - client->addNewMetadataUpdate(metadata_type); + addNewMetadataUpdate(metadata_type); } if (c != NULL) { - ALOGV("[%d] notify (%p, %d, %d, %d)", client->mConnId, spListener.get(), msg, ext1, ext2); + ALOGV("[%d] notify (%d, %d, %d)", mConnId, msg, ext1, ext2); c->notify(msg, ext1, ext2, obj); } } @@ -1542,8 +1537,8 @@ status_t MediaPlayerService::Client::releaseDrm() #if CALLBACK_ANTAGONIZER const int Antagonizer::interval = 10000; // 10 msecs -Antagonizer::Antagonizer(notify_callback_f cb, const wp &client) : - mExit(false), mActive(false), mClient(client), mCb(cb) +Antagonizer::Antagonizer(const sp &listener) : + mExit(false), mActive(false), mListener(listener) { createThread(callbackThread, this); } @@ -1563,7 +1558,7 @@ int Antagonizer::callbackThread(void* user) while (!p->mExit) { if (p->mActive) { ALOGV("send event"); - p->mCb(p->mClient, 0, 0, 0); + p->mListener->notify(0, 0, 0, 0); } usleep(interval); } diff --git a/media/libmediaplayerservice/MediaPlayerService.h b/media/libmediaplayerservice/MediaPlayerService.h index f043f32611..34e5e2a0ac 100644 --- a/media/libmediaplayerservice/MediaPlayerService.h +++ b/media/libmediaplayerservice/MediaPlayerService.h @@ -51,7 +51,7 @@ class MediaRecorderClient; #if CALLBACK_ANTAGONIZER class Antagonizer { public: - Antagonizer(notify_callback_f cb, const wp &client); + Antagonizer(const sp &listener); void start() { mActive = true; } void stop() { mActive = false; } void kill(); @@ -59,12 +59,11 @@ private: static const int interval; Antagonizer(); static int callbackThread(void* cookie); - Mutex mLock; - Condition mCondition; - bool mExit; - bool mActive; - wp mClient; - notify_callback_f mCb; + Mutex mLock; + Condition mCondition; + bool mExit; + bool mActive; + sp mListener; }; #endif @@ -215,7 +214,6 @@ class MediaPlayerService : public BnMediaPlayerService }; // AudioOutput - public: static void instantiate(); @@ -365,8 +363,7 @@ private: status_t setDataSource_post(const sp& p, status_t status); - static void notify(const wp &cookie, int msg, - int ext1, int ext2, const Parcel *obj); + void notify(int msg, int ext1, int ext2, const Parcel *obj); pid_t pid() const { return mPid; } virtual status_t dump(int fd, const Vector& args); @@ -437,23 +434,38 @@ private: status_t setAudioAttributes_l(const Parcel &request); - mutable Mutex mLock; - sp mPlayer; - sp mService; - sp mClient; - sp mAudioOutput; - pid_t mPid; - status_t mStatus; - bool mLoop; - int32_t mConnId; - audio_session_t mAudioSessionId; - audio_attributes_t * mAudioAttributes; - uid_t mUid; - sp mConnectedWindow; - sp mConnectedWindowBinder; - struct sockaddr_in mRetransmitEndpoint; - bool mRetransmitEndpointValid; - sp mNextClient; + class Listener : public MediaPlayerBase::Listener { + public: + Listener(const wp &client) : mClient(client) {} + virtual ~Listener() {} + virtual void notify(int msg, int ext1, int ext2, const Parcel *obj) { + sp client = mClient.promote(); + if (client != NULL) { + client->notify(msg, ext1, ext2, obj); + } + } + private: + wp mClient; + }; + + mutable Mutex mLock; + sp mPlayer; + sp mService; + sp mClient; + sp mAudioOutput; + pid_t mPid; + status_t mStatus; + bool mLoop; + int32_t mConnId; + audio_session_t mAudioSessionId; + audio_attributes_t * mAudioAttributes; + uid_t mUid; + sp mConnectedWindow; + sp mConnectedWindowBinder; + struct sockaddr_in mRetransmitEndpoint; + bool mRetransmitEndpointValid; + sp mNextClient; + sp mListener; // Metadata filters. media::Metadata::Filter mMetadataAllow; // protected by mLock @@ -468,7 +480,7 @@ private: sp mExtractorDeathListener; sp mCodecDeathListener; #if CALLBACK_ANTAGONIZER - Antagonizer* mAntagonizer; + Antagonizer* mAntagonizer; #endif }; // Client diff --git a/media/libmediaplayerservice/include/MediaPlayerInterface.h b/media/libmediaplayerservice/include/MediaPlayerInterface.h index c21ef42361..0ef38090f3 100644 --- a/media/libmediaplayerservice/include/MediaPlayerInterface.h +++ b/media/libmediaplayerservice/include/MediaPlayerInterface.h @@ -66,14 +66,17 @@ enum player_type { // 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 MediaPlayer object -typedef void (*notify_callback_f)(const wp &listener, - int msg, int ext1, int ext2, const Parcel *obj); - // abstract base class - use MediaPlayerInterface class MediaPlayerBase : public RefBase { public: + // callback mechanism for passing messages to MediaPlayer object + class Listener : public RefBase { + public: + virtual void notify(int msg, int ext1, int ext2, const Parcel *obj) = 0; + virtual ~Listener() {} + }; + // AudioSink: abstraction layer for audio output class AudioSink : public RefBase { public: @@ -152,7 +155,7 @@ public: virtual sp getVolumeShaperState(int id); }; - MediaPlayerBase() : mClient(0), mNotify(0) {} + MediaPlayerBase() {} virtual ~MediaPlayerBase() {} virtual status_t initCheck() = 0; virtual bool hardwareOutput() = 0; @@ -263,22 +266,22 @@ public: }; void setNotifyCallback( - const wp &client, notify_callback_f notifyFunc) { + const sp &listener) { Mutex::Autolock autoLock(mNotifyLock); - mClient = client; mNotify = notifyFunc; + mListener = listener; } void sendEvent(int msg, int ext1=0, int ext2=0, const Parcel *obj=NULL) { - notify_callback_f notifyCB; - wp client; + sp listener; { Mutex::Autolock autoLock(mNotifyLock); - notifyCB = mNotify; - client = mClient; + listener = mListener; } - if (notifyCB) notifyCB(client, msg, ext1, ext2, obj); + if (listener != NULL) { + listener->notify(msg, ext1, ext2, obj); + } } virtual status_t dump(int /* fd */, const Vector& /* args */) const { @@ -297,8 +300,7 @@ private: friend class MediaPlayerService; Mutex mNotifyLock; - wp mClient; - notify_callback_f mNotify; + sp mListener; }; // Implement this class for media players that use the AudioFlinger software mixer