From 8e86ed32b799fc967f727b7027e8db2f094766e2 Mon Sep 17 00:00:00 2001 From: Chris Pearce Date: Sat, 11 Sep 2010 11:29:11 +1200 Subject: [PATCH] Bug 589626 - Make video buffing logic consistent. r=kinetik a=blocking2.0 --- content/media/nsBuiltinDecoderReader.h | 9 ++ .../media/nsBuiltinDecoderStateMachine.cpp | 137 ++++++++++-------- content/media/nsBuiltinDecoderStateMachine.h | 11 ++ content/media/nsMediaDecoder.cpp | 26 +++- 4 files changed, 124 insertions(+), 59 deletions(-) diff --git a/content/media/nsBuiltinDecoderReader.h b/content/media/nsBuiltinDecoderReader.h index 8905d1c2524b..4ef5888b400d 100644 --- a/content/media/nsBuiltinDecoderReader.h +++ b/content/media/nsBuiltinDecoderReader.h @@ -330,6 +330,15 @@ template class MediaQueue : private nsDeque { return GetSize() == 0 && mEndOfStream; } + // Returns PR_TRUE if the media queue has had its last sample added to it. + // This happens when the media stream has been completely decoded. Note this + // does not mean that the corresponding stream has finished playback. + PRBool IsFinished() { + MonitorAutoEnter mon(mMonitor); + return mEndOfStream; + } + + // Informs the media queue that it won't be receiving any more samples. void Finish() { MonitorAutoEnter mon(mMonitor); mEndOfStream = PR_TRUE; diff --git a/content/media/nsBuiltinDecoderStateMachine.cpp b/content/media/nsBuiltinDecoderStateMachine.cpp index 531ea5950390..f0e6f0028044 100644 --- a/content/media/nsBuiltinDecoderStateMachine.cpp +++ b/content/media/nsBuiltinDecoderStateMachine.cpp @@ -99,6 +99,11 @@ const PRUint32 SILENCE_BYTES_CHUNK = 32 * 1024; // less than LOW_VIDEO_FRAMES frames. static const PRUint32 LOW_VIDEO_FRAMES = 1; +// If we've got more than AMPLE_VIDEO_FRAMES decoded video frames waiting in +// the video queue, we will not decode any more video frames until some have +// been consumed by the play state machine thread. +static const PRUint32 AMPLE_VIDEO_FRAMES = 10; + // Arbitrary "frame duration" when playing only audio. static const int AUDIO_DURATION_MS = 40; @@ -162,19 +167,20 @@ nsBuiltinDecoderStateMachine::~nsBuiltinDecoderStateMachine() PRBool nsBuiltinDecoderStateMachine::HasFutureAudio() const { mDecoder->GetMonitor().AssertCurrentThreadIn(); - PRBool aboveLowAudioThreshold = PR_FALSE; - if (mAudioEndTime != -1) { - aboveLowAudioThreshold = mAudioEndTime - GetMediaTime() > LOW_AUDIO_MS; - } - return HasAudio() && - !mAudioCompleted && - (mReader->mAudioQueue.GetSize() > 0 || aboveLowAudioThreshold); + NS_ASSERTION(HasAudio(), "Should only call HasFutureAudio() when we have audio"); + // We've got audio ready to play if: + // 1. We've not completed playback of audio, and + // 2. we either have more than the threshold of decoded audio available, or + // we've completely decoded all audio (but not finished playing it yet + // as per 1). + return !mAudioCompleted && + (AudioDecodedMs() > LOW_AUDIO_MS || mReader->mAudioQueue.IsFinished()); } PRBool nsBuiltinDecoderStateMachine::HaveNextFrameData() const { - return ((!HasAudio() || mReader->mAudioQueue.AtEndOfStream()) && - mReader->mVideoQueue.GetSize() > 0) || - HasFutureAudio(); + mDecoder->GetMonitor().AssertCurrentThreadIn(); + return (!HasAudio() || HasFutureAudio()) && + (!HasVideo() || mReader->mVideoQueue.GetSize() > 0); } void nsBuiltinDecoderStateMachine::DecodeLoop() @@ -201,12 +207,7 @@ void nsBuiltinDecoderStateMachine::DecodeLoop() // Once we've decoded more than videoPumpThreshold video frames, we'll // no longer be considered to be "pumping video". - const unsigned videoPumpThreshold = 5; - - // If we've got more than videoWaitThreshold decoded video frames waiting in - // the video queue, we will not decode any more video frames until they've - // been consumed by the play state machine thread. - const unsigned videoWaitThreshold = 10; + const unsigned videoPumpThreshold = AMPLE_VIDEO_FRAMES / 2; // After the audio decode fills with more than audioPumpThresholdMs ms // of decoded audio, we'll start to check whether the audio or video decode @@ -221,12 +222,6 @@ void nsBuiltinDecoderStateMachine::DecodeLoop() // Wait for more data to download if we've exhausted all our // buffered data. MonitorAutoEnter mon(mDecoder->GetMonitor()); - while (!mStopDecodeThreads && - mBufferExhausted && - mState != DECODER_STATE_SHUTDOWN) - { - mon.Wait(); - } if (mState == DECODER_STATE_SHUTDOWN || mStopDecodeThreads) break; } @@ -234,7 +229,7 @@ void nsBuiltinDecoderStateMachine::DecodeLoop() PRUint32 videoQueueSize = mReader->mVideoQueue.GetSize(); // Don't decode any more frames if we've filled our buffers. // Limits memory consumption. - if (videoQueueSize > videoWaitThreshold) { + if (videoQueueSize > AMPLE_VIDEO_FRAMES) { videoWait = PR_TRUE; } @@ -244,7 +239,8 @@ void nsBuiltinDecoderStateMachine::DecodeLoop() if (videoPump && videoQueueSize >= videoPumpThreshold) { videoPump = PR_FALSE; } - if (!videoPump && + if (audioPlaying && + !videoPump && videoPlaying && videoQueueSize < LOW_VIDEO_FRAMES) { @@ -283,9 +279,7 @@ void nsBuiltinDecoderStateMachine::DecodeLoop() videoPlaying = mReader->DecodeVideoFrame(skipToNextKeyframe, currentTime); { MonitorAutoEnter mon(mDecoder->GetMonitor()); - if (mDecoder->mDecoderPosition > initialDownloadPosition) { - mBufferExhausted = PR_TRUE; - } + mBufferExhausted = mDecoder->mDecoderPosition > initialDownloadPosition; } } { @@ -299,9 +293,7 @@ void nsBuiltinDecoderStateMachine::DecodeLoop() audioPlaying = mReader->DecodeAudioData(); { MonitorAutoEnter mon(mDecoder->GetMonitor()); - if (mDecoder->mDecoderPosition > initialDownloadPosition) { - mBufferExhausted = PR_TRUE; - } + mBufferExhausted = mDecoder->mDecoderPosition > initialDownloadPosition; } } @@ -325,11 +317,15 @@ void nsBuiltinDecoderStateMachine::DecodeLoop() if (mState == DECODER_STATE_SHUTDOWN || mStopDecodeThreads) { break; } + if ((!HasAudio() || (audioWait && audioPlaying)) && (!HasVideo() || (videoWait && videoPlaying))) { // All active bitstreams' decode is well ahead of the playback - // position, we may as well wait have for the playback to catch up. + // position, we may as well wait for the playback to catch up. + // Set mBufferExhausted to PR_FALSE, as we'll receive more data + // while we wait. + mBufferExhausted = PR_FALSE; mon.Wait(); } } @@ -855,6 +851,39 @@ nsBuiltinDecoderStateMachine::StartDecodeThreads() return NS_OK; } +PRInt64 nsBuiltinDecoderStateMachine::AudioDecodedMs() const +{ + NS_ASSERTION(HasAudio(), + "Should only call AudioDecodedMs() when we have audio"); + // The amount of audio we have decoded is the amount of audio data we've + // already decoded and pushed to the hardware, plus the amount of audio + // data waiting to be pushed to the hardware. + PRInt64 pushed = (mAudioEndTime != -1) ? (mAudioEndTime - GetMediaTime()) : 0; + return pushed + mReader->mAudioQueue.Duration(); +} + +PRBool nsBuiltinDecoderStateMachine::HasLowDecodedData() const +{ + // We consider ourselves low on decoded data if we're low on audio, or + // if we're only playing video and we're low on video frames. + return (HasAudio() && AudioDecodedMs() < LOW_AUDIO_MS) + || + (!HasAudio() && + HasVideo() && + (PRUint32)mReader->mVideoQueue.GetSize() < LOW_VIDEO_FRAMES); +} + +PRBool nsBuiltinDecoderStateMachine::HasAmpleDecodedData() const +{ + return (!HasAudio() || + AudioDecodedMs() >= AMPLE_AUDIO_MS || + mReader->mAudioQueue.IsFinished()) + && + (!HasVideo() || + (PRUint32)mReader->mVideoQueue.GetSize() > AMPLE_VIDEO_FRAMES || + mReader->mVideoQueue.AtEndOfStream()); +} + nsresult nsBuiltinDecoderStateMachine::Run() { NS_ASSERTION(IsCurrentThread(mDecoder->mStateMachineThread), @@ -945,25 +974,16 @@ nsresult nsBuiltinDecoderStateMachine::Run() continue; if (mBufferExhausted && + HasLowDecodedData() && mDecoder->GetState() == nsBuiltinDecoder::PLAY_STATE_PLAYING && - !mDecoder->GetCurrentStream()->IsDataCachedToEndOfStream(mDecoder->mDecoderPosition) && - !mDecoder->GetCurrentStream()->IsSuspendedByCache() && - ((HasAudio() && mReader->mAudioQueue.Duration() < LOW_AUDIO_MS) || - (HasVideo() && (PRUint32)mReader->mVideoQueue.GetSize() < LOW_VIDEO_FRAMES))) + !stream->IsDataCachedToEndOfStream(mDecoder->mDecoderPosition) && + !stream->IsSuspended()) { - // There is at most one frame in the queue and there's - // more data to load. Let's buffer to make sure we can play a - // decent amount of video in the future. + // We're low on decoded data, and/or our decode has caught up with + // the download. Let's buffer to make sure we can play a decent + // amount of video in the future. StartBuffering(); - } else { - if (mBufferExhausted) { - // This will wake up the decode thread and force it to try to - // decode video and audio. This guarantees we make progress. - mBufferExhausted = PR_FALSE; - mDecoder->GetMonitor().NotifyAll(); - } } - } break; @@ -1066,17 +1086,20 @@ nsresult nsBuiltinDecoderStateMachine::Run() case DECODER_STATE_BUFFERING: { - TimeStamp now = TimeStamp::Now(); - nsMediaStream* stream = mDecoder->GetCurrentStream(); - if (!mDecoder->CanPlayThrough() && - now - mBufferingStart < TimeDuration::FromSeconds(BUFFERING_WAIT) && - stream->GetCachedDataEnd(mDecoder->mDecoderPosition) < mBufferingEndOffset && - !stream->IsDataCachedToEndOfStream(mDecoder->mDecoderPosition) && - !stream->IsSuspendedByCache()) { + // We will remain in the buffering state if we've not decoded enough + // data to begin playback, or if we've not downloaded a reasonable + // amount of data inside our buffering time. + TimeDuration elapsed = TimeStamp::Now() - mBufferingStart; + if ((!mDecoder->CanPlayThrough() || !HasAmpleDecodedData()) && + elapsed < TimeDuration::FromSeconds(BUFFERING_WAIT) && + stream->GetCachedDataEnd(mDecoder->mDecoderPosition) < mBufferingEndOffset && + !stream->IsDataCachedToEndOfStream(mDecoder->mDecoderPosition) && + !stream->IsSuspended()) + { LOG(PR_LOG_DEBUG, - ("In buffering: buffering data until %d bytes available or %f seconds", - PRUint32(mBufferingEndOffset - mDecoder->GetCurrentStream()->GetCachedDataEnd(mDecoder->mDecoderPosition)), - BUFFERING_WAIT - (now - mBufferingStart).ToSeconds())); + ("In buffering: buffering data until %u bytes available or %f seconds", + PRUint32(mBufferingEndOffset - stream->GetCachedDataEnd(mDecoder->mDecoderPosition)), + BUFFERING_WAIT - elapsed.ToSeconds())); Wait(1000); if (mState == DECODER_STATE_SHUTDOWN) continue; @@ -1089,7 +1112,6 @@ nsresult nsBuiltinDecoderStateMachine::Run() } if (mState != DECODER_STATE_BUFFERING) { - mBufferExhausted = PR_FALSE; // Notify to allow blocked decoder thread to continue mDecoder->GetMonitor().NotifyAll(); UpdateReadyState(); @@ -1099,7 +1121,6 @@ nsresult nsBuiltinDecoderStateMachine::Run() } } } - break; } diff --git a/content/media/nsBuiltinDecoderStateMachine.h b/content/media/nsBuiltinDecoderStateMachine.h index ddc1e003ec0f..9e696158a25f 100644 --- a/content/media/nsBuiltinDecoderStateMachine.h +++ b/content/media/nsBuiltinDecoderStateMachine.h @@ -242,6 +242,17 @@ public: protected: + // Returns the number of unplayed ms of audio we've got decoded and/or + // pushed to the hardware waiting to play. This is how much audio we can + // play without having to run the audio decoder. + PRInt64 AudioDecodedMs() const; + + // Returns PR_TRUE if we're running low on decoded data. + PRBool HasLowDecodedData() const; + + // Returns PR_TRUE if we've got plenty of decoded data. + PRBool HasAmpleDecodedData() const; + // Returns PR_TRUE when there's decoded audio waiting to play. // The decoder monitor must be held. PRBool HasFutureAudio() const; diff --git a/content/media/nsMediaDecoder.cpp b/content/media/nsMediaDecoder.cpp index 64bbeff52a31..607330f4ec5b 100644 --- a/content/media/nsMediaDecoder.cpp +++ b/content/media/nsMediaDecoder.cpp @@ -68,6 +68,14 @@ // Number of milliseconds of no data before a stall event is fired as defined by spec #define STALL_MS 3000 +// Number of estimated seconds worth of data we need to have buffered +// ahead of the current playback position before we allow the media decoder +// to report that it can play through the entire media without the decode +// catching up with the download. Having this margin make the +// nsMediaDecoder::CanPlayThrough() calculation more stable in the case of +// fluctuating bitrates. +#define CAN_PLAY_THROUGH_MARGIN 20 + nsMediaDecoder::nsMediaDecoder() : mElement(0), mRGBWidth(-1), @@ -289,5 +297,21 @@ PRBool nsMediaDecoder::CanPlayThrough() double timeToDownload = (bytesToDownload + gDownloadSizeSafetyMargin)/stats.mDownloadRate; double timeToPlay = bytesToPlayback/stats.mPlaybackRate; - return timeToDownload <= timeToPlay; + + if (timeToDownload > timeToPlay) { + // Estimated time to download is greater than the estimated time to play. + // We probably can't play through without having to stop to buffer. + return PR_FALSE; + } + + // Estimated time to download is less than the estimated time to play. + // We can probably play through without having to buffer, but ensure that + // we've got a reasonable amount of data buffered after the current + // playback position, so that if the bitrate of the media fluctuates, or if + // our download rate or decode rate estimation is otherwise inaccurate, + // we don't suddenly discover that we need to buffer. This is particularly + // required near the start of the media, when not much data is downloaded. + PRInt64 readAheadMargin = stats.mPlaybackRate * CAN_PLAY_THROUGH_MARGIN; + return stats.mTotalBytes == stats.mDownloadPosition || + stats.mDownloadPosition > stats.mPlaybackPosition + readAheadMargin; }