Bug 1311924 - Handle play state changes in state objects of MDSM. r=kaku
authorJW Wang <jwwang@mozilla.com>
Fri, 21 Oct 2016 14:53:18 +0800
changeset 319709 3e8ddc7e657519e979f374c1a97e2aec4c97505d
parent 319708 0c8e6757abd6e3f0194249a6e6d324b70c15dc77
child 319710 2199f815eaa9478cc781bed08291162f20e59bd5
push id30877
push userarchaeopteryx@coole-files.de
push dateThu, 27 Oct 2016 22:28:26 +0000
treeherdermozilla-central@6d9707e3343f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskaku
bugs1311924
milestone52.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1311924 - Handle play state changes in state objects of MDSM. r=kaku Note we remove the comments that are no longer valid and we call ScheduleStateMachine() only for DecodingState and CompletedState. 1. DecodingFirstFrameState doesn't implement Step(). It doesn't make sense to schedule next cycles. 2. BufferingState doesn't care about the play state since it will never start playback. MozReview-Commit-ID: 6KRxkzsi5WX
dom/media/MediaDecoderStateMachine.cpp
--- a/dom/media/MediaDecoderStateMachine.cpp
+++ b/dom/media/MediaDecoderStateMachine.cpp
@@ -214,16 +214,18 @@ public:
   virtual bool HandleAudioCaptured() { return false; }
 
   virtual RefPtr<ShutdownPromise> HandleShutdown();
 
   virtual void HandleVideoSuspendTimeout() = 0;
 
   virtual void HandleResumeVideoDecoding();
 
+  virtual void HandlePlayStateChanged(MediaDecoder::PlayState aPlayState) {}
+
   virtual void DumpDebugInfo() {}
 
 protected:
   using Master = MediaDecoderStateMachine;
   explicit StateObject(Master* aPtr) : mMaster(aPtr) {}
   TaskQueue* OwnerThread() const { return mMaster->mTaskQueue; }
   MediaResource* Resource() const { return mMaster->mResource; }
   MediaDecoderReaderWrapper* Reader() const { return mMaster->mReader; }
@@ -625,16 +627,24 @@ public:
   {
     if (mMaster->HasVideo()) {
       mMaster->mVideoDecodeSuspended = true;
       mMaster->mOnPlaybackEvent.Notify(MediaEventType::EnterVideoSuspend);
       Reader()->SetVideoBlankDecode(true);
     }
   }
 
+  void HandlePlayStateChanged(MediaDecoder::PlayState aPlayState) override
+  {
+    if (aPlayState == MediaDecoder::PLAY_STATE_PLAYING) {
+      // Schedule Step() to check if we can start playback.
+      mMaster->ScheduleStateMachine();
+    }
+  }
+
   void DumpDebugInfo() override
   {
     SDUMP("mIsPrerolling=%d", mIsPrerolling);
   }
 
 private:
   void MaybeStartBuffering();
 
@@ -1046,16 +1056,24 @@ public:
     return true;
   }
 
   void HandleVideoSuspendTimeout() override
   {
     // Do nothing since no decoding is going on.
   }
 
+  void HandlePlayStateChanged(MediaDecoder::PlayState aPlayState) override
+  {
+    if (aPlayState == MediaDecoder::PLAY_STATE_PLAYING) {
+      // Schedule Step() to check if we can start playback.
+      mMaster->ScheduleStateMachine();
+    }
+  }
+
 private:
   bool mSentPlaybackEndedEvent = false;
 };
 
 /**
  * Purpose: release all resources allocated by MDSM.
  *
  * Transition to:
@@ -2452,44 +2470,25 @@ MediaDecoderStateMachine::Shutdown()
 }
 
 void MediaDecoderStateMachine::PlayStateChanged()
 {
   MOZ_ASSERT(OnTaskQueue());
 
   if (mPlayState != MediaDecoder::PLAY_STATE_PLAYING) {
     mVideoDecodeSuspendTimer.Reset();
-    return;
-  }
-
-  // Once we start playing, we don't want to minimize our prerolling, as we
-  // assume the user is likely to want to keep playing in future. This needs to
-  // happen before we invoke StartDecoding().
-  if (mMinimizePreroll) {
+  } else if (mMinimizePreroll) {
+    // Once we start playing, we don't want to minimize our prerolling, as we
+    // assume the user is likely to want to keep playing in future. This needs to
+    // happen before we invoke StartDecoding().
     mMinimizePreroll = false;
     DispatchDecodeTasksIfNeeded();
   }
 
-  // Some state transitions still happen synchronously on the main thread. So
-  // if the main thread invokes Play() and then Seek(), the seek will initiate
-  // synchronously on the main thread, and the asynchronous PlayInternal task
-  // will arrive when it's no longer valid. The proper thing to do is to move
-  // all state transitions to the state machine task queue, but for now we just
-  // make sure that none of the possible main-thread state transitions (Seek(),
-  // SetDormant(), and Shutdown()) have not occurred.
-  if (mState != DECODER_STATE_DECODING &&
-      mState != DECODER_STATE_DECODING_FIRSTFRAME &&
-      mState != DECODER_STATE_BUFFERING &&
-      mState != DECODER_STATE_COMPLETED)
-  {
-    DECODER_LOG("Unexpected state - Bailing out of PlayInternal()");
-    return;
-  }
-
-  ScheduleStateMachine();
+  mStateObj->HandlePlayStateChanged(mPlayState);
 }
 
 void MediaDecoderStateMachine::VisibilityChanged()
 {
   MOZ_ASSERT(OnTaskQueue());
   DECODER_LOG("VisibilityChanged: mIsVisible=%d, "
               "mVideoDecodeSuspended=%c, mIsReaderSuspended=%d",
               mIsVisible.Ref(), mVideoDecodeSuspended ? 'T' : 'F', mIsReaderSuspended.Ref());