Bug 1050667 - fix the non-synchronous waiting state between MediaDecoderStataMachine and MediaOmxReader and OmxDecoder. r=sotaro
authorBenjamin Chen <bechen@mozilla.com>
Mon, 06 Oct 2014 11:03:14 +0800
changeset 208933 539cf3404ad2985f2d86feee0638766232ec387a
parent 208932 f36f3a13dd61321be9cd4338ac281c70877c91ae
child 208934 c2c6ea9cc42854b4e539a04c6cead7414edf4986
child 208994 e2a94b3f6394dd48cdb2e89355ac34116b9e839d
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewerssotaro
bugs1050667
milestone35.0a1
Bug 1050667 - fix the non-synchronous waiting state between MediaDecoderStataMachine and MediaOmxReader and OmxDecoder. r=sotaro
content/media/MediaDecoderReader.h
content/media/MediaDecoderStateMachine.cpp
content/media/MediaDecoderStateMachine.h
content/media/omx/MediaOmxReader.cpp
content/media/omx/MediaOmxReader.h
content/media/omx/OmxDecoder.cpp
content/media/omx/OmxDecoder.h
--- a/content/media/MediaDecoderReader.h
+++ b/content/media/MediaDecoderReader.h
@@ -83,16 +83,19 @@ public:
   // If aSkipToKeyframe is true, the decode should skip ahead to the
   // the next keyframe at or after aTimeThreshold microseconds.
   virtual void RequestVideoData(bool aSkipToNextKeyframe,
                                 int64_t aTimeThreshold);
 
   virtual bool HasAudio() = 0;
   virtual bool HasVideo() = 0;
 
+  // A function that is called before ReadMetadata() call.
+  virtual void PreReadMetadata() {};
+
   // Read header data for all bitstreams in the file. Fills aInfo with
   // the data required to present the media, and optionally fills *aTags
   // with tag metadata from the file.
   // Returns NS_OK on success, or NS_ERROR_FAILURE on failure.
   virtual nsresult ReadMetadata(MediaInfo* aInfo,
                                 MetadataTags** aTags) = 0;
 
   // Moves the decode head to aTime microseconds. aStartTime and aEndTime
