Bug 1324629. Part 2 - remove video callback. r?kaku draft
authorJW Wang <jwwang@mozilla.com>
Tue, 20 Dec 2016 13:17:05 +0800
changeset 452800 fbdd4a258d129a8a24f5d6c2a25f7b440edb76e6
parent 452799 ce014d0306e0afbfec2cbeaa0b61dd97f1e5d53a
child 452801 4ae8ac13d1f7cfb0fbbe0f58e67d8c347b0f8aca
push id39496
push userjwwang@mozilla.com
push dateThu, 22 Dec 2016 08:00:57 +0000
reviewerskaku
bugs1324629
milestone53.0a1
Bug 1324629. Part 2 - remove video callback. r?kaku MozReview-Commit-ID: 92t6GoznxL5
dom/media/MediaDecoderReaderWrapper.cpp
dom/media/MediaDecoderReaderWrapper.h
dom/media/MediaDecoderStateMachine.cpp
dom/media/MediaDecoderStateMachine.h
--- a/dom/media/MediaDecoderReaderWrapper.cpp
+++ b/dom/media/MediaDecoderReaderWrapper.cpp
@@ -49,53 +49,36 @@ MediaDecoderReaderWrapper::RequestAudioD
                      __func__, &MediaDecoderReader::RequestAudioData)
     ->Then(mOwnerThread, __func__,
            [startTime] (MediaData* aAudio) {
              aAudio->AdjustForStartTime(startTime);
            },
            [] (const MediaResult& aError) {});
 }
 
-void
+RefPtr<MediaDecoderReaderWrapper::MediaDataPromise>
 MediaDecoderReaderWrapper::RequestVideoData(bool aSkipToNextKeyframe,
                                             media::TimeUnit aTimeThreshold)
 {
   MOZ_ASSERT(mOwnerThread->IsCurrentThreadIn());
   MOZ_ASSERT(!mShutdown);
 
-  // Time the video decode and send this value back to callbacks who accept
-  // a TimeStamp as its second parameter.
-  TimeStamp videoDecodeStartTime = TimeStamp::Now();
-
   if (aTimeThreshold.ToMicroseconds() > 0) {
     aTimeThreshold += StartTime();
   }
 
-  auto p = InvokeAsync(mReader->OwnerThread(), mReader.get(), __func__,
-                       &MediaDecoderReader::RequestVideoData,
-                       aSkipToNextKeyframe, aTimeThreshold.ToMicroseconds());
-
-  RefPtr<MediaDecoderReaderWrapper> self = this;
-  mVideoDataRequest.Begin(p->Then(mOwnerThread, __func__,
-    [self, videoDecodeStartTime] (MediaData* aVideoSample) {
-      self->mVideoDataRequest.Complete();
-      aVideoSample->AdjustForStartTime(self->StartTime().ToMicroseconds());
-      self->mVideoCallback.Notify(AsVariant(MakeTuple(aVideoSample, videoDecodeStartTime)));
-    },
-    [self] (const MediaResult& aError) {
-      self->mVideoDataRequest.Complete();
-      self->mVideoCallback.Notify(AsVariant(aError));
-    }));
-}
-
-bool
-MediaDecoderReaderWrapper::IsRequestingVideoData() const
-{
-  MOZ_ASSERT(mOwnerThread->IsCurrentThreadIn());
-  return mVideoDataRequest.Exists();
+  int64_t startTime = StartTime().ToMicroseconds();
+  return InvokeAsync(mReader->OwnerThread(), mReader.get(), __func__,
+                     &MediaDecoderReader::RequestVideoData,
+                     aSkipToNextKeyframe, aTimeThreshold.ToMicroseconds())
+    ->Then(mOwnerThread, __func__,
+           [startTime] (MediaData* aVideo) {
+             aVideo->AdjustForStartTime(startTime);
+           },
+           [] (const MediaResult& aError) {});
 }
 
 bool
 MediaDecoderReaderWrapper::IsWaitingAudioData() const
 {
   MOZ_ASSERT(mOwnerThread->IsCurrentThreadIn());
   return mAudioWaitRequest.Exists();
 }
