Bug 493187. Call nsOggDecoder::UpdateReadyStateForData more often so we don't fail to notice state changes, and make it smarter about examining the frame queue. r=doublec
authorRobert O'Callahan <robert@ocallahan.org>
Tue, 19 May 2009 10:47:08 +1200
changeset 28526 269ec95e8516b72411052a9be2bb2acd948dab62
parent 28525 8ae3e659599bb803131169a8d57d57d484cb23f4
child 28527 c7c6a38b8076b07c5ac9306dc7b3041a8ae3896b
push id7108
push userrocallahan@mozilla.com
push dateTue, 19 May 2009 01:17:50 +0000
treeherdermozilla-central@058122b0bf3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdoublec
bugs493187
milestone1.9.2a1pre
Bug 493187. Call nsOggDecoder::UpdateReadyStateForData more often so we don't fail to notice state changes, and make it smarter about examining the frame queue. r=doublec
content/media/video/src/nsOggDecoder.cpp
content/media/video/test/Makefile.in
content/media/video/test/test_bug493187.html
--- a/content/media/video/src/nsOggDecoder.cpp
+++ b/content/media/video/src/nsOggDecoder.cpp
@@ -183,74 +183,81 @@ public:
 
   // A queue of decoded video frames. 
   class FrameQueue
   {
   public:
     FrameQueue() :
       mHead(0),
       mTail(0),
-      mEmpty(PR_TRUE)
+      mCount(0)
     {
     }
 
     void Push(FrameData* frame)
     {
       NS_ASSERTION(!IsFull(), "FrameQueue is full");
       mQueue[mTail] = frame;
       mTail = (mTail+1) % OGGPLAY_BUFFER_SIZE;
-      mEmpty = PR_FALSE;
+      ++mCount;
     }
 
-    FrameData* Peek()
+    FrameData* Peek() const
     {
-      NS_ASSERTION(!mEmpty, "FrameQueue is empty");
+      NS_ASSERTION(mCount > 0, "FrameQueue is empty");
 
       return mQueue[mHead];
     }
 
     FrameData* Pop()
     {
-      NS_ASSERTION(!mEmpty, "FrameQueue is empty");
+      NS_ASSERTION(mCount, "FrameQueue is empty");
 
       FrameData* result = mQueue[mHead];
       mHead = (mHead + 1) % OGGPLAY_BUFFER_SIZE;
-      mEmpty = mHead == mTail;
+      --mCount;
       return result;
     }
 
     PRBool IsEmpty() const
     {
-      return mEmpty;
+      return mCount == 0;
+    }
+
+    PRInt32 GetCount() const
+    {
+      return mCount;
     }
 
     PRBool IsFull() const
     {
-      return !mEmpty && mHead == mTail;
+      return mCount == OGGPLAY_BUFFER_SIZE;
     }
 
     float ResetTimes(float aPeriod)
     {
       float time = 0.0;
-      if (!mEmpty) {
+      if (mCount > 0) {
         PRInt32 current = mHead;
         do {
           mQueue[current]->mTime = time;
           time += aPeriod;
           current = (current + 1) % OGGPLAY_BUFFER_SIZE;
         } while (current != mTail);
       }
       return time;
     }
 
   private:
     FrameData* mQueue[OGGPLAY_BUFFER_SIZE];
     PRInt32 mHead;
     PRInt32 mTail;
-    PRPackedBool mEmpty;
+    // This isn't redundant with mHead/mTail, since when mHead == mTail
+    // it's ambiguous whether the queue is full or empty
+    PRInt32 mCount;
   };
 
   // Enumeration for the valid states
   enum State {
     DECODER_STATE_DECODING_METADATA,
     DECODER_STATE_DECODING_FIRSTFRAME,
     DECODER_STATE_DECODING,
     DECODER_STATE_SEEKING,
@@ -335,18 +342,18 @@ public:
   // is currently queued. This is called from the main thread and must
   // be called with the decode monitor held.
   void ClearPositionChangeFlag();
 
   // Must be called with the decode monitor held. Can be called by main
   // thread.
   PRBool HaveNextFrameData() const {
     return !mDecodedFrames.IsEmpty() &&
-      (mState == DECODER_STATE_DECODING ||
-       mState == DECODER_STATE_COMPLETED);
+      (mDecodedFrames.Peek()->mDecodedFrameTime > mCurrentFrameTime ||
+       mDecodedFrames.GetCount() > 1);
   }
 
   // Must be called with the decode monitor held. Can be called by main
   // thread.
   PRBool IsBuffering() const {
     PR_ASSERT_CURRENT_THREAD_IN_MONITOR(mDecoder->GetMonitor());
     return mState == nsOggDecodeStateMachine::DECODER_STATE_BUFFERING;
   }
@@ -603,17 +610,22 @@ public:
           decoder->mReader->Stream()->GetCachedDataEnd(decoder->mDecoderPosition);
 
         mon.Exit();
         r = oggplay_step_decoding(mPlayer);
         mon.Enter();
 
         if (InStopDecodingState())
           break;
-        
+
+        // If PlayFrame is waiting, wake it up so we can run the
+        // decoder loop and move frames from the oggplay queue to our
+        // queue.
+        mon.NotifyAll();
+
         // Check whether decoding the last frame required us to read data
         // that wasn't available at the start of the frame. That means
         // we should probably start buffering.
         if (decoder->mDecoderPosition > initialDownloadPosition) {
           mDecodeStateMachine->mBufferExhausted = PR_TRUE;
         }
       }
     } while (!mDecodeStateMachine->mDecodingCompleted &&
@@ -1016,16 +1028,26 @@ void nsOggDecodeStateMachine::UpdatePlay
   }
 }
 
 void nsOggDecodeStateMachine::QueueDecodedFrames()
 {
   //  NS_ASSERTION(PR_InMonitor(mDecoder->GetMonitor()), "QueueDecodedFrames() called without acquiring decoder monitor");
   FrameData* frame;
   while (!mDecodedFrames.IsFull() && (frame = NextFrame())) {
+    if (mDecodedFrames.GetCount() < 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<nsIRunnable> event = 
+        NS_NEW_RUNNABLE_METHOD(nsOggDecoder, mDecoder, UpdateReadyStateForData);
+      NS_DispatchToMainThread(event, NS_DISPATCH_NORMAL);
+    }
     mDecodedFrames.Push(frame);
   }
 }
 
 void nsOggDecodeStateMachine::ClearPositionChangeFlag()
 {
   //  NS_ASSERTION(PR_InMonitor(mDecoder->GetMonitor()), "ClearPositionChangeFlag() called without acquiring decoder monitor");
   mPositionChangeQueued = PR_FALSE;
@@ -1189,17 +1211,18 @@ nsresult nsOggDecodeStateMachine::Run()
           }
 
           mBufferExhausted = PR_FALSE;
           mDecodingCompleted = PR_FALSE;
           nsCOMPtr<nsIRunnable> event = new nsOggStepDecodeEvent(this, mPlayer);
           mStepDecodeThread->Dispatch(event, NS_DISPATCH_NORMAL);
         }
 
