Bug 1129376 - Fix crash by moving ConfigureMediaCodec from binder thread to out own thread. r=sotaro
authorBenjamin Chen <bechen@mozilla.com>
Mon, 02 Mar 2015 03:57:46 +0800
changeset 231864 ffce258b2fee84adf3e19fed420f169b10e1db02
parent 231863 fbafae134491cdb44ae84cf1f73f3d327e90b285
child 231865 f222c7e1530824b6ad7436fdf7795f0160b88a49
push idunknown
push userunknown
push dateunknown
reviewerssotaro
bugs1129376
milestone39.0a1
Bug 1129376 - Fix crash by moving ConfigureMediaCodec from binder thread to out own thread. r=sotaro
dom/media/omx/MediaCodecReader.cpp
dom/media/omx/MediaCodecReader.h
--- a/dom/media/omx/MediaCodecReader.cpp
+++ b/dom/media/omx/MediaCodecReader.cpp
@@ -40,21 +40,16 @@
 #include "VideoFrameContainer.h"
 #include "VideoUtils.h"
 
 using namespace android;
 using namespace mozilla::layers;
 
 namespace mozilla {
 
-enum {
-  kNotifyCodecReserved = 'core',
-  kNotifyCodecCanceled = 'coca',
-};
-
 static const int64_t sInvalidDurationUs = INT64_C(-1);
 static const int64_t sInvalidTimestampUs = INT64_C(-1);
 
 // Try not to spend more than this much time (in seconds) in a single call
 // to GetCodecOutputData.
 static const double sMaxAudioDecodeDurationS = 0.1;
 static const double sMaxVideoDecodeDurationS = 0.1;
 
@@ -67,59 +62,40 @@ IsValidDurationUs(int64_t aDuration)
 }
 
 inline bool
 IsValidTimestampUs(int64_t aTimestamp)
 {
   return aTimestamp >= INT64_C(0);
 }
 
-MediaCodecReader::MessageHandler::MessageHandler(MediaCodecReader* aReader)
-  : mReader(aReader)
-{
-}
-
-MediaCodecReader::MessageHandler::~MessageHandler()
-{
-  mReader = nullptr;
-}
-
-void
-MediaCodecReader::MessageHandler::onMessageReceived(
-  const sp<AMessage>& aMessage)
-{
-  if (mReader) {
-    mReader->onMessageReceived(aMessage);
-  }
-}
-
 MediaCodecReader::VideoResourceListener::VideoResourceListener(
   MediaCodecReader* aReader)
   : mReader(aReader)
 {
 }
 
 MediaCodecReader::VideoResourceListener::~VideoResourceListener()
 {
   mReader = nullptr;
 }
 
 void
 MediaCodecReader::VideoResourceListener::codecReserved()
 {
   if (mReader) {
-    mReader->codecReserved(mReader->mVideoTrack);
+    mReader->VideoCodecReserved();
   }
 }
 
 void
 MediaCodecReader::VideoResourceListener::codecCanceled()
 {
   if (mReader) {
-    mReader->codecCanceled(mReader->mVideoTrack);
+    mReader->VideoCodecCanceled();
   }
 }
 
 MediaCodecReader::TrackInputCopier::~TrackInputCopier()
 {
 }
 
 bool