@@ -167,33 +150,30 @@ MediaDecoderReaderWrapper::ResetDecode(T
 {
   MOZ_ASSERT(mOwnerThread->IsCurrentThreadIn());
 
   if (aTracks.contains(TrackInfo::kAudioTrack)) {
     mAudioWaitRequest.DisconnectIfExists();
   }
 
   if (aTracks.contains(TrackInfo::kVideoTrack)) {
-    mVideoDataRequest.DisconnectIfExists();
     mVideoWaitRequest.DisconnectIfExists();
   }
 
   nsCOMPtr<nsIRunnable> r =
     NewRunnableMethod<TrackSet>(mReader,
                                 &MediaDecoderReader::ResetDecode,
                                 aTracks);
   mReader->OwnerThread()->Dispatch(r.forget());
 }
 
 RefPtr<ShutdownPromise>
 MediaDecoderReaderWrapper::Shutdown()
 {
   MOZ_ASSERT(mOwnerThread->IsCurrentThreadIn());
-  MOZ_ASSERT(!mVideoDataRequest.Exists());
-
   mShutdown = true;
   return InvokeAsync(mReader->OwnerThread(), mReader.get(), __func__,
                      &MediaDecoderReader::Shutdown);
 }
 
 void
 MediaDecoderReaderWrapper::OnMetadataRead(MetadataHolder* aMetadata)
 {
--- a/dom/media/MediaDecoderReaderWrapper.h
+++ b/dom/media/MediaDecoderReaderWrapper.h
@@ -33,39 +33,38 @@ class MediaDecoderReaderWrapper {
   typedef MediaDecoderReader::MetadataPromise MetadataPromise;
   typedef MediaDecoderReader::MediaDataPromise MediaDataPromise;
   typedef MediaDecoderReader::SeekPromise SeekPromise;
   typedef MediaDecoderReader::WaitForDataPromise WaitForDataPromise;
   typedef MediaDecoderReader::TrackSet TrackSet;
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(MediaDecoderReaderWrapper);
 
 private:
-  MediaCallbackExc<VideoCallbackData> mVideoCallback;
   MediaCallbackExc<WaitCallbackData> mAudioWaitCallback;
   MediaCallbackExc<WaitCallbackData> mVideoWaitCallback;
 
 public:
   MediaDecoderReaderWrapper(AbstractThread* aOwnerThread,
                             MediaDecoderReader* aReader);
 
   media::TimeUnit StartTime() const;
   RefPtr<MetadataPromise> ReadMetadata();
 
-  decltype(mVideoCallback)& VideoCallback() { return mVideoCallback; }
   decltype(mAudioWaitCallback)& AudioWaitCallback() { return mAudioWaitCallback; }
   decltype(mVideoWaitCallback)& VideoWaitCallback() { return mVideoWaitCallback; }
 
   // NOTE: please set callbacks before requesting audio/video data!
   RefPtr<MediaDataPromise> RequestAudioData();
-  void RequestVideoData(bool aSkipToNextKeyframe, media::TimeUnit aTimeThreshold);
+
+  RefPtr<MediaDataPromise>
+  RequestVideoData(bool aSkipToNextKeyframe, media::TimeUnit aTimeThreshold);
 
   // NOTE: please set callbacks before invoking WaitForData()!
   void WaitForData(MediaData::Type aType);
 
-  bool IsRequestingVideoData() const;
   bool IsWaitingAudioData() const;
   bool IsWaitingVideoData() const;
 
   RefPtr<SeekPromise> Seek(const SeekTarget& aTarget);
   RefPtr<ShutdownPromise> Shutdown();
 
   void ReleaseResources();
   void ResetDecode(TrackSet aTracks);
@@ -116,16 +115,15 @@ private:
   MozPromiseRequestHolder<WaitForDataPromise>& WaitRequestRef(MediaData::Type aType);
 
   const RefPtr<AbstractThread> mOwnerThread;
   const RefPtr<MediaDecoderReader> mReader;
 
   bool mShutdown = false;
   Maybe<media::TimeUnit> mStartTime;
 
-  MozPromiseRequestHolder<MediaDataPromise> mVideoDataRequest;
   MozPromiseRequestHolder<WaitForDataPromise> mAudioWaitRequest;
   MozPromiseRequestHolder<WaitForDataPromise> mVideoWaitRequest;
 };
 
 } // namespace mozilla
 
 #endif // MediaDecoderReaderWrapper_h_
--- a/dom/media/MediaDecoderStateMachine.cpp
+++ b/dom/media/MediaDecoderStateMachine.cpp
@@ -1067,19 +1067,19 @@ private:
     MOZ_ASSERT(!mMaster->IsRequestingAudioData());
     MOZ_ASSERT(!Reader()->IsWaitingAudioData());
     mMaster->RequestAudioData();
   }
 
   void RequestVideoData()
   {
     MOZ_ASSERT(!mDoneVideoSeeking);
-    MOZ_ASSERT(!Reader()->IsRequestingVideoData());
+    MOZ_ASSERT(!mMaster->IsRequestingVideoData());
     MOZ_ASSERT(!Reader()->IsWaitingVideoData());
-    Reader()->RequestVideoData(false, media::TimeUnit());
+    mMaster->RequestVideoData(false, media::TimeUnit());
   }
 
   void AdjustFastSeekIfNeeded(MediaData* aSample)
   {
     if (mSeekJob.mTarget->IsFast() &&
         mSeekJob.mTarget->GetTime() > mCurrentTimeBeforeSeek &&
         aSample->mTime < mCurrentTimeBeforeSeek.ToMicroseconds()) {
       // We are doing a fastSeek, but we ended up *before* the previous
@@ -1275,17 +1275,17 @@ private:
   {
     auto currentTime = mCurrentTime;
     DiscardFrames(VideoQueue(), [currentTime] (int64_t aSampleTime) {
       return aSampleTime <= currentTime;
     });
 
     if (!NeedMoreVideo()) {
       FinishSeek();
-    } else if (!Reader()->IsRequestingVideoData() &&
+    } else if (!mMaster->IsRequestingVideoData() &&
                !Reader()->IsWaitingVideoData()) {
       RequestVideoData();
     }
   }
 
   class AysncNextFrameSeekTask : public Runnable
   {
   public:
@@ -1425,17 +1425,17 @@ private:
   {
     // The HTMLMediaElement.currentTime should be updated to the seek target
     // which has been updated to the next frame's time.
     return mSeekJob.mTarget->GetTime().ToMicroseconds();
   }
 
   void RequestVideoData()
   {
-    Reader()->RequestVideoData(false, media::TimeUnit());
+    mMaster->RequestVideoData(false, media::TimeUnit());
   }
 
   bool NeedMoreVideo() const
   {
     // Need to request video when we have none and video queue is not finished.
     return VideoQueue().GetSize() == 0 &&
            !VideoQueue().IsFinished();
   }
@@ -2136,17 +2136,17 @@ BufferingState::Step()
                "Don't yet have a strategy for non-heuristic + non-WaitForData");
     mMaster->DispatchDecodeTasksIfNeeded();
     MOZ_ASSERT(mMaster->mMinimizePreroll ||
                !mMaster->OutOfDecodedAudio() ||
                mMaster->IsRequestingAudioData() ||
                Reader()->IsWaitingAudioData());
     MOZ_ASSERT(mMaster->mMinimizePreroll ||
                !mMaster->OutOfDecodedVideo() ||
-               Reader()->IsRequestingVideoData() ||
+               mMaster->IsRequestingVideoData() ||
                Reader()->IsWaitingVideoData());
     SLOG("In buffering mode, waiting to be notified: outOfAudio: %d, "
          "mAudioStatus: %s, outOfVideo: %d, mVideoStatus: %s",
          mMaster->OutOfDecodedAudio(), mMaster->AudioRequestStatus(),
          mMaster->OutOfDecodedVideo(), mMaster->VideoRequestStatus());
     return;
   }
 
@@ -2183,16 +2183,17 @@ ShutdownState::Enter()
 
   if (master->IsPlaying()) {
     master->StopPlayback();
   }
 
   // To break the cycle-reference between MediaDecoderReaderWrapper and MDSM.
   master->CancelMediaDecoderReaderWrapperCallback();
   master->mAudioDataRequest.DisconnectIfExists();
+  master->mVideoDataRequest.DisconnectIfExists();
 
   master->Reset();
 
   master->mMediaSink->Shutdown();
 
   // Prevent dangling pointers by disconnecting the listeners.
   master->mAudioQueueListener.Disconnect();
   master->mVideoQueueListener.Disconnect();
@@ -2575,31 +2576,41 @@ void
 MediaDecoderStateMachine::OnAudioNotDecoded(const MediaResult& aError)
 {
   MOZ_ASSERT(OnTaskQueue());
   mAudioDataRequest.Complete();
   OnNotDecoded(MediaData::AUDIO_DATA, aError);
 }
 
 void
+MediaDecoderStateMachine::OnVideoNotDecoded(const MediaResult& aError)
+{
+  MOZ_ASSERT(OnTaskQueue());
+  mVideoDataRequest.Complete();
+  OnNotDecoded(MediaData::VIDEO_DATA, aError);
+}
+
+void
 MediaDecoderStateMachine::OnNotDecoded(MediaData::Type aType,
                                        const MediaResult& aError)
 {
   MOZ_ASSERT(OnTaskQueue());
   SAMPLE_LOG("OnNotDecoded (aType=%u, aError=%u)", aType, aError.Code());
   mStateObj->HandleNotDecoded(aType, aError);
 }
 
 void
 MediaDecoderStateMachine::OnVideoDecoded(MediaData* aVideo,
                                          TimeStamp aDecodeStartTime)
 {
   MOZ_ASSERT(OnTaskQueue());
   MOZ_ASSERT(aVideo);
 
+  mVideoDataRequest.Complete();
+
   // Handle abnormal or negative timestamps.
   mDecodedVideoEndTime = std::max(mDecodedVideoEndTime, aVideo->GetEndTime());
 
   SAMPLE_LOG("OnVideoDecoded [%lld,%lld]", aVideo->mTime, aVideo->GetEndTime());
 
   mStateObj->HandleVideoDecoded(aVideo, aDecodeStartTime);
 }
 
@@ -2700,27 +2711,16 @@ nsresult MediaDecoderStateMachine::Init(
   return NS_OK;
 }
 
 void
 MediaDecoderStateMachine::SetMediaDecoderReaderWrapperCallback()
 {
   MOZ_ASSERT(OnTaskQueue());
 
-  mVideoCallback = mReader->VideoCallback().Connect(
-    mTaskQueue, [this] (VideoCallbackData aData) {
-    typedef Tuple<MediaData*, TimeStamp> Type;
-    if (aData.is<Type>()) {
-      auto&& v = aData.as<Type>();
-      OnVideoDecoded(Get<0>(v), Get<1>(v));
-    } else {
-      OnNotDecoded(MediaData::VIDEO_DATA, aData.as<MediaResult>());
-    }
-  });
-
   mAudioWaitCallback = mReader->AudioWaitCallback().Connect(
     mTaskQueue, [this] (WaitCallbackData aData) {
     if (aData.is<MediaData::Type>()) {
       OnAudioWaited(aData.as<MediaData::Type>());
     } else {
       OnNotWaited(aData.as<WaitForDataRejectValue>());
     }
   });
@@ -2734,17 +2734,16 @@ MediaDecoderStateMachine::SetMediaDecode
     }
   });
 }
 
 void
 MediaDecoderStateMachine::CancelMediaDecoderReaderWrapperCallback()
 {
   MOZ_ASSERT(OnTaskQueue());
-  mVideoCallback.Disconnect();
   mAudioWaitCallback.Disconnect();
   mVideoWaitCallback.Disconnect();
 }
 
 void MediaDecoderStateMachine::StopPlayback()
 {
   MOZ_ASSERT(OnTaskQueue());
   DECODER_LOG("StopPlayback()");
@@ -3113,43 +3112,45 @@ MediaDecoderStateMachine::EnsureVideoDec
 
   if (mState != DECODER_STATE_DECODING &&
       mState != DECODER_STATE_DECODING_FIRSTFRAME &&
       mState != DECODER_STATE_BUFFERING) {
     return;
   }
 
   if (!IsVideoDecoding() ||
-      mReader->IsRequestingVideoData() ||
+      IsRequestingVideoData() ||
       mReader->IsWaitingVideoData()) {
     return;
   }
 
-  RequestVideoData();
+  RequestVideoData(NeedToSkipToNextKeyframe(),
+                   media::TimeUnit::FromMicroseconds(GetMediaTime()));
 }
 
 void
