Bug 1050667 - Fix the non-synchronous waiting state between MediaDecoderStataMachine and MediaOmxReader and OmxDecoder. r=sotaro, a=bajaj
authorBenjamin Chen <bechen@mozilla.com>
Mon, 06 Oct 2014 11:03:14 +0800
changeset 220736 70cd37b5bcc329de7f958d8cc88e4076e703a5be
parent 220735 73aadf193e9495d710d4285f991c4e89f2cf0761
child 220737 714fffd056da87fec868ef6b1758f203c0155cd0
push id51
push userryanvm@gmail.com
push dateTue, 21 Oct 2014 20:35:44 +0000
treeherdermozilla-b2g34_v2_1@70cd37b5bcc3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssotaro, bajaj
bugs1050667
milestone34.0
Bug 1050667 - Fix the non-synchronous waiting state between MediaDecoderStataMachine and MediaOmxReader and OmxDecoder. r=sotaro, a=bajaj
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
@@ -1414,21 +1414,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,23 @@ 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;
   {
     ReentrantMonitorAutoExit exitMon(mDecoder->GetReentrantMonitor());
     res = mReader->ReadMetadata(&info, getter_Transfers(mMetadataTags));
   }
 
   if (NS_SUCCEEDED(res)) {
--- a/content/media/MediaDecoderStateMachine.h
+++ b/content/media/MediaDecoderStateMachine.h
@@ -327,19 +327,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();
@@ -636,16 +636,20 @@ protected:
 
   // Update mDecoder's playback offset.
   void OnPlaybackOffsetUpdate(int64_t aPlaybackOffset);
 
   // Called by the AudioSink to signal that all outstanding work is complete
   // 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
   // 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
@@ -37,16 +37,17 @@ extern PRLogModuleInfo* gMediaDecoderLog
 
 MediaOmxReader::MediaOmxReader(AbstractMediaDecoder *aDecoder)
   : MediaOmxCommonReader(aDecoder)
   , mHasVideo(false)
   , mHasAudio(false)
   , mVideoSeekTimeUs(-1)
   , mAudioSeekTimeUs(-1)
   , mSkipCount(0)
+  , mIsWaitingResources(false)
 {
 #ifdef PR_LOGGING
   if (!gMediaDecoderLog) {
     gMediaDecoderLog = PR_NewLogModule("MediaDecoder");
   }
 #endif
 
   mAudioChannel = dom::AudioChannelService::GetDefaultAudioChannel();
@@ -74,20 +75,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();
@@ -124,37 +131,50 @@ 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;
 
   // Initialize the internal OMX Decoder.
   nsresult rv = InitOmxDecoder();
   if (NS_FAILED(rv)) {
     return rv;
   }
 
-  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;
+  }
 
   // Set the total duration (the max of the audio and video track).
   int64_t durationUs;
   mOmxDecoder->GetDuration(&durationUs);
   if (durationUs) {
     ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());
     mDecoder->SetMediaDuration(durationUs);
   }
--- a/content/media/omx/MediaOmxReader.h
+++ b/content/media/omx/MediaOmxReader.h
@@ -36,26 +36,36 @@ class MediaOmxReader : public MediaOmxCo
   int64_t mVideoSeekTimeUs;
   int64_t mAudioSeekTimeUs;
   int32_t mSkipCount;
 
 protected:
   android::sp<android::OmxDecoder> mOmxDecoder;
   android::sp<android::MediaExtractor> mExtractor;
 
+
+  // 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);
 
@@ -68,21 +78,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
@@ -312,29 +312,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();
 
   bool NotifyDataArrived(const char* aBuffer, uint32_t aLength, int64_t aOffset);