Bug 1421179. P2 - mPlaybackStatistics should accumulate bytes as playback position progresses. draft
authorJW Wang <jwwang@mozilla.com>
Fri, 01 Dec 2017 11:26:03 +0800
changeset 706212 52c5478f9e28f67d957868b0d3852a8a2f9ad707
parent 706075 0d65bbc2f8c61ff296a03548d2f6d349229970fa
child 706213 9fe3797b80193e42cb9627e808b7f33a34b84ec3
push id91732
push userjwwang@mozilla.com
push dateFri, 01 Dec 2017 13:09:17 +0000
bugs1421179
milestone59.0a1
Bug 1421179. P2 - mPlaybackStatistics should accumulate bytes as playback position progresses. The original code accumulates bytes as the underlying decoder moves on which is wrong. MozReview-Commit-ID: 72hTwOHwKRh
dom/media/ChannelMediaDecoder.cpp
dom/media/ChannelMediaDecoder.h
dom/media/MediaDecoder.cpp
dom/media/MediaDecoder.h
dom/media/MediaDecoderStateMachine.cpp
dom/media/MediaDecoderStateMachine.h
--- a/dom/media/ChannelMediaDecoder.cpp
+++ b/dom/media/ChannelMediaDecoder.cpp
@@ -315,19 +315,16 @@ ChannelMediaDecoder::NotifyBytesConsumed
   MOZ_DIAGNOSTIC_ASSERT(!IsShutdown());
   AbstractThread::AutoEnter context(AbstractMainThread());
 
   if (mIgnoreProgressData) {
     return;
   }
 
   MOZ_ASSERT(GetStateMachine());
-  if (aOffset >= mDecoderPosition) {
-    mPlaybackStatistics.AddBytes(aBytes);
-  }
   mDecoderPosition = aOffset + aBytes;
 }
 
 void
 ChannelMediaDecoder::SeekingChanged()
 {
   // Stop updating the bytes downloaded for progress notifications when
   // seeking to prevent wild changes to the progress notification.
@@ -351,22 +348,33 @@ ChannelMediaDecoder::IsLiveStream()
 }
 
 void
 ChannelMediaDecoder::OnPlaybackEvent(MediaPlaybackEvent&& aEvent)
 {
   MOZ_ASSERT(NS_IsMainThread());
   switch (aEvent.mType) {
     case MediaPlaybackEvent::PlaybackStarted:
+      mPlaybackPosition = aEvent.mData.as<int64_t>();
       mPlaybackStatistics.Start();
       break;
-    case MediaPlaybackEvent::PlaybackStopped:
+    case MediaPlaybackEvent::PlaybackProgressed: {
+      int64_t newPos = aEvent.mData.as<int64_t>();
+      mPlaybackStatistics.AddBytes(newPos - mPlaybackPosition);
+      mPlaybackPosition = newPos;
+      break;
+    }
+    case MediaPlaybackEvent::PlaybackStopped: {
+      int64_t newPos = aEvent.mData.as<int64_t>();
+      mPlaybackStatistics.AddBytes(newPos - mPlaybackPosition);
+      mPlaybackPosition = newPos;
       mPlaybackStatistics.Stop();
       ComputePlaybackRate();
       break;
+    }
     default:
       break;
   }
   MediaDecoder::OnPlaybackEvent(Move(aEvent));
 }
 
 void
 ChannelMediaDecoder::DurationChanged()
--- a/dom/media/ChannelMediaDecoder.h
+++ b/dom/media/ChannelMediaDecoder.h
@@ -153,13 +153,19 @@ private:
   double mPlaybackBytesPerSecond = 0;
 
   // True if mPlaybackBytesPerSecond is a reliable estimate.
   bool mPlaybackRateReliable = true;
 
   // True when our media stream has been pinned. We pin the stream
   // while seeking.
   bool mPinnedForSeek = false;
+
+  // Current playback position in the stream. This is (approximately)
+  // where we're up to playing back the stream. This is not adjusted
+  // during decoder seek operations, but it's updated at the end when we
+  // start playing back again.
+  int64_t mPlaybackPosition = 0;
 };
 
 } // namespace mozilla
 
 #endif // ChannelMediaDecoder_h_