@@ -295,17 +271,16 @@ MediaCodecReader::MediaCodecReader(Abstr
   , mIsWaitingResources(false)
   , mTextureClientIndexesLock("MediaCodecReader::mTextureClientIndexesLock")
   , mColorConverterBufferSize(0)
   , mParserMonitor("MediaCodecReader::mParserMonitor")
   , mParseDataFromCache(true)
   , mNextParserPosition(INT64_C(0))
   , mParsedDataLength(INT64_C(0))
 {
-  mHandler = new MessageHandler(this);
   mVideoListener = new VideoResourceListener(this);
 }
 
 MediaCodecReader::~MediaCodecReader()
 {
 }
 
 nsresult
@@ -694,16 +669,24 @@ MediaCodecReader::ReadMetadata(MediaInfo
   // relies on IsWaitingMediaResources() function. And the waiting state will be
   // changed by binder thread, so we store the waiting state in a cache value to
   // make them in the same waiting state.
   UpdateIsWaitingMediaResources();
   if (IsWaitingMediaResources()) {
     return NS_OK;
   }
 
+  // Configure video codec after the codecReserved.
+  if (mVideoTrack.mSource != nullptr) {
+    if (!ConfigureMediaCodec(mVideoTrack)) {
+      DestroyMediaCodec(mVideoTrack);
+      return NS_ERROR_FAILURE;
+    }
+  }
+
   // TODO: start streaming
 
   if (!UpdateDuration()) {
     return NS_ERROR_FAILURE;
   }
 
   if (!UpdateAudioInfo()) {
     return NS_ERROR_FAILURE;
@@ -1098,18 +1081,18 @@ MediaCodecReader::GetAudioOffloadTrack()
 }
 
 bool
 MediaCodecReader::ReallocateResources()
 {
   if (CreateLooper() &&
       CreateExtractor() &&
       CreateMediaSources() &&
-      CreateMediaCodecs() &&
-      CreateTaskQueues()) {
+      CreateTaskQueues() &&
+      CreateMediaCodecs()) {
     return true;
   }
 
   ReleaseResources();
   return false;
 }
 
 void
@@ -1145,19 +1128,16 @@ MediaCodecReader::CreateLooper()
   if (mLooper != nullptr) {
     return true;
   }
 
   // Create ALooper
   sp<ALooper> looper = new ALooper;
   looper->setName("MediaCodecReader::mLooper");
 
-  // Register AMessage handler to ALooper.
-  looper->registerHandler(mHandler);
-
   // Start ALooper thread.
   if (looper->start() != OK) {
     return false;
   }
 
   mLooper = looper;
 
   return true;
@@ -1165,21 +1145,16 @@ MediaCodecReader::CreateLooper()
 
 void
 MediaCodecReader::DestroyLooper()
 {
   if (mLooper == nullptr) {
     return;
   }
 
-  // Unregister AMessage handler from ALooper.
-  if (mHandler != nullptr) {
-    mLooper->unregisterHandler(mHandler->id());
-  }
-
   // Stop ALooper thread.
   mLooper->stop();
 
   // Clear ALooper
   mLooper = nullptr;
 }
 
 bool
@@ -1299,23 +1274,21 @@ MediaCodecReader::ShutdownTaskQueues()
     mVideoTrack.mReleaseBufferTaskQueue->AwaitShutdownAndIdle();
     mVideoTrack.mReleaseBufferTaskQueue = nullptr;
   }
 }
 
 bool
 MediaCodecReader::CreateTaskQueues()
 {
-  if (mAudioTrack.mSource != nullptr && mAudioTrack.mCodec != nullptr &&
-      !mAudioTrack.mTaskQueue) {
+  if (mAudioTrack.mSource != nullptr && !mAudioTrack.mTaskQueue) {
     mAudioTrack.mTaskQueue = CreateFlushableMediaDecodeTaskQueue();
     NS_ENSURE_TRUE(mAudioTrack.mTaskQueue, false);
   }
-  if (mVideoTrack.mSource != nullptr && mVideoTrack.mCodec != nullptr &&
-      !mVideoTrack.mTaskQueue) {
+  if (mVideoTrack.mSource != nullptr && !mVideoTrack.mTaskQueue) {
     mVideoTrack.mTaskQueue = CreateFlushableMediaDecodeTaskQueue();
     NS_ENSURE_TRUE(mVideoTrack.mTaskQueue, false);
     mVideoTrack.mReleaseBufferTaskQueue = CreateMediaDecodeTaskQueue();
     NS_ENSURE_TRUE(mVideoTrack.mReleaseBufferTaskQueue, false);
   }
 
   return true;
 }
@@ -1938,64 +1911,27 @@ MediaCodecReader::GetColorConverterBuffe
 
 void
 MediaCodecReader::ClearColorConverterBuffer()
 {
   mColorConverterBuffer = nullptr;
   mColorConverterBufferSize = 0;
 }
 
-// Called on MediaCodecReader::mLooper thread.
+// Called on Binder thread.
 void
-MediaCodecReader::onMessageReceived(const sp<AMessage>& aMessage)
+MediaCodecReader::VideoCodecReserved()
 {
-  switch (aMessage->what()) {
-
-    case kNotifyCodecReserved:
-    {
-      // Our decode may have acquired the hardware resource that it needs
-      // to start. Notify the state machine to resume loading metadata.
-      mDecoder->NotifyWaitingForResourcesStatusChanged();
-      break;
-    }
-
-    case kNotifyCodecCanceled:
-    {
-      ReleaseCriticalResources();
-      break;
-    }
-
-    default:
-      TRESPASS();
-      break;
-  }
+  mDecoder->NotifyWaitingForResourcesStatusChanged();
 }
 
 // Called on Binder thread.
 void
-MediaCodecReader::codecReserved(Track& aTrack)
+MediaCodecReader::VideoCodecCanceled()
 {
-  if (!ConfigureMediaCodec(aTrack)) {
-    DestroyMediaCodec(aTrack);
-    return;
-  }
-
-  if (mHandler != nullptr) {
-    // post kNotifyCodecReserved to MediaCodecReader::mLooper thread.
-    sp<AMessage> notify = new AMessage(kNotifyCodecReserved, mHandler->id());
-    notify->post();
-  }
-}
-
-// Called on Binder thread.
-void
-MediaCodecReader::codecCanceled(Track& aTrack)
-{
-  DestroyMediaCodec(aTrack);
-
-  if (mHandler != nullptr) {
-    // post kNotifyCodecCanceled to MediaCodecReader::mLooper thread.
-    sp<AMessage> notify = new AMessage(kNotifyCodecCanceled, mHandler->id());
-    notify->post();
+  if (mVideoTrack.mTaskQueue) {
+    RefPtr<nsIRunnable> task =
+      NS_NewRunnableMethod(this, &MediaCodecReader::ReleaseCriticalResources);
+    mVideoTrack.mTaskQueue->Dispatch(task);
   }
 }
 
 } // namespace mozilla
--- a/dom/media/omx/MediaCodecReader.h
+++ b/dom/media/omx/MediaCodecReader.h
@@ -174,50 +174,31 @@ protected:
   };
 
   // Receive a message from MessageHandler.
   // Called on MediaCodecReader::mLooper thread.
   void onMessageReceived(const android::sp<android::AMessage>& aMessage);
 
   // Receive a notify from ResourceListener.
   // Called on Binder thread.
-  virtual void codecReserved(Track& aTrack);
-  virtual void codecCanceled(Track& aTrack);
+  virtual void VideoCodecReserved();
+  virtual void VideoCodecCanceled();
 
   virtual bool CreateExtractor();
 
   // Check the underlying HW resource is available and store the result in
   // mIsWaitingResources.
   void UpdateIsWaitingMediaResources();
 
   android::sp<android::MediaExtractor> mExtractor;
   // A cache value updated by UpdateIsWaitingMediaResources(), makes the
   // "waiting resources state" is synchronous to StateMachine.
   bool mIsWaitingResources;
 
 private:
-  // An intermediary class that can be managed by android::sp<T>.
-  // Redirect onMessageReceived() to MediaCodecReader.
-  class MessageHandler : public android::AHandler
-  {
-  public:
-    MessageHandler(MediaCodecReader* aReader);
-    ~MessageHandler();
-
-    virtual void onMessageReceived(const android::sp<android::AMessage>& aMessage);
-
-  private:
-    // Forbidden
-    MessageHandler() = delete;
-    MessageHandler(const MessageHandler& rhs) = delete;
-    const MessageHandler& operator=(const MessageHandler& rhs) = delete;
-
-    MediaCodecReader *mReader;
-  };
-  friend class MessageHandler;
 
   // An intermediary class that can be managed by android::sp<T>.
   // Redirect codecReserved() and codecCanceled() to MediaCodecReader.
   class VideoResourceListener : public android::MediaCodecProxy::CodecResourceListener
   {
   public:
     VideoResourceListener(MediaCodecReader* aReader);
     ~VideoResourceListener();
@@ -427,17 +408,16 @@ private:
   static PLDHashOperator ReleaseTextureClient(TextureClient* aClient,
                                               size_t& aIndex,
                                               void* aUserArg);
   PLDHashOperator ReleaseTextureClient(TextureClient* aClient,
                                        size_t& aIndex);
 
   void ReleaseAllTextureClients();
 
-  android::sp<MessageHandler> mHandler;
   android::sp<VideoResourceListener> mVideoListener;
 
   android::sp<android::ALooper> mLooper;
   android::sp<android::MetaData> mMetaData;
 
   Mutex mTextureClientIndexesLock;
   nsDataHashtable<nsPtrHashKey<TextureClient>, size_t> mTextureClientIndexes;