Bug 1311924 - Handle play state changes in state objects of MDSM. draft
authorJW Wang <jwwang@mozilla.com>
Fri, 21 Oct 2016 14:53:18 +0800
changeset 430062 538cfd840a1fdf0e5be28173e93683d48f00734a
parent 429594 f9f3cc95d7282f1fd83f66dd74acbcdbfe821915
child 535112 b0cc1a9ef67cf6ce7ed0377f437435fa66cc3bb2
push id33724
push userjwwang@mozilla.com
push dateThu, 27 Oct 2016 02:54:05 +0000
bugs1311924
milestone52.0a1
Bug 1311924 - Handle play state changes in state objects of MDSM. 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());