Simplify VideoDecodeEngine interface by making everything synchronous.

content/renderer/media/rtc_video_decoder.cc

Index: content/renderer/media/rtc_video_decoder.cc
diff --git a/content/renderer/media/rtc_video_decoder.cc b/content/renderer/media/rtc_video_decoder.cc
index a2a5784d1f789f88a4530a7d152f4f24f085d17b..9549627257d77087a47980ad79e64063974d5fff 100644
--- a/content/renderer/media/rtc_video_decoder.cc
+++ b/content/renderer/media/rtc_video_decoder.cc
@@ -55,7 +55,7 @@ void RTCVideoDecoder::Initialize(DemuxerStream* demuxer_stream,
   DCHECK_EQ(MessageLoop::current(), message_loop_);
 
   lock_.Acquire();
-  frame_queue_available_.clear();
+  frame_ready_cb_.Reset();  // is this needed?

[acolwell GONE FROM CHROMIUM, 2011/11/01 18:31:03]

I don't think this is needed. As far as I know Initialize() is only called once
at the beginning.

[scherkus (not reviewing), 2011/11/03 04:55:59]

Done.

   lock_.Release();
 
   state_ = kNormal;
@@ -117,31 +117,21 @@ void RTCVideoDecoder::Seek(base::TimeDelta time, const FilterStatusCB& cb) {
   }
 
   DCHECK_EQ(MessageLoop::current(), message_loop_);
-
-  state_ = kSeeking;
-  // Create output buffer pool and pass the frames to renderer
-  // so that the renderer can complete the seeking.
-  for (size_t i = 0; i < Limits::kMaxVideoFrames; ++i) {
-    VideoFrameReady(VideoFrame::CreateBlackFrame(
-        visible_size_.width(), visible_size_.height()));
-  }
-
   state_ = kNormal;
-
   cb.Run(PIPELINE_OK);
 }
 
-void RTCVideoDecoder::ProduceVideoFrame(
-    scoped_refptr<VideoFrame> video_frame) {
+void RTCVideoDecoder::Read(const FrameReadyCB& callback) {
   if (MessageLoop::current() != message_loop_) {
     message_loop_->PostTask(
         FROM_HERE,
-        base::Bind(&RTCVideoDecoder::ProduceVideoFrame, this, video_frame));
+        base::Bind(&RTCVideoDecoder::Read, this, callback));
     return;
   }
   DCHECK_EQ(MessageLoop::current(), message_loop_);
   base::AutoLock auto_lock(lock_);
-  frame_queue_available_.push_back(video_frame);
+  CHECK(frame_ready_cb_.is_null());
+  frame_ready_cb_ = callback;
 }
 
 gfx::Size RTCVideoDecoder::natural_size() {
@@ -158,42 +148,34 @@ bool RTCVideoDecoder::SetSize(int width, int height, int reserved) {
 }
 
 bool RTCVideoDecoder::RenderFrame(const cricket::VideoFrame* frame) {
+  // Called from libjingle thread.
   DCHECK(frame);
 
   if (state_ != kNormal)
     return true;
 
-  // This is called from another thread.
-  scoped_refptr<VideoFrame> video_frame;
+  FrameReadyCB frame_ready_cb;
   {
     base::AutoLock auto_lock(lock_);
-    if (frame_queue_available_.size() == 0) {
+    if (frame_ready_cb_.is_null()) {
       return true;
     }
-    video_frame = frame_queue_available_.front();
-    frame_queue_available_.pop_front();
+    std::swap(frame_ready_cb, frame_ready_cb_);
   }
 
-  // Check if there's a size change.
-  // TODO(vrk): Remove casts when media::VideoFrame is updated with gfx::Sizes
-  // for width/height.
-  if (video_frame->width() != static_cast<size_t>(visible_size_.width()) ||
-      video_frame->height() != static_cast<size_t>(visible_size_.height())) {
-    // Allocate new buffer based on the new size.
-    video_frame = VideoFrame::CreateFrame(VideoFrame::YV12,
-                                          visible_size_.width(),
-                                          visible_size_.height(),
-                                          kNoTimestamp,
-                                          kNoTimestamp);
-  }
+  // Always allocate a new frame.
+  //
+  // TODO(scherkus): migrate this to proper buffer recycling.
+  scoped_refptr<media::VideoFrame> video_frame =
+      VideoFrame::CreateFrame(VideoFrame::YV12,
+                              visible_size_.width(),
+                              visible_size_.height(),
+                              host()->GetTime(),
+                              base::TimeDelta::FromMilliseconds(30));

[acolwell GONE FROM CHROMIUM, 2011/11/01 18:31:03]

Do we really want to hard code 30ms for the duration? Are we guaranteed that we
will always produce frames at 33.33fps or less?

[scherkus (not reviewing), 2011/11/03 04:55:59]

That's a question for wjia to answer (will cc him when we're done with the bulk
of this review)

 
-  // Only YV12 frames are supported.
-  DCHECK(video_frame->format() == VideoFrame::YV12);
   // Aspect ratio unsupported; DCHECK when there are non-square pixels.
-  DCHECK(frame->GetPixelWidth() == 1);
-  DCHECK(frame->GetPixelHeight() == 1);
-  video_frame->SetTimestamp(host()->GetTime());
-  video_frame->SetDuration(base::TimeDelta::FromMilliseconds(30));
+  DCHECK_EQ(frame->GetPixelWidth(), 1u);
+  DCHECK_EQ(frame->GetPixelHeight(), 1u);
 
   int y_rows = frame->GetHeight();
   int uv_rows = frame->GetHeight() / 2;  // YV12 format.
@@ -201,13 +183,6 @@ bool RTCVideoDecoder::RenderFrame(const cricket::VideoFrame* frame) {
   CopyUPlane(frame->GetUPlane(), frame->GetUPitch(), uv_rows, video_frame);
   CopyVPlane(frame->GetVPlane(), frame->GetVPitch(), uv_rows, video_frame);
 
-  if (MessageLoop::current() != message_loop_) {
-    message_loop_->PostTask(
-        FROM_HERE,
-        base::Bind(&RTCVideoDecoder::VideoFrameReady, this, video_frame));
-  } else {
-    VideoFrameReady(video_frame);
-  }
-
+  frame_ready_cb.Run(video_frame);
   return true;
 }