am ca61a8f0: Merge "MediaCodecSource: stop puller from caller\'s thread instead of looper" into lmp-dev

* commit 'ca61a8f0bc22606cb11b99bdb10a146f941d556e':
  MediaCodecSource: stop puller from caller's thread instead of looper

3 files changed, 45 insertions, 36 deletions

diff --git a/include/media/stagefright/MediaCodecSource.h b/include/media/stagefright/MediaCodecSource.h
index 4b18a0b..e1b2830 100644
--- a/include/media/stagefright/MediaCodecSource.h
+++ b/include/media/stagefright/MediaCodecSource.h
@@ -106,7 +106,6 @@ private:
     bool mStarted;
     bool mStopping;
     bool mDoMoreWorkPending;
-    bool mPullerReachedEOS;
     sp<AMessage> mEncoderActivityNotify;
     sp<IGraphicBufferProducer> mGraphicBufferProducer;
     Vector<sp<ABuffer> > mEncoderInputBuffers;
@@ -123,7 +122,7 @@ private:
     Mutex mOutputBufferLock;
     Condition mOutputBufferCond;
     List<MediaBuffer*> mOutputBufferQueue;
-    bool mEncodedReachedEOS;
+    bool mEncoderReachedEOS;
     status_t mErrorCode;
 
     DISALLOW_EVIL_CONSTRUCTORS(MediaCodecSource);
diff --git a/media/libstagefright/AudioSource.cpp b/media/libstagefright/AudioSource.cpp
index a67fabe..804f131 100644
--- a/media/libstagefright/AudioSource.cpp
+++ b/media/libstagefright/AudioSource.cpp
@@ -155,12 +155,12 @@ status_t AudioSource::reset() {
     }
 
     mStarted = false;
+    mFrameAvailableCondition.signal();
+
     mRecord->stop();
     waitOutstandingEncodingFrames_l();
     releaseQueuedFrames_l();
 
-    mFrameAvailableCondition.signal();
-
     return OK;
 }
 
