Fix VideoRendererBase end of stream logic.

media/filters/video_renderer_base.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/filters/video_renderer_base.h"
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/callback_helpers.h"
 #include "base/threading/platform_thread.h"
@@ skipping 191 matching lines @@
       frame_available_.TimedWait(kIdleTimeDelta);
       continue;
     }
 
     // Remain idle until we have the next frame ready for rendering.
     if (ready_frames_.empty()) {
       frame_available_.TimedWait(kIdleTimeDelta);
       continue;
     }
 
+    // If the next frame is end of stream then we are truly at the end of the
+    // video stream.
+    if (ready_frames_.front()->IsEndOfStream()) {

[DaleCurtis, 2012/08/06 21:44:35]

Not super familiar with this code, but won't this cause us to signal ended early
sometimes? The old code ensured that current_frame_ had been on screen for the
proper length of time before bailing out.

[scherkus (not reviewing), 2012/08/06 23:36:46]

I wondered about that in acolwell's previous change...

How would we handle a 10 frame video that was 1 fps? Would we play for 10
seconds or 9 seconds?

Is this simply the outcome of r149274?

+      state_ = kEnded;
+      ended_cb_.Run();
+      ready_frames_.clear();
+
+      // No need to sleep here as we idle when |state_ != kPlaying|.
+      continue;
+    }
+
     // Remain idle until we've initialized |current_frame_| via prerolling.
     if (!current_frame_) {
-      // This can happen if our preroll only contains end of stream frames.
-      if (ready_frames_.front()->IsEndOfStream()) {
-        state_ = kEnded;
-        ended_cb_.Run();
-        ready_frames_.clear();
-
-        // No need to sleep here as we idle when |state_ != kPlaying|.
-        continue;
-      }
-
       frame_available_.TimedWait(kIdleTimeDelta);
       continue;
     }
 
     // Calculate how long until we should advance the frame, which is
     // typically negative but for playback rates < 1.0f may be long enough
     // that it makes more sense to idle and check again.
     base::TimeDelta remaining_time =
         CalculateSleepDuration(ready_frames_.front(), playback_rate_);
 
     // Sleep up to a maximum of our idle time until we're within the time to
     // render the next frame.
     if (remaining_time.InMicroseconds() > 0) {
       remaining_time = std::min(remaining_time, kIdleTimeDelta);
       frame_available_.TimedWait(remaining_time);
       continue;
     }
 
 
     // We're almost there!
     //
     // At this point we've rendered |current_frame_| for the proper amount
     // of time and also have the next frame that ready for rendering.
 
-
-    // If the next frame is end of stream then we are truly at the end of the
-    // video stream.
-    //
-    // TODO(scherkus): deduplicate this end of stream check after we get rid of
-    // |current_frame_|.
-    if (ready_frames_.front()->IsEndOfStream()) {
-      state_ = kEnded;
-      ended_cb_.Run();
-      ready_frames_.clear();
-
-      // No need to sleep here as we idle when |state_ != kPlaying|.
-      continue;
-    }
-
     // We cannot update |current_frame_| until we've completed the pending
     // paint. Furthermore, the pending paint might be really slow: check to
     // see if we have any ready frames that we can drop if they've already
     // expired.
     if (pending_paint_) {
       while (!ready_frames_.empty()) {
         // Can't drop anything if we're at the end.
         if (ready_frames_.front()->IsEndOfStream())
           break;
 
@@ skipping 10 matching lines @@
         // Frame dropped: read again.
         ++frames_dropped;
         ready_frames_.pop_front();
         AttemptRead_Locked();
       }
       // Continue waiting for the current paint to finish.
       frame_available_.TimedWait(kIdleTimeDelta);
       continue;
     }
 
-
     // Congratulations! You've made it past the video frame timing gauntlet.
     //
     // We can now safely update the current frame, request another frame, and
     // signal to the client that a new frame is available.
     DCHECK(!pending_paint_);
     DCHECK(!ready_frames_.empty());
     SetCurrentFrameToNextReadyFrame();
     AttemptRead_Locked();
 
     base::AutoUnlock auto_unlock(lock_);
@@ skipping 253 matching lines @@
 
 int VideoRendererBase::NumFrames_Locked() const {
   lock_.AssertAcquired();
   int outstanding_frames =
       (current_frame_ ? 1 : 0) + (last_available_frame_ ? 1 : 0) +
       (current_frame_ && (current_frame_ == last_available_frame_) ? -1 : 0);
   return ready_frames_.size() + outstanding_frames;
 }
 
 }  // namespace media