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 319710 3e8ddc7e657519e979f374c1a97e2aec4c97505d
parent 319709 0c8e6757abd6e3f0194249a6e6d324b70c15dc77
child 319711 2199f815eaa9478cc781bed08291162f20e59bd5
push id20748
push userphilringnalda@gmail.com
push dateFri, 28 Oct 2016 03:39:55 +0000
treeherderfx-team@715360440695 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskaku
bugs1311924
milestone52.0a1
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());