From 4026dc6f848957f2116bf7c06b4f28350de50115 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Fri, 6 Oct 2017 15:48:38 +0800 Subject: [PATCH] Bug 1405025. P1 - ensure 'seeking' is fired before 'waiting'. r=jya Use MediaEventSource instead of state-mirroring to notify nextFrameStatus changes so we have more control over the order of events. MozReview-Commit-ID: 3DGtMbghEQm --HG-- extra : rebase_source : 774fc3da290c033769871a1bd7230177ff24d5bf extra : intermediate-source : 6583b9281492be1a3bb0771b600cd80efd487af8 extra : source : 00570c319bfbd94970d4c637c7bf81b52d79ca02 --- dom/media/MediaDecoder.cpp | 37 ++++++++++++++++++++--- dom/media/MediaDecoder.h | 9 ++++-- dom/media/MediaDecoderStateMachine.cpp | 41 ++++---------------------- dom/media/MediaDecoderStateMachine.h | 16 ++++------ 4 files changed, 50 insertions(+), 53 deletions(-) diff --git a/dom/media/MediaDecoder.cpp b/dom/media/MediaDecoder.cpp index 501adc19e905..46dd8d9e7182 100644 --- a/dom/media/MediaDecoder.cpp +++ b/dom/media/MediaDecoder.cpp @@ -374,7 +374,6 @@ MediaDecoder::MediaDecoder(MediaDecoderInit& aInit) , mHasSuspendTaint(aInit.mHasSuspendTaint) , mPlaybackRate(aInit.mPlaybackRate) , INIT_MIRROR(mBuffered, TimeIntervals()) - , INIT_MIRROR(mNextFrameStatus, MediaDecoderOwner::NEXT_FRAME_UNAVAILABLE) , INIT_MIRROR(mCurrentPosition, TimeUnit::Zero()) , INIT_MIRROR(mStateMachineDuration, NullableTimeUnit()) , INIT_MIRROR(mPlaybackPosition, 0) @@ -404,7 +403,6 @@ MediaDecoder::MediaDecoder(MediaDecoderInit& aInit) // readyState mWatchManager.Watch(mPlayState, &MediaDecoder::UpdateReadyState); - mWatchManager.Watch(mNextFrameStatus, &MediaDecoder::UpdateReadyState); // ReadyState computation depends on MediaDecoder::CanPlayThrough, which // depends on the download rate. mWatchManager.Watch(mBuffered, &MediaDecoder::UpdateReadyState); @@ -452,6 +450,7 @@ MediaDecoder::Shutdown() mOnEncrypted.Disconnect(); mOnWaitingForKey.Disconnect(); mOnDecodeWarning.Disconnect(); + mOnNextFrameStatus.Disconnect(); mDecoderStateMachine->BeginShutdown() ->Then(mAbstractMainThread, __func__, this, @@ -557,6 +556,36 @@ MediaDecoder::OnDecoderDoctorEvent(DecoderDoctorEvent aEvent) diags.StoreEvent(doc, aEvent, __func__); } +static const char* +NextFrameStatusToStr(MediaDecoderOwner::NextFrameStatus aStatus) +{ + switch (aStatus) { + case MediaDecoderOwner::NEXT_FRAME_AVAILABLE: + return "NEXT_FRAME_AVAILABLE"; + case MediaDecoderOwner::NEXT_FRAME_UNAVAILABLE: + return "NEXT_FRAME_UNAVAILABLE"; + case MediaDecoderOwner::NEXT_FRAME_UNAVAILABLE_BUFFERING: + return "NEXT_FRAME_UNAVAILABLE_BUFFERING"; + case MediaDecoderOwner::NEXT_FRAME_UNAVAILABLE_SEEKING: + return "NEXT_FRAME_UNAVAILABLE_SEEKING"; + case MediaDecoderOwner::NEXT_FRAME_UNINITIALIZED: + return "NEXT_FRAME_UNINITIALIZED"; + } + return "UNKNOWN"; +} + +void +MediaDecoder::OnNextFrameStatus(MediaDecoderOwner::NextFrameStatus aStatus) +{ + MOZ_ASSERT(NS_IsMainThread()); + MOZ_DIAGNOSTIC_ASSERT(!IsShutdown()); + if (mNextFrameStatus != aStatus) { + LOG("Changed mNextFrameStatus to %s", NextFrameStatusToStr(aStatus)); + mNextFrameStatus = aStatus; + UpdateReadyState(); + } +} + void MediaDecoder::FinishShutdown() { @@ -606,6 +635,8 @@ MediaDecoder::SetStateMachineParameters() mAbstractMainThread, this, &MediaDecoder::OnDecoderDoctorEvent); mOnMediaNotSeekable = mDecoderStateMachine->OnMediaNotSeekable().Connect( mAbstractMainThread, this, &MediaDecoder::OnMediaNotSeekable); + mOnNextFrameStatus = mDecoderStateMachine->OnNextFrameStatus().Connect( + mAbstractMainThread, this, &MediaDecoder::OnNextFrameStatus); mOnEncrypted = mReader->OnEncrypted().Connect( mAbstractMainThread, GetOwner(), &MediaDecoderOwner::DispatchEncrypted); @@ -1247,7 +1278,6 @@ MediaDecoder::ConnectMirrors(MediaDecoderStateMachine* aObject) MOZ_ASSERT(aObject); mStateMachineDuration.Connect(aObject->CanonicalDuration()); mBuffered.Connect(aObject->CanonicalBuffered()); - mNextFrameStatus.Connect(aObject->CanonicalNextFrameStatus()); mCurrentPosition.Connect(aObject->CanonicalCurrentPosition()); mPlaybackPosition.Connect(aObject->CanonicalPlaybackOffset()); mIsAudioDataAudible.Connect(aObject->CanonicalIsAudioDataAudible()); @@ -1259,7 +1289,6 @@ MediaDecoder::DisconnectMirrors() MOZ_ASSERT(NS_IsMainThread()); mStateMachineDuration.DisconnectIfConnected(); mBuffered.DisconnectIfConnected(); - mNextFrameStatus.DisconnectIfConnected(); mCurrentPosition.DisconnectIfConnected(); mPlaybackPosition.DisconnectIfConnected(); mIsAudioDataAudible.DisconnectIfConnected(); diff --git a/dom/media/MediaDecoder.h b/dom/media/MediaDecoder.h index 6d0d5e3787f5..90e237e7fbec 100644 --- a/dom/media/MediaDecoder.h +++ b/dom/media/MediaDecoder.h @@ -494,6 +494,8 @@ private: mMediaSeekable = false; } + void OnNextFrameStatus(MediaDecoderOwner::NextFrameStatus); + void FinishShutdown(); void ConnectMirrors(MediaDecoderStateMachine* aObject); @@ -579,6 +581,9 @@ protected: // disabled. bool mHasSuspendTaint; + MediaDecoderOwner::NextFrameStatus mNextFrameStatus = + MediaDecoderOwner::NEXT_FRAME_UNAVAILABLE; + // A listener to receive metadata updates from MDSM. MediaEventListener mTimedMetadataListener; @@ -592,6 +597,7 @@ protected: MediaEventListener mOnEncrypted; MediaEventListener mOnWaitingForKey; MediaEventListener mOnDecodeWarning; + MediaEventListener mOnNextFrameStatus; protected: // PlaybackRate and pitch preservation status we should start at. @@ -600,9 +606,6 @@ protected: // Buffered range, mirrored from the reader. Mirror mBuffered; - // NextFrameStatus, mirrored from the state machine. - Mirror mNextFrameStatus; - // NB: Don't use mCurrentPosition directly, but rather CurrentPosition(). Mirror mCurrentPosition; diff --git a/dom/media/MediaDecoderStateMachine.cpp b/dom/media/MediaDecoderStateMachine.cpp index 04409c65ec42..994272291ee4 100644 --- a/dom/media/MediaDecoderStateMachine.cpp +++ b/dom/media/MediaDecoderStateMachine.cpp @@ -869,7 +869,7 @@ public: mMaster->StopPlayback(); mMaster->UpdatePlaybackPositionInternal(mSeekJob.mTarget->GetTime()); mMaster->mOnPlaybackEvent.Notify(MediaEventType::SeekStarted); - mMaster->UpdateNextFrameStatus( + mMaster->mOnNextFrameStatus.Notify( MediaDecoderOwner::NEXT_FRAME_UNAVAILABLE_SEEKING); } @@ -1144,8 +1144,9 @@ protected: MOZ_ASSERT_IF(aReject.mType == MediaData::VIDEO_DATA, !mMaster->IsWaitingVideoData()); // Fire 'waiting' to notify the player that we are waiting for data. - mMaster->UpdateNextFrameStatus( + mMaster->mOnNextFrameStatus.Notify( MediaDecoderOwner::NEXT_FRAME_UNAVAILABLE_SEEKING); + Reader() ->WaitForData(aReject.mType) ->Then(OwnerThread(), __func__, @@ -1795,7 +1796,7 @@ public: mBufferingStart = TimeStamp::Now(); mMaster->ScheduleStateMachineIn(TimeUnit::FromMicroseconds(USECS_PER_S)); - mMaster->UpdateNextFrameStatus( + mMaster->mOnNextFrameStatus.Notify( MediaDecoderOwner::NEXT_FRAME_UNAVAILABLE_BUFFERING); } @@ -1901,7 +1902,7 @@ public: bool hasNextFrame = (!mMaster->HasAudio() || !mMaster->mAudioCompleted) && (!mMaster->HasVideo() || !mMaster->mVideoCompleted); - mMaster->UpdateNextFrameStatus( + mMaster->mOnNextFrameStatus.Notify( hasNextFrame ? MediaDecoderOwner::NEXT_FRAME_AVAILABLE : MediaDecoderOwner::NEXT_FRAME_UNAVAILABLE); @@ -2281,7 +2282,7 @@ DecodingState::Enter() } }); - mMaster->UpdateNextFrameStatus(MediaDecoderOwner::NEXT_FRAME_AVAILABLE); + mMaster->mOnNextFrameStatus.Notify(MediaDecoderOwner::NEXT_FRAME_AVAILABLE); mDecodeStartTime = TimeStamp::Now(); @@ -2573,7 +2574,6 @@ ShutdownState::Enter() master->mMediaPrincipalHandle.DisconnectIfConnected(); master->mDuration.DisconnectAll(); - master->mNextFrameStatus.DisconnectAll(); master->mCurrentPosition.DisconnectAll(); master->mPlaybackOffset.DisconnectAll(); master->mIsAudioDataAudible.DisconnectAll(); @@ -2626,7 +2626,6 @@ MediaDecoderStateMachine::MediaDecoderStateMachine(MediaDecoder* aDecoder, INIT_MIRROR(mSameOriginMedia, false), INIT_MIRROR(mMediaPrincipalHandle, PRINCIPAL_HANDLE_NONE), INIT_CANONICAL(mDuration, NullableTimeUnit()), - INIT_CANONICAL(mNextFrameStatus, MediaDecoderOwner::NEXT_FRAME_UNAVAILABLE), INIT_CANONICAL(mCurrentPosition, TimeUnit::Zero()), INIT_CANONICAL(mPlaybackOffset, 0), INIT_CANONICAL(mIsAudioDataAudible, false) @@ -3487,34 +3486,6 @@ MediaDecoderStateMachine::UpdatePlaybackPositionPeriodically() ScheduleStateMachineIn(TimeUnit::FromMicroseconds(delay)); } -/* static */ const char* -MediaDecoderStateMachine::ToStr(NextFrameStatus aStatus) -{ - switch (aStatus) { - case MediaDecoderOwner::NEXT_FRAME_AVAILABLE: - return "NEXT_FRAME_AVAILABLE"; - case MediaDecoderOwner::NEXT_FRAME_UNAVAILABLE: - return "NEXT_FRAME_UNAVAILABLE"; - case MediaDecoderOwner::NEXT_FRAME_UNAVAILABLE_BUFFERING: - return "NEXT_FRAME_UNAVAILABLE_BUFFERING"; - case MediaDecoderOwner::NEXT_FRAME_UNAVAILABLE_SEEKING: - return "NEXT_FRAME_UNAVAILABLE_SEEKING"; - case MediaDecoderOwner::NEXT_FRAME_UNINITIALIZED: - return "NEXT_FRAME_UNINITIALIZED"; - } - return "UNKNOWN"; -} - -void -MediaDecoderStateMachine::UpdateNextFrameStatus(NextFrameStatus aStatus) -{ - MOZ_ASSERT(OnTaskQueue()); - if (aStatus != mNextFrameStatus) { - LOG("Changed mNextFrameStatus to %s", ToStr(aStatus)); - mNextFrameStatus = aStatus; - } -} - void MediaDecoderStateMachine::ScheduleStateMachine() { diff --git a/dom/media/MediaDecoderStateMachine.h b/dom/media/MediaDecoderStateMachine.h index c7a455994c8e..c0b3da68d2d5 100644 --- a/dom/media/MediaDecoderStateMachine.h +++ b/dom/media/MediaDecoderStateMachine.h @@ -252,6 +252,9 @@ public: MediaEventSource& OnDecoderDoctorEvent() { return mOnDecoderDoctorEvent; } + MediaEventSource& + OnNextFrameStatus() { return mOnNextFrameStatus; } + size_t SizeOfVideoQueue() const; size_t SizeOfAudioQueue() const; @@ -275,7 +278,6 @@ private: class ShutdownState; static const char* ToStateStr(State aState); - static const char* ToStr(NextFrameStatus aStatus); const char* ToStateStr(); nsCString GetDebugInfo(); @@ -383,8 +385,6 @@ protected: // Returns true if we have less than aThreshold of buffered data available. bool HasLowBufferedData(const media::TimeUnit& aThreshold); - void UpdateNextFrameStatus(NextFrameStatus aStatus); - // Return the current time, either the audio clock if available (if the media // has audio, and the playback is possible), or a clock for the video. // Called on the state machine thread. @@ -661,6 +661,8 @@ private: MediaEventProducer mOnDecoderDoctorEvent; + MediaEventProducer mOnNextFrameStatus; + const bool mIsMSE; private: @@ -692,10 +694,6 @@ private: // decoding the first frame. Canonical mDuration; - // The status of our next frame. Mirrored on the main thread and used to - // compute ready state. - Canonical mNextFrameStatus; - // 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. @@ -714,10 +712,6 @@ public: { return &mDuration; } - AbstractCanonical* CanonicalNextFrameStatus() - { - return &mNextFrameStatus; - } AbstractCanonical* CanonicalCurrentPosition() { return &mCurrentPosition;