-MediaDecoderStateMachine::RequestVideoData()
+MediaDecoderStateMachine::RequestVideoData(bool aSkipToNextKeyframe,
+                                           const media::TimeUnit& aCurrentTime)
 {
   MOZ_ASSERT(OnTaskQueue());
-  MOZ_ASSERT(mState != DECODER_STATE_SEEKING);
-
-  bool skipToNextKeyFrame = NeedToSkipToNextKeyframe();
-
-  media::TimeUnit currentTime = media::TimeUnit::FromMicroseconds(GetMediaTime());
-
   SAMPLE_LOG("Queueing video task - queued=%i, decoder-queued=%o, skip=%i, time=%lld",
-             VideoQueue().GetSize(), mReader->SizeOfVideoQueueInFrames(), skipToNextKeyFrame,
-             currentTime.ToMicroseconds());
-
-  // MediaDecoderReaderWrapper::RequestVideoData() records the decoding start
-  // time and sent it back to MDSM::OnVideoDecoded() so that if the decoding is
-  // slow, we can increase our low audio threshold to reduce the chance of an
-  // audio underrun while we're waiting for a video decode to complete.
-  mReader->RequestVideoData(skipToNextKeyFrame, currentTime);
+             VideoQueue().GetSize(), mReader->SizeOfVideoQueueInFrames(), aSkipToNextKeyframe,
+             aCurrentTime.ToMicroseconds());
+
+  TimeStamp videoDecodeStartTime = TimeStamp::Now();
+  mVideoDataRequest.Begin(
+    mReader->RequestVideoData(aSkipToNextKeyframe, aCurrentTime)->Then(
+      OwnerThread(), __func__,
+      [this, videoDecodeStartTime] (MediaData* aVideo) {
+        OnVideoDecoded(aVideo, videoDecodeStartTime);
+      },
+      [this] (const MediaResult& aError) {
+        OnVideoNotDecoded(aError);
+      })
+  );
 }
 
 void
 MediaDecoderStateMachine::StartMediaSink()
 {
   MOZ_ASSERT(OnTaskQueue());
   if (!mMediaSink->IsStarted()) {
     mAudioCompleted = false;
@@ -3369,16 +3370,17 @@ MediaDecoderStateMachine::Reset(TrackSet
     // crash for no samples to be popped.
     StopMediaSink();
   }
 
   if (aTracks.contains(TrackInfo::kVideoTrack)) {
     mDecodedVideoEndTime = 0;
     mVideoCompleted = false;
     VideoQueue().Reset();
+    mVideoDataRequest.DisconnectIfExists();
   }
 
   if (aTracks.contains(TrackInfo::kAudioTrack)) {
     mDecodedAudioEndTime = 0;
     mAudioCompleted = false;
     AudioQueue().Reset();
     mAudioDataRequest.DisconnectIfExists();
   }
@@ -3791,17 +3793,17 @@ MediaDecoderStateMachine::AudioRequestSt
   }
   return "idle";
 }
 
 const char*
 MediaDecoderStateMachine::VideoRequestStatus() const
 {
   MOZ_ASSERT(OnTaskQueue());
-  if (mReader->IsRequestingVideoData()) {
+  if (IsRequestingVideoData()) {
     MOZ_DIAGNOSTIC_ASSERT(!mReader->IsWaitingVideoData());
     return "pending";
   } else if (mReader->IsWaitingVideoData()) {
     return "waiting";
   }
   return "idle";
 }
 
--- a/dom/media/MediaDecoderStateMachine.h
+++ b/dom/media/MediaDecoderStateMachine.h
@@ -325,16 +325,17 @@ private:
 
   // Returns true if we're currently playing. The decoder monitor must
   // be held.
   bool IsPlaying() const;
 
   void OnAudioDecoded(MediaData* aAudio);
   void OnVideoDecoded(MediaData* aVideo, TimeStamp aDecodeStartTime);
   void OnAudioNotDecoded(const MediaResult& aError);
+  void OnVideoNotDecoded(const MediaResult& aError);
   void OnNotDecoded(MediaData::Type aType, const MediaResult& aError);
   void OnAudioWaited(MediaData::Type aType);
   void OnVideoWaited(MediaData::Type aType);
   void OnNotWaited(const WaitForDataRejectValue& aRejection);
 
   // Resets all state related to decoding and playback, emptying all buffers
   // and aborting all pending operations on the decode task queue.
   void Reset(TrackSet aTracks = TrackSet(TrackInfo::kAudioTrack,
@@ -468,19 +469,21 @@ protected:
   void EnsureVideoDecodeTaskQueued();
 
   // Start a task to decode audio.
   // The decoder monitor must be held.
   void RequestAudioData();
 
   // Start a task to decode video.
   // The decoder monitor must be held.
-  void RequestVideoData();
+  void RequestVideoData(bool aSkipToNextKeyframe,
+                        const media::TimeUnit& aCurrentTime);
 
   bool IsRequestingAudioData() const { return mAudioDataRequest.Exists(); }
+  bool IsRequestingVideoData() const { return mVideoDataRequest.Exists(); }
 
   // Re-evaluates the state and determines whether we need to dispatch
   // events to run the decode, or if not whether we should set the reader
   // to idle mode. This is threadsafe, and can be called from any thread.
   // The decoder monitor must be held.
   void DispatchDecodeTasksIfNeeded();
 
   // Returns the "media time". This is the absolute time which the media
@@ -652,22 +655,22 @@ private:
   {
     MOZ_ASSERT(OnTaskQueue());
     return GetAmpleVideoFrames() / 2;
   }
 
   // Only one of a given pair of ({Audio,Video}DataPromise, WaitForDataPromise)
   // should exist at any given moment.
 
-  MediaEventListener mVideoCallback;
   MediaEventListener mAudioWaitCallback;
   MediaEventListener mVideoWaitCallback;
 
   using MediaDataPromise = MediaDecoderReader::MediaDataPromise;
   MozPromiseRequestHolder<MediaDataPromise> mAudioDataRequest;
+  MozPromiseRequestHolder<MediaDataPromise> mVideoDataRequest;
 
   const char* AudioRequestStatus() const;
   const char* VideoRequestStatus() const;
 
   void OnSuspendTimerResolved();
   void OnSuspendTimerRejected();
 
   // True if we shouldn't play our audio (but still write it to any capturing