-        // Get the decoded frame and store it in our queue of decoded frames
+        // Get the decoded frames and store them in our queue of decoded frames
+        QueueDecodedFrames();
         while (mDecodedFrames.IsEmpty()) {
           mon.Wait(PR_MillisecondsToInterval(PRInt64(mCallbackPeriod*500)));
           if (mState != DECODER_STATE_DECODING)
             break;
           QueueDecodedFrames();
         }
 
         if (mState != DECODER_STATE_DECODING)
--- a/content/media/video/test/Makefile.in
+++ b/content/media/video/test/Makefile.in
@@ -65,16 +65,17 @@ ifdef MOZ_OGG
 		dynamic_redirect.sjs \
 		test_access_control.html \
 		file_access_controls.html \
 		test_autobuffer2.html \
 		test_bug448534.html \
 		test_bug461281.html \
 		test_bug468190.html \
 		test_bug482461.html \
+		test_bug493187.html \
 		test_can_play_type_ogg.html \
 		test_closing_connections.html \
 		test_contentDuration1.html \
 		test_contentDuration2.html \
 		test_contentDuration3.html \
 		test_contentDuration4.html \
 		test_contentDuration5.html \
 		test_contentDuration6.html \
new file mode 100644
--- /dev/null
+++ b/content/media/video/test/test_bug493187.html
@@ -0,0 +1,45 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=493187
+-->
+
+<head>
+  <title>Bug 493187 - enter HAVE_FUTURE_DATA when seeking within buffered data even if new data isn't arriving</title>
+  <script type="text/javascript" src="/MochiKit/packed.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+  <script type="text/javascript" src="use_large_cache.js"></script>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=493187">Mozilla Bug 493187</a>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+var v;
+var startedSeek = false;
+var completed = false;
+
+function start() {
+  v = document.getElementById('v');
+  v.currentTime = 1.0;
+}
+
+function startSeeking() {
+  startedSeek = true;
+}
+
+function canPlayThrough() {
+  if (startedSeek && !completed) {
+    ok(true, "Got canplaythrough after seek");
+    completed = true;
+    SimpleTest.finish();
+  }
+}
+
+SimpleTest.waitForExplicitFinish();
+</script>
+</pre>
+<video id='v' src='seek.ogv' oncanplaythrough='canPlayThrough()' onseeking='startSeeking()'
+       autobuffer onload='start()'></video>
+</body>
+</html>