Fix a deadlock caused by the AudioPlayer notifying the observer that the stream had ended at the same time the observer was shutting down the AudioPlayer.

related-to-bug: 2414536

4 files changed, 73 insertions, 52 deletions

diff --git a/include/media/stagefright/AudioPlayer.h b/include/media/stagefright/AudioPlayer.h
index 26fcc95..843e051 100644
--- a/include/media/stagefright/AudioPlayer.h
+++ b/include/media/stagefright/AudioPlayer.h
@@ -41,9 +41,6 @@ public:
     // Caller retains ownership of "source".
     void setSource(const sp<MediaSource> &source);
 
-    void setListenerCallback(
-            void (*notify)(void *cookie, int what), void *cookie);
-
     // Return time in us.
     virtual int64_t getRealTimeUs();
 
@@ -63,6 +60,9 @@ public:
 
     status_t seekTo(int64_t time_us);
 
+    bool isSeeking();
+    bool reachedEOS();
+
 private:
     sp<MediaSource> mSource;
     AudioTrack *mAudioTrack;
@@ -80,13 +80,11 @@ private:
     int64_t mPositionTimeRealUs;
 
     bool mSeeking;
+    bool mReachedEOS;
     int64_t mSeekTimeUs;
 
     bool mStarted;
 
-    void (*mListenerCallback)(void *cookie, int what);
-    void *mListenerCookie;
-
     sp<MediaPlayerBase::AudioSink> mAudioSink;
 
     static void AudioCallback(int event, void *user, void *info);
diff --git a/media/libstagefright/AudioPlayer.cpp b/media/libstagefright/AudioPlayer.cpp
index 01578c1..31157ce 100644
--- a/media/libstagefright/AudioPlayer.cpp
+++ b/media/libstagefright/AudioPlayer.cpp
@@ -37,6 +37,7 @@ AudioPlayer::AudioPlayer(const sp<MediaPlayerBase::AudioSink> &audioSink)
       mPositionTimeMediaUs(-1),
       mPositionTimeRealUs(-1),
       mSeeking(false),
+      mReachedEOS(false),
       mStarted(false),
       mAudioSink(audioSink) {
 }
@@ -47,12 +48,6 @@ AudioPlayer::~AudioPlayer() {
     }
 }
 
