Refactor WebRtcVideoCapturer.

Pass incoming frames directly to VideoCapturer::OnFrame (after
conversion to cricket::VideoFrame), without using SignalFrameCaptured
or WebRtcCapturedFrame.

BUG=webrtc:5682

Review-Url: https://codereview.webrtc.org/2258933003
Cr-Commit-Position: refs/heads/master@{#13861}

webrtc/media/base/testutils.cc

@@ -210,17 +210,19 @@ bool RtpTestUtility::VerifyPacket(const RtpDumpPacket* dump,
 
 // Implementation of VideoCaptureListener.
 VideoCapturerListener::VideoCapturerListener(VideoCapturer* capturer)
-    : last_capture_state_(CS_STARTING),
+    : capturer_(capturer),
+      last_capture_state_(CS_STARTING),
       frame_count_(0),
-      frame_fourcc_(0),
       frame_width_(0),
       frame_height_(0),
-      frame_size_(0),
       resolution_changed_(false) {
   capturer->SignalStateChange.connect(this,
       &VideoCapturerListener::OnStateChange);
-  capturer->SignalFrameCaptured.connect(this,
-      &VideoCapturerListener::OnFrameCaptured);
+  capturer->AddOrUpdateSink(this, rtc::VideoSinkWants());
+}
+
+VideoCapturerListener::~VideoCapturerListener() {
+  capturer_->RemoveSink(this);
 }
 
 void VideoCapturerListener::OnStateChange(VideoCapturer* capturer,
@@ -228,15 +230,12 @@ void VideoCapturerListener::OnStateChange(VideoCapturer* capturer,
   last_capture_state_ = result;
 }
 
-void VideoCapturerListener::OnFrameCaptured(VideoCapturer* capturer,
-                                            const CapturedFrame* frame) {
+void VideoCapturerListener::OnFrame(const VideoFrame& frame) {
   ++frame_count_;
   if (1 == frame_count_) {
-    frame_fourcc_ = frame->fourcc;
-    frame_width_ = frame->width;
-    frame_height_ = frame->height;
-    frame_size_ = frame->data_size;
-  } else if (frame_width_ != frame->width || frame_height_ != frame->height) {
+    frame_width_ = frame.width();
+    frame_height_ = frame.height();
+  } else if (frame_width_ != frame.width() || frame_height_ != frame.height()) {
     resolution_changed_ = true;
   }
 }

webrtc/media/base/testutils.h

@@ -114,28 +114,28 @@ class RtpTestUtility {
 };
 
 // Test helper for testing VideoCapturer implementations.
-class VideoCapturerListener : public sigslot::has_slots<> {
+class VideoCapturerListener
+    : public sigslot::has_slots<>,
+      public rtc::VideoSinkInterface<cricket::VideoFrame> {
  public:
   explicit VideoCapturerListener(VideoCapturer* cap);
+  ~VideoCapturerListener();
 
   CaptureState last_capture_state() const { return last_capture_state_; }
   int frame_count() const { return frame_count_; }
-  uint32_t frame_fourcc() const { return frame_fourcc_; }
   int frame_width() const { return frame_width_; }
   int frame_height() const { return frame_height_; }
-  uint32_t frame_size() const { return frame_size_; }
   bool resolution_changed() const { return resolution_changed_; }
 
   void OnStateChange(VideoCapturer* capturer, CaptureState state);
-  void OnFrameCaptured(VideoCapturer* capturer, const CapturedFrame* frame);
+  void OnFrame(const VideoFrame& frame) override;
 
  private:
+  VideoCapturer* capturer_;
   CaptureState last_capture_state_;
   int frame_count_;
-  uint32_t frame_fourcc_;
   int frame_width_;
   int frame_height_;
-  uint32_t frame_size_;
   bool resolution_changed_;
 };
 

webrtc/media/engine/fakewebrtcvideocapturemodule.h

@@ -75,7 +75,7 @@ class FakeWebRtcVideoCaptureModule : public webrtc::VideoCaptureModule {
     return -1;  // not implemented
   }
   bool SetApplyRotation(bool enable) override {
-    return false;  // not implemented
+    return true;  // ignored
   }
   bool GetApplyRotation() override {
     return true;  // Rotation compensation is turned on.

webrtc/media/engine/webrtcvideocapturer.cc

@@ -349,22 +349,19 @@ void WebRtcVideoCapturer::OnIncomingCapturedFrame(
   // This can only happen between Start() and Stop().
   RTC_DCHECK(start_thread_);
   RTC_DCHECK(async_invoker_);
-  if (start_thread_->IsCurrent()) {
-    SignalFrameCapturedOnStartThread(sample);
-  } else {
-    // This currently happens on with at least VideoCaptureModuleV4L2 and
-    // possibly other implementations of WebRTC's VideoCaptureModule.
-    // In order to maintain the threading contract with the upper layers and
-    // consistency with other capturers such as in Chrome, we need to do a
-    // thread hop.
-    // Note that Stop() can cause the async invoke call to be cancelled.
-    async_invoker_->AsyncInvoke<void>(
-        RTC_FROM_HERE, start_thread_,
-        // Note that Bind captures by value, so there's an intermediate copy
-        // of sample.
-        rtc::Bind(&WebRtcVideoCapturer::SignalFrameCapturedOnStartThread, this,
-                  sample));
+
+  ++captured_frames_;
+  // Log the size and pixel aspect ratio of the first captured frame.
+  if (1 == captured_frames_) {
+    LOG(LS_INFO) << "Captured frame size "
+                 << sample.width() << "x" << sample.height()
+                 << ". Expected format " << GetCaptureFormat()->ToString();
   }
+
+  OnFrame(cricket::WebRtcVideoFrame(
+              sample.video_frame_buffer(), sample.rotation(),
+              sample.render_time_ms() * rtc::kNumMicrosecsPerMillisec, 0),
+          sample.width(), sample.height());
 }
 
 void WebRtcVideoCapturer::OnCaptureDelayChanged(const int32_t id,
@@ -372,49 +369,4 @@ void WebRtcVideoCapturer::OnCaptureDelayChanged(const int32_t id,
   LOG(LS_INFO) << "Capture delay changed to " << delay << " ms";
 }
 
-void WebRtcVideoCapturer::SignalFrameCapturedOnStartThread(
-    const webrtc::VideoFrame& frame) {
-  // This can only happen between Start() and Stop().
-  RTC_DCHECK(start_thread_);
-  RTC_DCHECK(start_thread_->IsCurrent());
-  RTC_DCHECK(async_invoker_);
-
-  ++captured_frames_;
-  // Log the size and pixel aspect ratio of the first captured frame.
-  if (1 == captured_frames_) {
-    LOG(LS_INFO) << "Captured frame size "
-                 << frame.width() << "x" << frame.height()
-                 << ". Expected format " << GetCaptureFormat()->ToString();
-  }
-
-  // Signal down stream components on captured frame.
-  // The CapturedFrame class doesn't support planes. We have to ExtractBuffer
-  // to one block for it.
-  size_t length =
-      webrtc::CalcBufferSize(webrtc::kI420, frame.width(), frame.height());
-  capture_buffer_.resize(length);
-  // TODO(magjed): Refactor the WebRtcCapturedFrame to avoid memory copy or
-  // take over ownership of the buffer held by |frame| if that's possible.
-  webrtc::ExtractBuffer(frame, length, &capture_buffer_[0]);
-  WebRtcCapturedFrame webrtc_frame(frame, &capture_buffer_[0], length);
-  SignalFrameCaptured(this, &webrtc_frame);
-}
-
-// WebRtcCapturedFrame
-WebRtcCapturedFrame::WebRtcCapturedFrame(const webrtc::VideoFrame& sample,
-                                         void* buffer,
-                                         size_t length) {
-  width = sample.width();
-  height = sample.height();
-  fourcc = FOURCC_I420;
-  // TODO(hellner): Support pixel aspect ratio (for OSX).
-  pixel_width = 1;
-  pixel_height = 1;
-  // Convert units from VideoFrame RenderTimeMs to CapturedFrame (nanoseconds).
-  time_stamp = sample.render_time_ms() * rtc::kNumNanosecsPerMillisec;
-  data_size = rtc::checked_cast<uint32_t>(length);
-  data = buffer;
-  rotation = sample.rotation();
-}
-
 }  // namespace cricket

webrtc/media/engine/webrtcvideocapturer.h

@@ -87,13 +87,6 @@ class WebRtcVideoCapturer : public VideoCapturer,
   std::unique_ptr<rtc::AsyncInvoker> async_invoker_;
 };
 
-struct WebRtcCapturedFrame : public CapturedFrame {
- public:
-  WebRtcCapturedFrame(const webrtc::VideoFrame& frame,
-                      void* buffer,
-                      size_t length);
-};
-
 }  // namespace cricket
 
 #endif  // WEBRTC_MEDIA_WEBRTC_WEBRTCVIDEOCAPTURER_H_

webrtc/media/engine/webrtcvideocapturer_unittest.cc

@@ -35,8 +35,7 @@ class WebRtcVideoCapturerTest : public testing::Test {
  public:
   WebRtcVideoCapturerTest()
       : factory_(new FakeWebRtcVcmFactory),
-        capturer_(new cricket::WebRtcVideoCapturer(factory_)),
-        listener_(capturer_.get()) {
+        capturer_(new cricket::WebRtcVideoCapturer(factory_)) {
     factory_->device_info.AddDevice(kTestDeviceName, kTestDeviceId);
     // add a VGA/I420 capability
     webrtc::VideoCaptureCapability vga;
@@ -50,7 +49,6 @@ class WebRtcVideoCapturerTest : public testing::Test {
  protected:
   FakeWebRtcVcmFactory* factory_;  // owned by capturer_
   std::unique_ptr<cricket::WebRtcVideoCapturer> capturer_;
-  cricket::VideoCapturerListener listener_;
 };
 
 TEST_F(WebRtcVideoCapturerTest, TestNotOpened) {
@@ -83,28 +81,29 @@ TEST_F(WebRtcVideoCapturerTest, TestInitVcm) {
 
 TEST_F(WebRtcVideoCapturerTest, TestCapture) {
   EXPECT_TRUE(capturer_->Init(cricket::Device(kTestDeviceName, kTestDeviceId)));
+  cricket::VideoCapturerListener listener(capturer_.get());
   cricket::VideoFormat format(
       capturer_->GetSupportedFormats()->at(0));
   EXPECT_EQ(cricket::CS_STARTING, capturer_->Start(format));
   EXPECT_TRUE(capturer_->IsRunning());
   ASSERT_TRUE(capturer_->GetCaptureFormat() != NULL);
   EXPECT_EQ(format, *capturer_->GetCaptureFormat());
-  EXPECT_EQ_WAIT(cricket::CS_RUNNING, listener_.last_capture_state(), 1000);
+  EXPECT_EQ_WAIT(cricket::CS_RUNNING, listener.last_capture_state(), 1000);
   factory_->modules[0]->SendFrame(640, 480);
-  EXPECT_TRUE_WAIT(listener_.frame_count() > 0, 5000);
-  EXPECT_EQ(capturer_->GetCaptureFormat()->fourcc, listener_.frame_fourcc());
-  EXPECT_EQ(640, listener_.frame_width());
-  EXPECT_EQ(480, listener_.frame_height());
+  EXPECT_TRUE_WAIT(listener.frame_count() > 0, 5000);
+  EXPECT_EQ(640, listener.frame_width());
+  EXPECT_EQ(480, listener.frame_height());
   EXPECT_EQ(cricket::CS_FAILED, capturer_->Start(format));
   capturer_->Stop();
   EXPECT_FALSE(capturer_->IsRunning());
   EXPECT_TRUE(capturer_->GetCaptureFormat() == NULL);
-  EXPECT_EQ_WAIT(cricket::CS_STOPPED, listener_.last_capture_state(), 1000);
+  EXPECT_EQ_WAIT(cricket::CS_STOPPED, listener.last_capture_state(), 1000);
 }
 
 TEST_F(WebRtcVideoCapturerTest, TestCaptureVcm) {
   EXPECT_TRUE(capturer_->Init(factory_->Create(0,
       reinterpret_cast<const char*>(kTestDeviceId.c_str()))));
+  cricket::VideoCapturerListener listener(capturer_.get());
   EXPECT_TRUE(capturer_->GetSupportedFormats()->empty());
   VideoFormat format;
   EXPECT_TRUE(capturer_->GetBestCaptureFormat(kDefaultVideoFormat, &format));
@@ -116,12 +115,11 @@ TEST_F(WebRtcVideoCapturerTest, TestCaptureVcm) {
   EXPECT_TRUE(capturer_->IsRunning());
   ASSERT_TRUE(capturer_->GetCaptureFormat() != NULL);
   EXPECT_EQ(format, *capturer_->GetCaptureFormat());
-  EXPECT_EQ_WAIT(cricket::CS_RUNNING, listener_.last_capture_state(), 1000);
+  EXPECT_EQ_WAIT(cricket::CS_RUNNING, listener.last_capture_state(), 1000);
   factory_->modules[0]->SendFrame(640, 480);
-  EXPECT_TRUE_WAIT(listener_.frame_count() > 0, 5000);
-  EXPECT_EQ(capturer_->GetCaptureFormat()->fourcc, listener_.frame_fourcc());
-  EXPECT_EQ(640, listener_.frame_width());
-  EXPECT_EQ(480, listener_.frame_height());
+  EXPECT_TRUE_WAIT(listener.frame_count() > 0, 5000);
+  EXPECT_EQ(640, listener.frame_width());
+  EXPECT_EQ(480, listener.frame_height());
   EXPECT_EQ(cricket::CS_FAILED, capturer_->Start(format));
   capturer_->Stop();
   EXPECT_FALSE(capturer_->IsRunning());