Bug 1295921 - PC: Implement blocking image get for SurfaceFromElement. r?jwwang draft
authorDan Glastonbury <dglastonbury@mozilla.com>
Tue, 27 Sep 2016 15:30:59 +1000
changeset 450832 c727af5916c4847841afd4b11c5ea646b7a7538a
parent 450831 f04eae06ddd9467003bb538780ed260f3e15b52b
child 450833 913cd310b0d1c1f1303e67b73c5d189bc7da8b06
push id38957
push userbmo:dglastonbury@mozilla.com
push dateMon, 19 Dec 2016 02:15:56 +0000
reviewersjwwang
bugs1295921
milestone53.0a1
Bug 1295921 - PC: Implement blocking image get for SurfaceFromElement. r?jwwang Connect all the pieces: * HTMLMediaElement::GetCurrentImage() waits on next rendered video frame before locking ImageContainer. * MediaDecoder::WaitOnNextRenderedVideoFrame(): * Sets suspend taint flag and returns if ImageContainer has a frame. * Otherwise, sets up monitor protected flag, sets suspend taint, and dispatches change tasks to MDSM TaskQueue before waiting on done flag. * MDSM resumes decoding, and * Signals done flag when after VideoSink::RenderVideoFrames adds new frame to ImageContainer, or * If resuming seeking fails, signals done flag to unblock main thread. MozReview-Commit-ID: HlGf5UM70EJ
dom/html/HTMLMediaElement.cpp
dom/media/MediaDecoder.cpp
dom/media/MediaDecoder.h
dom/media/MediaDecoderStateMachine.cpp
dom/media/MediaDecoderStateMachine.h
--- a/dom/html/HTMLMediaElement.cpp
+++ b/dom/html/HTMLMediaElement.cpp
@@ -1464,30 +1464,31 @@ HTMLMediaElement::SetVisible(bool aVisib
   }
 
   mDecoder->SetForcedHidden(!aVisible);
 }
 
 layers::Image*
 HTMLMediaElement::GetCurrentImage()
 {
-  // Mark the decoder owned by the element as tainted so that the video decode
-  // isn't suspended.
-  mHasSuspendTaint = true;
-  if (mDecoder) {
-    mDecoder->SetSuspendTaint(true);
-  }
-
-  // TODO(djg): In a follow-up patch, handle case when video decode is
-  // suspended.
   ImageContainer* container = GetImageContainer();
   if (!container) {
     return nullptr;
   }
 
+  // Mark the decoder owned by the element as tainted so that the video decode
+  // won't be suspended. If the decoder is already in suspended state, this will
+  // resume decoding.
+  mHasSuspendTaint = true;
+  if (mDecoder) {
+    // If element has a decoder, block and get next decoded image, otherwise use
+    // standard, non-blocking behavior.
+    mDecoder->WaitOnNextRenderedVideoFrame();
+  }
+
   AutoLockImage lockImage(container);
   return lockImage.GetImage();
 }
 
 bool
 HTMLMediaElement::HasSuspendTaint() const
 {
   MOZ_ASSERT(!mDecoder || (mDecoder->HasSuspendTaint() == mHasSuspendTaint));
--- a/dom/media/MediaDecoder.cpp
+++ b/dom/media/MediaDecoder.cpp
@@ -1176,16 +1176,44 @@ MediaDecoder::SeekingStarted()
   mOwner->SeekStarted();
 }
 
 void
 MediaDecoder::DispatchPendingTasks()
 {
   mDecoderStateMachine->DispatchPendingTasks();
 }
+
+void
+MediaDecoder::WaitOnNextRenderedVideoFrame()
+{
+  // Image container doesn't have a valid frame. Wait for the decoder
+  // to produce a new frame.
+  using WaitForFrame = MediaDecoderStateMachine::WaitForFrame;
+  WaitForFrame state{ "WaitOnNextRenderedVideoFrame" };
+
+  // Dispatch the variable setting before changing the suspend taint
+  // that'll trigger the decoder to start up.
+  mDecoderStateMachine->DispatchSetWaitForFrameState(&state);
+  SetSuspendTaint(true);
+
+  // Decoder is marked as tainted, but it might be suspended already. The
+  // called AutoLockImage will cause the MainThread to block until the frame
+  // is decoded.  To achieve this, need to go into a stable state where any
+  // pending variable state updates has been dispatched to MDSM.  This is done
+  // by DispatchPendingTasks.
+  DispatchPendingTasks();
+
+ {
+    MonitorAutoLock lock(state.mMon);
+    // Should this time-out and count the number of time outs?
+    while (!state.mDone) {
+      state.mMon.Wait();
+    }
+  }
 }
 
 void
 MediaDecoder::ChangeState(PlayState aState)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(!IsShutdown(), "SHUTDOWN is the final state.");
 
