Fix black video frames when seeking (which also fixes flashing poster issue).

media/filters/video_renderer_base.cc

 // Copyright (c) 2010 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 "base/callback.h"
 #include "media/base/buffers.h"
 #include "media/base/callback.h"
 #include "media/base/filter_host.h"
 #include "media/base/limits.h"
 #include "media/base/video_frame.h"
@@ skipping 15 matching lines @@
 static const int kIdleMilliseconds = 10;
 
 VideoRendererBase::VideoRendererBase()
     : width_(0),
       height_(0),
       frame_available_(&lock_),
       state_(kUninitialized),
       thread_(kNullThreadHandle),
       pending_reads_(0),
       pending_paint_(false),
+      pending_paint_with_last_available_(false),
       playback_rate_(0) {
 }
 
 VideoRendererBase::~VideoRendererBase() {
   AutoLock auto_lock(lock_);
   DCHECK(state_ == kUninitialized || state_ == kStopped);
 }
 
 // static
 bool VideoRendererBase::ParseMediaFormat(
@@ skipping 44 matching lines @@
   state_ = kPaused;
 }
 
 void VideoRendererBase::Flush(FilterCallback* callback) {
   DCHECK_EQ(state_, kPaused);
 
   AutoLock auto_lock(lock_);
   flush_callback_.reset(callback);
   state_ = kFlushing;
 
   if (pending_paint_ == false)

[scherkus (not reviewing), 2010/12/20 22:49:55]

ditto?

[sjl, 2010/12/21 22:13:40]

Do you mean '&& pending_paint_with_last_available_ == false' ?

If so, I don't believe this is actually needed as FlushBuffers() only affects
current_frame_. It can still run even if the last_available_frame_ is being
painted.

     FlushBuffers();
 }
 
 void VideoRendererBase::Stop(FilterCallback* callback) {
   DCHECK_EQ(pending_reads_, 0);
 
   {
     AutoLock auto_lock(lock_);
     state_ = kStopped;
 
@@ skipping 207 matching lines @@
         AutoUnlock auto_unlock(lock_);
         // Notify subclass that |current_frame_| has been updated.
         OnFrameAvailable();
       }
     }
   }
 }
 
 void VideoRendererBase::GetCurrentFrame(scoped_refptr<VideoFrame>* frame_out) {
   AutoLock auto_lock(lock_);
-  DCHECK(!pending_paint_);
+  DCHECK(!pending_paint_ && !pending_paint_with_last_available_);
 
   if (!current_frame_.get() || current_frame_->IsEndOfStream()) {
-    *frame_out = NULL;
-    return;
+    if (!last_available_frame_.get() ||
+        last_available_frame_->IsEndOfStream()) {
+      *frame_out = NULL;
+      return;
+    }
   }
 
   // We should have initialized and have the current frame.
   DCHECK(state_ != kUninitialized && state_ != kStopped && state_ != kError);
-  *frame_out = current_frame_;
-  pending_paint_ = true;
+
+  if (current_frame_) {
+    *frame_out = current_frame_;
+    last_available_frame_ = current_frame_;
+    pending_paint_ = true;
+  } else {
+    DCHECK(last_available_frame_.get() != NULL);
+    *frame_out = last_available_frame_;
+    pending_paint_with_last_available_ = true;
+  }
 }
 
 void VideoRendererBase::PutCurrentFrame(scoped_refptr<VideoFrame> frame) {
   AutoLock auto_lock(lock_);
 
   // Note that we do not claim |pending_paint_| when we return NULL frame, in
   // that case, |current_frame_| could be changed before PutCurrentFrame.
-  DCHECK(pending_paint_ || frame.get() == NULL);
-  DCHECK(current_frame_.get() == frame.get() || frame.get() == NULL);
+  if (pending_paint_) {
+    DCHECK(current_frame_.get() == frame.get());
+    DCHECK(pending_paint_with_last_available_ == false);
+    pending_paint_ = false;
+  } else if (pending_paint_with_last_available_) {
+    DCHECK(last_available_frame_.get() == frame.get());
+    pending_paint_with_last_available_ = false;
+  } else {
+    DCHECK(frame.get() == NULL);
+  }
 
-  pending_paint_ = false;
   // We had cleared the |pending_paint_| flag, there are chances that current
   // frame is timed-out. We will wake up our main thread to advance the current
   // frame when this is true.
   frame_available_.Signal();
   if (state_ == kFlushing)
     FlushBuffers();
 }
 
 void VideoRendererBase::ConsumeVideoFrame(scoped_refptr<VideoFrame> frame) {
   AutoLock auto_lock(lock_);
@@ skipping 89 matching lines @@
   // provide buffer pools. In the future, we may want to implement real
   // buffer pool to recycle buffers.
   while (!frames_queue_done_.empty()) {
     scoped_refptr<VideoFrame> video_frame = frames_queue_done_.front();
     frames_queue_done_.pop_front();
     ReadInput(video_frame);
   }
 }
 
 void VideoRendererBase::FlushBuffers() {
   DCHECK(!pending_paint_);

[scherkus (not reviewing), 2010/12/20 22:49:55]

add a DCHECK for pending_paint_with_last_available_ here?

[sjl, 2010/12/21 22:13:40]

As above, it's ok if we have a pending paint on last_available_frame_.

 
   // We should never put EOF frame into "done queue".
   while (!frames_queue_ready_.empty()) {
     scoped_refptr<VideoFrame> video_frame = frames_queue_ready_.front();
     if (!video_frame->IsEndOfStream()) {
       frames_queue_done_.push_back(video_frame);
     }
     frames_queue_ready_.pop_front();
   }
   if (current_frame_.get() && !current_frame_->IsEndOfStream()) {
@@ skipping 51 matching lines @@
     previous_time_ = now;
   }
 
   // Scale our sleep based on the playback rate.
   // TODO(scherkus): floating point badness and degrade gracefully.
   return base::TimeDelta::FromMicroseconds(
       static_cast<int64>(sleep.InMicroseconds() / playback_rate));
 }
 
 }  // namespace media