Bug 1050667 - fix the non-synchronous waiting state between MediaDecoderStataMachine and MediaOmxReader and OmxDecoder. r=sotaro

content/media/MediaDecoderStateMachine.cpp

@@ -1419,11 +1419,21 @@ void MediaDecoderStateMachine::StartWaitForResources()
 void MediaDecoderStateMachine::NotifyWaitingForResourcesStatusChanged()
 {
   AssertCurrentThreadInMonitor();
-  if (mState != DECODER_STATE_WAIT_FOR_RESOURCES ||
+  DECODER_LOG("NotifyWaitingForResourcesStatusChanged");
-      mReader->IsWaitingMediaResources()) {
+  RefPtr<nsIRunnable> task(
+    NS_NewRunnableMethod(this,
+      &MediaDecoderStateMachine::DoNotifyWaitingForResourcesStatusChanged));
+  mDecodeTaskQueue->Dispatch(task);
+}
+
+void MediaDecoderStateMachine::DoNotifyWaitingForResourcesStatusChanged()
+{
+  NS_ASSERTION(OnDecodeThread(), "Should be on decode thread.");
+  ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());
+  if (mState != DECODER_STATE_WAIT_FOR_RESOURCES) {
     return;
   }
-  DECODER_LOG("NotifyWaitingForResourcesStatusChanged");
+  DECODER_LOG("DoNotifyWaitingForResourcesStatusChanged");
   // The reader is no longer waiting for resources (say a hardware decoder),
   // we can now proceed to decode metadata.
   SetState(DECODER_STATE_DECODING_NONE);

content/media/MediaDecoderStateMachine.h

@@ -327,9 +327,9 @@ public:
   // be held.
   bool IsPlaying();
 
+  // Dispatch DoNotifyWaitingForResourcesStatusChanged task to mDecodeTaskQueue.
   // Called when the reader may have acquired the hardware resources required
-  // to begin decoding. The state machine may move into DECODING_METADATA if
+  // to begin decoding. The decoder monitor must be held while calling this.
-  // appropriate. The decoder monitor must be held while calling this.
   void NotifyWaitingForResourcesStatusChanged();
 
   // Notifies the state machine that should minimize the number of samples
@@ -636,6 +636,10 @@ protected:
   // and the sink is shutting down.
   void OnAudioSinkComplete();
 
+  // The state machine may move into DECODING_METADATA if we are in
+  // DECODER_STATE_WAIT_FOR_RESOURCES.
+  void DoNotifyWaitingForResourcesStatusChanged();
+
   // The decoder object that created this state machine. The state machine
   // holds a strong reference to the decoder to ensure that the decoder stays
   // alive once media element has started the decoder shutdown process, and has

content/media/omx/MediaOmxReader.cpp

@@ -135,6 +135,7 @@ MediaOmxReader::MediaOmxReader(AbstractMediaDecoder *aDecoder)
   , mSkipCount(0)
   , mUseParserDuration(false)
   , mLastParserDuration(-1)
+  , mIsWaitingResources(false)
 {
 #ifdef PR_LOGGING
   if (!gMediaDecoderLog) {
@@ -172,10 +173,16 @@ void MediaOmxReader::Shutdown()
 
 bool MediaOmxReader::IsWaitingMediaResources()
 {
-  if (!mOmxDecoder.get()) {
+  return mIsWaitingResources;
-    return false;
+}
+
+void MediaOmxReader::UpdateIsWaitingMediaResources()
+{
+  if (mOmxDecoder.get()) {
+    mIsWaitingResources = mOmxDecoder->IsWaitingMediaResources();
+  } else {
+    mIsWaitingResources = false;
   }
-  return mOmxDecoder->IsWaitingMediaResources();
 }
 
 bool MediaOmxReader::IsDormantNeeded()
@@ -245,13 +252,21 @@ nsresult MediaOmxReader::ReadMetadata(MediaInfo* aInfo,
     ProcessCachedData(0, true);
   }
 
-  if (!mOmxDecoder->TryLoad()) {
+  if (!mOmxDecoder->AllocateMediaResources()) {
     return NS_ERROR_FAILURE;
   }
-
+  // Bug 1050667, both MediaDecoderStateMachine and MediaOmxReader
+  // 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;
   }
+  // After resources are available, set the metadata.
+  if (!mOmxDecoder->EnsureMetadata()) {
+    return NS_ERROR_FAILURE;
+  }
 
   if (isMP3 && mMP3FrameParser.IsMP3()) {
     int64_t duration = mMP3FrameParser.GetDuration();

content/media/omx/MediaOmxReader.h

@@ -44,6 +44,11 @@ protected:
   android::sp<android::OmxDecoder> mOmxDecoder;
   android::sp<android::MediaExtractor> mExtractor;
   MP3FrameParser mMP3FrameParser;
+
+  // A cache value updated by UpdateIsWaitingMediaResources(), makes the
+  // "waiting resources state" is synchronous to StateMachine.
+  bool mIsWaitingResources;
+
   // Called by ReadMetadata() during MediaDecoderStateMachine::DecodeMetadata()
   // on decode thread. It create and initialize the OMX decoder including
   // setting up custom extractor. The extractor provide the essential
@@ -54,6 +59,11 @@ protected:
   // to activate the decoder automatically.
   virtual void EnsureActive();
 
+  // Check the underlying HW resources are available and store the result in
+  // mIsWaitingResources. The result might be changed by binder thread,
+  // Can only called by ReadMetadata.
+  void UpdateIsWaitingMediaResources();
+
 public:
   MediaOmxReader(AbstractMediaDecoder* aDecoder);
   ~MediaOmxReader();
@@ -76,7 +86,8 @@ public:
     return mHasVideo;
   }
 
-  virtual bool IsWaitingMediaResources();
+  // Return mIsWaitingResources.
+  virtual bool IsWaitingMediaResources() MOZ_OVERRIDE;
 
   virtual bool IsDormantNeeded();
   virtual void ReleaseMediaResources();

content/media/omx/OmxDecoder.cpp

@@ -162,19 +162,7 @@ bool OmxDecoder::Init(sp<MediaExtractor>& extractor) {
   return true;
 }
 
-bool OmxDecoder::TryLoad() {
+bool OmxDecoder::EnsureMetadata() {
-
-  if (!AllocateMediaResources()) {
-    return false;
-  }
-
-  //check if video is waiting resources
-  if (mVideoSource.get()) {
-    if (mVideoSource->IsWaitingResources()) {
-      return true;
-    }
-  }
-
   // calculate duration
   int64_t totalDurationUs = 0;
   int64_t durationUs = 0;

content/media/omx/OmxDecoder.h

@@ -152,9 +152,16 @@ public:
   // Note: RTSP requires a custom extractor because it doesn't have a container.
   bool Init(sp<MediaExtractor>& extractor);
 
-  bool TryLoad();
   bool IsDormantNeeded();
+
+  // Called after resources(video/audio codec) are allocated, set the
+  // mDurationUs and video/audio metadata.
+  bool EnsureMetadata();
+
+  // Only called by MediaOmxDecoder, do not call this function arbitrarily.
+  // See bug 1050667.
   bool IsWaitingMediaResources();
+
   bool AllocateMediaResources();
   void ReleaseMediaResources();
   bool SetVideoFormat();