--- a/dom/media/MediaDecoder.cpp
+++ b/dom/media/MediaDecoder.cpp
@@ -398,17 +398,16 @@ MediaDecoder::MediaDecoder(MediaDecoderI
   , mElementVisibility(Visibility::UNTRACKED)
   , mIsElementInTree(false)
   , mForcedHidden(false)
   , mHasSuspendTaint(aInit.mHasSuspendTaint)
   , mPlaybackRate(aInit.mPlaybackRate)
   , INIT_MIRROR(mBuffered, TimeIntervals())
   , INIT_MIRROR(mCurrentPosition, TimeUnit::Zero())
   , INIT_MIRROR(mStateMachineDuration, NullableTimeUnit())
-  , INIT_MIRROR(mPlaybackPosition, 0)
   , INIT_MIRROR(mIsAudioDataAudible, false)
   , INIT_CANONICAL(mVolume, aInit.mVolume)
   , INIT_CANONICAL(mPreservesPitch, aInit.mPreservesPitch)
   , INIT_CANONICAL(mLooping, aInit.mLooping)
   , INIT_CANONICAL(mPlayState, PLAY_STATE_LOADING)
   , INIT_CANONICAL(mLogicallySeeking, false)
   , INIT_CANONICAL(mSameOriginMedia, false)
   , INIT_CANONICAL(mMediaPrincipalHandle, PRINCIPAL_HANDLE_NONE)
@@ -1295,28 +1294,26 @@ MediaDecoder::SetLooping(bool aLooping)
 void
 MediaDecoder::ConnectMirrors(MediaDecoderStateMachine* aObject)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(aObject);
   mStateMachineDuration.Connect(aObject->CanonicalDuration());
   mBuffered.Connect(aObject->CanonicalBuffered());
   mCurrentPosition.Connect(aObject->CanonicalCurrentPosition());
-  mPlaybackPosition.Connect(aObject->CanonicalPlaybackOffset());
   mIsAudioDataAudible.Connect(aObject->CanonicalIsAudioDataAudible());
 }
 
 void
 MediaDecoder::DisconnectMirrors()
 {
   MOZ_ASSERT(NS_IsMainThread());
   mStateMachineDuration.DisconnectIfConnected();
   mBuffered.DisconnectIfConnected();
   mCurrentPosition.DisconnectIfConnected();
-  mPlaybackPosition.DisconnectIfConnected();
   mIsAudioDataAudible.DisconnectIfConnected();
 }
 
 void
 MediaDecoder::SetStateMachine(MediaDecoderStateMachine* aStateMachine)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT_IF(aStateMachine, !mDecoderStateMachine);
--- a/dom/media/MediaDecoder.h
+++ b/dom/media/MediaDecoder.h
@@ -599,22 +599,16 @@ protected:
   Mirror<media::TimeIntervals> mBuffered;
 
   // NB: Don't use mCurrentPosition directly, but rather CurrentPosition().
   Mirror<media::TimeUnit> mCurrentPosition;
 
   // Duration of the media resource according to the state machine.
   Mirror<media::NullableTimeUnit> mStateMachineDuration;
 
-  // Current playback position in the stream. This is (approximately)
-  // where we're up to playing back the stream. This is not adjusted
-  // during decoder seek operations, but it's updated at the end when we
-  // start playing back again.
-  Mirror<int64_t> mPlaybackPosition;
-
   // Used to distinguish whether the audio is producing sound.
   Mirror<bool> mIsAudioDataAudible;
 
   // Volume of playback.  0.0 = muted. 1.0 = full volume.
   Canonical<double> mVolume;
 
   Canonical<bool> mPreservesPitch;
 