diff --git a/media/libstagefright/MediaCodecSource.cpp b/media/libstagefright/MediaCodecSource.cpp
index 9868ecf..1a80dcc 100644
--- a/media/libstagefright/MediaCodecSource.cpp
+++ b/media/libstagefright/MediaCodecSource.cpp
@@ -54,7 +54,7 @@ struct MediaCodecSource::Puller : public AHandler {
     Puller(const sp<MediaSource> &source);
 
     status_t start(const sp<MetaData> &meta, const sp<AMessage> &notify);
-    void stopAsync();
+    void stop();
 
     void pause();
     void resume();
@@ -139,8 +139,17 @@ status_t MediaCodecSource::Puller::start(const sp<MetaData> &meta,
     return postSynchronouslyAndReturnError(msg);
 }
 
-void MediaCodecSource::Puller::stopAsync() {
-    ALOGV("puller (%s) stopAsync", mIsAudio ? "audio" : "video");
+void MediaCodecSource::Puller::stop() {
+    // Stop source from caller's thread instead of puller's looper.
+    // mSource->stop() is thread-safe, doing it outside the puller's
+    // looper allows us to at least stop if source gets stuck.
+    // If source gets stuck in read(), the looper would never
+    // be able to process the stop(), which could lead to ANR.
+
+    ALOGV("source (%s) stopping", mIsAudio ? "audio" : "video");
+    mSource->stop();
+    ALOGV("source (%s) stopped", mIsAudio ? "audio" : "video");
+
     (new AMessage(kWhatStop, id()))->post();
 }
 
@@ -194,9 +203,6 @@ void MediaCodecSource::Puller::onMessageReceived(const sp<AMessage> &msg) {
 
         case kWhatStop:
         {
-            ALOGV("source (%s) stopping", mIsAudio ? "audio" : "video");
-            mSource->stop();
-            ALOGV("source (%s) stopped", mIsAudio ? "audio" : "video");
             ++mPullGeneration;
 
             handleEOS();
@@ -283,7 +289,21 @@ status_t MediaCodecSource::start(MetaData* params) {
 
 status_t MediaCodecSource::stop() {
     sp<AMessage> msg = new AMessage(kWhatStop, mReflector->id());
-    return postSynchronouslyAndReturnError(msg);
+    status_t err = postSynchronouslyAndReturnError(msg);
+
+    // mPuller->stop() needs to be done outside MediaCodecSource's looper,
+    // as it contains a synchronous call to stop the underlying MediaSource,
+    // which often waits for all outstanding MediaBuffers to return, but
+    // MediaBuffers are only returned when MediaCodecSource looper gets
+    // to process them.
+
+    if (mPuller != NULL) {
+        ALOGI("puller (%s) stopping", mIsVideo ? "video" : "audio");
+        mPuller->stop();
+        ALOGI("puller (%s) stopped", mIsVideo ? "video" : "audio");
+    }
+
+    return err;
 }
 
 status_t MediaCodecSource::pause() {
@@ -301,10 +321,10 @@ status_t MediaCodecSource::read(
     Mutex::Autolock autolock(mOutputBufferLock);
 
     *buffer = NULL;
-    while (mOutputBufferQueue.size() == 0 && !mEncodedReachedEOS) {
+    while (mOutputBufferQueue.size() == 0 && !mEncoderReachedEOS) {
         mOutputBufferCond.wait(mOutputBufferLock);
     }
-    if (!mEncodedReachedEOS) {
+    if (!mEncoderReachedEOS) {
         *buffer = *mOutputBufferQueue.begin();
         mOutputBufferQueue.erase(mOutputBufferQueue.begin());
         return OK;
@@ -330,9 +350,8 @@ MediaCodecSource::MediaCodecSource(
       mStarted(false),
       mStopping(false),
       mDoMoreWorkPending(false),
-      mPullerReachedEOS(false),
       mFirstSampleTimeUs(-1ll),
-      mEncodedReachedEOS(false),
+      mEncoderReachedEOS(false),
       mErrorCode(OK) {
     CHECK(mLooper != NULL);
 
@@ -434,7 +453,7 @@ status_t MediaCodecSource::initEncoder() {
         return err;
     }
 
-    mEncodedReachedEOS = false;
+    mEncoderReachedEOS = false;
     mErrorCode = OK;
 
     return OK;
@@ -465,10 +484,6 @@ void MediaCodecSource::releaseEncoder() {
     mEncoderOutputBuffers.clear();
 }
 
-bool MediaCodecSource::reachedEOS() {
-    return mEncodedReachedEOS && ((mPuller == NULL) || mPullerReachedEOS);
-}
-
 status_t MediaCodecSource::postSynchronouslyAndReturnError(
         const sp<AMessage> &msg) {
     sp<AMessage> response;
@@ -486,8 +501,8 @@ status_t MediaCodecSource::postSynchronouslyAndReturnError(
 }
 
 void MediaCodecSource::signalEOS(status_t err) {
-    if (!mEncodedReachedEOS) {
-        ALOGI("encoder (%s) reached EOS", mIsVideo ? "video" : "audio");
+    if (!mEncoderReachedEOS) {
+        ALOGV("encoder (%s) reached EOS", mIsVideo ? "video" : "audio");
         {
             Mutex::Autolock autoLock(mOutputBufferLock);
             // release all unread media buffers
@@ -496,16 +511,15 @@ void MediaCodecSource::signalEOS(status_t err) {
                 (*it)->release();
             }
             mOutputBufferQueue.clear();
-            mEncodedReachedEOS = true;
+            mEncoderReachedEOS = true;
             mErrorCode = err;
             mOutputBufferCond.signal();
         }
 
         releaseEncoder();
     }
-    if (mStopping && reachedEOS()) {
-        ALOGI("MediaCodecSource (%s) fully stopped",
-                mIsVideo ? "video" : "audio");
+    if (mStopping && mEncoderReachedEOS) {
+        ALOGI("encoder (%s) stopped", mIsVideo ? "video" : "audio");
         // posting reply to everyone that's waiting
         List<uint32_t>::iterator it;
         for (it = mStopReplyIDQueue.begin();
@@ -755,7 +769,6 @@ status_t MediaCodecSource::onStart(MetaData *params) {
                 kWhatPullerNotify, mReflector->id());
         err = mPuller->start(params, notify);
         if (err != OK) {
-            mPullerReachedEOS = true;
             return err;
         }
     }
@@ -774,9 +787,9 @@ void MediaCodecSource::onMessageReceived(const sp<AMessage> &msg) {
         CHECK(msg->findPointer("accessUnit", (void**)&mbuf));
 
         if (mbuf == NULL) {
-            ALOGI("puller (%s) reached EOS",
+            ALOGV("puller (%s) reached EOS",
                     mIsVideo ? "video" : "audio");
-            mPullerReachedEOS = true;
+            signalEOS();
         }
 
         if (mEncoder == NULL) {
@@ -785,9 +798,8 @@ void MediaCodecSource::onMessageReceived(const sp<AMessage> &msg) {
 
             if (mbuf != NULL) {
                 mbuf->release();
-            } else {
-                signalEOS();
             }
+
             break;
         }
 
@@ -833,14 +845,14 @@ void MediaCodecSource::onMessageReceived(const sp<AMessage> &msg) {
     }
     case kWhatStop:
     {
-        ALOGI("MediaCodecSource (%s) stopping", mIsVideo ? "video" : "audio");
+        ALOGI("encoder (%s) stopping", mIsVideo ? "video" : "audio");
 
         uint32_t replyID;
         CHECK(msg->senderAwaitsResponse(&replyID));
 
-        if (reachedEOS()) {
+        if (mEncoderReachedEOS) {
             // if we already reached EOS, reply and return now
-            ALOGI("MediaCodecSource (%s) already stopped",
+            ALOGI("encoder (%s) already stopped",
                     mIsVideo ? "video" : "audio");
             (new AMessage)->postReply(replyID);
             break;
@@ -860,8 +872,6 @@ void MediaCodecSource::onMessageReceived(const sp<AMessage> &msg) {
         if (mFlags & FLAG_USE_SURFACE_INPUT) {
             mEncoder->signalEndOfInputStream();
         } else {
-            CHECK(mPuller != NULL);
-            mPuller->stopAsync();
             signalEOS();
         }
         break;