Bug 589626 - Make video buffing logic consistent. r=kinetik a=blocking2.0

content/media/nsBuiltinDecoderReader.h

@@ -330,6 +330,15 @@ template <class T> 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;    

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;
+  NS_ASSERTION(HasAudio(), "Should only call HasFutureAudio() when we have audio");
-  if (mAudioEndTime != -1) {
+  // We've got audio ready to play if:
-    aboveLowAudioThreshold = mAudioEndTime - GetMediaTime() > LOW_AUDIO_MS;
+  // 1. We've not completed playback of audio, and
-  }
+  // 2. we either have more than the threshold of decoded audio available, or
-  return HasAudio() &&
+  //    we've completely decoded all audio (but not finished playing it yet
-    !mAudioCompleted &&
+  //    as per 1).
-    (mReader->mAudioQueue.GetSize() > 0 || aboveLowAudioThreshold);
+  return !mAudioCompleted &&
+         (AudioDecodedMs() > LOW_AUDIO_MS || mReader->mAudioQueue.IsFinished());
 }
 
 PRBool nsBuiltinDecoderStateMachine::HaveNextFrameData() const {
-    return ((!HasAudio() || mReader->mAudioQueue.AtEndOfStream()) && 
+  mDecoder->GetMonitor().AssertCurrentThreadIn();
-             mReader->mVideoQueue.GetSize() > 0) ||
+  return (!HasAudio() || HasFutureAudio()) &&
-            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;
+  const unsigned videoPumpThreshold = AMPLE_VIDEO_FRAMES / 2;
-
-  // 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;
 
   // 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 = mDecoder->mDecoderPosition > initialDownloadPosition;
-          mBufferExhausted = PR_TRUE;
-        }
       }
     }
     {
@@ -299,9 +293,7 @@ void nsBuiltinDecoderStateMachine::DecodeLoop()
       audioPlaying = mReader->DecodeAudioData();
       {
         MonitorAutoEnter mon(mDecoder->GetMonitor());
-        if (mDecoder->mDecoderPosition > initialDownloadPosition) {
+        mBufferExhausted = mDecoder->mDecoderPosition > initialDownloadPosition;
-          mBufferExhausted = PR_TRUE;
-        }
       }
     }
 
@@ -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) &&
+            !stream->IsDataCachedToEndOfStream(mDecoder->mDecoderPosition) &&
-            !mDecoder->GetCurrentStream()->IsSuspendedByCache() &&
+            !stream->IsSuspended())
-            ((HasAudio() && mReader->mAudioQueue.Duration() < LOW_AUDIO_MS) ||
-             (HasVideo() && (PRUint32)mReader->mVideoQueue.GetSize() < LOW_VIDEO_FRAMES)))
         {
-          // There is at most one frame in the queue and there's
+          // We're low on decoded data, and/or our decode has caught up with
-          // more data to load. Let's buffer to make sure we can play a
+          // the download. Let's buffer to make sure we can play a decent
-          // decent amount of video in the future.
+          // 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();
+        // We will remain in the buffering state if we've not decoded enough
-        nsMediaStream* stream = mDecoder->GetCurrentStream();
+        // data to begin playback, or if we've not downloaded a reasonable
-        if (!mDecoder->CanPlayThrough() &&
+        // amount of data inside our buffering time.
-            now - mBufferingStart < TimeDuration::FromSeconds(BUFFERING_WAIT) &&
+        TimeDuration elapsed = TimeStamp::Now() - mBufferingStart;
-            stream->GetCachedDataEnd(mDecoder->mDecoderPosition) < mBufferingEndOffset &&
+        if ((!mDecoder->CanPlayThrough() || !HasAmpleDecodedData()) &&
-            !stream->IsDataCachedToEndOfStream(mDecoder->mDecoderPosition) &&
+             elapsed < TimeDuration::FromSeconds(BUFFERING_WAIT) &&
-            !stream->IsSuspendedByCache()) {
+             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",
+              ("In buffering: buffering data until %u bytes available or %f seconds",
-               PRUint32(mBufferingEndOffset - mDecoder->GetCurrentStream()->GetCachedDataEnd(mDecoder->mDecoderPosition)),
+               PRUint32(mBufferingEndOffset - stream->GetCachedDataEnd(mDecoder->mDecoderPosition)),
-               BUFFERING_WAIT - (now - mBufferingStart).ToSeconds()));
+               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;
       }
 

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;

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;
 }