Simplify VideoDecodeEngine interface by making everything synchronous.

content/renderer/media/rtc_video_decoder.cc

 // Copyright (c) 2011 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 "content/renderer/media/rtc_video_decoder.h"
 
 #include <deque>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 37 matching lines @@
         FROM_HERE,
         base::Bind(&RTCVideoDecoder::Initialize, this,
                    make_scoped_refptr(demuxer_stream),
                    filter_callback, stat_callback));
     return;
   }
 
   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;
 
   filter_callback.Run();
 
   // TODO(acolwell): Implement stats.
 }
 
 void RTCVideoDecoder::Play(const base::Closure& callback) {
@@ skipping 41 matching lines @@
 
 void RTCVideoDecoder::Seek(base::TimeDelta time, const FilterStatusCB& cb) {
   if (MessageLoop::current() != message_loop_) {
      message_loop_->PostTask(FROM_HERE,
                              base::Bind(&RTCVideoDecoder::Seek, this,
                                         time, cb));
      return;
   }
 
   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() {
   // TODO(vrk): Return natural size when aspect ratio support is implemented.
   return visible_size_;
 }
 
 bool RTCVideoDecoder::SetSize(int width, int height, int reserved) {
   visible_size_.SetSize(width, height);
 
   // TODO(vrk): Provide natural size when aspect ratio support is implemented.
   host()->SetNaturalVideoSize(visible_size_);
   return true;
 }
 
 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.
   CopyYPlane(frame->GetYPlane(), frame->GetYPitch(), y_rows, video_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;
 }