--- a/dom/media/MediaDecoderStateMachine.cpp
+++ b/dom/media/MediaDecoderStateMachine.cpp
@@ -2629,17 +2629,16 @@ ShutdownState::Enter()
   master->mVolume.DisconnectIfConnected();
   master->mPreservesPitch.DisconnectIfConnected();
   master->mLooping.DisconnectIfConnected();
   master->mSameOriginMedia.DisconnectIfConnected();
   master->mMediaPrincipalHandle.DisconnectIfConnected();
 
   master->mDuration.DisconnectAll();
   master->mCurrentPosition.DisconnectAll();
-  master->mPlaybackOffset.DisconnectAll();
   master->mIsAudioDataAudible.DisconnectAll();
 
   // Shut down the watch manager to stop further notifications.
   master->mWatchManager.Shutdown();
 
   return Reader()->Shutdown()->Then(
     OwnerThread(), __func__, master,
     &MediaDecoderStateMachine::FinishShutdown,
@@ -2682,17 +2681,16 @@ MediaDecoderStateMachine::MediaDecoderSt
   INIT_MIRROR(mPlayState, MediaDecoder::PLAY_STATE_LOADING),
   INIT_MIRROR(mVolume, 1.0),
   INIT_MIRROR(mPreservesPitch, true),
   INIT_MIRROR(mLooping, false),
   INIT_MIRROR(mSameOriginMedia, false),
   INIT_MIRROR(mMediaPrincipalHandle, PRINCIPAL_HANDLE_NONE),
   INIT_CANONICAL(mDuration, NullableTimeUnit()),
   INIT_CANONICAL(mCurrentPosition, TimeUnit::Zero()),
-  INIT_CANONICAL(mPlaybackOffset, 0),
   INIT_CANONICAL(mIsAudioDataAudible, false)
 #ifdef XP_WIN
   , mShouldUseHiResTimers(Preferences::GetBool("media.hi-res-timers.enabled", true))
 #endif
 {
   MOZ_COUNT_CTOR(MediaDecoderStateMachine);
   NS_ASSERTION(NS_IsMainThread(), "Should be on main thread.");
 
@@ -2828,24 +2826,24 @@ MediaDecoderStateMachine::PushVideo(Vide
   aSample->mFrameID = ++mCurrentFrameID;
   VideoQueue().Push(aSample);
 }
 
 void
 MediaDecoderStateMachine::OnAudioPopped(const RefPtr<AudioData>& aSample)
 {
   MOZ_ASSERT(OnTaskQueue());
-  mPlaybackOffset = std::max(mPlaybackOffset.Ref(), aSample->mOffset);
+  mPlaybackOffset = std::max(mPlaybackOffset, aSample->mOffset);
 }
 
 void
 MediaDecoderStateMachine::OnVideoPopped(const RefPtr<VideoData>& aSample)
 {
   MOZ_ASSERT(OnTaskQueue());
-  mPlaybackOffset = std::max(mPlaybackOffset.Ref(), aSample->mOffset);
+  mPlaybackOffset = std::max(mPlaybackOffset, aSample->mOffset);
 }
 
 bool
 MediaDecoderStateMachine::IsAudioDecoding()
 {
   MOZ_ASSERT(OnTaskQueue());
   return HasAudio() && !AudioQueue().IsFinished();
 }
@@ -2901,19 +2899,19 @@ nsresult MediaDecoderStateMachine::Init(
 }
 
 void
 MediaDecoderStateMachine::StopPlayback()
 {
   MOZ_ASSERT(OnTaskQueue());
   LOG("StopPlayback()");
 
-  mOnPlaybackEvent.Notify(MediaPlaybackEvent::PlaybackStopped);
-
   if (IsPlaying()) {
+    mOnPlaybackEvent.Notify(MediaPlaybackEvent{
+      MediaPlaybackEvent::PlaybackStopped, mPlaybackOffset });
     mMediaSink->SetPlaying(false);
     MOZ_ASSERT(!IsPlaying());
 #ifdef XP_WIN
     if (mHiResTimersRequested) {
       mHiResTimersRequested = false;
       timeEndPeriod(1);
     }
 #endif
@@ -2932,17 +2930,16 @@ void MediaDecoderStateMachine::MaybeStar
   }
 
   if (mPlayState != MediaDecoder::PLAY_STATE_PLAYING) {
     LOG("Not starting playback [mPlayState=%d]", mPlayState.Ref());
     return;
   }
 
   LOG("MaybeStartPlayback() starting playback");
-  mOnPlaybackEvent.Notify(MediaPlaybackEvent::PlaybackStarted);
   StartMediaSink();
 
 #ifdef XP_WIN
   if (!mHiResTimersRequested && mShouldUseHiResTimers) {
     mHiResTimersRequested = true;
     // Ensure high precision timers are enabled on Windows, otherwise the state
     // machine isn't woken up at reliable intervals to set the next frame, and we
     // drop frames while painting. Note that each call must be matched by a
@@ -2953,16 +2950,19 @@ void MediaDecoderStateMachine::MaybeStar
     timeBeginPeriod(1);
   }
 #endif
 
   if (!IsPlaying()) {
     mMediaSink->SetPlaying(true);
     MOZ_ASSERT(IsPlaying());
   }
+
+  mOnPlaybackEvent.Notify(
+    MediaPlaybackEvent{ MediaPlaybackEvent::PlaybackStarted, mPlaybackOffset });
 }
 
 void
 MediaDecoderStateMachine::UpdatePlaybackPositionInternal(const TimeUnit& aTime)
 {
   MOZ_ASSERT(OnTaskQueue());
   LOGV("UpdatePlaybackPositionInternal(%" PRId64 ")", aTime.ToMicroseconds());
 
@@ -3311,16 +3311,24 @@ MediaDecoderStateMachine::StartMediaSink
     }
     if (videoPromise) {
       videoPromise->Then(
         OwnerThread(), __func__, this,
         &MediaDecoderStateMachine::OnMediaSinkVideoComplete,
         &MediaDecoderStateMachine::OnMediaSinkVideoError)
       ->Track(mMediaSinkVideoPromise);
     }
+    // Remember the initial offset when playback starts. This will be used
+    // to calculate the rate at which bytes are consumed as playback moves on.
+    RefPtr<MediaData> sample = mAudioQueue.PeekFront();
+    mPlaybackOffset = sample ? sample->mOffset : 0;
+    sample = mVideoQueue.PeekFront();
+    if (sample && sample->mOffset > mPlaybackOffset) {
+      mPlaybackOffset = sample->mOffset;
+    }
   }
 }
 
 bool
 MediaDecoderStateMachine::HasLowDecodedAudio()
 {
   MOZ_ASSERT(OnTaskQueue());
   return IsAudioDecoding() && GetDecodedAudioDuration()
@@ -3489,18 +3497,16 @@ MediaDecoderStateMachine::ResetDecode(Tr
   if (aTracks.contains(TrackInfo::kAudioTrack)) {
     mDecodedAudioEndTime = TimeUnit::Zero();
     mAudioCompleted = false;
     AudioQueue().Reset();
     mAudioDataRequest.DisconnectIfExists();
     mAudioWaitRequest.DisconnectIfExists();
   }
 
-  mPlaybackOffset = 0;
-
   mReader->ResetDecode(aTracks);
 }
 
 media::TimeUnit
 MediaDecoderStateMachine::GetClock(TimeStamp* aTimeStamp) const
 {
   MOZ_ASSERT(OnTaskQueue());
   auto clockTime = mMediaSink->GetPosition(aTimeStamp);
@@ -3546,16 +3552,24 @@ MediaDecoderStateMachine::UpdatePlayback
   }
   // Note we have to update playback position before releasing the monitor.
   // Otherwise, MediaDecoder::AddOutputStream could kick in when we are outside
   // the monitor and get a staled value from GetCurrentTimeUs() which hits the
   // assertion in GetClock().
 
   int64_t delay = std::max<int64_t>(1, AUDIO_DURATION_USECS / mPlaybackRate);
   ScheduleStateMachineIn(TimeUnit::FromMicroseconds(delay));
+
+  // Notify the listener as we progress in the playback offset. Note it would
+  // be too intensive to send notifications for each popped audio/video sample.
+  // It is good enough to send 'PlaybackProgressed' events every 40us (defined
+  // by AUDIO_DURATION_USECS), and we ensure 'PlaybackProgressed' events are
+  // always sent after 'PlaybackStarted' and before 'PlaybackStopped'.
+  mOnPlaybackEvent.Notify(MediaPlaybackEvent{
+    MediaPlaybackEvent::PlaybackProgressed, mPlaybackOffset });
 }
 
 void
 MediaDecoderStateMachine::ScheduleStateMachine()
 {
   MOZ_ASSERT(OnTaskQueue());
   if (mDispatchedStateMachine) {
     return;
--- a/dom/media/MediaDecoderStateMachine.h
+++ b/dom/media/MediaDecoderStateMachine.h
@@ -115,16 +115,17 @@ class TaskQueue;
 extern LazyLogModule gMediaDecoderLog;
 
 struct MediaPlaybackEvent
 {
   enum EventType
   {
     PlaybackStarted,
     PlaybackStopped,
+    PlaybackProgressed,
     PlaybackEnded,
     SeekStarted,
     Loop,
     Invalidate,
     EnterVideoSuspend,
     ExitVideoSuspend,
     StartVideoSuspendTimer,
     CancelVideoSuspendTimer,
@@ -135,16 +136,23 @@ struct MediaPlaybackEvent
   using DataType = Variant<Nothing, int64_t>;
   DataType mData;
 
   MOZ_IMPLICIT MediaPlaybackEvent(EventType aType)
     : mType(aType)
     , mData(Nothing{})
   {
   }
+
+  template<typename T>
+  MediaPlaybackEvent(EventType aType, T&& aArg)
+    : mType(aType)
+    , mData(Forward<T>(aArg))
+  {
+  }
 };
 
 enum class VideoDecodeMode : uint8_t
 {
   Normal,
   Suspend
 };
 
@@ -679,16 +687,19 @@ private:
   MediaEventProducer<DecoderDoctorEvent> mOnDecoderDoctorEvent;
 
   MediaEventProducer<NextFrameStatus> mOnNextFrameStatus;
 
   const bool mIsMSE;
 
   bool mSeamlessLoopingAllowed;
 
+  // Current playback position in the stream in bytes.
+  int64_t mPlaybackOffset = 0;
+
 private:
   // The buffered range. Mirrored from the decoder thread.
   Mirror<media::TimeIntervals> mBuffered;
 
   // The current play state, mirrored from the main thread.
   Mirror<MediaDecoder::PlayState> mPlayState;
 
   // Volume of playback. 0.0 = muted. 1.0 = full volume.
@@ -713,37 +724,30 @@ private:
   // decoding the first frame.
   Canonical<media::NullableTimeUnit> mDuration;
 
   // The time of the current frame, corresponding to the "current
   // playback position" in HTML5. This is referenced from 0, which is the initial
   // playback position.
   Canonical<media::TimeUnit> mCurrentPosition;
 
-  // Current playback position in the stream in bytes.
-  Canonical<int64_t> mPlaybackOffset;
-
   // Used to distinguish whether the audio is producing sound.
   Canonical<bool> mIsAudioDataAudible;
 
 public:
   AbstractCanonical<media::TimeIntervals>* CanonicalBuffered() const;
 
   AbstractCanonical<media::NullableTimeUnit>* CanonicalDuration()
   {
     return &mDuration;
   }
   AbstractCanonical<media::TimeUnit>* CanonicalCurrentPosition()
   {
     return &mCurrentPosition;
   }
-  AbstractCanonical<int64_t>* CanonicalPlaybackOffset()
-  {
-    return &mPlaybackOffset;
-  }
   AbstractCanonical<bool>* CanonicalIsAudioDataAudible()
   {
     return &mIsAudioDataAudible;
   }
 
 #ifdef XP_WIN
   // Whether we've called timeBeginPeriod(1) to request high resolution
   // timers. We request high resolution timers when playback starts, and