Bug 871753 - Part 1: Fix deadlock during youtube video playback. r=doublec, a=tef+

content/media/omx/OmxDecoder.cpp

@@ -8,6 +8,7 @@
 
 #include "base/basictypes.h"
 #include <cutils/properties.h>
+#include <stagefright/foundation/AMessage.h>
 #include <stagefright/MediaExtractor.h>
 #include <stagefright/MetaData.h>
 #include <stagefright/OMXClient.h>
@@ -57,7 +58,11 @@ VideoGraphicBuffer::Unlock()
 {
   android::sp<android::OmxDecoder> omxDecoder = mOmxDecoder.promote();
   if (omxDecoder.get()) {
-    omxDecoder->ReleaseVideoBuffer(mMediaBuffer);
+    // Post kNotifyPostReleaseVideoBuffer message to OmxDecoder via ALooper.
+    // The message is delivered to OmxDecoder on ALooper thread.
+    // MediaBuffer::release() could take a very long time.
+    // PostReleaseVideoBuffer() prevents long time locking.
+    omxDecoder->PostReleaseVideoBuffer(mMediaBuffer);
   } else {
     NS_WARNING("OmxDecoder is not present");
     if (mMediaBuffer) {
@@ -144,10 +149,24 @@ OmxDecoder::OmxDecoder(MediaResource *aResource,
   mPaused(false),
   mAudioMetadataRead(false)
 {
+  mLooper = new ALooper;
+  mLooper->setName("OmxDecoder");
+
+  mReflector = new AHandlerReflector<OmxDecoder>(this);
+  // Register AMessage handler to ALooper.
+  mLooper->registerHandler(mReflector);
+  // Start ALooper thread.
+  mLooper->start();
 }
 
 OmxDecoder::~OmxDecoder()
 {
+  {
+    // Free all pending video buffers.
+    Mutex::Autolock autoLock(mSeekLock);
+    ReleaseAllPendingVideoBuffersLocked();
+  }
+
   ReleaseVideoBuffer();
   ReleaseAudioBuffer();
 
@@ -158,6 +177,11 @@ OmxDecoder::~OmxDecoder()
   if (mAudioSource.get()) {
     mAudioSource->stop();
   }
+
+  // unregister AMessage handler from ALooper.
+  mLooper->unregisterHandler(mReflector->id());
+  // Stop ALooper thread.
+  mLooper->stop();
 }
 
 class AutoStopMediaSource {
@@ -670,32 +694,6 @@ bool OmxDecoder::ReadAudio(AudioFrame *aFrame, int64_t aSeekTimeUs)
   return true;
 }
 
-bool OmxDecoder::ReleaseVideoBuffer(MediaBuffer *aBuffer)
-{
-  Mutex::Autolock autoLock(mSeekLock);
-
-  if (!aBuffer) {
-    return false;
-  }
-
-  if (mIsVideoSeeking == true) {
-    mPendingVideoBuffers.push(aBuffer);
-  } else {
-    aBuffer->release();
-  }
-  return true;
-}
-
-void OmxDecoder::ReleaseAllPendingVideoBuffersLocked()
-{
-  int size = mPendingVideoBuffers.size();
-  for (int i = 0; i < size; i++) {
-    MediaBuffer *buffer = mPendingVideoBuffers[i];
-    buffer->release();
-  }
-  mPendingVideoBuffers.clear();
-}
-
 nsresult OmxDecoder::Play() {
   if (!mPaused) {
     return NS_OK;
@@ -724,3 +722,52 @@ void OmxDecoder::Pause() {
   }
   mPaused = true;
 }
+
+// Called on ALooper thread.
+void OmxDecoder::onMessageReceived(const sp<AMessage> &msg)
+{
+  Mutex::Autolock autoLock(mSeekLock);
+
+  // Free pending video buffers when OmxDecoder is not seeking video.
+  // If OmxDecoder is in seeking video, the buffers are freed on seek exit. 
+  if (mIsVideoSeeking != true) {
+    ReleaseAllPendingVideoBuffersLocked();
+  }
+}
+
+void OmxDecoder::PostReleaseVideoBuffer(MediaBuffer *aBuffer)
+{
+  {
+    Mutex::Autolock autoLock(mPendingVideoBuffersLock);
+    mPendingVideoBuffers.push(aBuffer);
+  }
+
+  sp<AMessage> notify =
+            new AMessage(kNotifyPostReleaseVideoBuffer, mReflector->id());
+  // post AMessage to OmxDecoder via ALooper.
+  notify->post();
+}
+
+void OmxDecoder::ReleaseAllPendingVideoBuffersLocked()
+{
+  Vector<MediaBuffer *> releasingVideoBuffers;
+  {
+    Mutex::Autolock autoLock(mPendingVideoBuffersLock);
+
+    int size = mPendingVideoBuffers.size();
+    for (int i = 0; i < size; i++) {
+      MediaBuffer *buffer = mPendingVideoBuffers[i];
+      releasingVideoBuffers.push(buffer);
+    }
+    mPendingVideoBuffers.clear();
+  }
+  // Free all pending video buffers without holding mPendingVideoBuffersLock.
+  int size = releasingVideoBuffers.size();
+  for (int i = 0; i < size; i++) {
+    MediaBuffer *buffer;
+    buffer = releasingVideoBuffers[i];
+    buffer->release();
+  }
+  releasingVideoBuffers.clear();
+}
+

content/media/omx/OmxDecoder.h

@@ -1,3 +1,7 @@
+#include <stagefright/foundation/ABase.h>
+#include <stagefright/foundation/AHandlerReflector.h>
+#include <stagefright/foundation/ALooper.h>
+#include <stagefright/MediaSource.h>
 #include <stagefright/DataSource.h>
 #include <stagefright/MediaSource.h>
 #include <utils/RefBase.h>
@@ -77,6 +81,10 @@ class OmxDecoder : public RefBase {
     kHardwareCodecsOnly = 16,
   };
 
+  enum {
+    kNotifyPostReleaseVideoBuffer = 'noti'
+  };
+
   nsBuiltinDecoder *mDecoder;
   MediaResource *mResource;
   sp<GonkNativeWindow> mNativeWindow;
@@ -107,6 +115,9 @@ class OmxDecoder : public RefBase {
   // OMXCodec does not accept MediaBuffer during seeking. If MediaBuffer is
   //  returned to OMXCodec during seeking, OMXCodec calls assert.
   Vector<MediaBuffer *> mPendingVideoBuffers;
+  // The lock protects mPendingVideoBuffers.
+  Mutex mPendingVideoBuffersLock;
+
   // Show if OMXCodec is seeking.
   bool mIsVideoSeeking;
   // The lock protects video MediaBuffer release()'s pending operations called
@@ -115,6 +126,16 @@ class OmxDecoder : public RefBase {
   // Holding time should be minimum.
   Mutex mSeekLock;
 
+  // ALooper is a message loop used in stagefright.
+  // It creates a thread for messages and handles messages in the thread.
+  // ALooper is a clone of Looper in android Java.
+  // http://developer.android.com/reference/android/os/Looper.html
+  sp<ALooper> mLooper;
+  // deliver a message to a wrapped object(OmxDecoder).
+  // AHandlerReflector is similar to Handler in android Java.
+  // http://developer.android.com/reference/android/os/Handler.html
+  sp<AHandlerReflector<OmxDecoder> > mReflector;
+
   // 'true' if a read from the audio stream was done while reading the metadata
   bool mAudioMetadataRead;
 
@@ -173,13 +194,18 @@ public:
     return mResource;
   }
 
-  bool ReleaseVideoBuffer(MediaBuffer *aBuffer);
-
   //Change decoder into a playing state
   nsresult Play();
 
   //Change decoder into a paused state
   void Pause();
+
+  // Post kNotifyPostReleaseVideoBuffer message to OmxDecoder via ALooper.
+  void PostReleaseVideoBuffer(MediaBuffer *aBuffer);
+  // Receive a message from AHandlerReflector.
+  // Called on ALooper thread.
+  void onMessageReceived(const sp<AMessage> &msg);
+
 };
 
 }