diff --git a/content/media/nsBuiltinDecoderStateMachine.cpp b/content/media/nsBuiltinDecoderStateMachine.cpp index 51e211838e4a..796865e81b3a 100644 --- a/content/media/nsBuiltinDecoderStateMachine.cpp +++ b/content/media/nsBuiltinDecoderStateMachine.cpp @@ -82,6 +82,10 @@ extern PRLogModuleInfo* gBuiltinDecoderLog; // less than LOW_VIDEO_FRAMES frames. static const PRUint32 LOW_AUDIO_MS = 100; +// If more than this many ms of decoded audio is queued, we'll hold off +// decoding more audio. +const unsigned AMPLE_AUDIO_MS = 2000; + // If we have fewer than LOW_VIDEO_FRAMES decoded frames, and // we're not "pumping video", we'll skip the video up to the next keyframe // which is at or after the current playback position. @@ -128,6 +132,20 @@ nsBuiltinDecoderStateMachine::~nsBuiltinDecoderStateMachine() MOZ_COUNT_DTOR(nsBuiltinDecoderStateMachine); } +PRBool nsBuiltinDecoderStateMachine::HasFutureAudio() const { + mDecoder->GetMonitor().AssertCurrentThreadIn(); + return HasAudio() && + !mAudioCompleted && + (mReader->mAudioQueue.GetSize() > 0 || + mAudioEndTime - mCurrentFrameTime + mStartTime > LOW_AUDIO_MS); +} + +PRBool nsBuiltinDecoderStateMachine::HaveNextFrameData() const { + return ((!HasAudio() || mReader->mAudioQueue.AtEndOfStream()) && + mReader->mVideoQueue.GetSize() > 0) || + HasFutureAudio(); +} + void nsBuiltinDecoderStateMachine::DecodeLoop() { NS_ASSERTION(OnDecodeThread(), "Should be on decode thread."); @@ -164,10 +182,6 @@ void nsBuiltinDecoderStateMachine::DecodeLoop() // is falling behind. const unsigned audioPumpThresholdMs = 250; - // If more than this many ms of decoded audio is queued, we'll hold off - // decoding more audio. - const unsigned audioWaitThresholdMs = 2000; - // Main decode loop. while (videoPlaying || audioPlaying) { PRBool audioWait = !audioPlaying; @@ -206,23 +220,24 @@ void nsBuiltinDecoderStateMachine::DecodeLoop() skipToNextKeyframe = PR_TRUE; } - PRInt64 initialDownloadPosition = 0; - PRInt64 currentTime = 0; - { - MonitorAutoEnter mon(mDecoder->GetMonitor()); - initialDownloadPosition = - mDecoder->GetCurrentStream()->GetCachedDataEnd(mDecoder->mDecoderPosition); - currentTime = mCurrentFrameTime + mStartTime; - } - // Determine how much audio data is decoded ahead of the current playback // position. int audioQueueSize = mReader->mAudioQueue.GetSize(); - PRInt64 audioDecoded = mReader->mAudioQueue.Duration(); + PRInt64 initialDownloadPosition = 0; + PRInt64 currentTime = 0; + PRInt64 audioDecoded = 0; + { + MonitorAutoEnter mon(mDecoder->GetMonitor()); + currentTime = mCurrentFrameTime + mStartTime; + audioDecoded = mReader->mAudioQueue.Duration() + + mAudioEndTime - currentTime; + initialDownloadPosition = + mDecoder->GetCurrentStream()->GetCachedDataEnd(mDecoder->mDecoderPosition); + } // Don't decode any audio if the audio decode is way ahead, or if we're // skipping to the next video keyframe and the audio is marginally ahead. - if (audioDecoded > audioWaitThresholdMs || + if (audioDecoded > AMPLE_AUDIO_MS || (skipToNextKeyframe && audioDecoded > audioPumpThresholdMs)) { audioWait = PR_TRUE; } @@ -365,6 +380,7 @@ void nsBuiltinDecoderStateMachine::AudioLoop() LOG(PR_LOG_DEBUG, ("First audio sample has timestamp %lldms", mAudioStartTime)); } + PRInt64 audioEndTime = -1; { MonitorAutoEnter audioMon(mAudioMonitor); if (mAudioStream) { @@ -379,7 +395,7 @@ void nsBuiltinDecoderStateMachine::AudioLoop() mAudioStream->Write(sound->mAudioData, sound->AudioDataLength(), PR_TRUE); - mAudioEndTime = sound->mTime + sound->mDuration; + audioEndTime = sound->mTime + sound->mDuration; mDecoder->UpdatePlaybackOffset(sound->mOffset); } else { mReader->mAudioQueue.PushFront(sound); @@ -389,19 +405,47 @@ void nsBuiltinDecoderStateMachine::AudioLoop() } sound = nsnull; - if (mReader->mAudioQueue.AtEndOfStream()) { - // Last sample pushed to audio hardware, wait for the audio to finish, - // before the audio thread terminates. - MonitorAutoEnter audioMon(mAudioMonitor); - if (mAudioStream) { - mAudioStream->Drain(); + { + MonitorAutoEnter mon(mDecoder->GetMonitor()); + if (audioEndTime != -1) { + mAudioEndTime = audioEndTime; + } + PRInt64 audioAhead = mAudioEndTime - mCurrentFrameTime - mStartTime; + if (audioAhead > AMPLE_AUDIO_MS) { + // We've pushed enough audio onto the hardware that we've queued up a + // significant amount ahead of the playback position. The decode + // thread will be going to sleep, so we won't get any new samples + // anyway, so sleep until we need to push to the hardware again. + Wait(AMPLE_AUDIO_MS / 2); + // Kick the decode thread; since above we only do a NotifyAll when + // we pop an audio chunk of the queue, the decoder won't wake up if + // we've got no more decoded chunks to push to the hardware. We can + // hit this condition if the last sample in the stream doesn't have + // it's EOS flag set, and the decode thread sleeps just after decoding + // that packet, but before realising there's no more packets. + mon.NotifyAll(); } - LOG(PR_LOG_DEBUG, ("%p Reached audio stream end.", mDecoder)); } } + if (mReader->mAudioQueue.AtEndOfStream() && + mState != DECODER_STATE_SHUTDOWN && + !mStopDecodeThreads) + { + // Last sample pushed to audio hardware, wait for the audio to finish, + // before the audio thread terminates. + MonitorAutoEnter audioMon(mAudioMonitor); + if (mAudioStream) { + mAudioStream->Drain(); + } + LOG(PR_LOG_DEBUG, ("%p Reached audio stream end.", mDecoder)); + } { MonitorAutoEnter mon(mDecoder->GetMonitor()); mAudioCompleted = PR_TRUE; + UpdateReadyState(); + // Kick the decode and state machine threads; they may be sleeping waiting + // for this to finish. + mDecoder->GetMonitor().NotifyAll(); } LOG(PR_LOG_DEBUG, ("Audio stream finished playing, audio thread exit")); } @@ -939,13 +983,14 @@ nsresult nsBuiltinDecoderStateMachine::Run() continue; } - // Play the remaining media. - while (mState == DECODER_STATE_COMPLETED && - (mReader->mVideoQueue.GetSize() > 0 || - (HasAudio() && !mAudioCompleted))) - { + // Play the remaining media. We want to run AdvanceFrame() at least + // once to ensure the current playback position is advanced to the + // end of the media, and so that we update the readyState. + do { AdvanceFrame(); - } + } while (mState == DECODER_STATE_COMPLETED && + (mReader->mVideoQueue.GetSize() > 0 || + (HasAudio() && !mAudioCompleted))); if (mAudioStream) { // Close the audop stream so that next time audio is used a new stream diff --git a/content/media/nsBuiltinDecoderStateMachine.h b/content/media/nsBuiltinDecoderStateMachine.h index aff3fd074dad..bfa7b42f4010 100644 --- a/content/media/nsBuiltinDecoderStateMachine.h +++ b/content/media/nsBuiltinDecoderStateMachine.h @@ -193,12 +193,7 @@ public: } // Should be called by main thread. - PRBool HaveNextFrameData() const { - PRUint32 audioQueueSize = mReader->mAudioQueue.GetSize(); - return (mReader->mVideoQueue.GetSize() > 0 && - (!HasAudio() || audioQueueSize > 0)) || - audioQueueSize > 0; - } + PRBool HaveNextFrameData() const; // Must be called with the decode monitor held. PRBool IsBuffering() const { @@ -240,6 +235,10 @@ public: protected: + // Returns PR_TRUE when there's decoded audio waiting to play. + // The decoder monitor must be held. + PRBool HasFutureAudio() const; + // Waits on the decoder Monitor for aMs. If the decoder monitor is awoken // by a Notify() call, we'll continue waiting, unless we've moved into // shutdown state. This enables us to ensure that we wait for a specified diff --git a/content/media/test/manifest.js b/content/media/test/manifest.js index 8c3cb5f526a7..cda37542f5d3 100644 --- a/content/media/test/manifest.js +++ b/content/media/test/manifest.js @@ -37,8 +37,9 @@ var gPlayTests = [ // file with list chunk { name:"r16000_u8_c1_list.wav", type:"audio/x-wav", duration:4.2 }, - // Ogg stream with eof marker + // Ogg stream without eof marker { name:"bug461281.ogg", type:"application/ogg" }, + // oggz-chop stream { name:"bug482461.ogv", type:"video/ogg", duration:4.34 }, // With first frame a "duplicate" (empty) frame.