Bug 1158448 Part 1 - Replace MDSM::Play with a state-watcher on mPlayState and eliminate ApplyStateToStateMachine. r=sotaro
authorSotaro Ikeda <sikeda@mozilla.com>
Thu, 07 May 2015 08:19:33 -0700
changeset 242766 5edde270844458f6d0d2debe25cb075878fc1afd
parent 242765 34fae3ffbf628e83b0ed1a5fe94035b9b9d8d73c
child 242767 d55f825951b42409710cf2350e55623c6bcd0c86
push id59497
push usersikeda@mozilla.com
push dateThu, 07 May 2015 15:19:56 +0000
treeherdermozilla-inbound@d55f825951b4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssotaro
bugs1158448
milestone40.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 1158448 Part 1 - Replace MDSM::Play with a state-watcher on mPlayState and eliminate ApplyStateToStateMachine. r=sotaro
dom/media/MediaDecoder.cpp
dom/media/MediaDecoder.h
dom/media/MediaDecoderStateMachine.cpp
dom/media/MediaDecoderStateMachine.h
--- a/dom/media/MediaDecoder.cpp
+++ b/dom/media/MediaDecoder.cpp
@@ -1297,52 +1297,36 @@ void MediaDecoder::ChangeState(PlayState
   mPlayState = aState;
 
   if (mPlayState == PLAY_STATE_PLAYING) {
     ConstructMediaTracks();
   } else if (IsEnded()) {
     RemoveMediaTracks();
   }
 
-  ApplyStateToStateMachine(mPlayState);
+  // XXXbholley - We should refactor things and move this to the code that
+  // actually initiates the seek.
+  if (mPlayState == PLAY_STATE_SEEKING) {
+    mSeekRequest.Begin(ProxyMediaCall(mDecoderStateMachine->TaskQueue(),
+                                      mDecoderStateMachine.get(), __func__,
+                                      &MediaDecoderStateMachine::Seek, mRequestedSeekTarget)
+      ->RefableThen(AbstractThread::MainThread(), __func__, this,
+                    &MediaDecoder::OnSeekResolved, &MediaDecoder::OnSeekRejected));
+    mRequestedSeekTarget.Reset();
+  }
+
+  ScheduleStateMachineThread();
 
   CancelDormantTimer();
   // Start dormant timer if necessary
   StartDormantTimer();
 
   GetReentrantMonitor().NotifyAll();
 }
 
-void MediaDecoder::ApplyStateToStateMachine(PlayState aState)
-{
-  MOZ_ASSERT(NS_IsMainThread());
-  GetReentrantMonitor().AssertCurrentThreadIn();
-
-  if (mDecoderStateMachine) {
-    switch (aState) {
-      case PLAY_STATE_PLAYING:
-        mDecoderStateMachine->Play();
-        break;
-      case PLAY_STATE_SEEKING:
-        mSeekRequest.Begin(ProxyMediaCall(mDecoderStateMachine->TaskQueue(),
-                                          mDecoderStateMachine.get(), __func__,
-                                          &MediaDecoderStateMachine::Seek, mRequestedSeekTarget)
-          ->RefableThen(AbstractThread::MainThread(), __func__, this,
-                        &MediaDecoder::OnSeekResolved, &MediaDecoder::OnSeekRejected));
-        mRequestedSeekTarget.Reset();
-        break;
-      default:
-        // The state machine checks for things like PAUSED in RunStateMachine.
-        // Make sure to keep it in the loop.
-        ScheduleStateMachineThread();
-        break;
-    }
-  }
-}
-
 void MediaDecoder::PlaybackPositionChanged(MediaDecoderEventVisibility aEventVisibility)
 {
   MOZ_ASSERT(NS_IsMainThread());
   if (mShuttingDown)
     return;
 
   double lastTime = mCurrentTime;
 
--- a/dom/media/MediaDecoder.h
+++ b/dom/media/MediaDecoder.h
@@ -763,20 +763,16 @@ public:
    * thread.
    ******/
 
   // Change to a new play state. This updates the mState variable and
   // notifies any thread blocking on this object's monitor of the
   // change. Call on the main thread only.
   virtual void ChangeState(PlayState aState);
 
-  // Called by |ChangeState|, to update the state machine.
-  // Call on the main thread only and the lock must be obtained.
-  virtual void ApplyStateToStateMachine(PlayState aState);
-
   // May be called by the reader to notify this decoder that the metadata from
   // the media file has been read. Call on the decode thread only.
   void OnReadMetadataCompleted() override { }
 
   // Called when the metadata from the media file has been loaded by the
   // state machine. Call on the main thread only.
   virtual void MetadataLoaded(nsAutoPtr<MediaInfo> aInfo,
                               nsAutoPtr<MetadataTags> aTags,
--- a/dom/media/MediaDecoderStateMachine.cpp
+++ b/dom/media/MediaDecoderStateMachine.cpp
@@ -313,17 +313,17 @@ MediaDecoderStateMachine::Initialization
   mPreservesPitch.Connect(mDecoder->CanonicalPreservesPitch());
 
   // Initialize watchers.
   mWatchManager.Watch(mState, &MediaDecoderStateMachine::UpdateNextFrameStatus);
   mWatchManager.Watch(mAudioCompleted, &MediaDecoderStateMachine::UpdateNextFrameStatus);
   mWatchManager.Watch(mVolume, &MediaDecoderStateMachine::VolumeChanged);
   mWatchManager.Watch(mLogicalPlaybackRate, &MediaDecoderStateMachine::LogicalPlaybackRateChanged);
   mWatchManager.Watch(mPreservesPitch, &MediaDecoderStateMachine::PreservesPitchChanged);
-
+  mWatchManager.Watch(mPlayState, &MediaDecoderStateMachine::PlayStateChanged);
 }
 
 bool MediaDecoderStateMachine::HasFutureAudio() {
   MOZ_ASSERT(OnTaskQueue());
   AssertCurrentThreadInMonitor();
   NS_ASSERTION(HasAudio(), "Should only call HasFutureAudio() when we have audio");
   // We've got audio ready to play if:
   // 1. We've not completed playback of audio, and
@@ -1622,21 +1622,28 @@ void MediaDecoderStateMachine::NotifyWai
     ScheduleStateMachine();
   } else if (mState == DECODER_STATE_WAIT_FOR_CDM &&
              !mReader->IsWaitingOnCDMResource()) {
     SetState(DECODER_STATE_DECODING_FIRSTFRAME);
     EnqueueDecodeFirstFrameTask();
   }
 }
 