--- a/dom/media/MediaDecoder.h
+++ b/dom/media/MediaDecoder.h
@@ -364,18 +364,22 @@ private:
   // Returns true if the decoder can't participate in video decode suspending.
   bool HasSuspendTaint() const;
 
   /******
    * The following methods must only be called on the main
    * thread.
    ******/
 
-  // Dispatch any pending tasks in the main thread tail dispatcher in order.
-  void DispatchPendingTasks();
+  // Cause the decoder to wait for image container to hold a valid frame. If the
+  // image container doesn't contain any frames, WaitOnNextRenderedVideoFrame
+  // will block the thread until a frame is added into the image container. In
+  // the case that video decode is suspended and a seek failure occurs, the
+  // thread will unblock and image container will be empty.
+  void WaitOnNextRenderedVideoFrame();
 
   // 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 from MetadataLoaded(). Creates audio tracks and adds them to its
   // owner's audio track list, and implies to video tracks respectively.
@@ -572,16 +576,19 @@ private:
     mMediaSeekable = false;
   }
 
   void FinishShutdown();
 
   void ConnectMirrors(MediaDecoderStateMachine* aObject);
   void DisconnectMirrors();
 
+  // Dispatch any pending tasks in the main thread tail dispatcher in order.
+  void DispatchPendingTasks();
+
   MediaEventProducer<void> mDataArrivedEvent;
   MediaEventProducer<RefPtr<layers::KnowsCompositor>> mCompositorUpdatedEvent;
 
   // The state machine object for handling the decoding. It is safe to
   // call methods of this object from other threads. Its internal data
   // is synchronised on a monitor. The lifetime of this object is
   // after mPlayState is LOADING and before mPlayState is SHUTDOWN. It
   // is safe to access it during this period.
