Fix VideoRendererBase end of stream logic.

media/filters/video_renderer_base.cc

Index: media/filters/video_renderer_base.cc
diff --git a/media/filters/video_renderer_base.cc b/media/filters/video_renderer_base.cc
index e4d90c78d2fe70e6fccb40691f393122791d533e..a419f74bd2e73efac31fff228e629f5f7bec5a91 100644
--- a/media/filters/video_renderer_base.cc
+++ b/media/filters/video_renderer_base.cc
@@ -15,6 +15,10 @@
 
 namespace media {
 
+base::TimeDelta VideoRendererBase::kLastFrameDuration() {
+  return base::TimeDelta::FromMilliseconds(250);
+}
+
 VideoRendererBase::VideoRendererBase(const base::Closure& paint_cb,
                                      const SetOpaqueCB& set_opaque_cb,
                                      bool drop_frames)
@@ -484,7 +488,19 @@ void VideoRendererBase::AddReadyFrame(const scoped_refptr<VideoFrame>& frame) {
   // frame rate.  Another way for this to happen is for the container to state a
   // smaller duration than the largest packet timestamp.
   base::TimeDelta duration = get_duration_cb_.Run();
-  if (frame->GetTimestamp() > duration || frame->IsEndOfStream()) {
+  if (frame->IsEndOfStream()) {
+    base::TimeDelta end_timestamp = kNoTimestamp();
+    if (!ready_frames_.empty()) {
+      end_timestamp =
+          std::min(duration,
+                   ready_frames_.back()->GetTimestamp() + kLastFrameDuration());
+    } else if (current_frame_) {
+      end_timestamp =
+          std::min(duration,
+                   current_frame_->GetTimestamp() + kLastFrameDuration());
+    }
+    frame->SetTimestamp(end_timestamp);

[DaleCurtis, 2012/08/07 20:32:14]

Since it wasn't immediately obvious to me: when end_timestamp == kNoTimestamp()
we'll end up sleeping for (kNoTimestamp() - get_time_cb_.Run()), which I assume
is a negative number thus line 236 will skip sleeping => ending immediately.

OOC could all this be be collapsed to:

frame->SetTimestamp(std::min(
    duration, get_time_cb_.Run() + kLastFrameDuration()));

?

[acolwell GONE FROM CHROMIUM, 2012/08/07 21:10:28]

I'm pretty sure the only way to get kNoTimestamp is if the first frame we get is
an EOS which means current_frame_ will never get set. This will trigger the
logic on line 220 and it get to line 236. Skipping sleeping would also be ok in
this case since we never got any valid frames to display so there is no need to
wait.
You can't use get_time_cb_ in the normal cases because ready_frames_ is
typically 4 frames deep and we shouldn't make any assumptions about how far
apart the frames are.

+  } else  if (frame->GetTimestamp() > duration) {

[DaleCurtis, 2012/08/07 20:32:14]

nit: extra space.

[acolwell GONE FROM CHROMIUM, 2012/08/07 21:10:28]

Done.

     frame->SetTimestamp(duration);
   }