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 204105 f92e54a91038697d7c29a109d9bd3deaeef89528
parent 204104 92038b8491c2fdf48e239b827f2ef5ae482860d8
child 204106 2da5a884c70b9ced113d3c45bf2cabd7c8db5eb7
push id424
push userryanvm@gmail.com
push dateTue, 21 Oct 2014 20:34:58 +0000
treeherdermozilla-b2g32_v2_0@f92e54a91038 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssotaro, bajaj
bugs1050667
milestone32.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
@@ -53,16 +53,19 @@ public:
   // than aTimeThreshold will be decoded (unless they're not keyframes
   // and aKeyframeSkip is true), but will not be added to the queue.
   virtual bool DecodeVideoFrame(bool &aKeyframeSkip,
                                 int64_t aTimeThreshold) = 0;
 
   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;
 
   // Stores the presentation time of the first frame we'd be able to play if
--- a/content/media/MediaDecoderStateMachine.cpp
+++ b/content/media/MediaDecoderStateMachine.cpp
@@ -1351,20 +1351,32 @@ void MediaDecoderStateMachine::StartWait
                "Should be on state machine or decode thread.");
   AssertCurrentThreadInMonitor();
   mState = DECODER_STATE_WAIT_FOR_RESOURCES;
 }
 
 void MediaDecoderStateMachine::NotifyWaitingForResourcesStatusChanged()
 {
   AssertCurrentThreadInMonitor();
-  if (mState != DECODER_STATE_WAIT_FOR_RESOURCES ||
-      mReader->IsWaitingMediaResources()) {
+  DECODER_LOG(PR_LOG_DEBUG,"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(PR_LOG_DEBUG, "DoNotifyWaitingForResourcesStatusChanged");
+
   // The reader is no longer waiting for resources (say a hardware decoder),
   // we can now proceed to decode metadata.
   mState = DECODER_STATE_DECODING_METADATA;
   EnqueueDecodeMetadataTask();
 }
 
 void MediaDecoderStateMachine::Play()
 {
@@ -1794,16 +1806,18 @@ MediaDecoderStateMachine::CallDecodeMeta
   }
 }
 
 nsresult MediaDecoderStateMachine::DecodeMetadata()
 {
   AssertCurrentThreadInMonitor();
   NS_ASSERTION(OnDecodeThread(), "Should be on decode thread.");
   DECODER_LOG(PR_LOG_DEBUG, "Decoding Media Headers");
+  mReader->PreReadMetadata();
+
   if (mState != DECODER_STATE_DECODING_METADATA) {
     return NS_ERROR_FAILURE;
   }
 
   nsresult res;
   MediaInfo info;
   MetadataTags* tags;
   {
--- a/content/media/MediaDecoderStateMachine.h
+++ b/content/media/MediaDecoderStateMachine.h
@@ -340,19 +340,19 @@ public:
   bool IsShutdown();
 
   void QueueMetadata(int64_t aPublishTime, int aChannels, int aRate, bool aHasAudio, bool aHasVideo, 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();
@@ -632,16 +632,20 @@ protected:
     return !mTimeout.IsNull();
   }
 
   // Returns true if we're not playing and the decode thread has filled its
   // decode buffers and is waiting. We can shut the decode thread down in this
   // case as it may not be needed again.
   bool IsPausedAndDecoderWaiting();
 
+  // 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
@@ -42,16 +42,17 @@ extern PRLogModuleInfo* gMediaDecoderLog
 
 MediaOmxReader::MediaOmxReader(AbstractMediaDecoder *aDecoder)
   : MediaDecoderReader(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();
@@ -66,20 +67,26 @@ MediaOmxReader::~MediaOmxReader()
 
 nsresult MediaOmxReader::Init(MediaDecoderReader* aCloneDonor)
 {
   return NS_OK;
 }
 
 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();
@@ -123,41 +130,54 @@ 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();
 #ifdef MOZ_AUDIO_OFFLOAD
   CheckAudioOffload();
 #endif
 
   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
@@ -38,26 +38,36 @@ class MediaOmxReader : public MediaDecod
   int32_t mSkipCount;
   dom::AudioChannel mAudioChannel;
   android::sp<android::MediaSource> mAudioOffloadTrack;
 
 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);
 
@@ -70,23 +80,25 @@ 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 ReleaseDecoder() MOZ_OVERRIDE;
 
+  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 void SetIdle() MOZ_OVERRIDE;
 
   void SetAudioChannel(dom::AudioChannel aAudioChannel) {
     mAudioChannel = aAudioChannel;
--- a/content/media/omx/OmxDecoder.cpp
+++ b/content/media/omx/OmxDecoder.cpp
@@ -367,29 +367,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
@@ -180,19 +180,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);