--- a/dom/media/MediaDecoderStateMachine.cpp
+++ b/dom/media/MediaDecoderStateMachine.cpp
@@ -801,16 +801,19 @@ public:
 
     DoSeek();
 
     return mSeekJob.mPromise.Ensure(__func__);
   }
 
   virtual void Exit() override
   {
+    // Unblock main thread if it is waiting for seek to finish.
+    mMaster->MarkWaitForFrameDone();
+
     mSeekTaskRequest.DisconnectIfExists();
     mSeekJob.RejectIfExists(__func__);
     mSeekTask->Discard();
   }
 
   State GetState() const override
   {
     return DECODER_STATE_SEEKING;
@@ -2221,16 +2224,17 @@ MediaDecoderStateMachine::MediaDecoderSt
   mLowAudioThresholdUsecs(detail::LOW_AUDIO_USECS),
   mAmpleAudioThresholdUsecs(detail::AMPLE_AUDIO_USECS),
   mAudioCaptured(false),
   mMinimizePreroll(false),
   mSentLoadedMetadataEvent(false),
   mSentFirstFrameLoadedEvent(false),
   mVideoDecodeSuspended(false),
   mVideoDecodeSuspendTimer(mTaskQueue),
+  mWaitForFrameState(nullptr),
   mOutputStreamManager(new OutputStreamManager()),
   mResource(aDecoder->GetResource()),
   mAudioOffloading(false),
   INIT_MIRROR(mBuffered, TimeIntervals()),
   INIT_MIRROR(mEstimatedDuration, NullableTimeUnit()),
   INIT_MIRROR(mExplicitDuration, Maybe<double>()),
   INIT_MIRROR(mPlayState, MediaDecoder::PLAY_STATE_LOADING),
   INIT_MIRROR(mNextPlayState, MediaDecoder::PLAY_STATE_PAUSED),
@@ -2900,24 +2904,67 @@ void MediaDecoderStateMachine::Visibilit
   // Resume from suspended decoding.
   if (mVideoDecodeSuspended) {
     mStateObj->HandleResumeVideoDecoding();
   }
 }
 
 void MediaDecoderStateMachine::SuspendTaintChanged()
 {
+  DECODER_LOG("SuspendTaintChanged: HasSuspendTaint=%c",
+              mHasSuspendTaint ? 'T' : 'F');
+
   MOZ_ASSERT(OnTaskQueue());
   MOZ_ASSERT(mHasSuspendTaint); // Suspend taint is only ever set.
 
   CancelSuspendTimer();
 
   // Resume from suspended decoding.
   if (mVideoDecodeSuspended) {
+    // Resume from suspended decoding.
     mStateObj->HandleResumeVideoDecoding();
+  } else {
+    // Decode isn't suspended, so clear the blocking main thread.
+    MarkWaitForFrameDone();
+  }
+}
+
+void
+MediaDecoderStateMachine::DispatchSetWaitForFrameState(WaitForFrame* aState)
+{
+  DECODER_LOG("DispatchSetWaitForFrameMonitor: state = %p", aState);
+
+  MOZ_ASSERT(NS_IsMainThread());
+  OwnerThread()->DispatchStateChange(
+    NewRunnableMethod<WaitForFrame*>(this,
+      &MediaDecoderStateMachine::SetWaitForFrameState, aState));
+}
+
+void
+MediaDecoderStateMachine::SetWaitForFrameState(WaitForFrame* aState)
+{
+  DECODER_LOG("SetWaitForFrameMonitor: state = %p", aState);
+
+  MOZ_ASSERT(OnTaskQueue());
+  MOZ_ASSERT(!mWaitForFrameState);
+  mWaitForFrameState = aState;
+}
+
+void
+MediaDecoderStateMachine::MarkWaitForFrameDone()
+{
+  DECODER_LOG("MarkWaitForFrameDone: state = %p", mWaitForFrameState);
+  MOZ_ASSERT(OnTaskQueue());
+
+  WaitForFrame* state = mWaitForFrameState;
+  if (state) {
+    MonitorAutoLock lock(state->mMon);
+    mWaitForFrameState = nullptr;
+    state->mDone = true;
+    lock.NotifyAll();
   }
 }
 
 void MediaDecoderStateMachine::BufferedRangeUpdated()
 {
   MOZ_ASSERT(OnTaskQueue());
 
   // While playing an unseekable stream of unknown duration, mObservedDuration
--- a/dom/media/MediaDecoderStateMachine.h
+++ b/dom/media/MediaDecoderStateMachine.h
@@ -231,16 +231,29 @@ public:
     MOZ_ASSERT(NS_IsMainThread());
     AbstractThread* mainThread = AbstractThread::MainThread();
     // Direct tasks need to be drained...
     mainThread->TailDispatcher().DrainDirectTasks();
     // ...before state and normal tasks.
     mainThread->TailDispatchTasksFor(OwnerThread());
   }
 
+  // WaitForFrame holds a monitor protected flag used to signal to the
+  // main thread that it can stop blocking.
+  struct WaitForFrame {
+    explicit WaitForFrame(const char* aDesc) :
+      mMon{aDesc}, mDone{false}
+    {}
+    Monitor mMon;
+    bool mDone;
+  };
+
+  // Set/Unset wait for next frame state.
+  void DispatchSetWaitForFrameState(WaitForFrame* aState);
+
   // Drop reference to mResource. Only called during shutdown dance.
   void BreakCycles() {
     MOZ_ASSERT(NS_IsMainThread());
     mResource = nullptr;
   }
 
   TimedMetadataEventSource& TimedMetadataEvent() {
     return mMetadataManager.TimedMetadataEvent();
@@ -446,16 +459,20 @@ protected:
   void PlayStateChanged();
 
   // Notification method invoked when mIsVisible changes.
   void VisibilityChanged();
 
   // Notification method invoked when mHasSuspendTaint changes.
   void SuspendTaintChanged();
 
+  void SetWaitForFrameState(WaitForFrame* aState);
+
+  void MarkWaitForFrameDone();
+
   // 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();
@@ -728,16 +745,18 @@ private:
   bool mMediaSeekable = true;
 
   // True if the media is seekable only in buffered ranges.
   bool mMediaSeekableOnlyInBufferedRanges = false;
 
   // Track enabling video decode suspension via timer
   DelayedScheduler mVideoDecodeSuspendTimer;
 
+  WaitForFrame* mWaitForFrameState;
+
   // Data about MediaStreams that are being fed by the decoder.
   const RefPtr<OutputStreamManager> mOutputStreamManager;
 
   // Media data resource from the decoder.
   RefPtr<MediaResource> mResource;
 
   // Track the complete & error for audio/video separately
   MozPromiseRequestHolder<GenericPromise> mMediaSinkAudioPromise;