--- a/content/media/MediaDecoderStateMachine.cpp
+++ b/content/media/MediaDecoderStateMachine.cpp
@@ -1416,21 +1416,31 @@ void MediaDecoderStateMachine::StartWait
   AssertCurrentThreadInMonitor();
   SetState(DECODER_STATE_WAIT_FOR_RESOURCES);
   DECODER_LOG("StartWaitForResources");
 }
 
 void MediaDecoderStateMachine::NotifyWaitingForResourcesStatusChanged()
 {
   AssertCurrentThreadInMonitor();
-  if (mState != DECODER_STATE_WAIT_FOR_RESOURCES ||
-      mReader->IsWaitingMediaResources()) {
+  DECODER_LOG("NotifyWaitingForResourcesStatusChanged");
+  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);
   ScheduleStateMachine();
 }
 
 void MediaDecoderStateMachine::Play()
 {
@@ -1905,16 +1915,18 @@ MediaDecoderStateMachine::CallDecodeMeta
 
 nsresult MediaDecoderStateMachine::DecodeMetadata()
 {
   AssertCurrentThreadInMonitor();
   NS_ASSERTION(OnDecodeThread(), "Should be on decode thread.");
   MOZ_ASSERT(mState == DECODER_STATE_DECODING_METADATA);
   DECODER_LOG("Decoding Media Headers");
 
+  mReader->PreReadMetadata();
+
   if (mReader->IsWaitingMediaResources()) {
     StartWaitForResources();
     return NS_OK;
   }
 
   nsresult res;
   MediaInfo info;
   {
--- a/content/media/MediaDecoderStateMachine.h
+++ b/content/media/MediaDecoderStateMachine.h
@@ -322,19 +322,19 @@ public:
   bool IsShutdown();
 
   void QueueMetadata(int64_t aPublishTime, MediaInfo* aInfo, MetadataTags* aTags);
 
   // Returns true if we're currently playing. The decoder monitor must
   // 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
-  // appropriate. The decoder monitor must be held while calling this.
+  // to begin decoding. The decoder monitor must be held while calling this.
   void NotifyWaitingForResourcesStatusChanged();
 
   // Notifies the state machine that should minimize the number of samples
   // decoded we preroll, until playback starts. The first time playback starts
   // the state machine is free to return to prerolling normally. Note
   // "prerolling" in this context refers to when we decode and buffer decoded
   // samples in advance of when they're needed for playback.
   void SetMinimizePrerollUntilPlaybackStarts();
@@ -634,16 +634,20 @@ protected:
 
   // Called by the AudioSink to signal that all outstanding work is complete
   // and the sink is shutting down.
   void OnAudioSinkComplete();
 
   // Called by the AudioSink to signal errors.
   void OnAudioSinkError();
 
+  // 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
   // dropped its reference to the decoder. This enables the state machine to
   // keep using the decoder's monitor until the state machine has finished
   // shutting down, without fear of the monitor being destroyed. After
   // shutting down, the state machine will then release this reference,
   // causing the decoder to be destroyed. This is accessed on the decode,
--- a/content/media/omx/MediaOmxReader.cpp
+++ b/content/media/omx/MediaOmxReader.cpp
@@ -142,16 +142,17 @@ MediaOmxReader::MediaOmxReader(AbstractM
   , mHasVideo(false)
   , mHasAudio(false)
   , mVideoSeekTimeUs(-1)
   , mAudioSeekTimeUs(-1)
   , mSkipCount(0)
   , mUseParserDuration(false)
   , mLastParserDuration(-1)
   , mIsShutdown(false)
+  , mIsWaitingResources(false)
 {
 #ifdef PR_LOGGING
   if (!gMediaDecoderLog) {
     gMediaDecoderLog = PR_NewLogModule("MediaDecoder");
   }
 #endif
 
   mAudioChannel = dom::AudioChannelService::GetDefaultAudioChannel();
@@ -183,20 +184,26 @@ void MediaOmxReader::Shutdown()
   ReleaseMediaResources();
   nsCOMPtr<nsIRunnable> event =
     NS_NewRunnableMethod(this, &MediaOmxReader::ReleaseDecoder);
   NS_DispatchToMainThread(event);
 }
 
 bool MediaOmxReader::IsWaitingMediaResources()
 {
-  if (!mOmxDecoder.get()) {
-    return false;
+  return mIsWaitingResources;
+}
+
+void MediaOmxReader::UpdateIsWaitingMediaResources()
+{
+  if (mOmxDecoder.get()) {
+    mIsWaitingResources = mOmxDecoder->IsWaitingMediaResources();
+  } else {
+    mIsWaitingResources = false;
   }
-  return mOmxDecoder->IsWaitingMediaResources();
 }
 
 bool MediaOmxReader::IsDormantNeeded()
 {
   if (!mOmxDecoder.get()) {
     return false;
   }
   return mOmxDecoder->IsDormantNeeded();
@@ -233,16 +240,21 @@ nsresult MediaOmxReader::InitOmxDecoder(
     mOmxDecoder = new OmxDecoder(mDecoder->GetResource(), mDecoder);
     if (!mOmxDecoder->Init(mExtractor)) {
       return NS_ERROR_FAILURE;
     }
   }
   return NS_OK;
 }
 
+void MediaOmxReader::PreReadMetadata()
+{
+  UpdateIsWaitingMediaResources();
+}
+
 nsresult MediaOmxReader::ReadMetadata(MediaInfo* aInfo,
                                       MetadataTags** aTags)
 {
   NS_ASSERTION(mDecoder->OnDecodeThread(), "Should be on decode thread.");
   EnsureActive();
 
   *aTags = nullptr;
 
@@ -256,23 +268,31 @@ nsresult MediaOmxReader::ReadMetadata(Me
   if (isMP3) {
     // When read sdcard's file on b2g platform at constructor,
     // the mDecoder->GetResource()->GetLength() would return -1.
     // Delay set the total duration on this function.
     mMP3FrameParser.SetLength(mDecoder->GetResource()->GetLength());
     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 consistent 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();
     // The MP3FrameParser may reported a duration;
     // return -1 if no frame has been parsed.
     if (duration >= 0) {
       ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());
       mUseParserDuration = true;
--- a/content/media/omx/MediaOmxReader.h
+++ b/content/media/omx/MediaOmxReader.h
@@ -42,26 +42,36 @@ class MediaOmxReader : public MediaOmxCo
   int64_t mLastParserDuration;
   int32_t mSkipCount;
   bool mUseParserDuration;
   bool mIsShutdown;
 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
   // information used for creating OMX decoder such as video/audio codec.
   virtual nsresult InitOmxDecoder();
 
   // Called inside DecodeVideoFrame, DecodeAudioData, ReadMetadata and Seek
   // 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();
 
   virtual nsresult Init(MediaDecoderReader* aCloneDonor);
 
   virtual void NotifyDataArrived(const char* aBuffer, uint32_t aLength, int64_t aOffset);
 
@@ -74,21 +84,23 @@ public:
     return mHasAudio;
   }
 
   virtual bool HasVideo()
   {
     return mHasVideo;
   }
 
-  virtual bool IsWaitingMediaResources();
+  // Return mIsWaitingResources.
+  virtual bool IsWaitingMediaResources() MOZ_OVERRIDE;
 
   virtual bool IsDormantNeeded();
   virtual void ReleaseMediaResources();
 
+  virtual void PreReadMetadata() MOZ_OVERRIDE;
   virtual nsresult ReadMetadata(MediaInfo* aInfo,
                                 MetadataTags** aTags);
   virtual nsresult Seek(int64_t aTime, int64_t aStartTime, int64_t aEndTime, int64_t aCurrentTime);
 
   virtual bool IsMediaSeekable() MOZ_OVERRIDE;
 
   virtual void SetIdle() MOZ_OVERRIDE;
 
--- a/content/media/omx/OmxDecoder.cpp
+++ b/content/media/omx/OmxDecoder.cpp
@@ -157,29 +157,17 @@ bool OmxDecoder::Init(sp<MediaExtractor>
     // mAudioTrack is be used by OMXCodec. For offloaded audio track, using same
     // object gives undetermined behavior. So get a new track
     mAudioOffloadTrack = extractor->getTrack(audioTrackIndex);
 #endif
   }
   return true;
 }
 
-bool OmxDecoder::TryLoad() {
-
-  if (!AllocateMediaResources()) {
-    return false;
-  }
-
-  //check if video is waiting resources
-  if (mVideoSource.get()) {
-    if (mVideoSource->IsWaitingResources()) {
-      return true;
-    }
-  }
-
+bool OmxDecoder::EnsureMetadata() {
   // calculate duration
   int64_t totalDurationUs = 0;
   int64_t durationUs = 0;
   if (mVideoTrack.get() && mVideoTrack->getFormat()->findInt64(kKeyDuration, &durationUs)) {
     if (durationUs > totalDurationUs)
       totalDurationUs = durationUs;
   }
   if (mAudioTrack.get()) {
--- a/content/media/omx/OmxDecoder.h
+++ b/content/media/omx/OmxDecoder.h
@@ -147,19 +147,26 @@ public:
   // MediaExtractor::getTrackMetaData().
   // In general cases, the extractor is created by a sp<DataSource> which
   // connect to a MediaResource like ChannelMediaResource.
   // Data is read from the MediaResource to create a suitable extractor which
   // extracts data from a container.
   // 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();
   bool SetAudioFormat();
 
   void ReleaseDecoder();
 
   void GetDuration(int64_t *durationUs) {