From 0f13ec1265250a87a5390c9a0403943bcf1bbf33 Mon Sep 17 00:00:00 2001 From: Per Date: Thu, 3 Mar 2016 09:22:28 +0100 Subject: [PATCH] Removed VideoSource dependency to ChannelManager. Instead VideoSource directly access the cricket::VideoCapturer via the worker_thread. Document: https://docs.google.com/a/google.com/document/d/1mEIw_0uDzyHjL3l8a82WKp6AvgR8Tlwn9JGvhbUjVpY/edit?usp=sharing BUG=webrtc:5426 R=nisse@webrtc.org, pthatcher@webrtc.org Review URL: https://codereview.webrtc.org/1759473003 . Cr-Commit-Position: refs/heads/master@{#11852} --- webrtc/api/peerconnection.cc | 10 ++-- webrtc/api/peerconnectionfactory.cc | 2 +- webrtc/api/videosource.cc | 79 +++++++++++++++++------------ webrtc/api/videosource.h | 12 +++-- webrtc/api/videosource_unittest.cc | 14 ++--- webrtc/api/videotrack_unittest.cc | 7 +-- 6 files changed, 63 insertions(+), 61 deletions(-) diff --git a/webrtc/api/peerconnection.cc b/webrtc/api/peerconnection.cc index 125300a3b5..1eca42a9e7 100644 --- a/webrtc/api/peerconnection.cc +++ b/webrtc/api/peerconnection.cc @@ -382,9 +382,9 @@ namespace webrtc { class RemoteMediaStreamFactory { public: explicit RemoteMediaStreamFactory(rtc::Thread* signaling_thread, - cricket::ChannelManager* channel_manager) + rtc::Thread* worker_thread) : signaling_thread_(signaling_thread), - channel_manager_(channel_manager) {} + worker_thread_(worker_thread) {} rtc::scoped_refptr CreateMediaStream( const std::string& stream_label) { @@ -404,7 +404,7 @@ class RemoteMediaStreamFactory { const std::string& track_id) { return AddTrack( stream, track_id, - VideoSource::Create(channel_manager_, new RemoteVideoCapturer(), + VideoSource::Create(worker_thread_, new RemoteVideoCapturer(), nullptr, true) .get()); } @@ -424,7 +424,7 @@ class RemoteMediaStreamFactory { } rtc::Thread* signaling_thread_; - cricket::ChannelManager* channel_manager_; + rtc::Thread* worker_thread_; }; bool ConvertRtcOptionsForOffer( @@ -633,7 +633,7 @@ bool PeerConnection::Initialize( media_controller_.reset(factory_->CreateMediaController(media_config)); remote_stream_factory_.reset(new RemoteMediaStreamFactory( - factory_->signaling_thread(), media_controller_->channel_manager())); + factory_->signaling_thread(), factory_->worker_thread())); session_.reset( new WebRtcSession(media_controller_.get(), factory_->signaling_thread(), diff --git a/webrtc/api/peerconnectionfactory.cc b/webrtc/api/peerconnectionfactory.cc index 9d0e7014ac..d63045f9ca 100644 --- a/webrtc/api/peerconnectionfactory.cc +++ b/webrtc/api/peerconnectionfactory.cc @@ -205,7 +205,7 @@ PeerConnectionFactory::CreateVideoSource( const MediaConstraintsInterface* constraints) { RTC_DCHECK(signaling_thread_->IsCurrent()); rtc::scoped_refptr source(VideoSource::Create( - channel_manager_.get(), capturer, constraints, false)); + worker_thread_, capturer, constraints, false)); return VideoSourceProxy::Create(signaling_thread_, source); } diff --git a/webrtc/api/videosource.cc b/webrtc/api/videosource.cc index 4e28cec018..0178d1ec59 100644 --- a/webrtc/api/videosource.cc +++ b/webrtc/api/videosource.cc @@ -16,7 +16,6 @@ #include "webrtc/api/mediaconstraintsinterface.h" #include "webrtc/base/arraysize.h" -#include "webrtc/pc/channelmanager.h" using cricket::CaptureState; using webrtc::MediaConstraintsInterface; @@ -26,12 +25,6 @@ namespace { const double kRoundingTruncation = 0.0005; -enum { - MSG_VIDEOCAPTURESTATECONNECT, - MSG_VIDEOCAPTURESTATEDISCONNECT, - MSG_VIDEOCAPTURESTATECHANGE, -}; - // Default resolution. If no constraint is specified, this is the resolution we // will use. static const cricket::VideoFormatPod kDefaultFormat = @@ -282,39 +275,40 @@ bool ExtractVideoOptions(const MediaConstraintsInterface* all_constraints, namespace webrtc { rtc::scoped_refptr VideoSource::Create( - cricket::ChannelManager* channel_manager, + rtc::Thread* worker_thread, cricket::VideoCapturer* capturer, const webrtc::MediaConstraintsInterface* constraints, bool remote) { - ASSERT(channel_manager != NULL); - ASSERT(capturer != NULL); + RTC_DCHECK(worker_thread != NULL); + RTC_DCHECK(capturer != NULL); rtc::scoped_refptr source(new rtc::RefCountedObject( - channel_manager, capturer, remote)); + worker_thread, capturer, remote)); source->Initialize(constraints); return source; } -VideoSource::VideoSource(cricket::ChannelManager* channel_manager, +VideoSource::VideoSource(rtc::Thread* worker_thread, cricket::VideoCapturer* capturer, bool remote) - : channel_manager_(channel_manager), + : signaling_thread_(rtc::Thread::Current()), + worker_thread_(worker_thread), video_capturer_(capturer), + started_(false), state_(kInitializing), remote_(remote) { - channel_manager_->SignalVideoCaptureStateChange.connect( + video_capturer_->SignalStateChange.connect( this, &VideoSource::OnStateChange); } VideoSource::~VideoSource() { - channel_manager_->StopVideoCapture(video_capturer_.get(), format_); - channel_manager_->SignalVideoCaptureStateChange.disconnect(this); + video_capturer_->SignalStateChange.disconnect(this); + Stop(); } void VideoSource::Initialize( const webrtc::MediaConstraintsInterface* constraints) { - std::vector formats = - channel_manager_->GetSupportedFormats(video_capturer_.get()); + *video_capturer_->GetSupportedFormats(); if (formats.empty()) { if (video_capturer_->IsScreencast()) { // The screen capturer can accept any resolution and we will derive the @@ -367,52 +361,71 @@ void VideoSource::Initialize( // the camera doesn't produce frames with the correct format? Or will // cricket::VideCapturer be able to re-scale / crop to the requested // resolution? - if (!channel_manager_->StartVideoCapture(video_capturer_.get(), format_)) { + if (!worker_thread_->Invoke( + rtc::Bind(&cricket::VideoCapturer::StartCapturing, + video_capturer_.get(), format_))) { SetState(kEnded); return; } + started_ = true; // Initialize hasn't succeeded until a successful state change has occurred. } void VideoSource::Stop() { - channel_manager_->StopVideoCapture(video_capturer_.get(), format_); + if (!started_) { + return; + } + started_ = false; + worker_thread_->Invoke( + rtc::Bind(&cricket::VideoCapturer::Stop, + video_capturer_.get())); } void VideoSource::Restart() { - if (!channel_manager_->StartVideoCapture(video_capturer_.get(), format_)) { + if (started_) { + return; + } + if (!worker_thread_->Invoke( + rtc::Bind(&cricket::VideoCapturer::StartCapturing, + video_capturer_.get(), format_))) { SetState(kEnded); return; } - for (auto* sink : sinks_) { - channel_manager_->AddVideoSink(video_capturer_.get(), sink); - } + started_ = true; } void VideoSource::AddSink( rtc::VideoSinkInterface* output) { - sinks_.push_back(output); - channel_manager_->AddVideoSink(video_capturer_.get(), output); + // TODO(perkj): Use fake rtc::VideoSinkWants for now. This will change once + // webrtc::VideoSourceInterface inherit rtc::VideoSourceInterface. + worker_thread_->Invoke( + rtc::Bind(&cricket::VideoCapturer::AddOrUpdateSink, + video_capturer_.get(), output, rtc::VideoSinkWants())); } void VideoSource::RemoveSink( rtc::VideoSinkInterface* output) { - sinks_.remove(output); - channel_manager_->RemoveVideoSink(video_capturer_.get(), output); + worker_thread_->Invoke( + rtc::Bind(&cricket::VideoCapturer::RemoveSink, + video_capturer_.get(), output)); } -// OnStateChange listens to the ChannelManager::SignalVideoCaptureStateChange. -// This signal is triggered for all video capturers. Not only the one we are -// interested in. +// OnStateChange listens to the cricket::VideoCapturer::SignalStateChange. void VideoSource::OnStateChange(cricket::VideoCapturer* capturer, cricket::CaptureState capture_state) { + if (rtc::Thread::Current() != signaling_thread_) { + invoker_.AsyncInvoke( + signaling_thread_, rtc::Bind(&VideoSource::OnStateChange, this, + capturer, capture_state)); + return; + } + if (capturer == video_capturer_.get()) { SetState(GetReadyState(capture_state)); } } void VideoSource::SetState(SourceState new_state) { - // TODO(hbos): Temporarily disabled VERIFY due to webrtc:4776. - // if (VERIFY(state_ != new_state)) { if (state_ != new_state) { state_ = new_state; FireOnChanged(); diff --git a/webrtc/api/videosource.h b/webrtc/api/videosource.h index e3ca0180d5..7ff479a45f 100644 --- a/webrtc/api/videosource.h +++ b/webrtc/api/videosource.h @@ -17,6 +17,7 @@ #include "webrtc/api/notifier.h" #include "webrtc/api/videosourceinterface.h" #include "webrtc/api/videotrackrenderers.h" +#include "webrtc/base/asyncinvoker.h" #include "webrtc/base/scoped_ptr.h" #include "webrtc/base/sigslot.h" #include "webrtc/media/base/videosinkinterface.h" @@ -48,7 +49,7 @@ class VideoSource : public Notifier, // |constraints| can be NULL and in that case the camera is opened using a // default resolution. static rtc::scoped_refptr Create( - cricket::ChannelManager* channel_manager, + rtc::Thread* worker_thread, cricket::VideoCapturer* capturer, const webrtc::MediaConstraintsInterface* constraints, bool remote); @@ -71,7 +72,7 @@ class VideoSource : public Notifier, virtual void RemoveSink(rtc::VideoSinkInterface* output); protected: - VideoSource(cricket::ChannelManager* channel_manager, + VideoSource(rtc::Thread* worker_thread, cricket::VideoCapturer* capturer, bool remote); virtual ~VideoSource(); @@ -82,12 +83,13 @@ class VideoSource : public Notifier, cricket::CaptureState capture_state); void SetState(SourceState new_state); - cricket::ChannelManager* channel_manager_; + rtc::Thread* signaling_thread_; + rtc::Thread* worker_thread_; + rtc::AsyncInvoker invoker_; rtc::scoped_ptr video_capturer_; + bool started_; rtc::scoped_ptr frame_input_; - std::list*> sinks_; - cricket::VideoFormat format_; cricket::VideoOptions options_; SourceState state_; diff --git a/webrtc/api/videosource_unittest.cc b/webrtc/api/videosource_unittest.cc index cd4ca3ae3d..e5f8b9f146 100644 --- a/webrtc/api/videosource_unittest.cc +++ b/webrtc/api/videosource_unittest.cc @@ -19,7 +19,6 @@ #include "webrtc/media/base/fakevideocapturer.h" #include "webrtc/media/base/fakevideorenderer.h" #include "webrtc/media/engine/webrtcvideoframe.h" -#include "webrtc/pc/channelmanager.h" using webrtc::FakeConstraints; using webrtc::VideoSource; @@ -111,9 +110,7 @@ class StateObserver : public ObserverInterface { class VideoSourceTest : public testing::Test { protected: - VideoSourceTest() - : channel_manager_(new cricket::ChannelManager( - new cricket::FakeMediaEngine(), rtc::Thread::Current())) { + VideoSourceTest() { InitCapturer(false); } void InitCapturer(bool is_screencast) { @@ -124,10 +121,6 @@ class VideoSourceTest : public testing::Test { void InitScreencast() { InitCapturer(true); } - void SetUp() { - ASSERT_TRUE(channel_manager_->Init()); - } - void CreateVideoSource() { CreateVideoSource(NULL); } @@ -136,7 +129,7 @@ class VideoSourceTest : public testing::Test { const webrtc::MediaConstraintsInterface* constraints) { // VideoSource take ownership of |capturer_| source_ = - VideoSource::Create(channel_manager_.get(), capturer_cleanup_.release(), + VideoSource::Create(rtc::Thread::Current(), capturer_cleanup_.release(), constraints, false); ASSERT_TRUE(source_.get() != NULL); @@ -150,7 +143,6 @@ class VideoSourceTest : public testing::Test { rtc::scoped_ptr capturer_cleanup_; TestVideoCapturer* capturer_; cricket::FakeVideoRenderer renderer_; - rtc::scoped_ptr channel_manager_; rtc::scoped_ptr state_observer_; rtc::scoped_refptr source_; }; @@ -200,7 +192,7 @@ TEST_F(VideoSourceTest, StopRestart) { // Test start stop with a remote VideoSource - the video source that has a // RemoteVideoCapturer and takes video frames from FrameInput. TEST_F(VideoSourceTest, StartStopRemote) { - source_ = VideoSource::Create(channel_manager_.get(), + source_ = VideoSource::Create(rtc::Thread::Current(), new webrtc::RemoteVideoCapturer(), NULL, true); ASSERT_TRUE(source_.get() != NULL); diff --git a/webrtc/api/videotrack_unittest.cc b/webrtc/api/videotrack_unittest.cc index df412589c7..0c3e8b0534 100644 --- a/webrtc/api/videotrack_unittest.cc +++ b/webrtc/api/videotrack_unittest.cc @@ -39,18 +39,13 @@ class VideoTrackTest : public testing::Test { public: VideoTrackTest() { static const char kVideoTrackId[] = "track_id"; - - channel_manager_.reset(new cricket::ChannelManager( - new cricket::FakeMediaEngine(), rtc::Thread::Current())); - EXPECT_TRUE(channel_manager_->Init()); video_track_ = VideoTrack::Create( kVideoTrackId, - VideoSource::Create(channel_manager_.get(), + VideoSource::Create(rtc::Thread::Current(), new webrtc::RemoteVideoCapturer(), NULL, true)); } protected: - rtc::scoped_ptr channel_manager_; rtc::scoped_refptr video_track_; };