From a768f5dfdf9e8ed808bc8681dace987f06ca46c3 Mon Sep 17 00:00:00 2001 From: Matthew Gregan Date: Mon, 15 Mar 2010 12:46:38 +1300 Subject: [PATCH] Bug 525401 - Make ready state transitions initiated by the decode thread more reliable by including the new state in the event. Fixes an ABA problem where we could play through without ever moving beyond HAVE_CURRENT_DATA. r=chris.double --- content/media/ogg/nsOggDecoder.cpp | 98 +++++++++++++++++++++------- content/media/ogg/nsOggDecoder.h | 6 ++ content/media/wave/nsWaveDecoder.cpp | 66 +++++++++++++++++-- content/media/wave/nsWaveDecoder.h | 6 ++ 4 files changed, 144 insertions(+), 32 deletions(-) diff --git a/content/media/ogg/nsOggDecoder.cpp b/content/media/ogg/nsOggDecoder.cpp index 19b41cde3a2..3e3a4c57ad3 100644 --- a/content/media/ogg/nsOggDecoder.cpp +++ b/content/media/ogg/nsOggDecoder.cpp @@ -371,6 +371,9 @@ public: // be called with the decode monitor held. void ClearPositionChangeFlag(); + // Called by decoder and main thread. + nsHTMLMediaElement::NextFrameStatus GetNextFrameStatus(); + // Must be called with the decode monitor held. Can be called by main // thread. PRBool HaveNextFrameData() const { @@ -407,6 +410,27 @@ protected: void HandleVideoData(FrameData* aFrame, int aTrackNum, OggPlayDataHeader* aVideoHeader); void HandleAudioData(FrameData* aFrame, OggPlayAudioData* aAudioData, int aSize); + void UpdateReadyState() { + PR_ASSERT_CURRENT_THREAD_IN_MONITOR(mDecoder->GetMonitor()); + + nsCOMPtr event; + switch (GetNextFrameStatus()) { + case nsHTMLMediaElement::NEXT_FRAME_UNAVAILABLE_BUFFERING: + event = NS_NEW_RUNNABLE_METHOD(nsOggDecoder, mDecoder, NextFrameUnavailableBuffering); + break; + case nsHTMLMediaElement::NEXT_FRAME_AVAILABLE: + event = NS_NEW_RUNNABLE_METHOD(nsOggDecoder, mDecoder, NextFrameAvailable); + break; + case nsHTMLMediaElement::NEXT_FRAME_UNAVAILABLE: + event = NS_NEW_RUNNABLE_METHOD(nsOggDecoder, mDecoder, NextFrameUnavailable); + break; + default: + PR_NOT_REACHED("unhandled frame state"); + } + + NS_DispatchToMainThread(event, NS_DISPATCH_NORMAL); + } + // These methods can only be called on the decoding thread. void LoadOggHeaders(nsChannelReader* aReader); @@ -1149,17 +1173,16 @@ void nsOggDecodeStateMachine::QueueDecodedFrames() PR_ASSERT_CURRENT_THREAD_IN_MONITOR(mDecoder->GetMonitor()); FrameData* frame; while (!mDecodedFrames.IsFull() && (frame = NextFrame())) { - if (mDecodedFrames.GetCount() < 2) { + PRUint32 oldFrameCount = mDecodedFrames.GetCount(); + mDecodedFrames.Push(frame); + if (oldFrameCount < 2) { // Transitioning from 0 to 1 frames or from 1 to 2 frames could // affect HaveNextFrameData and hence what UpdateReadyStateForData does. // This could change us from HAVE_CURRENT_DATA to HAVE_FUTURE_DATA // (or even HAVE_ENOUGH_DATA), so we'd better trigger an - // UpdateReadyStateForData. - nsCOMPtr event = - NS_NEW_RUNNABLE_METHOD(nsOggDecoder, mDecoder, UpdateReadyStateForData); - NS_DispatchToMainThread(event, NS_DISPATCH_NORMAL); + // update to the ready state. + UpdateReadyState(); } - mDecodedFrames.Push(frame); } } @@ -1169,6 +1192,17 @@ void nsOggDecodeStateMachine::ClearPositionChangeFlag() mPositionChangeQueued = PR_FALSE; } +nsHTMLMediaElement::NextFrameStatus nsOggDecodeStateMachine::GetNextFrameStatus() +{ + nsAutoMonitor mon(mDecoder->GetMonitor()); + if (IsBuffering() || IsSeeking()) { + return nsHTMLMediaElement::NEXT_FRAME_UNAVAILABLE_BUFFERING; + } else if (HaveNextFrameData()) { + return nsHTMLMediaElement::NEXT_FRAME_AVAILABLE; + } + return nsHTMLMediaElement::NEXT_FRAME_UNAVAILABLE; +} + void nsOggDecodeStateMachine::SetVolume(float volume) { PR_ASSERT_CURRENT_THREAD_IN_MONITOR(mDecoder->GetMonitor()); @@ -1554,9 +1588,7 @@ nsresult nsOggDecodeStateMachine::Run() // we just trigger UpdateReadyStateForData; when it runs, it // will check the current state and decide whether to tell // the element we're buffering or not. - nsCOMPtr event = - NS_NEW_RUNNABLE_METHOD(nsOggDecoder, mDecoder, UpdateReadyStateForData); - NS_DispatchToMainThread(event, NS_DISPATCH_NORMAL); + UpdateReadyState(); mBufferingStart = TimeStamp::Now(); PRPackedBool reliable; @@ -1699,9 +1731,7 @@ nsresult nsOggDecodeStateMachine::Run() mBufferExhausted = PR_FALSE; // Notify to allow blocked decoder thread to continue mon.NotifyAll(); - nsCOMPtr event = - NS_NEW_RUNNABLE_METHOD(nsOggDecoder, mDecoder, UpdateReadyStateForData); - NS_DispatchToMainThread(event, NS_DISPATCH_NORMAL); + UpdateReadyState(); if (mDecoder->GetState() == nsOggDecoder::PLAY_STATE_PLAYING) { if (!mPlaying) { ResumePlayback(); @@ -2390,23 +2420,41 @@ void nsOggDecoder::NotifyBytesConsumed(PRInt64 aBytes) } } -void nsOggDecoder::UpdateReadyStateForData() +void nsOggDecoder::NextFrameUnavailableBuffering() { + NS_ASSERTION(NS_IsMainThread(), "Should be called on main thread"); if (!mElement || mShuttingDown || !mDecodeStateMachine) return; - nsHTMLMediaElement::NextFrameStatus frameStatus; - { - nsAutoMonitor mon(mMonitor); - if (mDecodeStateMachine->IsBuffering() || - mDecodeStateMachine->IsSeeking()) { - frameStatus = nsHTMLMediaElement::NEXT_FRAME_UNAVAILABLE_BUFFERING; - } else if (mDecodeStateMachine->HaveNextFrameData()) { - frameStatus = nsHTMLMediaElement::NEXT_FRAME_AVAILABLE; - } else { - frameStatus = nsHTMLMediaElement::NEXT_FRAME_UNAVAILABLE; - } - } + mElement->UpdateReadyStateForData(nsHTMLMediaElement::NEXT_FRAME_UNAVAILABLE_BUFFERING); +} + +void nsOggDecoder::NextFrameAvailable() +{ + NS_ASSERTION(NS_IsMainThread(), "Should be called on main thread"); + if (!mElement || mShuttingDown || !mDecodeStateMachine) + return; + + mElement->UpdateReadyStateForData(nsHTMLMediaElement::NEXT_FRAME_AVAILABLE); +} + +void nsOggDecoder::NextFrameUnavailable() +{ + NS_ASSERTION(NS_IsMainThread(), "Should be called on main thread"); + if (!mElement || mShuttingDown || !mDecodeStateMachine) + return; + + mElement->UpdateReadyStateForData(nsHTMLMediaElement::NEXT_FRAME_UNAVAILABLE); +} + +void nsOggDecoder::UpdateReadyStateForData() +{ + NS_ASSERTION(NS_IsMainThread(), "Should be called on main thread"); + if (!mElement || mShuttingDown || !mDecodeStateMachine) + return; + + nsHTMLMediaElement::NextFrameStatus frameStatus = + mDecodeStateMachine->GetNextFrameStatus(); mElement->UpdateReadyStateForData(frameStatus); } diff --git a/content/media/ogg/nsOggDecoder.h b/content/media/ogg/nsOggDecoder.h index 58b0f81e7ac..3eaba083610 100644 --- a/content/media/ogg/nsOggDecoder.h +++ b/content/media/ogg/nsOggDecoder.h @@ -459,6 +459,12 @@ protected: // This must be called on the main thread only. void PlaybackPositionChanged(); + // Calls mElement->UpdateReadyStateForData, telling it which state we have + // entered. Main thread only. + void NextFrameUnavailableBuffering(); + void NextFrameAvailable(); + void NextFrameUnavailable(); + // Calls mElement->UpdateReadyStateForData, telling it whether we have // data for the next frame and if we're buffering. Main thread only. void UpdateReadyStateForData(); diff --git a/content/media/wave/nsWaveDecoder.cpp b/content/media/wave/nsWaveDecoder.cpp index 2256cc52f5f..6818368a6b7 100644 --- a/content/media/wave/nsWaveDecoder.cpp +++ b/content/media/wave/nsWaveDecoder.cpp @@ -161,7 +161,7 @@ public: // Called on the decoder thread void NotifyBytesConsumed(PRInt64 aBytes); - // Called by the main thread only + // Called by decoder and main thread. nsHTMLMediaElement::NextFrameStatus GetNextFrameStatus(); // Clear the flag indicating that a playback position change event is @@ -178,6 +178,27 @@ private: // this. PRBool ReadAll(char* aBuf, PRInt64 aSize, PRInt64* aBytesRead); + void UpdateReadyState() { + PR_ASSERT_CURRENT_THREAD_IN_MONITOR(mMonitor); + + nsCOMPtr event; + switch (GetNextFrameStatus()) { + case nsHTMLMediaElement::NEXT_FRAME_UNAVAILABLE_BUFFERING: + event = NS_NEW_RUNNABLE_METHOD(nsWaveDecoder, mDecoder, NextFrameUnavailableBuffering); + break; + case nsHTMLMediaElement::NEXT_FRAME_AVAILABLE: + event = NS_NEW_RUNNABLE_METHOD(nsWaveDecoder, mDecoder, NextFrameAvailable); + break; + case nsHTMLMediaElement::NEXT_FRAME_UNAVAILABLE: + event = NS_NEW_RUNNABLE_METHOD(nsWaveDecoder, mDecoder, NextFrameUnavailable); + break; + default: + PR_NOT_REACHED("unhandled frame state"); + } + + NS_DispatchToMainThread(event, NS_DISPATCH_NORMAL); + } + // Change the current state and wake the playback thread if it is waiting // on mMonitor. Used by public member functions called from both threads, // so must hold mMonitor. Threadsafe. @@ -529,9 +550,7 @@ nsWaveStateMachine::Run() monitor.Wait(PR_MillisecondsToInterval(1000)); } else { ChangeState(mNextState); - nsCOMPtr event = - NS_NEW_RUNNABLE_METHOD(nsWaveDecoder, mDecoder, UpdateReadyStateForData); - NS_DispatchToMainThread(event, NS_DISPATCH_NORMAL); + UpdateReadyState(); } break; @@ -590,9 +609,7 @@ nsWaveStateMachine::Run() mNextState = mState; ChangeState(STATE_BUFFERING); - nsCOMPtr event = - NS_NEW_RUNNABLE_METHOD(nsWaveDecoder, mDecoder, UpdateReadyStateForData); - NS_DispatchToMainThread(event, NS_DISPATCH_NORMAL); + UpdateReadyState(); break; } @@ -1485,9 +1502,44 @@ nsWaveDecoder::Observe(nsISupports* aSubject, const char* aTopic, const PRUnicha return NS_OK; } +void +nsWaveDecoder::NextFrameUnavailableBuffering() +{ + NS_ASSERTION(NS_IsMainThread(), "Should be called on main thread"); + if (!mElement || mShuttingDown || !mPlaybackStateMachine) + return; + + mElement->UpdateReadyStateForData(nsHTMLMediaElement::NEXT_FRAME_UNAVAILABLE_BUFFERING); +} + +void +nsWaveDecoder::NextFrameAvailable() +{ + NS_ASSERTION(NS_IsMainThread(), "Should be called on main thread"); + if (!mElement || mShuttingDown || !mPlaybackStateMachine) + return; + + if (!mMetadataLoadedReported) { + mElement->UpdateReadyStateForData(nsHTMLMediaElement::NEXT_FRAME_UNAVAILABLE); + } else { + mElement->UpdateReadyStateForData(nsHTMLMediaElement::NEXT_FRAME_AVAILABLE); + } +} + +void +nsWaveDecoder::NextFrameUnavailable() +{ + NS_ASSERTION(NS_IsMainThread(), "Should be called on main thread"); + if (!mElement || mShuttingDown || !mPlaybackStateMachine) + return; + + mElement->UpdateReadyStateForData(nsHTMLMediaElement::NEXT_FRAME_UNAVAILABLE); +} + void nsWaveDecoder::UpdateReadyStateForData() { + NS_ASSERTION(NS_IsMainThread(), "Should be called on main thread"); if (!mElement || mShuttingDown || !mPlaybackStateMachine) return; diff --git a/content/media/wave/nsWaveDecoder.h b/content/media/wave/nsWaveDecoder.h index a139e5315a4..95874597515 100644 --- a/content/media/wave/nsWaveDecoder.h +++ b/content/media/wave/nsWaveDecoder.h @@ -221,6 +221,12 @@ class nsWaveDecoder : public nsMediaDecoder // main thread only. virtual void Resume(); + // Calls mElement->UpdateReadyStateForData, telling it which state we have + // entered. Main thread only. + void NextFrameUnavailableBuffering(); + void NextFrameAvailable(); + void NextFrameUnavailable(); + // Change the element's ready state as necessary. Main thread only. void UpdateReadyStateForData();