-void AudioPlayer::setListenerCallback(
-        void (*notify)(void *cookie, int what), void *cookie) {
-    mListenerCallback = notify;
-    mListenerCookie = cookie;
-}
-
 void AudioPlayer::setSource(const sp<MediaSource> &source) {
     CHECK_EQ(mSource, NULL);
     mSource = source;
@@ -172,6 +167,7 @@ void AudioPlayer::stop() {
     mPositionTimeMediaUs = -1;
     mPositionTimeRealUs = -1;
     mSeeking = false;
+    mReachedEOS = false;
     mStarted = false;
 }
 
@@ -180,6 +176,16 @@ void AudioPlayer::AudioCallback(int event, void *user, void *info) {
     static_cast<AudioPlayer *>(user)->AudioCallback(event, info);
 }
 
+bool AudioPlayer::isSeeking() {
+    Mutex::Autolock autoLock(mLock);
+    return mSeeking;
+}
+
+bool AudioPlayer::reachedEOS() {
+    Mutex::Autolock autoLock(mLock);
+    return mReachedEOS;
+}
+
 // static
 void AudioPlayer::AudioSinkCallback(
         MediaPlayerBase::AudioSink *audioSink,
@@ -203,6 +209,11 @@ void AudioPlayer::fillBuffer(void *data, size_t size) {
         LOGV("AudioCallback");
     }
 
+    if (mReachedEOS) {
+        memset(data, 0, size);
+        return;
+    }
+
     size_t size_done = 0;
     size_t size_remaining = size;
     while (size_remaining > 0) {
@@ -227,24 +238,16 @@ void AudioPlayer::fillBuffer(void *data, size_t size) {
             CHECK((err == OK && mInputBuffer != NULL)
                    || (err != OK && mInputBuffer == NULL));
 
-            if (mSeeking) {
-                mSeeking = false;
+            Mutex::Autolock autoLock(mLock);
 
-                if (mListenerCallback) {
-                    (*mListenerCallback)(mListenerCookie, SEEK_COMPLETE);
-                }
-            }
+            mSeeking = false;
 
             if (err != OK) {
-                if (mListenerCallback) {
-                    (*mListenerCallback)(mListenerCookie, REACHED_EOS);
-                }
-
+                mReachedEOS = true;
                 memset((char *)data + size_done, 0, size_remaining);
                 break;
             }
 
-            Mutex::Autolock autoLock(mLock);
             CHECK(mInputBuffer->meta_data()->findInt64(
                         kKeyTime, &mPositionTimeMediaUs));
 
@@ -319,6 +322,7 @@ status_t AudioPlayer::seekTo(int64_t time_us) {
     Mutex::Autolock autoLock(mLock);
 
     mSeeking = true;
+    mReachedEOS = false;
     mSeekTimeUs = time_us;
 
     return OK;
diff --git a/media/libstagefright/AwesomePlayer.cpp b/media/libstagefright/AwesomePlayer.cpp
index 4e7738e..935b0e1 100644
--- a/media/libstagefright/AwesomePlayer.cpp
+++ b/media/libstagefright/AwesomePlayer.cpp
@@ -121,6 +121,8 @@ AwesomePlayer::AwesomePlayer()
     mStreamDoneEventPending = false;
     mBufferingEvent = new AwesomeEvent(this, 2);
     mBufferingEventPending = false;
+    mCheckAudioStatusEvent = new AwesomeEvent(this, 3);
+    mAudioStatusEventPending = false;
 
     mQueue.start();
 
@@ -140,6 +142,8 @@ void AwesomePlayer::cancelPlayerEvents(bool keepBufferingGoing) {
     mVideoEventPending = false;
     mQueue.cancelEvent(mStreamDoneEvent->eventID());
     mStreamDoneEventPending = false;
+    mQueue.cancelEvent(mCheckAudioStatusEvent->eventID());
+    mAudioStatusEventPending = false;
 
     if (!keepBufferingGoing) {
         mQueue.cancelEvent(mBufferingEvent->eventID());
@@ -283,29 +287,6 @@ void AwesomePlayer::reset_l() {
     mPrefetcher.clear();
 }
 
-// static
-void AwesomePlayer::AudioNotify(void *_me, int what) {
-    AwesomePlayer *me = (AwesomePlayer *)_me;
-
-    Mutex::Autolock autoLock(me->mLock);
-
-    switch (what) {
-        case AudioPlayer::REACHED_EOS:
-            me->postStreamDoneEvent_l();
-            break;
-
-        case AudioPlayer::SEEK_COMPLETE:
-        {
-            me->notifyListener_l(MEDIA_SEEK_COMPLETE);
-            break;
-        }
-
-        default:
-            CHECK(!"should not be here.");
-            break;
-    }
-}
-
 void AwesomePlayer::notifyListener_l(int msg, int ext1) {
     if (mListener != NULL) {
         sp<MediaPlayerBase> listener = mListener.promote();
@@ -373,10 +354,6 @@ status_t AwesomePlayer::play() {
         if (mAudioPlayer == NULL) {
             if (mAudioSink != NULL) {
                 mAudioPlayer = new AudioPlayer(mAudioSink);
-
-                mAudioPlayer->setListenerCallback(
-                        &AwesomePlayer::AudioNotify, this);
-
                 mAudioPlayer->setSource(mAudioSource);
                 status_t err = mAudioPlayer->start();
 
@@ -393,10 +370,15 @@ status_t AwesomePlayer::play() {
                 mTimeSource = mAudioPlayer;
 
                 deferredAudioSeek = true;
+
+                mWatchForAudioSeekComplete = false;
+                mWatchForAudioEOS = true;
             }
         } else {
             mAudioPlayer->resume();
         }
+
+        postCheckAudioStatusEvent_l();
     }
 
     if (mTimeSource == NULL && mAudioPlayer == NULL) {
@@ -561,6 +543,8 @@ void AwesomePlayer::seekAudioIfNecessary_l() {
     if (mSeeking && mVideoRenderer == NULL && mAudioPlayer != NULL) {
         mAudioPlayer->seekTo(mSeekTimeUs);
 
+        mWatchForAudioSeekComplete = true;
+        mWatchForAudioEOS = true;
         mSeeking = false;
     }
 }
@@ -652,6 +636,9 @@ void AwesomePlayer::onEvent(int32_t code) {
     } else if (code == 2) {
         onBufferingUpdate();
         return;
+    } else if (code == 3) {
+        onCheckAudioStatus();
+        return;
     }
 
     Mutex::Autolock autoLock(mLock);
@@ -718,6 +705,8 @@ void AwesomePlayer::onEvent(int32_t code) {
             LOGV("seeking audio to %lld us (%.2f secs).", timeUs, timeUs / 1E6);
 
             mAudioPlayer->seekTo(timeUs);
+            mWatchForAudioSeekComplete = true;
+            mWatchForAudioEOS = true;
         } else {
             // If we're playing video only, report seek complete now,
             // otherwise audio player will notify us later.
@@ -803,5 +792,30 @@ void AwesomePlayer::postBufferingEvent_l() {
     mQueue.postEventWithDelay(mBufferingEvent, 1000000ll);
 }
 
+void AwesomePlayer::postCheckAudioStatusEvent_l() {
+    if (mAudioStatusEventPending) {
+        return;
+    }
+    mAudioStatusEventPending = true;
+    mQueue.postEventWithDelay(mCheckAudioStatusEvent, 100000ll);
+}
+
+void AwesomePlayer::onCheckAudioStatus() {
+    Mutex::Autolock autoLock(mLock);
+    mAudioStatusEventPending = false;
+
+    if (mWatchForAudioSeekComplete && !mAudioPlayer->isSeeking()) {
+        mWatchForAudioSeekComplete = false;
+        notifyListener_l(MEDIA_SEEK_COMPLETE);
+    }
+
+    if (mWatchForAudioEOS && mAudioPlayer->reachedEOS()) {
+        mWatchForAudioEOS = false;
+        postStreamDoneEvent_l();
+    }
+
+    postCheckAudioStatusEvent_l();
+}
+
 }  // namespace android
 
diff --git a/media/libstagefright/include/AwesomePlayer.h b/media/libstagefright/include/AwesomePlayer.h
index 8bd6594..75e71e6 100644
--- a/media/libstagefright/include/AwesomePlayer.h
+++ b/media/libstagefright/include/AwesomePlayer.h
@@ -109,16 +109,22 @@ private:
     bool mSeeking;
     int64_t mSeekTimeUs;
 
+    bool mWatchForAudioSeekComplete;
+    bool mWatchForAudioEOS;
+
     sp<TimedEventQueue::Event> mVideoEvent;
     bool mVideoEventPending;
     sp<TimedEventQueue::Event> mStreamDoneEvent;
     bool mStreamDoneEventPending;
     sp<TimedEventQueue::Event> mBufferingEvent;
     bool mBufferingEventPending;
+    sp<TimedEventQueue::Event> mCheckAudioStatusEvent;
+    bool mAudioStatusEventPending;
 
     void postVideoEvent_l(int64_t delayUs = -1);
     void postBufferingEvent_l();
     void postStreamDoneEvent_l();
+    void postCheckAudioStatusEvent_l();
 
     MediaBuffer *mLastVideoBuffer;
     MediaBuffer *mVideoBuffer;
@@ -138,13 +144,12 @@ private:
     status_t setVideoSource(sp<MediaSource> source);
 
     void onEvent(int32_t code);
-
-    static void AudioNotify(void *me, int what);
     void onStreamDone();
 
     void notifyListener_l(int msg, int ext1 = 0);
 
     void onBufferingUpdate();
+    void onCheckAudioStatus();
 
     AwesomePlayer(const AwesomePlayer &);
     AwesomePlayer &operator=(const AwesomePlayer &);