corresponding change in CaptureVideoDecoder and RTCVideoDecoder due to pull...

corresponding change in CaptureVideoDecoder and RTCVideoDecoder due to pull model used in media VideoRenderer
1. Recent refactoring in media stack went back to pull model between VideoDecoder and VideoRenderer. Necessary change is needed in VideoDecoder subclasses.
2. fix racing condition during video capture shutdown. Now VideoCaptureImpl is guaranteed to be deleted after all events have been processed.

BUG=none
TEST=trybots
Review URL: http://codereview.chromium.org/8528045

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

content/renderer/media/capture_video_decoder.cc

@@ -67,6 +67,13 @@ void CaptureVideoDecoder::Pause(const base::Closure& callback) {
                  this, callback));
 }
 
+void CaptureVideoDecoder::Flush(const base::Closure& callback) {
+  message_loop_proxy_->PostTask(
+      FROM_HERE,
+      base::Bind(&CaptureVideoDecoder::FlushOnDecoderThread,
+                 this, callback));
+}
+
 void CaptureVideoDecoder::Stop(const base::Closure& callback) {
   message_loop_proxy_->PostTask(
       FROM_HERE,
@@ -158,6 +165,18 @@ void CaptureVideoDecoder::PauseOnDecoderThread(const base::Closure& callback) {
   media::VideoDecoder::Pause(callback);
 }
 
+void CaptureVideoDecoder::FlushOnDecoderThread(const base::Closure& callback) {
+  DVLOG(1) << "FlushOnDecoderThread";
+  DCHECK(message_loop_proxy_->BelongsToCurrentThread());
+  if (!read_cb_.is_null()) {
+    scoped_refptr<media::VideoFrame> video_frame =
+        media::VideoFrame::CreateBlackFrame(natural_size_.width(),
+                                            natural_size_.height());
+    DeliverFrame(video_frame);
+  }
+  media::VideoDecoder::Flush(callback);
+}
+
 void CaptureVideoDecoder::StopOnDecoderThread(const base::Closure& callback) {
   DVLOG(1) << "StopOnDecoderThread";
   DCHECK(message_loop_proxy_->BelongsToCurrentThread());

content/renderer/media/capture_video_decoder.h

@@ -5,8 +5,6 @@
 #ifndef CONTENT_RENDERER_MEDIA_CAPTURE_VIDEO_DECODER_H_
 #define CONTENT_RENDERER_MEDIA_CAPTURE_VIDEO_DECODER_H_
 
-#include <deque>
-
 #include "base/time.h"
 #include "content/common/content_export.h"
 #include "media/base/demuxer_stream.h"
@@ -37,6 +35,7 @@ class CONTENT_EXPORT CaptureVideoDecoder
   virtual void Seek(base::TimeDelta time,
                     const media::FilterStatusCB& cb) OVERRIDE;
   virtual void Pause(const base::Closure& callback) OVERRIDE;
+  virtual void Flush(const base::Closure& callback) OVERRIDE;
   virtual void Stop(const base::Closure& callback) OVERRIDE;
 
   // Decoder implementation.
@@ -74,6 +73,7 @@ class CONTENT_EXPORT CaptureVideoDecoder
   void SeekOnDecoderThread(base::TimeDelta time,
                            const media::FilterStatusCB& cb);
   void PauseOnDecoderThread(const base::Closure& callback);
+  void FlushOnDecoderThread(const base::Closure& callback);
   void StopOnDecoderThread(const base::Closure& callback);
 
   void InitializeOnDecoderThread(

content/renderer/media/capture_video_decoder_unittest.cc

@@ -152,6 +152,16 @@ class CaptureVideoDecoderTest : public ::testing::Test {
     message_loop_->RunAllPending();
   }
 
+  void Flush() {
+    // Issue a read.
+    EXPECT_CALL(*this, FrameReady(_));
+    decoder_->Read(read_cb_);
+
+    decoder_->Pause(media::NewExpectedClosure());
+    decoder_->Flush(media::NewExpectedClosure());
+    message_loop_->RunAllPending();
+  }
+
   void Stop() {
     EXPECT_CALL(*vc_impl_, StopCapture(capture_client()))
         .Times(1)
@@ -198,6 +208,7 @@ TEST_F(CaptureVideoDecoderTest, Play) {
   Initialize();
   Start();
   Play();
+  Flush();
   Stop();
 }
 

content/renderer/media/rtc_video_decoder.cc

@@ -4,8 +4,6 @@
 
 #include "content/renderer/media/rtc_video_decoder.h"
 
-#include <deque>
-
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/message_loop.h"
@@ -87,6 +85,34 @@ void RTCVideoDecoder::Pause(const base::Closure& callback) {
   VideoDecoder::Pause(callback);
 }
 
+void RTCVideoDecoder::Flush(const base::Closure& callback) {
+  if (MessageLoop::current() != message_loop_) {
+    message_loop_->PostTask(FROM_HERE,
+                            base::Bind(&RTCVideoDecoder::Flush,
+                                       this, callback));
+    return;
+  }
+
+  DCHECK_EQ(MessageLoop::current(), message_loop_);
+
+  ReadCB read_cb;
+  {
+    base::AutoLock auto_lock(lock_);
+    if (!read_cb_.is_null()) {
+      std::swap(read_cb, read_cb_);
+    }
+  }
+
+  if (!read_cb.is_null()) {
+    scoped_refptr<media::VideoFrame> video_frame =
+        media::VideoFrame::CreateBlackFrame(visible_size_.width(),
+                                            visible_size_.height());
+    read_cb.Run(video_frame);
+  }
+
+  VideoDecoder::Flush(callback);
+}
+
 void RTCVideoDecoder::Stop(const base::Closure& callback) {
   if (MessageLoop::current() != message_loop_) {
     message_loop_->PostTask(FROM_HERE,
@@ -141,6 +167,10 @@ bool RTCVideoDecoder::SetSize(int width, int height, int reserved) {
   return true;
 }
 
+// TODO(wjia): this function can be split into two parts so that the |lock_|
+// can be removed.
+// First creates media::VideoFrame, then post a task onto |message_loop_|
+// to deliver that frame.
 bool RTCVideoDecoder::RenderFrame(const cricket::VideoFrame* frame) {
   // Called from libjingle thread.
   DCHECK(frame);

content/renderer/media/rtc_video_decoder.h

@@ -5,7 +5,6 @@
 #ifndef CONTENT_RENDERER_MEDIA_RTC_VIDEO_DECODER_H_
 #define CONTENT_RENDERER_MEDIA_RTC_VIDEO_DECODER_H_
 
-#include <deque>
 #include <string>
 
 #include "base/compiler_specific.h"
@@ -36,6 +35,7 @@ class CONTENT_EXPORT RTCVideoDecoder
   virtual void Seek(base::TimeDelta time,
                     const media::FilterStatusCB& cb) OVERRIDE;
   virtual void Pause(const base::Closure& callback) OVERRIDE;
+  virtual void Flush(const base::Closure& callback) OVERRIDE;
   virtual void Stop(const base::Closure& callback) OVERRIDE;
 
   // Decoder implementation.
@@ -54,6 +54,7 @@ class CONTENT_EXPORT RTCVideoDecoder
   friend class RTCVideoDecoderTest;
   FRIEND_TEST_ALL_PREFIXES(RTCVideoDecoderTest, Initialize_Successful);
   FRIEND_TEST_ALL_PREFIXES(RTCVideoDecoderTest, DoSeek);
+  FRIEND_TEST_ALL_PREFIXES(RTCVideoDecoderTest, DoFlush);
   FRIEND_TEST_ALL_PREFIXES(RTCVideoDecoderTest, DoRenderFrame);
   FRIEND_TEST_ALL_PREFIXES(RTCVideoDecoderTest, DoSetSize);
 

content/renderer/media/rtc_video_decoder_unittest.cc

@@ -114,6 +114,8 @@ class RTCVideoDecoderTest : public testing::Test {
   RTCVideoDecoderTest() {
     decoder_ = new RTCVideoDecoder(&message_loop_, kUrl);
     renderer_ = new MockVideoRenderer();
+    read_cb_ = base::Bind(&RTCVideoDecoderTest::FrameReady,
+                          base::Unretained(this));
 
     DCHECK(decoder_);
 
@@ -149,6 +151,7 @@ class RTCVideoDecoderTest : public testing::Test {
   MockStatisticsCallback stats_callback_object_;
   StrictMock<MockFilterHost> host_;
   MessageLoop message_loop_;
+  media::VideoDecoder::ReadCB read_cb_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(RTCVideoDecoderTest);
@@ -180,6 +183,20 @@ TEST_F(RTCVideoDecoderTest, DoSeek) {
   EXPECT_EQ(RTCVideoDecoder::kNormal, decoder_->state_);
 }
 
+TEST_F(RTCVideoDecoderTest, DoFlush) {
+  const base::TimeDelta kZero;
+
+  InitializeDecoderSuccessfully();
+
+  EXPECT_CALL(*this, FrameReady(_));
+  decoder_->Read(read_cb_);
+  decoder_->Pause(media::NewExpectedClosure());
+  decoder_->Flush(media::NewExpectedClosure());
+
+  message_loop_.RunAllPending();
+  EXPECT_EQ(RTCVideoDecoder::kPaused, decoder_->state_);
+}
+
 TEST_F(RTCVideoDecoderTest, DoRenderFrame) {
   const base::TimeDelta kZero;
   EXPECT_CALL(host_, GetTime()).WillRepeatedly(Return(base::TimeDelta()));

content/renderer/media/video_capture_impl.cc

@@ -79,20 +79,21 @@ void VideoCaptureImpl::Init() {
 }
 
 void VideoCaptureImpl::DeInit(base::Closure task) {
+  ml_proxy_->PostTask(FROM_HERE,
+      base::Bind(&VideoCaptureImpl::DoDeInit,
+                 base::Unretained(this), task));
+}
+
+void VideoCaptureImpl::DoDeInit(base::Closure task) {
   if (state_ == kStarted)
     Send(new VideoCaptureHostMsg_Stop(device_id_));
 
   base::MessageLoopProxy* io_message_loop_proxy =
       ChildProcess::current()->io_message_loop_proxy();
 
-  if (!io_message_loop_proxy->BelongsToCurrentThread()) {
-    io_message_loop_proxy->PostTask(FROM_HERE,
-        base::Bind(&VideoCaptureImpl::RemoveDelegateOnIOThread,
-                   base::Unretained(this), task));
-    return;
-  }
-
-  RemoveDelegateOnIOThread(task);
+  io_message_loop_proxy->PostTask(FROM_HERE,
+      base::Bind(&VideoCaptureImpl::RemoveDelegateOnIOThread,
+                 base::Unretained(this), task));
 }
 
 void VideoCaptureImpl::StartCapture(
@@ -237,14 +238,14 @@ void VideoCaptureImpl::DoStopCapture(
   if (clients_.find(handler) == clients_.end())
     return;
 
-  handler->OnStopped(this);
-  handler->OnRemoved(this);
   clients_.erase(handler);
 
   if (clients_.empty()) {
     DVLOG(1) << "StopCapture: No more client, stopping ...";
     StopDevice();
   }
+  handler->OnStopped(this);
+  handler->OnRemoved(this);
 }
 
 void VideoCaptureImpl::DoFeedBuffer(scoped_refptr<VideoFrameBuffer> buffer) {
@@ -420,8 +421,8 @@ void VideoCaptureImpl::AddDelegateOnIOThread() {
 }
 
 void VideoCaptureImpl::RemoveDelegateOnIOThread(base::Closure task) {
-  base::ScopedClosureRunner task_runner(task);
   message_filter_->RemoveDelegate(this);
+  ml_proxy_->PostTask(FROM_HERE, task);
 }
 
 void VideoCaptureImpl::Send(IPC::Message* message) {

content/renderer/media/video_capture_impl.h

@@ -68,6 +68,7 @@ class CONTENT_EXPORT VideoCaptureImpl
 
   void Init();
   void DeInit(base::Closure task);
+  void DoDeInit(base::Closure task);
   void StopDevice();
   void RestartCapture();
   void StartCaptureInternal();