Simplify VideoDecodeEngine interface by making everything synchronous.

Although I plan to remove VideoDecodeEngine entirely it requires detangling some of the code first.

Other noteworthy changes:
  - It's no longer valid to call VideoFrameReady(NULL), instead FFmpegVideoDecoder will raise an error the moment it finds one
  - Buffer recycling has been vanquished (for now), with video frames always allocated in the decoder
  - Produce/ConsumeVideoFrame() has been replaced by Read()
  - Video decode byte statistics are only updated if more than 0 bytes were decoded
  - FFmpegVideoDecodeEngine no longer attempts to preroll

Review URL: http://codereview.chromium.org/8417019

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@108612 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 76 insertions, 135 deletions

diff --git a/content/renderer/media/capture_video_decoder.cc b/content/renderer/media/capture_video_decoder.cc
index fadcac7..4d1f683 100644
--- a/content/renderer/media/capture_video_decoder.cc
+++ b/content/renderer/media/capture_video_decoder.cc
@@ -22,6 +22,7 @@ CaptureVideoDecoder::CaptureVideoDecoder(
     : message_loop_proxy_(message_loop_proxy),
       vc_manager_(vc_manager),
       capability_(capability),
+      natural_size_(capability.width, capability.height),
       state_(kUnInitialized),
       video_stream_id_(video_stream_id),
       capture_engine_(NULL) {
@@ -41,16 +42,15 @@ void CaptureVideoDecoder::Initialize(
                  filter_callback, stat_callback));
 }
 
-void CaptureVideoDecoder::ProduceVideoFrame(
-    scoped_refptr<media::VideoFrame> video_frame) {
+void CaptureVideoDecoder::Read(const ReadCB& callback) {
   message_loop_proxy_->PostTask(
       FROM_HERE,
-      base::Bind(&CaptureVideoDecoder::ProduceVideoFrameOnDecoderThread,
-                 this, video_frame));
+      base::Bind(&CaptureVideoDecoder::ReadOnDecoderThread,
+                 this, callback));
 }
 
