From 792469709d9edea8af82c74d56116bcdfc2428a1 Mon Sep 17 00:00:00 2001 From: nisse Date: Tue, 23 Aug 2016 05:50:09 -0700 Subject: [PATCH] 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 | 23 +++--- webrtc/media/base/testutils.h | 12 ++-- .../engine/fakewebrtcvideocapturemodule.h | 2 +- webrtc/media/engine/webrtcvideocapturer.cc | 72 ++++--------------- webrtc/media/engine/webrtcvideocapturer.h | 7 -- .../engine/webrtcvideocapturer_unittest.cc | 26 ++++--- 6 files changed, 42 insertions(+), 100 deletions(-) diff --git a/webrtc/media/base/testutils.cc b/webrtc/media/base/testutils.cc index 9e2d174f61..321719d3e9 100644 --- a/webrtc/media/base/testutils.cc +++ b/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; } } diff --git a/webrtc/media/base/testutils.h b/webrtc/media/base/testutils.h index 2e338d95ae..d69e10e6c0 100644 --- a/webrtc/media/base/testutils.h +++ b/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 { 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_; }; diff --git a/webrtc/media/engine/fakewebrtcvideocapturemodule.h b/webrtc/media/engine/fakewebrtcvideocapturemodule.h index 2bc715b1e1..d81597858b 100644 --- a/webrtc/media/engine/fakewebrtcvideocapturemodule.h +++ b/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. diff --git a/webrtc/media/engine/webrtcvideocapturer.cc b/webrtc/media/engine/webrtcvideocapturer.cc index 1c46546d89..ba44b1324e 100644 --- a/webrtc/media/engine/webrtcvideocapturer.cc +++ b/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( - 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(length); - data = buffer; - rotation = sample.rotation(); -} - } // namespace cricket diff --git a/webrtc/media/engine/webrtcvideocapturer.h b/webrtc/media/engine/webrtcvideocapturer.h index 1efa4ad66f..01defefdfe 100644 --- a/webrtc/media/engine/webrtcvideocapturer.h +++ b/webrtc/media/engine/webrtcvideocapturer.h @@ -87,13 +87,6 @@ class WebRtcVideoCapturer : public VideoCapturer, std::unique_ptr 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_ diff --git a/webrtc/media/engine/webrtcvideocapturer_unittest.cc b/webrtc/media/engine/webrtcvideocapturer_unittest.cc index 62ac942582..718ab4c116 100644 --- a/webrtc/media/engine/webrtcvideocapturer_unittest.cc +++ b/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 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(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());