-void MediaDecoderStateMachine::PlayInternal()
+void MediaDecoderStateMachine::PlayStateChanged()
 {
   MOZ_ASSERT(OnTaskQueue());
   ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());
 
+  // This method used to be a Play() method invoked by MediaDecoder when the
+  // play state became PLAY_STATE_PLAYING. As such, it doesn't have any work to
+  // do for other state changes. That could change.
+  if (mPlayState != MediaDecoder::PLAY_STATE_PLAYING) {
+    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) {
     mMinimizePreroll = false;
     DispatchDecodeTasksIfNeeded();
   }
 
--- a/dom/media/MediaDecoderStateMachine.h
+++ b/dom/media/MediaDecoderStateMachine.h
@@ -221,32 +221,16 @@ public:
   // Can result in a durationchangeevent. aDuration is in microseconds.
   void UpdateEstimatedDuration(int64_t aDuration);
 
   // Functions used by assertions to ensure we're calling things
   // on the appropriate threads.
   bool OnDecodeTaskQueue() const;
   bool OnTaskQueue() const;
 
-  // Cause state transitions. These methods obtain the decoder monitor
-  // to synchronise the change of state, and to notify other threads
-  // that the state has changed.
-  void Play()
-  {
-    MOZ_ASSERT(NS_IsMainThread());
-    RefPtr<nsRunnable> r = NS_NewRunnableMethod(this, &MediaDecoderStateMachine::PlayInternal);
-    TaskQueue()->Dispatch(r);
-  }
-
-private:
-  // The actual work for the above, which happens asynchronously on the state
-  // machine thread.
-  void PlayInternal();
-public:
-
   // Seeks to the decoder to aTarget asynchronously.
   // Must be called on the state machine thread.
   nsRefPtr<MediaDecoder::SeekPromise> Seek(SeekTarget aTarget);
 
   // Returns the current playback position in seconds.
   // Called from the main thread to get the current frame time. The decoder
   // monitor must be obtained before calling this.
   double GetCurrentTime() const;
@@ -588,16 +572,19 @@ protected:
   // Stops the audio thread. The decoder monitor must be held with exactly
   // one lock count. Called on the state machine thread.
   void StopAudioThread();
 
   // Starts the audio thread. The decoder monitor must be held with exactly
   // one lock count. Called on the state machine thread.
   nsresult StartAudioThread();
 
+  // Notification method invoked when mPlayState changes.
+  void PlayStateChanged();
+
   // Sets internal state which causes playback of media to pause.
   // The decoder monitor must be held.
   void StopPlayback();
 
   // If the conditions are right, sets internal state which causes playback
   // of media to begin or resume.
   // Must be called with the decode monitor held.
   void MaybeStartPlayback();