-gfx::Size CaptureVideoDecoder::natural_size() {
-  return gfx::Size(capability_.width, capability_.height);
+const gfx::Size& CaptureVideoDecoder::natural_size() {
+  return natural_size_;
 }
 
 void CaptureVideoDecoder::Play(const base::Closure& callback) {
@@ -131,17 +131,15 @@ void CaptureVideoDecoder::InitializeOnDecoderThread(
 
   capture_engine_ = vc_manager_->AddDevice(video_stream_id_, this);
 
-  available_frames_.clear();
-
   statistics_callback_ = stat_callback;
   filter_callback.Run();
   state_ = kNormal;
 }
 
-void CaptureVideoDecoder::ProduceVideoFrameOnDecoderThread(
-    scoped_refptr<media::VideoFrame> video_frame) {
+void CaptureVideoDecoder::ReadOnDecoderThread(const ReadCB& callback) {
   DCHECK(message_loop_proxy_->BelongsToCurrentThread());
-  available_frames_.push_back(video_frame);
+  CHECK(read_cb_.is_null());
+  read_cb_ = callback;
 }
 
 void CaptureVideoDecoder::PlayOnDecoderThread(const base::Closure& callback) {
@@ -170,14 +168,6 @@ void CaptureVideoDecoder::SeekOnDecoderThread(base::TimeDelta time,
   VLOG(1) << "SeekOnDecoderThread.";
   DCHECK(message_loop_proxy_->BelongsToCurrentThread());
 
-  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 < media::Limits::kMaxVideoFrames; ++i) {
-    VideoFrameReady(media::VideoFrame::CreateBlackFrame(capability_.width,
-                                                        capability_.height));
-  }
-
   cb.Run(media::PIPELINE_OK);
   state_ = kNormal;
   capture_engine_->StartCapture(this, capability_);
@@ -197,34 +187,27 @@ void CaptureVideoDecoder::OnBufferReadyOnDecoderThread(
     scoped_refptr<media::VideoCapture::VideoFrameBuffer> buf) {
   DCHECK(message_loop_proxy_->BelongsToCurrentThread());
 
-  if (available_frames_.size() == 0 || kNormal != state_) {
+  if (read_cb_.is_null() || kNormal != state_) {
     capture->FeedBuffer(buf);
     return;
   }
 
-  scoped_refptr<media::VideoFrame> video_frame = available_frames_.front();
-  available_frames_.pop_front();
-
   if (buf->width != capability_.width || buf->height != capability_.height) {
     capability_.width = buf->width;
     capability_.height = buf->height;
-    host()->SetNaturalVideoSize(
-        gfx::Size(capability_.width, capability_.height));
+    natural_size_.SetSize(buf->width, buf->height);
+    host()->SetNaturalVideoSize(natural_size_);
   }
 
-  // Check if there's a size change.
-  if (static_cast<int>(video_frame->width()) != capability_.width ||
-      static_cast<int>(video_frame->height()) != capability_.height) {
-    // Allocate new buffer based on the new size.
-    video_frame = media::VideoFrame::CreateFrame(media::VideoFrame::YV12,
-                                                 capability_.width,
-                                                 capability_.height,
-                                                 media::kNoTimestamp,
-                                                 media::kNoTimestamp);
-  }
-
-  video_frame->SetTimestamp(buf->timestamp - start_time_);
-  video_frame->SetDuration(base::TimeDelta::FromMilliseconds(33));
+  // Always allocate a new frame.
+  //
+  // TODO(scherkus): migrate this to proper buffer recycling.
+  scoped_refptr<media::VideoFrame> video_frame =
+      media::VideoFrame::CreateFrame(media::VideoFrame::YV12,
+                                     natural_size_.width(),
+                                     natural_size_.height(),
+                                     buf->timestamp - start_time_,
+                                     base::TimeDelta::FromMilliseconds(33));
 
   uint8* buffer = buf->memory_pointer;
 
@@ -241,6 +224,14 @@ void CaptureVideoDecoder::OnBufferReadyOnDecoderThread(
   buffer += uv_width * uv_height;
   CopyVPlane(buffer, uv_width, uv_height, video_frame);
 
-  VideoFrameReady(video_frame);
+  DeliverFrame(video_frame);
   capture->FeedBuffer(buf);
 }
+
+void CaptureVideoDecoder::DeliverFrame(
+    const scoped_refptr<media::VideoFrame>& video_frame) {
+  // Reset the callback before running to protect against reentrancy.
+  ReadCB read_cb = read_cb_;
+  read_cb_.Reset();
+  read_cb.Run(video_frame);
+}
diff --git a/content/renderer/media/capture_video_decoder.h b/content/renderer/media/capture_video_decoder.h
index 0c874ab..f3e79be 100644
--- a/content/renderer/media/capture_video_decoder.h
+++ b/content/renderer/media/capture_video_decoder.h
@@ -44,9 +44,8 @@ class CONTENT_EXPORT CaptureVideoDecoder
       media::DemuxerStream* demuxer_stream,
       const base::Closure& filter_callback,
       const media::StatisticsCallback& stat_callback) OVERRIDE;
-  virtual void ProduceVideoFrame(
-      scoped_refptr<media::VideoFrame> video_frame) OVERRIDE;
-  virtual gfx::Size natural_size() OVERRIDE;
+  virtual void Read(const ReadCB& callback) OVERRIDE;
+  virtual const gfx::Size& natural_size() OVERRIDE;
 
   // VideoCapture::EventHandler implementation.
   virtual void OnStarted(media::VideoCapture* capture) OVERRIDE;
@@ -67,7 +66,6 @@ class CONTENT_EXPORT CaptureVideoDecoder
   enum DecoderState {
     kUnInitialized,
     kNormal,
-    kSeeking,
     kStopped,
     kPaused
   };
@@ -82,19 +80,22 @@ class CONTENT_EXPORT CaptureVideoDecoder
       media::DemuxerStream* demuxer_stream,
       const base::Closure& filter_callback,
       const media::StatisticsCallback& stat_callback);
-  void ProduceVideoFrameOnDecoderThread(
-      scoped_refptr<media::VideoFrame> video_frame);
+  void ReadOnDecoderThread(const ReadCB& callback);
 
   void OnStoppedOnDecoderThread(media::VideoCapture* capture);
   void OnBufferReadyOnDecoderThread(
       media::VideoCapture* capture,
       scoped_refptr<media::VideoCapture::VideoFrameBuffer> buf);
 
+  // Delivers the frame to |read_cb_| and resets the callback.
+  void DeliverFrame(const scoped_refptr<media::VideoFrame>& video_frame);
+
   scoped_refptr<base::MessageLoopProxy> message_loop_proxy_;
   scoped_refptr<VideoCaptureImplManager> vc_manager_;
   media::VideoCapture::VideoCaptureCapability capability_;
+  gfx::Size natural_size_;
   DecoderState state_;
-  std::deque<scoped_refptr<media::VideoFrame> > available_frames_;
+  ReadCB read_cb_;
   base::Closure pending_stop_cb_;
   media::StatisticsCallback statistics_callback_;
 
diff --git a/content/renderer/media/capture_video_decoder_unittest.cc b/content/renderer/media/capture_video_decoder_unittest.cc
index 65c7893..1b7cd11 100644
--- a/content/renderer/media/capture_video_decoder_unittest.cc
+++ b/content/renderer/media/capture_video_decoder_unittest.cc
@@ -21,10 +21,6 @@ using ::testing::StrictMock;
 
 static const media::VideoCaptureSessionId kVideoStreamId = 1;
 
-ACTION_P(ReturnFrameFromRenderer, decoder) {
-  decoder->ProduceVideoFrame(arg0);
-}
-
 ACTION_P3(CreateDataBufferFromCapture, decoder, vc_impl, data_buffer_number) {
   for (int i = 0; i < data_buffer_number; i++) {
     media::VideoCapture::VideoFrameBuffer* buffer;
@@ -98,14 +94,12 @@ class CaptureVideoDecoderTest : public ::testing::Test {
 
     decoder_ = new CaptureVideoDecoder(message_loop_proxy_,
                                        kVideoStreamId, vc_manager_, capability);
-    renderer_ = new media::MockVideoRenderer();
-
     decoder_->set_host(&host_);
-    decoder_->set_consume_video_frame_callback(
-        base::Bind(&media::MockVideoRenderer::ConsumeVideoFrame,
-                   base::Unretained(renderer_.get())));
     EXPECT_CALL(statistics_callback_object_, OnStatistics(_))
         .Times(AnyNumber());
+
+    read_cb_ = base::Bind(&CaptureVideoDecoderTest::FrameReady,
+                          base::Unretained(this));
   }
 
   virtual ~CaptureVideoDecoderTest() {
@@ -117,14 +111,16 @@ class CaptureVideoDecoderTest : public ::testing::Test {
                       base::Unretained(&statistics_callback_object_));
   }
 
+  MOCK_METHOD1(FrameReady, void(scoped_refptr<media::VideoFrame>));
+
   // Fixture members.
   scoped_refptr<CaptureVideoDecoder> decoder_;
   scoped_refptr<MockVideoCaptureImplManager> vc_manager_;
-  scoped_refptr<media::MockVideoRenderer> renderer_;
   media::MockStatisticsCallback statistics_callback_object_;
   StrictMock<media::MockFilterHost> host_;
   scoped_ptr<MessageLoop> message_loop_;
   scoped_refptr<base::MessageLoopProxy> message_loop_proxy_;
+  media::VideoDecoder::ReadCB read_cb_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(CaptureVideoDecoderTest);
@@ -146,8 +142,9 @@ TEST_F(CaptureVideoDecoderTest, Play) {
                        NewStatisticsCallback());
   message_loop_->RunAllPending();
 
-  EXPECT_CALL(*renderer_, ConsumeVideoFrame(_))
-      .WillRepeatedly(ReturnFrameFromRenderer(decoder_.get()));
+  EXPECT_CALL(*this, FrameReady(_));
+  decoder_->Read(read_cb_);
+
   EXPECT_CALL(*vc_impl, StartCapture(capture_client, _))
       .Times(1)
       .WillOnce(CreateDataBufferFromCapture(capture_client, vc_impl.get(),
diff --git a/content/renderer/media/rtc_video_decoder.cc b/content/renderer/media/rtc_video_decoder.cc
index a2a5784..3888d54 100644
--- a/content/renderer/media/rtc_video_decoder.cc
+++ b/content/renderer/media/rtc_video_decoder.cc
@@ -53,13 +53,7 @@ void RTCVideoDecoder::Initialize(DemuxerStream* demuxer_stream,
   }
 
   DCHECK_EQ(MessageLoop::current(), message_loop_);
-
-  lock_.Acquire();
-  frame_queue_available_.clear();
-  lock_.Release();
-
   state_ = kNormal;
-
   filter_callback.Run();
 
   // TODO(acolwell): Implement stats.
@@ -117,34 +111,24 @@ 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 ReadCB& 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(read_cb_.is_null());
+  read_cb_ = callback;
 }
 
-gfx::Size RTCVideoDecoder::natural_size() {
+const gfx::Size& RTCVideoDecoder::natural_size() {
   // TODO(vrk): Return natural size when aspect ratio support is implemented.
   return visible_size_;
 }
@@ -158,42 +142,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;
+  ReadCB read_cb;
   {
     base::AutoLock auto_lock(lock_);
-    if (frame_queue_available_.size() == 0) {
+    if (read_cb_.is_null()) {
       return true;
     }
-    video_frame = frame_queue_available_.front();
-    frame_queue_available_.pop_front();
+    std::swap(read_cb, read_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));
 
-  // 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 +177,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);
-  }
-
+  read_cb.Run(video_frame);
   return true;
 }
diff --git a/content/renderer/media/rtc_video_decoder.h b/content/renderer/media/rtc_video_decoder.h
index eb59e22..bfcea41 100644
--- a/content/renderer/media/rtc_video_decoder.h
+++ b/content/renderer/media/rtc_video_decoder.h
@@ -43,9 +43,8 @@ class CONTENT_EXPORT RTCVideoDecoder
       media::DemuxerStream* demuxer_stream,
       const base::Closure& filter_callback,
       const media::StatisticsCallback& stat_callback) OVERRIDE;
-  virtual void ProduceVideoFrame(
-      scoped_refptr<media::VideoFrame> video_frame) OVERRIDE;
-  virtual gfx::Size natural_size() OVERRIDE;
+  virtual void Read(const ReadCB& callback) OVERRIDE;
+  virtual const gfx::Size& natural_size() OVERRIDE;
 
   // cricket::VideoRenderer implementation
   virtual bool SetSize(int width, int height, int reserved) OVERRIDE;
@@ -61,7 +60,6 @@ class CONTENT_EXPORT RTCVideoDecoder
   enum DecoderState {
     kUnInitialized,
     kNormal,
-    kSeeking,
     kPaused,
     kStopped
   };
@@ -70,8 +68,9 @@ class CONTENT_EXPORT RTCVideoDecoder
   gfx::Size visible_size_;
   std::string url_;
   DecoderState state_;
-  std::deque<scoped_refptr<media::VideoFrame> > frame_queue_available_;
-  // Used for accessing frame queue from another thread.
+  ReadCB read_cb_;
+
+  // Used for accessing |read_cb_| from another thread.
   base::Lock lock_;
 
   DISALLOW_COPY_AND_ASSIGN(RTCVideoDecoder);
diff --git a/content/renderer/media/rtc_video_decoder_unittest.cc b/content/renderer/media/rtc_video_decoder_unittest.cc
index 3ff5a75..8d5c8fa 100644
--- a/content/renderer/media/rtc_video_decoder_unittest.cc
+++ b/content/renderer/media/rtc_video_decoder_unittest.cc
@@ -141,6 +141,8 @@ class RTCVideoDecoderTest : public testing::Test {
                       base::Unretained(&stats_callback_object_));
   }
 
+  MOCK_METHOD1(FrameReady, void(scoped_refptr<media::VideoFrame>));
+
   // Fixture members.
   scoped_refptr<RTCVideoDecoder> decoder_;
   scoped_refptr<MockVideoRenderer> renderer_;
@@ -171,13 +173,7 @@ TEST_F(RTCVideoDecoderTest, DoSeek) {
 
   InitializeDecoderSuccessfully();
 
-  decoder_->set_consume_video_frame_callback(
-      base::Bind(&MockVideoRenderer::ConsumeVideoFrame,
-                 base::Unretained(renderer_.get())));
-
-  // Expect Seek and verify the results.
-  EXPECT_CALL(*renderer_.get(), ConsumeVideoFrame(_))
-      .Times(Limits::kMaxVideoFrames);
+  // Expect seek and verify the results.
   decoder_->Seek(kZero, NewExpectedStatusCB(PIPELINE_OK));
 
   message_loop_.RunAllPending();
@@ -190,18 +186,6 @@ TEST_F(RTCVideoDecoderTest, DoRenderFrame) {
 
   InitializeDecoderSuccessfully();
 
-  // Pass the frame back to decoder
-  decoder_->set_consume_video_frame_callback(
-      base::Bind(&RTCVideoDecoder::ProduceVideoFrame,
-                 base::Unretained(decoder_.get())));
-  decoder_->Seek(kZero, NewExpectedStatusCB(PIPELINE_OK));
-
-  decoder_->set_consume_video_frame_callback(
-      base::Bind(&MockVideoRenderer::ConsumeVideoFrame,
-                 base::Unretained(renderer_.get())));
-  EXPECT_CALL(*renderer_.get(), ConsumeVideoFrame(_))
-      .Times(Limits::kMaxVideoFrames);
-
   NullVideoFrame video_frame;
 
   for (size_t i = 0; i < Limits::kMaxVideoFrames; ++i) {