From 5a4a75ae48382d8a85105d7fcd173718db8501c5 Mon Sep 17 00:00:00 2001 From: deadbeef Date: Thu, 2 Jun 2016 16:23:38 -0700 Subject: [PATCH] Combining SetVideoSend and SetSource into one method. This means there's only one thread hop to the worker thread. At the video engine level, SetOptions and SetSource are combined into one method (all within the same critical section) which ensures that no frame will be encoded while SetVideoSend is only partially finished. BUG=webrtc:5691 Review-Url: https://codereview.webrtc.org/1838413002 Cr-Commit-Position: refs/heads/master@{#13022} --- webrtc/api/mediastreamprovider.h | 17 +- webrtc/api/rtpsender.cc | 27 ++-- webrtc/api/rtpsender.h | 2 + webrtc/api/rtpsenderreceiver_unittest.cc | 78 +++++---- webrtc/api/webrtcsession.cc | 33 ++-- webrtc/api/webrtcsession.h | 11 +- webrtc/api/webrtcsession_unittest.cc | 5 +- webrtc/media/base/fakemediaengine.h | 13 +- webrtc/media/base/mediachannel.h | 15 +- webrtc/media/base/videoengine_unittest.h | 54 ++++--- webrtc/media/engine/webrtcvideoengine2.cc | 136 +++++++--------- webrtc/media/engine/webrtcvideoengine2.h | 18 +-- .../engine/webrtcvideoengine2_unittest.cc | 149 +++++++++--------- webrtc/pc/channel.cc | 13 +- webrtc/pc/channel.h | 9 +- webrtc/pc/channel_unittest.cc | 20 +-- 16 files changed, 282 insertions(+), 318 deletions(-) diff --git a/webrtc/api/mediastreamprovider.h b/webrtc/api/mediastreamprovider.h index b23e17bd4b..b508c39f7e 100644 --- a/webrtc/api/mediastreamprovider.h +++ b/webrtc/api/mediastreamprovider.h @@ -80,18 +80,21 @@ class AudioProviderInterface { // of a video track connected to a certain PeerConnection. class VideoProviderInterface { public: - virtual bool SetSource( - uint32_t ssrc, - rtc::VideoSourceInterface* source) = 0; // Enable/disable the video playout of a remote video track with |ssrc|. virtual void SetVideoPlayout( uint32_t ssrc, bool enable, rtc::VideoSinkInterface* sink) = 0; - // Enable sending video on the local video track with |ssrc|. - virtual void SetVideoSend(uint32_t ssrc, - bool enable, - const cricket::VideoOptions* options) = 0; + // Enable/disable sending video on the local video track with |ssrc|. + // TODO(deadbeef): Make |options| a reference parameter. + // TODO(deadbeef): Eventually, |enable| and |options| will be contained + // in |source|. When that happens, remove those parameters and rename + // this to SetVideoSource. + virtual void SetVideoSend( + uint32_t ssrc, + bool enable, + const cricket::VideoOptions* options, + rtc::VideoSourceInterface* source) = 0; virtual RtpParameters GetVideoRtpSendParameters(uint32_t ssrc) const = 0; virtual bool SetVideoRtpSendParameters(uint32_t ssrc, diff --git a/webrtc/api/rtpsender.cc b/webrtc/api/rtpsender.cc index 5577b9c750..33d7ee37a9 100644 --- a/webrtc/api/rtpsender.cc +++ b/webrtc/api/rtpsender.cc @@ -280,7 +280,7 @@ bool VideoRtpSender::SetTrack(MediaStreamTrackInterface* track) { // Attach to new track. bool prev_can_send_track = can_send_track(); // Keep a reference to the old track to keep it alive until we call - // SetSource. + // SetVideoSend. rtc::scoped_refptr old_track = track_; track_ = video_track; if (track_) { @@ -290,17 +290,9 @@ bool VideoRtpSender::SetTrack(MediaStreamTrackInterface* track) { // Update video provider. if (can_send_track()) { - // TODO(deadbeef): If SetTrack is called with a disabled track, and the - // previous track was enabled, this could cause a frame from the new track - // to slip out. Really, what we need is for SetSource and SetVideoSend - // to be combined into one atomic operation, all the way down to - // WebRtcVideoSendStream. - - provider_->SetSource(ssrc_, track_); SetVideoSend(); } else if (prev_can_send_track) { - provider_->SetSource(ssrc_, nullptr); - provider_->SetVideoSend(ssrc_, false, nullptr); + ClearVideoSend(); } return true; } @@ -312,12 +304,10 @@ void VideoRtpSender::SetSsrc(uint32_t ssrc) { } // If we are already sending with a particular SSRC, stop sending. if (can_send_track()) { - provider_->SetSource(ssrc_, nullptr); - provider_->SetVideoSend(ssrc_, false, nullptr); + ClearVideoSend(); } ssrc_ = ssrc; if (can_send_track()) { - provider_->SetSource(ssrc_, track_); SetVideoSend(); } } @@ -332,8 +322,7 @@ void VideoRtpSender::Stop() { track_->UnregisterObserver(this); } if (can_send_track()) { - provider_->SetSource(ssrc_, nullptr); - provider_->SetVideoSend(ssrc_, false, nullptr); + ClearVideoSend(); } stopped_ = true; } @@ -346,7 +335,13 @@ void VideoRtpSender::SetVideoSend() { options.is_screencast = rtc::Optional(source->is_screencast()); options.video_noise_reduction = source->needs_denoising(); } - provider_->SetVideoSend(ssrc_, track_->enabled(), &options); + provider_->SetVideoSend(ssrc_, track_->enabled(), &options, track_); +} + +void VideoRtpSender::ClearVideoSend() { + RTC_DCHECK(ssrc_ != 0); + RTC_DCHECK(provider_ != nullptr); + provider_->SetVideoSend(ssrc_, false, nullptr, nullptr); } RtpParameters VideoRtpSender::GetParameters() const { diff --git a/webrtc/api/rtpsender.h b/webrtc/api/rtpsender.h index ffe5daeb01..c8a46a80e4 100644 --- a/webrtc/api/rtpsender.h +++ b/webrtc/api/rtpsender.h @@ -171,6 +171,8 @@ class VideoRtpSender : public ObserverInterface, // Helper function to construct options for // VideoProviderInterface::SetVideoSend. void SetVideoSend(); + // Helper function to call SetVideoSend with "stop sending" parameters. + void ClearVideoSend(); std::string id_; std::string stream_id_; diff --git a/webrtc/api/rtpsenderreceiver_unittest.cc b/webrtc/api/rtpsenderreceiver_unittest.cc index 3db9d5e00e..a08e1b4c63 100644 --- a/webrtc/api/rtpsenderreceiver_unittest.cc +++ b/webrtc/api/rtpsenderreceiver_unittest.cc @@ -80,17 +80,15 @@ class MockAudioProvider : public AudioProviderInterface { class MockVideoProvider : public VideoProviderInterface { public: virtual ~MockVideoProvider() {} - MOCK_METHOD2(SetSource, - bool(uint32_t ssrc, - rtc::VideoSourceInterface* source)); MOCK_METHOD3(SetVideoPlayout, void(uint32_t ssrc, bool enable, rtc::VideoSinkInterface* sink)); - MOCK_METHOD3(SetVideoSend, + MOCK_METHOD4(SetVideoSend, void(uint32_t ssrc, bool enable, - const cricket::VideoOptions* options)); + const cricket::VideoOptions* options, + rtc::VideoSourceInterface* source)); MOCK_CONST_METHOD1(GetVideoRtpSendParameters, RtpParameters(uint32_t ssrc)); MOCK_METHOD2(SetVideoRtpSendParameters, @@ -126,8 +124,8 @@ class RtpSenderReceiverTest : public testing::Test { void CreateVideoRtpSender() { AddVideoTrack(); - EXPECT_CALL(video_provider_, SetSource(kVideoSsrc, video_track_.get())); - EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, true, _)); + EXPECT_CALL(video_provider_, + SetVideoSend(kVideoSsrc, true, _, video_track_.get())); video_rtp_sender_ = new VideoRtpSender(stream_->GetVideoTracks()[0], stream_->label(), &video_provider_); video_rtp_sender_->SetSsrc(kVideoSsrc); @@ -140,8 +138,8 @@ class RtpSenderReceiverTest : public testing::Test { } void DestroyVideoRtpSender() { - EXPECT_CALL(video_provider_, SetSource(kVideoSsrc, NULL)).Times(1); - EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, false, _)).Times(1); + EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, false, _, nullptr)) + .Times(1); video_rtp_sender_ = nullptr; } @@ -240,10 +238,12 @@ TEST_F(RtpSenderReceiverTest, RemoteAudioTrackDisable) { TEST_F(RtpSenderReceiverTest, LocalVideoTrackDisable) { CreateVideoRtpSender(); - EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, false, _)); + EXPECT_CALL(video_provider_, + SetVideoSend(kVideoSsrc, false, _, video_track_.get())); video_track_->set_enabled(false); - EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, true, _)); + EXPECT_CALL(video_provider_, + SetVideoSend(kVideoSsrc, true, _, video_track_.get())); video_track_->set_enabled(true); DestroyVideoRtpSender(); @@ -358,13 +358,13 @@ TEST_F(RtpSenderReceiverTest, VideoSenderEarlyWarmupSsrcThenTrack) { rtc::scoped_refptr sender = new VideoRtpSender(&video_provider_); sender->SetSsrc(kVideoSsrc); - EXPECT_CALL(video_provider_, SetSource(kVideoSsrc, video_track_.get())); - EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, true, _)); + EXPECT_CALL(video_provider_, + SetVideoSend(kVideoSsrc, true, _, video_track_.get())); sender->SetTrack(video_track_); // Calls expected from destructor. - EXPECT_CALL(video_provider_, SetSource(kVideoSsrc, nullptr)).Times(1); - EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, false, _)).Times(1); + EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, false, _, nullptr)) + .Times(1); } // Test that a video sender calls the expected methods on the provider once @@ -374,13 +374,13 @@ TEST_F(RtpSenderReceiverTest, VideoSenderEarlyWarmupTrackThenSsrc) { rtc::scoped_refptr sender = new VideoRtpSender(&video_provider_); sender->SetTrack(video_track_); - EXPECT_CALL(video_provider_, SetSource(kVideoSsrc, video_track_.get())); - EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, true, _)); + EXPECT_CALL(video_provider_, + SetVideoSend(kVideoSsrc, true, _, video_track_.get())); sender->SetSsrc(kVideoSsrc); // Calls expected from destructor. - EXPECT_CALL(video_provider_, SetSource(kVideoSsrc, nullptr)).Times(1); - EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, false, _)).Times(1); + EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, false, _, nullptr)) + .Times(1); } // Test that the sender is disconnected from the provider when its SSRC is @@ -405,20 +405,19 @@ TEST_F(RtpSenderReceiverTest, AudioSenderSsrcSetToZero) { // set to 0. TEST_F(RtpSenderReceiverTest, VideoSenderSsrcSetToZero) { AddVideoTrack(); - EXPECT_CALL(video_provider_, SetSource(kVideoSsrc, video_track_.get())); - EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, true, _)); + EXPECT_CALL(video_provider_, + SetVideoSend(kVideoSsrc, true, _, video_track_.get())); rtc::scoped_refptr sender = new VideoRtpSender(video_track_, kStreamLabel1, &video_provider_); sender->SetSsrc(kVideoSsrc); - EXPECT_CALL(video_provider_, SetSource(kVideoSsrc, nullptr)).Times(1); - EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, false, _)).Times(1); + EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, false, _, nullptr)) + .Times(1); sender->SetSsrc(0); // Make sure it's SetSsrc that called methods on the provider, and not the // destructor. - EXPECT_CALL(video_provider_, SetSource(_, _)).Times(0); - EXPECT_CALL(video_provider_, SetVideoSend(_, _, _)).Times(0); + EXPECT_CALL(video_provider_, SetVideoSend(_, _, _, _)).Times(0); } TEST_F(RtpSenderReceiverTest, AudioSenderTrackSetToNull) { @@ -449,28 +448,24 @@ TEST_F(RtpSenderReceiverTest, VideoSenderTrackSetToNull) { FakeVideoTrackSource::Create()); rtc::scoped_refptr track = VideoTrack::Create(kVideoTrackId, source); - EXPECT_CALL(video_provider_, SetSource(kVideoSsrc, track.get())); - EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, true, _)); + EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, true, _, track.get())); rtc::scoped_refptr sender = new VideoRtpSender(track, kStreamLabel1, &video_provider_); sender->SetSsrc(kVideoSsrc); - // Expect that SetSource will be called before the reference to the track + // Expect that SetVideoSend will be called before the reference to the track // is released. - EXPECT_CALL(video_provider_, SetSource(kVideoSsrc, nullptr)) + EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, false, _, nullptr)) .Times(1) .WillOnce(InvokeWithoutArgs([&track] { EXPECT_LT(2, track->AddRef()); track->Release(); - return true; })); - EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, false, _)).Times(1); EXPECT_TRUE(sender->SetTrack(nullptr)); // Make sure it's SetTrack that called methods on the provider, and not the // destructor. - EXPECT_CALL(video_provider_, SetSource(_, _)).Times(0); - EXPECT_CALL(video_provider_, SetVideoSend(_, _, _)).Times(0); + EXPECT_CALL(video_provider_, SetVideoSend(_, _, _, _)).Times(0); } TEST_F(RtpSenderReceiverTest, AudioSenderSsrcChanged) { @@ -492,21 +487,22 @@ TEST_F(RtpSenderReceiverTest, AudioSenderSsrcChanged) { TEST_F(RtpSenderReceiverTest, VideoSenderSsrcChanged) { AddVideoTrack(); - EXPECT_CALL(video_provider_, SetSource(kVideoSsrc, video_track_.get())); - EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, true, _)); + EXPECT_CALL(video_provider_, + SetVideoSend(kVideoSsrc, true, _, video_track_.get())); rtc::scoped_refptr sender = new VideoRtpSender(video_track_, kStreamLabel1, &video_provider_); sender->SetSsrc(kVideoSsrc); - EXPECT_CALL(video_provider_, SetSource(kVideoSsrc, nullptr)).Times(1); - EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, false, _)).Times(1); - EXPECT_CALL(video_provider_, SetSource(kVideoSsrc2, video_track_.get())); - EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc2, true, _)); + EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, false, _, nullptr)) + .Times(1); + EXPECT_CALL(video_provider_, + SetVideoSend(kVideoSsrc2, true, _, video_track_.get())) + .Times(1); sender->SetSsrc(kVideoSsrc2); // Calls expected from destructor. - EXPECT_CALL(video_provider_, SetSource(kVideoSsrc2, nullptr)).Times(1); - EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc2, false, _)).Times(1); + EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc2, false, _, nullptr)) + .Times(1); } TEST_F(RtpSenderReceiverTest, AudioSenderCanSetParameters) { diff --git a/webrtc/api/webrtcsession.cc b/webrtc/api/webrtcsession.cc index 0699308554..a7c712c6a1 100644 --- a/webrtc/api/webrtcsession.cc +++ b/webrtc/api/webrtcsession.cc @@ -1254,21 +1254,6 @@ bool WebRtcSession::SetAudioRtpReceiveParameters( return voice_channel_->SetRtpReceiveParameters(ssrc, parameters); } -bool WebRtcSession::SetSource( - uint32_t ssrc, - rtc::VideoSourceInterface* source) { - ASSERT(signaling_thread()->IsCurrent()); - - if (!video_channel_) { - // |video_channel_| doesnt't exist. Probably because the remote end doesnt't - // support video. - LOG(LS_WARNING) << "Video not used in this call."; - return false; - } - video_channel_->SetSource(ssrc, source); - return true; -} - void WebRtcSession::SetVideoPlayout( uint32_t ssrc, bool enable, @@ -1286,19 +1271,21 @@ void WebRtcSession::SetVideoPlayout( } } -void WebRtcSession::SetVideoSend(uint32_t ssrc, - bool enable, - const cricket::VideoOptions* options) { +void WebRtcSession::SetVideoSend( + uint32_t ssrc, + bool enable, + const cricket::VideoOptions* options, + rtc::VideoSourceInterface* source) { ASSERT(signaling_thread()->IsCurrent()); if (!video_channel_) { LOG(LS_WARNING) << "SetVideoSend: No video channel exists."; return; } - if (!video_channel_->SetVideoSend(ssrc, enable, options)) { - // Allow that MuteStream fail if |enable| is false but assert otherwise. - // This in the normal case when the underlying media channel has already - // been deleted. - ASSERT(enable == false); + if (!video_channel_->SetVideoSend(ssrc, enable, options, source)) { + // Allow that MuteStream fail if |enable| is false and |source| is NULL but + // assert otherwise. This in the normal case when the underlying media + // channel has already been deleted. + ASSERT(enable == false && source == nullptr); } } diff --git a/webrtc/api/webrtcsession.h b/webrtc/api/webrtcsession.h index dd47229a61..9ba7605e67 100644 --- a/webrtc/api/webrtcsession.h +++ b/webrtc/api/webrtcsession.h @@ -252,16 +252,15 @@ class WebRtcSession : public AudioProviderInterface, const RtpParameters& parameters) override; // Implements VideoMediaProviderInterface. - bool SetSource( - uint32_t ssrc, - rtc::VideoSourceInterface* source) override; void SetVideoPlayout( uint32_t ssrc, bool enable, rtc::VideoSinkInterface* sink) override; - void SetVideoSend(uint32_t ssrc, - bool enable, - const cricket::VideoOptions* options) override; + void SetVideoSend( + uint32_t ssrc, + bool enable, + const cricket::VideoOptions* options, + rtc::VideoSourceInterface* source) override; RtpParameters GetVideoRtpSendParameters(uint32_t ssrc) const override; bool SetVideoRtpSendParameters(uint32_t ssrc, diff --git a/webrtc/api/webrtcsession_unittest.cc b/webrtc/api/webrtcsession_unittest.cc index f2ac38c438..088694cb16 100644 --- a/webrtc/api/webrtcsession_unittest.cc +++ b/webrtc/api/webrtcsession_unittest.cc @@ -253,7 +253,6 @@ class WebRtcSessionForTest : public webrtc::WebRtcSession { using webrtc::WebRtcSession::SetAudioPlayout; using webrtc::WebRtcSession::SetAudioSend; - using webrtc::WebRtcSession::SetSource; using webrtc::WebRtcSession::SetVideoPlayout; using webrtc::WebRtcSession::SetVideoSend; @@ -3540,9 +3539,9 @@ TEST_F(WebRtcSessionTest, SetVideoSend) { uint32_t send_ssrc = channel->send_streams()[0].first_ssrc(); EXPECT_FALSE(channel->IsStreamMuted(send_ssrc)); cricket::VideoOptions* options = NULL; - session_->SetVideoSend(send_ssrc, false, options); + session_->SetVideoSend(send_ssrc, false, options, nullptr); EXPECT_TRUE(channel->IsStreamMuted(send_ssrc)); - session_->SetVideoSend(send_ssrc, true, options); + session_->SetVideoSend(send_ssrc, true, options, nullptr); EXPECT_FALSE(channel->IsStreamMuted(send_ssrc)); } diff --git a/webrtc/media/base/fakemediaengine.h b/webrtc/media/base/fakemediaengine.h index b9be39bdcf..883b474a49 100644 --- a/webrtc/media/base/fakemediaengine.h +++ b/webrtc/media/base/fakemediaengine.h @@ -547,20 +547,19 @@ class FakeVideoMediaChannel : public RtpHelper { } bool SetSend(bool send) override { return set_sending(send); } - bool SetVideoSend(uint32_t ssrc, bool enable, - const VideoOptions* options) override { + bool SetVideoSend( + uint32_t ssrc, + bool enable, + const VideoOptions* options, + rtc::VideoSourceInterface* source) override { if (!RtpHelper::MuteStream(ssrc, !enable)) { return false; } if (enable && options) { return SetOptions(*options); } - return true; - } - void SetSource( - uint32_t ssrc, - rtc::VideoSourceInterface* source) override { sources_[ssrc] = source; + return true; } bool HasSource(uint32_t ssrc) const { diff --git a/webrtc/media/base/mediachannel.h b/webrtc/media/base/mediachannel.h index 3e66bcb746..bb30798249 100644 --- a/webrtc/media/base/mediachannel.h +++ b/webrtc/media/base/mediachannel.h @@ -991,18 +991,17 @@ class VideoMediaChannel : public MediaChannel { virtual bool GetSendCodec(VideoCodec* send_codec) = 0; // Starts or stops transmission (and potentially capture) of local video. virtual bool SetSend(bool send) = 0; - // Configure stream for sending. - virtual bool SetVideoSend(uint32_t ssrc, - bool enable, - const VideoOptions* options) = 0; + // Configure stream for sending and register a source. + // The |ssrc| must correspond to a registered send stream. + virtual bool SetVideoSend( + uint32_t ssrc, + bool enable, + const VideoOptions* options, + rtc::VideoSourceInterface* source) = 0; // Sets the sink object to be used for the specified stream. // If SSRC is 0, the renderer is used for the 'default' stream. virtual bool SetSink(uint32_t ssrc, rtc::VideoSinkInterface* sink) = 0; - // Register a source. The |ssrc| must correspond to a registered send stream. - virtual void SetSource( - uint32_t ssrc, - rtc::VideoSourceInterface* source) = 0; // Gets quality stats for the channel. virtual bool GetStats(VideoMediaInfo* info) = 0; }; diff --git a/webrtc/media/base/videoengine_unittest.h b/webrtc/media/base/videoengine_unittest.h index 394b9c9a8e..fcc77d70c1 100644 --- a/webrtc/media/base/videoengine_unittest.h +++ b/webrtc/media/base/videoengine_unittest.h @@ -104,7 +104,8 @@ class VideoMediaChannelTest : public testing::Test, cricket::VideoFormat::FpsToInterval(30), cricket::FOURCC_I420); EXPECT_EQ(cricket::CS_RUNNING, video_capturer_->Start(format)); - channel_->SetSource(kSsrc, video_capturer_.get()); + EXPECT_TRUE( + channel_->SetVideoSend(kSsrc, true, nullptr, video_capturer_.get())); } virtual cricket::FakeVideoCapturer* CreateFakeVideoCapturer() { @@ -141,7 +142,8 @@ class VideoMediaChannelTest : public testing::Test, cricket::FOURCC_I420); EXPECT_EQ(cricket::CS_RUNNING, video_capturer_2_->Start(format)); - channel_->SetSource(kSsrc + 2, video_capturer_2_.get()); + EXPECT_TRUE(channel_->SetVideoSend(kSsrc + 2, true, nullptr, + video_capturer_2_.get())); } virtual void TearDown() { channel_.reset(); @@ -352,7 +354,8 @@ class VideoMediaChannelTest : public testing::Test, // Test that SetSend works. void SetSend() { EXPECT_FALSE(channel_->sending()); - channel_->SetSource(kSsrc, video_capturer_.get()); + EXPECT_TRUE( + channel_->SetVideoSend(kSsrc, true, nullptr, video_capturer_.get())); EXPECT_TRUE(SetOneCodec(DefaultCodec())); EXPECT_FALSE(channel_->sending()); EXPECT_TRUE(SetSend(true)); @@ -546,7 +549,7 @@ class VideoMediaChannelTest : public testing::Test, EXPECT_EQ(cricket::CS_RUNNING, capturer->Start(format)); EXPECT_TRUE(channel_->AddSendStream( cricket::StreamParams::CreateLegacy(5678))); - channel_->SetSource(5678, capturer.get()); + EXPECT_TRUE(channel_->SetVideoSend(5678, true, nullptr, capturer.get())); EXPECT_TRUE(channel_->AddRecvStream( cricket::StreamParams::CreateLegacy(5678))); EXPECT_TRUE(channel_->SetSink(5678, &renderer2)); @@ -582,7 +585,7 @@ class VideoMediaChannelTest : public testing::Test, EXPECT_EQ(kTestWidth, info.senders[1].send_frame_width); EXPECT_EQ(kTestHeight, info.senders[1].send_frame_height); // The capturer must be unregistered here as it runs out of it's scope next. - channel_->SetSource(5678, NULL); + channel_->SetVideoSend(5678, true, nullptr, nullptr); } // Test that we can set the bandwidth. @@ -619,7 +622,8 @@ class VideoMediaChannelTest : public testing::Test, EXPECT_TRUE(SetDefaultCodec()); EXPECT_TRUE(channel_->AddSendStream( cricket::StreamParams::CreateLegacy(999))); - channel_->SetSource(999u, video_capturer_.get()); + EXPECT_TRUE( + channel_->SetVideoSend(999u, true, nullptr, video_capturer_.get())); EXPECT_TRUE(SetSend(true)); EXPECT_TRUE(WaitAndSendFrame(0)); EXPECT_TRUE_WAIT(NumRtpPackets() > 0, kTimeout); @@ -685,7 +689,8 @@ class VideoMediaChannelTest : public testing::Test, EXPECT_TRUE(channel_->AddSendStream( cricket::StreamParams::CreateLegacy(789u))); - channel_->SetSource(789u, video_capturer_.get()); + EXPECT_TRUE( + channel_->SetVideoSend(789u, true, nullptr, video_capturer_.get())); EXPECT_EQ(rtp_packets, NumRtpPackets()); // Wait 30ms to guarantee the engine does not drop the frame. EXPECT_TRUE(WaitAndSendFrame(30)); @@ -754,7 +759,7 @@ class VideoMediaChannelTest : public testing::Test, // test which is related to screencast logic. cricket::VideoOptions video_options; video_options.is_screencast = rtc::Optional(true); - channel_->SetVideoSend(kSsrc, true, &video_options); + channel_->SetVideoSend(kSsrc, true, &video_options, nullptr); cricket::VideoFormat format(480, 360, cricket::VideoFormat::FpsToInterval(30), @@ -768,7 +773,7 @@ class VideoMediaChannelTest : public testing::Test, int captured_frames = 1; for (int iterations = 0; iterations < 2; ++iterations) { - channel_->SetSource(kSsrc, capturer.get()); + EXPECT_TRUE(channel_->SetVideoSend(kSsrc, true, nullptr, capturer.get())); rtc::Thread::Current()->ProcessMessages(time_between_send); EXPECT_TRUE(capturer->CaptureCustomFrame(format.width, format.height, cricket::FOURCC_I420)); @@ -783,7 +788,7 @@ class VideoMediaChannelTest : public testing::Test, EXPECT_EQ(format.height, renderer_.height()); captured_frames = renderer_.num_rendered_frames() + 1; EXPECT_FALSE(renderer_.black_frame()); - channel_->SetSource(kSsrc, NULL); + EXPECT_TRUE(channel_->SetVideoSend(kSsrc, true, nullptr, nullptr)); // Make sure a black frame is generated within the specified timeout. // The black frame should be the resolution of the previous frame to // prevent expensive encoder reconfigurations. @@ -805,8 +810,9 @@ class VideoMediaChannelTest : public testing::Test, } } - // Tests that if RemoveCapturer is called without a capturer ever being - // added, the plugin shouldn't crash (and no black frame should be sent). + // Tests that if SetVideoSend is called with a NULL capturer after the + // capturer was already removed, the application doesn't crash (and no black + // frame is sent). void RemoveCapturerWithoutAdd() { EXPECT_TRUE(SetOneCodec(DefaultCodec())); EXPECT_TRUE(SetSend(true)); @@ -818,12 +824,12 @@ class VideoMediaChannelTest : public testing::Test, // tightly. rtc::Thread::Current()->ProcessMessages(30); // Remove the capturer. - channel_->SetSource(kSsrc, NULL); + EXPECT_TRUE(channel_->SetVideoSend(kSsrc, true, nullptr, nullptr)); // Wait for one black frame for removing the capturer. EXPECT_FRAME_WAIT(2, 640, 400, kTimeout); - // No capturer was added, so this SetSource should be a NOP. - channel_->SetSource(kSsrc, NULL); + // No capturer was added, so this SetVideoSend shouldn't do anything. + EXPECT_TRUE(channel_->SetVideoSend(kSsrc, true, nullptr, nullptr)); rtc::Thread::Current()->ProcessMessages(300); // Verify no more frames were sent. EXPECT_EQ(2, renderer_.num_rendered_frames()); @@ -865,11 +871,11 @@ class VideoMediaChannelTest : public testing::Test, EXPECT_EQ(cricket::CS_RUNNING, capturer2->Start(capture_format)); // State for all the streams. EXPECT_TRUE(SetOneCodec(DefaultCodec())); - // A limitation in the lmi implementation requires that SetSource() is + // A limitation in the lmi implementation requires that SetVideoSend() is // called after SetOneCodec(). // TODO(hellner): this seems like an unnecessary constraint, fix it. - channel_->SetSource(1, capturer1.get()); - channel_->SetSource(2, capturer2.get()); + EXPECT_TRUE(channel_->SetVideoSend(1, true, nullptr, capturer1.get())); + EXPECT_TRUE(channel_->SetVideoSend(2, true, nullptr, capturer2.get())); EXPECT_TRUE(SetSend(true)); // Test capturer associated with engine. const int kTestWidth = 160; @@ -884,13 +890,11 @@ class VideoMediaChannelTest : public testing::Test, EXPECT_FRAME_ON_RENDERER_WAIT( renderer2, 1, kTestWidth, kTestHeight, kTimeout); // Successfully remove the capturer. - channel_->SetSource(kSsrc, NULL); - // Fail to re-remove the capturer. - channel_->SetSource(kSsrc, NULL); + EXPECT_TRUE(channel_->SetVideoSend(kSsrc, true, nullptr, nullptr)); // The capturers must be unregistered here as it runs out of it's scope // next. - channel_->SetSource(1, NULL); - channel_->SetSource(2, NULL); + EXPECT_TRUE(channel_->SetVideoSend(1, true, nullptr, nullptr)); + EXPECT_TRUE(channel_->SetVideoSend(2, true, nullptr, nullptr)); } void HighAspectHighHeightCapturer() { @@ -923,13 +927,13 @@ class VideoMediaChannelTest : public testing::Test, EXPECT_EQ(cricket::CS_RUNNING, capturer->Start(capture_format)); // Capture frame to not get same frame timestamps as previous capturer. capturer->CaptureFrame(); - channel_->SetSource(kSsrc, capturer.get()); + EXPECT_TRUE(channel_->SetVideoSend(kSsrc, true, nullptr, capturer.get())); EXPECT_TRUE(rtc::Thread::Current()->ProcessMessages(30)); EXPECT_TRUE(capturer->CaptureCustomFrame(kWidth, kHeight, cricket::FOURCC_ARGB)); EXPECT_GT_FRAME_ON_RENDERER_WAIT( renderer, 2, kScaledWidth, kScaledHeight, kTimeout); - channel_->SetSource(kSsrc, NULL); + EXPECT_TRUE(channel_->SetVideoSend(kSsrc, true, nullptr, nullptr)); } // Tests that we can adapt video resolution with 16:10 aspect ratio properly. diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc index 07d47ab10f..c6e9968998 100644 --- a/webrtc/media/engine/webrtcvideoengine2.cc +++ b/webrtc/media/engine/webrtcvideoengine2.cc @@ -1048,19 +1048,29 @@ bool WebRtcVideoChannel2::SetSend(bool send) { } // TODO(nisse): The enable argument was used for mute logic which has -// been moved to VideoBroadcaster. So delete this method, and use -// SetOptions instead. -bool WebRtcVideoChannel2::SetVideoSend(uint32_t ssrc, bool enable, - const VideoOptions* options) { +// been moved to VideoBroadcaster. So remove the argument from this +// method. +bool WebRtcVideoChannel2::SetVideoSend( + uint32_t ssrc, + bool enable, + const VideoOptions* options, + rtc::VideoSourceInterface* source) { TRACE_EVENT0("webrtc", "SetVideoSend"); + RTC_DCHECK(ssrc != 0); LOG(LS_INFO) << "SetVideoSend (ssrc= " << ssrc << ", enable = " << enable - << "options: " << (options ? options->ToString() : "nullptr") - << ")."; + << ", options: " << (options ? options->ToString() : "nullptr") + << ", source = " << (source ? "(source)" : "nullptr") << ")"; - if (enable && options) { - SetOptions(ssrc, *options); + rtc::CritScope stream_lock(&stream_crit_); + const auto& kv = send_streams_.find(ssrc); + if (kv == send_streams_.end()) { + // Allow unknown ssrc only if source is null. + RTC_CHECK(source == nullptr); + LOG(LS_ERROR) << "No sending stream on ssrc " << ssrc; + return false; } - return true; + + return kv->second->SetVideoSend(enable, options, source); } bool WebRtcVideoChannel2::ValidateSendSsrcAvailability( @@ -1287,7 +1297,8 @@ bool WebRtcVideoChannel2::RemoveRecvStream(uint32_t ssrc) { bool WebRtcVideoChannel2::SetSink(uint32_t ssrc, rtc::VideoSinkInterface* sink) { - LOG(LS_INFO) << "SetSink: ssrc:" << ssrc << " " << (sink ? "(ptr)" : "NULL"); + LOG(LS_INFO) << "SetSink: ssrc:" << ssrc << " " + << (sink ? "(ptr)" : "nullptr"); if (ssrc == 0) { default_unsignalled_ssrc_handler_.SetDefaultSink(this, sink); return true; @@ -1355,23 +1366,6 @@ void WebRtcVideoChannel2::FillBandwidthEstimationStats( video_media_info->bw_estimations.push_back(bwe_info); } -void WebRtcVideoChannel2::SetSource( - uint32_t ssrc, - rtc::VideoSourceInterface* source) { - LOG(LS_INFO) << "SetSource: " << ssrc << " -> " - << (source ? "(source)" : "NULL"); - RTC_DCHECK(ssrc != 0); - - rtc::CritScope stream_lock(&stream_crit_); - const auto& kv = send_streams_.find(ssrc); - if (kv == send_streams_.end()) { - // Allow unknown ssrc only if source is null. - RTC_CHECK(source == nullptr); - } else { - kv->second->SetSource(source); - } -} - void WebRtcVideoChannel2::OnPacketReceived( rtc::CopyOnWriteBuffer* packet, const rtc::PacketTime& packet_time) { @@ -1458,19 +1452,6 @@ void WebRtcVideoChannel2::OnNetworkRouteChanged( call_->OnNetworkRouteChanged(transport_name, network_route); } -// TODO(pbos): Remove SetOptions in favor of SetSendParameters. -void WebRtcVideoChannel2::SetOptions(uint32_t ssrc, - const VideoOptions& options) { - LOG(LS_INFO) << "SetOptions: ssrc " << ssrc << ": " << options.ToString(); - - rtc::CritScope stream_lock(&stream_crit_); - const auto& kv = send_streams_.find(ssrc); - if (kv == send_streams_.end()) { - return; - } - kv->second->SetOptions(options); -} - void WebRtcVideoChannel2::SetInterface(NetworkInterface* iface) { MediaChannel::SetInterface(iface); // Set the RTP recv/send buffer to a bigger size @@ -1639,24 +1620,40 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::OnFrame( stream_->Input()->IncomingCapturedFrame(video_frame); } -void WebRtcVideoChannel2::WebRtcVideoSendStream::SetSource( +bool WebRtcVideoChannel2::WebRtcVideoSendStream::SetVideoSend( + bool enable, + const VideoOptions* options, rtc::VideoSourceInterface* source) { - TRACE_EVENT0("webrtc", "WebRtcVideoSendStream::SetSource"); + TRACE_EVENT0("webrtc", "WebRtcVideoSendStream::SetVideoSend"); RTC_DCHECK(thread_checker_.CalledOnValidThread()); - if (!source && !source_) - return; - DisconnectSource(); + // Ignore |options| pointer if |enable| is false. + bool options_present = enable && options; + bool source_changing = source_ != source; + if (source_changing) { + DisconnectSource(); + } - { + if (options_present || source_changing) { rtc::CritScope cs(&lock_); - // Reset timestamps to realign new incoming frames to a webrtc timestamp. A - // new capturer may have a different timestamp delta than the previous one. - first_frame_timestamp_ms_ = rtc::Optional(); + if (options_present) { + VideoOptions old_options = parameters_.options; + parameters_.options.SetAll(*options); + // Reconfigure encoder settings on the naext frame or stream + // recreation if the options changed. + if (parameters_.options != old_options) { + pending_encoder_reconfiguration_ = true; + } + } - if (source == NULL) { - if (stream_ != NULL) { + if (source_changing) { + // Reset timestamps to realign new incoming frames to a webrtc timestamp. + // A new source may have a different timestamp delta than the previous + // one. + first_frame_timestamp_ms_ = rtc::Optional(); + + if (source == nullptr && stream_ != nullptr) { LOG(LS_VERBOSE) << "Disabling capturer, sending black frame."; // Force this black frame not to be dropped due to timestamp order // check. As IncomingCapturedFrame will drop the frame if this frame's @@ -1668,14 +1665,16 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::SetSource( CreateBlackFrame(last_dimensions_.width, last_dimensions_.height, last_frame_timestamp_ms_, last_rotation_)); } + source_ = source; } } - source_ = source; + // |source_->AddOrUpdateSink| may not be called while holding |lock_| since // that might cause a lock order inversion. - if (source_) { + if (source_changing && source_) { source_->AddOrUpdateSink(this, sink_wants_); } + return true; } void WebRtcVideoChannel2::WebRtcVideoSendStream::DisconnectSource() { @@ -1688,9 +1687,9 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::DisconnectSource() { // that might cause a lock order inversion. source_->RemoveSink(this); source_ = nullptr; - // Reset |cpu_restricted_counter_| if the capturer is changed. It is not + // Reset |cpu_restricted_counter_| if the source is changed. It is not // possible to know if the video resolution is restricted by CPU usage after - // the capturer is changed since the next capturer might be screen capture + // the source is changed since the next source might be screen capture // with another resolution and frame rate. cpu_restricted_counter_ = 0; } @@ -1700,19 +1699,6 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::GetSsrcs() const { return ssrcs_; } -void WebRtcVideoChannel2::WebRtcVideoSendStream::SetOptions( - const VideoOptions& options) { - rtc::CritScope cs(&lock_); - - VideoOptions old_options = parameters_.options; - parameters_.options.SetAll(options); - // Reconfigure encoder settings on the next frame or stream - // recreation if the options changed. - if (parameters_.options != old_options) { - pending_encoder_reconfiguration_ = true; - } -} - webrtc::VideoCodecType CodecTypeFromName(const std::string& name) { if (CodecNamesEq(name, kVp8CodecName)) { return webrtc::kVideoCodecVP8; @@ -1848,7 +1834,7 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::SetSendParameters( } } // release |lock_| - // |capturer_->AddOrUpdateSink| may not be called while holding |lock_| since + // |source_->AddOrUpdateSink| may not be called while holding |lock_| since // that might cause a lock order inversion. if (params.rtp_header_extensions) { sink_wants_.rotation_applied = !ContainsHeaderExtension( @@ -2043,14 +2029,14 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::OnLoadUpdate(Load load) { return; } // The input video frame size will have a resolution with less than or - // equal to |max_pixel_count| depending on how the capturer can scale the + // equal to |max_pixel_count| depending on how the source can scale the // input frame size. max_pixel_count = rtc::Optional( (last_dimensions_.height * last_dimensions_.width * 3) / 5); // Increase |number_of_cpu_adapt_changes_| if // sink_wants_.max_pixel_count will be changed since - // last time |capturer_->AddOrUpdateSink| was called. That is, this will - // result in a new request for the capturer to change resolution. + // last time |source_->AddOrUpdateSink| was called. That is, this will + // result in a new request for the source to change resolution. if (!sink_wants_.max_pixel_count || *sink_wants_.max_pixel_count > *max_pixel_count) { ++number_of_cpu_adapt_changes_; @@ -2060,13 +2046,13 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::OnLoadUpdate(Load load) { RTC_DCHECK(load == kUnderuse); // The input video frame size will have a resolution with "one step up" // pixels than |max_pixel_count_step_up| where "one step up" depends on - // how the capturer can scale the input frame size. + // how the source can scale the input frame size. max_pixel_count_step_up = rtc::Optional(last_dimensions_.height * last_dimensions_.width); // Increase |number_of_cpu_adapt_changes_| if // sink_wants_.max_pixel_count_step_up will be changed since - // last time |capturer_->AddOrUpdateSink| was called. That is, this will - // result in a new request for the capturer to change resolution. + // last time |source_->AddOrUpdateSink| was called. That is, this will + // result in a new request for the source to change resolution. if (sink_wants_.max_pixel_count || (sink_wants_.max_pixel_count_step_up && *sink_wants_.max_pixel_count_step_up < *max_pixel_count_step_up)) { diff --git a/webrtc/media/engine/webrtcvideoengine2.h b/webrtc/media/engine/webrtcvideoengine2.h index 2d1b1923ed..f34844cebf 100644 --- a/webrtc/media/engine/webrtcvideoengine2.h +++ b/webrtc/media/engine/webrtcvideoengine2.h @@ -156,9 +156,11 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport { const webrtc::RtpParameters& parameters) override; bool GetSendCodec(VideoCodec* send_codec) override; bool SetSend(bool send) override; - bool SetVideoSend(uint32_t ssrc, - bool mute, - const VideoOptions* options) override; + bool SetVideoSend( + uint32_t ssrc, + bool enable, + const VideoOptions* options, + rtc::VideoSourceInterface* source) override; bool AddSendStream(const StreamParams& sp) override; bool RemoveSendStream(uint32_t ssrc) override; bool AddRecvStream(const StreamParams& sp) override; @@ -167,9 +169,6 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport { bool SetSink(uint32_t ssrc, rtc::VideoSinkInterface* sink) override; bool GetStats(VideoMediaInfo* info) override; - void SetSource( - uint32_t ssrc, - rtc::VideoSourceInterface* source) override; void OnPacketReceived(rtc::CopyOnWriteBuffer* packet, const rtc::PacketTime& packet_time) override; @@ -226,7 +225,6 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport { ChangedRecvParameters* changed_params) const; void SetMaxSendBandwidth(int bps); - void SetOptions(uint32_t ssrc, const VideoOptions& options); void ConfigureReceiverRtp(webrtc::VideoReceiveStream::Config* config, const StreamParams& sp) const; @@ -260,14 +258,14 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport { const VideoSendParameters& send_params); virtual ~WebRtcVideoSendStream(); - void SetOptions(const VideoOptions& options); - // TODO(pbos): Move logic from SetOptions into this method. void SetSendParameters(const ChangedSendParameters& send_params); bool SetRtpParameters(const webrtc::RtpParameters& parameters); webrtc::RtpParameters GetRtpParameters() const; void OnFrame(const cricket::VideoFrame& frame) override; - void SetSource(rtc::VideoSourceInterface* source); + bool SetVideoSend(bool mute, + const VideoOptions* options, + rtc::VideoSourceInterface* source); void DisconnectSource(); void SetSend(bool send); diff --git a/webrtc/media/engine/webrtcvideoengine2_unittest.cc b/webrtc/media/engine/webrtcvideoengine2_unittest.cc index 177ab9f223..3f43e5779b 100644 --- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc +++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc @@ -257,7 +257,7 @@ TEST_F(WebRtcVideoEngine2Test, CVOSetHeaderExtensionBeforeCapturer) { EXPECT_TRUE(channel->SetSendParameters(parameters)); // Set capturer. - channel->SetSource(kSsrc, &capturer); + EXPECT_TRUE(channel->SetVideoSend(kSsrc, true, nullptr, &capturer)); // Verify capturer has turned off applying rotation. EXPECT_FALSE(capturer.apply_rotation()); @@ -288,7 +288,7 @@ TEST_F(WebRtcVideoEngine2Test, CVOSetHeaderExtensionBeforeAddSendStream) { EXPECT_TRUE(channel->AddSendStream(StreamParams::CreateLegacy(kSsrc))); // Set capturer. - channel->SetSource(kSsrc, &capturer); + EXPECT_TRUE(channel->SetVideoSend(kSsrc, true, nullptr, &capturer)); // Verify capturer has turned off applying rotation. EXPECT_FALSE(capturer.apply_rotation()); @@ -309,7 +309,7 @@ TEST_F(WebRtcVideoEngine2Test, CVOSetHeaderExtensionAfterCapturer) { EXPECT_TRUE(channel->AddSendStream(StreamParams::CreateLegacy(kSsrc))); // Set capturer. - channel->SetSource(kSsrc, &capturer); + EXPECT_TRUE(channel->SetVideoSend(kSsrc, true, nullptr, &capturer)); // Verify capturer has turned on applying rotation. EXPECT_TRUE(capturer.apply_rotation()); @@ -369,7 +369,7 @@ TEST_F(WebRtcVideoEngine2Test, UseExternalFactoryForVp8WhenSupported) { EXPECT_TRUE(channel->SetSend(true)); cricket::FakeVideoCapturer capturer; - channel->SetSource(kSsrc, &capturer); + EXPECT_TRUE(channel->SetVideoSend(kSsrc, true, nullptr, &capturer)); EXPECT_EQ(cricket::CS_RUNNING, capturer.Start(capturer.GetSupportedFormats()->front())); EXPECT_TRUE(capturer.CaptureFrame()); @@ -460,7 +460,7 @@ TEST_F(WebRtcVideoEngine2Test, PropagatesInputFrameTimestamp) { channel->AddSendStream(cricket::StreamParams::CreateLegacy(kSsrc))); FakeVideoCapturer capturer; - channel->SetSource(kSsrc, &capturer); + EXPECT_TRUE(channel->SetVideoSend(kSsrc, true, nullptr, &capturer)); capturer.Start(cricket::VideoFormat(1280, 720, cricket::VideoFormat::FpsToInterval(60), cricket::FOURCC_I420)); @@ -522,7 +522,7 @@ TEST_F(WebRtcVideoEngine2Test, FakeVideoSendStream* stream = fake_call->GetVideoSendStreams()[0]; FakeVideoCapturer capturer1; - channel->SetSource(kSsrc, &capturer1); + EXPECT_TRUE(channel->SetVideoSend(kSsrc, true, nullptr, &capturer1)); cricket::CapturedFrame frame; frame.width = 1280; @@ -547,7 +547,7 @@ TEST_F(WebRtcVideoEngine2Test, // Reset input source, should still be continuous even though input-frame // timestamp is less than before. FakeVideoCapturer capturer2; - channel->SetSource(kSsrc, &capturer2); + EXPECT_TRUE(channel->SetVideoSend(kSsrc, true, nullptr, &capturer2)); rtc::Thread::Current()->SleepMs(1); // Deliver with a timestamp (10 seconds) before the previous initial one, @@ -607,7 +607,7 @@ TEST_F(WebRtcVideoEngine2Test, UsesSimulcastAdapterForVp8Factories) { EXPECT_TRUE(channel->SetSend(true)); cricket::FakeVideoCapturer capturer; - channel->SetSource(ssrcs.front(), &capturer); + EXPECT_TRUE(channel->SetVideoSend(ssrcs.front(), true, nullptr, &capturer)); EXPECT_EQ(cricket::CS_RUNNING, capturer.Start(capturer.GetSupportedFormats()->front())); EXPECT_TRUE(capturer.CaptureFrame()); @@ -626,7 +626,7 @@ TEST_F(WebRtcVideoEngine2Test, UsesSimulcastAdapterForVp8Factories) { prev_width = codec_settings.width; } - channel->SetSource(ssrcs.front(), NULL); + EXPECT_TRUE(channel->SetVideoSend(ssrcs.front(), true, nullptr, nullptr)); channel.reset(); ASSERT_EQ(0u, encoder_factory.encoders().size()); @@ -689,7 +689,7 @@ TEST_F(WebRtcVideoEngine2Test, // encoder adapter at a low-enough size that it'll only create a single // encoder layer. cricket::FakeVideoCapturer capturer; - channel->SetSource(ssrcs.front(), &capturer); + EXPECT_TRUE(channel->SetVideoSend(ssrcs.front(), true, nullptr, &capturer)); EXPECT_EQ(cricket::CS_RUNNING, capturer.Start(capturer.GetSupportedFormats()->front())); EXPECT_TRUE(capturer.CaptureFrame()); @@ -745,7 +745,7 @@ TEST_F(WebRtcVideoEngine2Test, SimulcastDisabledForH264) { cricket::VideoFormat format( 1280, 720, cricket::VideoFormat::FpsToInterval(30), cricket::FOURCC_I420); cricket::FakeVideoCapturer capturer; - channel->SetSource(ssrcs[0], &capturer); + EXPECT_TRUE(channel->SetVideoSend(ssrcs[0], true, nullptr, &capturer)); EXPECT_EQ(cricket::CS_RUNNING, capturer.Start(format)); EXPECT_TRUE(capturer.CaptureFrame()); @@ -754,7 +754,7 @@ TEST_F(WebRtcVideoEngine2Test, SimulcastDisabledForH264) { ASSERT_TRUE(encoder_factory.encoders()[0]->WaitForInitEncode()); EXPECT_EQ(webrtc::kVideoCodecH264, encoder->GetCodecSettings().codecType); EXPECT_EQ(1u, encoder->GetCodecSettings().numberOfSimulcastStreams); - channel->SetSource(ssrcs[0], nullptr); + EXPECT_TRUE(channel->SetVideoSend(ssrcs[0], true, nullptr, nullptr)); } // Test that external codecs are added to the end of the supported codec list. @@ -1075,7 +1075,7 @@ class WebRtcVideoChannel2Test : public WebRtcVideoEngine2Test { bool enabled) { cricket::VideoOptions options; options.video_noise_reduction = rtc::Optional(enabled); - channel_->SetVideoSend(ssrc, true, &options); + EXPECT_TRUE(channel_->SetVideoSend(ssrc, true, &options, capturer)); // Options only take effect on the next frame. EXPECT_TRUE(capturer->CaptureFrame()); @@ -1558,7 +1558,7 @@ TEST_F(WebRtcVideoChannel2Test, ReconfiguresEncodersWhenNotSending) { EXPECT_EQ(144u, streams[0].height); cricket::FakeVideoCapturer capturer; - channel_->SetSource(last_ssrc_, &capturer); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, nullptr, &capturer)); EXPECT_EQ(cricket::CS_RUNNING, capturer.Start(capturer.GetSupportedFormats()->front())); EXPECT_TRUE(capturer.CaptureFrame()); @@ -1570,7 +1570,7 @@ TEST_F(WebRtcVideoChannel2Test, ReconfiguresEncodersWhenNotSending) { // No frames should have been actually put in there though. EXPECT_EQ(0, stream->GetNumberOfSwappedFrames()); - channel_->SetSource(last_ssrc_, NULL); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, nullptr, nullptr)); } TEST_F(WebRtcVideoChannel2Test, UsesCorrectSettingsForScreencast) { @@ -1581,13 +1581,12 @@ TEST_F(WebRtcVideoChannel2Test, UsesCorrectSettingsForScreencast) { EXPECT_TRUE(channel_->SetSendParameters(parameters)); AddSendStream(); + cricket::FakeVideoCapturer capturer; VideoOptions min_bitrate_options; min_bitrate_options.screencast_min_bitrate_kbps = rtc::Optional(kScreenshareMinBitrateKbps); - channel_->SetVideoSend(last_ssrc_, true, &min_bitrate_options); - - cricket::FakeVideoCapturer capturer; - channel_->SetSource(last_ssrc_, &capturer); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, &min_bitrate_options, + &capturer)); cricket::VideoFormat capture_format_hd = capturer.GetSupportedFormats()->front(); EXPECT_EQ(1280, capture_format_hd.width); @@ -1611,13 +1610,13 @@ TEST_F(WebRtcVideoChannel2Test, UsesCorrectSettingsForScreencast) { EXPECT_EQ(0, encoder_config.min_transmit_bitrate_bps) << "Non-screenshare shouldn't use min-transmit bitrate."; - channel_->SetSource(last_ssrc_, nullptr); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, nullptr, nullptr)); // Removing a capturer triggers a black frame to be sent. EXPECT_EQ(2, send_stream->GetNumberOfSwappedFrames()); - channel_->SetSource(last_ssrc_, &capturer); VideoOptions screencast_options; screencast_options.is_screencast = rtc::Optional(true); - EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, &screencast_options)); + EXPECT_TRUE( + channel_->SetVideoSend(last_ssrc_, true, &screencast_options, &capturer)); EXPECT_TRUE(capturer.CaptureFrame()); // Send stream not recreated after option change. ASSERT_EQ(send_stream, fake_call_->GetVideoSendStreams().front()); @@ -1634,7 +1633,7 @@ TEST_F(WebRtcVideoChannel2Test, UsesCorrectSettingsForScreencast) { EXPECT_EQ(capture_format_hd.height, encoder_config.streams.front().height); EXPECT_TRUE(encoder_config.streams[0].temporal_layer_thresholds_bps.empty()); - channel_->SetSource(last_ssrc_, NULL); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, nullptr, nullptr)); } TEST_F(WebRtcVideoChannel2Test, NoRecreateStreamForScreencast) { @@ -1644,7 +1643,7 @@ TEST_F(WebRtcVideoChannel2Test, NoRecreateStreamForScreencast) { EXPECT_TRUE(channel_->SetSend(true)); cricket::FakeVideoCapturer capturer; - channel_->SetSource(kSsrc, &capturer); + EXPECT_TRUE(channel_->SetVideoSend(kSsrc, true, nullptr, &capturer)); EXPECT_EQ(cricket::CS_RUNNING, capturer.Start(capturer.GetSupportedFormats()->front())); EXPECT_TRUE(capturer.CaptureFrame()); @@ -1661,7 +1660,7 @@ TEST_F(WebRtcVideoChannel2Test, NoRecreateStreamForScreencast) { * encoder, but no change of the send stream. */ struct VideoOptions video_options; video_options.is_screencast = rtc::Optional(true); - channel_->SetVideoSend(kSsrc, true, &video_options); + EXPECT_TRUE(channel_->SetVideoSend(kSsrc, true, &video_options, &capturer)); EXPECT_TRUE(capturer.CaptureFrame()); ASSERT_EQ(1, fake_call_->GetNumCreatedSendStreams()); @@ -1674,7 +1673,7 @@ TEST_F(WebRtcVideoChannel2Test, NoRecreateStreamForScreencast) { /* Switch back. */ video_options.is_screencast = rtc::Optional(false); - channel_->SetVideoSend(kSsrc, true, &video_options); + EXPECT_TRUE(channel_->SetVideoSend(kSsrc, true, &video_options, &capturer)); EXPECT_TRUE(capturer.CaptureFrame()); ASSERT_EQ(1, fake_call_->GetNumCreatedSendStreams()); @@ -1685,7 +1684,7 @@ TEST_F(WebRtcVideoChannel2Test, NoRecreateStreamForScreencast) { EXPECT_EQ(webrtc::VideoEncoderConfig::ContentType::kRealtimeVideo, encoder_config.content_type); - channel_->SetSource(kSsrc, NULL); + EXPECT_TRUE(channel_->SetVideoSend(kSsrc, true, nullptr, nullptr)); } TEST_F(WebRtcVideoChannel2Test, @@ -1698,9 +1697,8 @@ TEST_F(WebRtcVideoChannel2Test, AddSendStream(); VideoOptions options; options.is_screencast = rtc::Optional(true); - channel_->SetVideoSend(last_ssrc_, true, &options); cricket::FakeVideoCapturer capturer; - channel_->SetSource(last_ssrc_, &capturer); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, &options, &capturer)); cricket::VideoFormat capture_format_hd = capturer.GetSupportedFormats()->front(); EXPECT_EQ(cricket::CS_RUNNING, capturer.Start(capture_format_hd)); @@ -1722,7 +1720,7 @@ TEST_F(WebRtcVideoChannel2Test, EXPECT_EQ(kConferenceScreencastTemporalBitrateBps, encoder_config.streams[0].temporal_layer_thresholds_bps[0]); - channel_->SetSource(last_ssrc_, NULL); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, nullptr, nullptr)); } TEST_F(WebRtcVideoChannel2Test, SuspendBelowMinBitrateDisabledByDefault) { @@ -1772,7 +1770,7 @@ TEST_F(WebRtcVideoChannel2Test, VerifyVp8SpecificSettings) { cricket::FakeVideoCapturer capturer; EXPECT_EQ(cricket::CS_RUNNING, capturer.Start(capturer.GetSupportedFormats()->front())); - channel_->SetSource(last_ssrc_, &capturer); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, nullptr, &capturer)); channel_->SetSend(true); EXPECT_TRUE(capturer.CaptureFrame()); @@ -1796,9 +1794,9 @@ TEST_F(WebRtcVideoChannel2Test, VerifyVp8SpecificSettings) { EXPECT_TRUE(vp8_settings.automaticResizeOn); EXPECT_TRUE(vp8_settings.frameDroppingOn); - channel_->SetSource(last_ssrc_, NULL); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, nullptr, nullptr)); stream = SetUpSimulcast(true, false); - channel_->SetSource(last_ssrc_, &capturer); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, nullptr, &capturer)); channel_->SetSend(true); EXPECT_TRUE(capturer.CaptureFrame()); @@ -1811,7 +1809,7 @@ TEST_F(WebRtcVideoChannel2Test, VerifyVp8SpecificSettings) { // In screen-share mode, denoising is forced off and simulcast disabled. VideoOptions options; options.is_screencast = rtc::Optional(true); - EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, &options)); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, &options, &capturer)); stream = SetDenoisingOption(last_ssrc_, &capturer, false); @@ -1829,7 +1827,7 @@ TEST_F(WebRtcVideoChannel2Test, VerifyVp8SpecificSettings) { EXPECT_FALSE(vp8_settings.automaticResizeOn); EXPECT_FALSE(vp8_settings.frameDroppingOn); - channel_->SetSource(last_ssrc_, NULL); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, nullptr, nullptr)); } // Test that setting the same options doesn't result in the encoder being @@ -1839,21 +1837,20 @@ TEST_F(WebRtcVideoChannel2Test, SetIdenticalOptionsDoesntReconfigureEncoder) { cricket::FakeVideoCapturer capturer; FakeVideoSendStream* send_stream = AddSendStream(); - channel_->SetSource(last_ssrc_, &capturer); EXPECT_EQ(cricket::CS_RUNNING, capturer.Start(capturer.GetSupportedFormats()->front())); - EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, &options)); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, &options, &capturer)); EXPECT_TRUE(capturer.CaptureFrame()); // Expect 2 reconfigurations at this point, from the initial configuration // and from the dimensions of the first frame. EXPECT_EQ(2, send_stream->num_encoder_reconfigurations()); // Set the options one more time and expect no additional reconfigurations. - EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, &options)); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, &options, &capturer)); EXPECT_TRUE(capturer.CaptureFrame()); EXPECT_EQ(2, send_stream->num_encoder_reconfigurations()); - channel_->SetSource(last_ssrc_, nullptr); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, nullptr, nullptr)); } class Vp9SettingsTest : public WebRtcVideoChannel2Test { @@ -1891,7 +1888,7 @@ TEST_F(Vp9SettingsTest, VerifyVp9SpecificSettings) { cricket::FakeVideoCapturer capturer; EXPECT_EQ(cricket::CS_RUNNING, capturer.Start(capturer.GetSupportedFormats()->front())); - channel_->SetSource(last_ssrc_, &capturer); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, nullptr, &capturer)); channel_->SetSend(true); EXPECT_TRUE(capturer.CaptureFrame()); @@ -1917,7 +1914,7 @@ TEST_F(Vp9SettingsTest, VerifyVp9SpecificSettings) { // In screen-share mode, denoising is forced off. VideoOptions options; options.is_screencast = rtc::Optional(true); - EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, &options)); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, &options, &capturer)); stream = SetDenoisingOption(last_ssrc_, &capturer, false); @@ -1932,7 +1929,7 @@ TEST_F(Vp9SettingsTest, VerifyVp9SpecificSettings) { EXPECT_FALSE(vp9_settings.denoisingOn); EXPECT_FALSE(vp9_settings.frameDroppingOn); - channel_->SetSource(last_ssrc_, NULL); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, nullptr, nullptr)); } class Vp9SettingsTestWithFieldTrial : public Vp9SettingsTest { @@ -1951,7 +1948,7 @@ class Vp9SettingsTestWithFieldTrial : public Vp9SettingsTest { cricket::FakeVideoCapturer capturer; EXPECT_EQ(cricket::CS_RUNNING, capturer.Start(capturer.GetSupportedFormats()->front())); - channel_->SetSource(last_ssrc_, &capturer); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, nullptr, &capturer)); channel_->SetSend(true); EXPECT_TRUE(capturer.CaptureFrame()); @@ -1961,7 +1958,7 @@ class Vp9SettingsTestWithFieldTrial : public Vp9SettingsTest { EXPECT_EQ(num_spatial_layers, vp9_settings.numberOfSpatialLayers); EXPECT_EQ(num_temporal_layers, vp9_settings.numberOfTemporalLayers); - channel_->SetSource(last_ssrc_, NULL); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, nullptr, nullptr)); } }; @@ -2026,7 +2023,7 @@ TEST_F(WebRtcVideoChannel2Test, AdaptsOnOveruseAndChangeResolution) { AddSendStream(); cricket::FakeVideoCapturer capturer; - channel_->SetSource(last_ssrc_, &capturer); + ASSERT_TRUE(channel_->SetVideoSend(last_ssrc_, true, nullptr, &capturer)); ASSERT_EQ(cricket::CS_RUNNING, capturer.Start(capturer.GetSupportedFormats()->front())); ASSERT_TRUE(channel_->SetSend(true)); @@ -2084,7 +2081,7 @@ TEST_F(WebRtcVideoChannel2Test, AdaptsOnOveruseAndChangeResolution) { EXPECT_EQ(1284, send_stream->GetLastWidth()); EXPECT_EQ(724, send_stream->GetLastHeight()); - channel_->SetSource(last_ssrc_, NULL); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, nullptr, nullptr)); } TEST_F(WebRtcVideoChannel2Test, PreviousAdaptationDoesNotApplyToScreenshare) { @@ -2100,12 +2097,12 @@ TEST_F(WebRtcVideoChannel2Test, PreviousAdaptationDoesNotApplyToScreenshare) { AddSendStream(); cricket::FakeVideoCapturer capturer; - channel_->SetSource(last_ssrc_, &capturer); ASSERT_EQ(cricket::CS_RUNNING, capturer.Start(capturer.GetSupportedFormats()->front())); ASSERT_TRUE(channel_->SetSend(true)); cricket::VideoOptions camera_options; - channel_->SetVideoSend(last_ssrc_, true /* enable */, &camera_options); + channel_->SetVideoSend(last_ssrc_, true /* enable */, &camera_options, + &capturer); ASSERT_EQ(1u, fake_call_->GetVideoSendStreams().size()); FakeVideoSendStream* send_stream = fake_call_->GetVideoSendStreams().front(); @@ -2129,24 +2126,24 @@ TEST_F(WebRtcVideoChannel2Test, PreviousAdaptationDoesNotApplyToScreenshare) { cricket::FakeVideoCapturer screen_share(true); ASSERT_EQ(cricket::CS_RUNNING, screen_share.Start(screen_share.GetSupportedFormats()->front())); - channel_->SetSource(last_ssrc_, &screen_share); cricket::VideoOptions screenshare_options; screenshare_options.is_screencast = rtc::Optional(true); - channel_->SetVideoSend(last_ssrc_, true /* enable */, &screenshare_options); + channel_->SetVideoSend(last_ssrc_, true /* enable */, &screenshare_options, + &screen_share); EXPECT_TRUE(screen_share.CaptureCustomFrame(1284, 724, cricket::FOURCC_I420)); EXPECT_EQ(3, send_stream->GetNumberOfSwappedFrames()); EXPECT_EQ(1284, send_stream->GetLastWidth()); EXPECT_EQ(724, send_stream->GetLastHeight()); // Switch back to the normal capturer. Expect the frame to be CPU adapted. - channel_->SetSource(last_ssrc_, &capturer); - channel_->SetVideoSend(last_ssrc_, true /* enable */, &camera_options); + channel_->SetVideoSend(last_ssrc_, true /* enable */, &camera_options, + &capturer); EXPECT_TRUE(capturer.CaptureCustomFrame(1280, 720, cricket::FOURCC_I420)); EXPECT_EQ(4, send_stream->GetNumberOfSwappedFrames()); EXPECT_EQ(1280 * 3 / 4, send_stream->GetLastWidth()); EXPECT_EQ(720 * 3 / 4, send_stream->GetLastHeight()); - channel_->SetSource(last_ssrc_, NULL); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, nullptr, nullptr)); } void WebRtcVideoChannel2Test::TestCpuAdaptation(bool enable_overuse, @@ -2166,12 +2163,10 @@ void WebRtcVideoChannel2Test::TestCpuAdaptation(bool enable_overuse, AddSendStream(); + cricket::FakeVideoCapturer capturer; VideoOptions options; options.is_screencast = rtc::Optional(is_screenshare); - EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, &options)); - - cricket::FakeVideoCapturer capturer; - channel_->SetSource(last_ssrc_, &capturer); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, &options, &capturer)); EXPECT_EQ(cricket::CS_RUNNING, capturer.Start(capturer.GetSupportedFormats()->front())); @@ -2192,7 +2187,7 @@ void WebRtcVideoChannel2Test::TestCpuAdaptation(bool enable_overuse, EXPECT_EQ(codec.width, send_stream->GetLastWidth()); EXPECT_EQ(codec.height, send_stream->GetLastHeight()); - channel_->SetSource(last_ssrc_, NULL); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, nullptr, nullptr)); return; } @@ -2221,7 +2216,7 @@ void WebRtcVideoChannel2Test::TestCpuAdaptation(bool enable_overuse, EXPECT_EQ(codec.width, send_stream->GetLastWidth()); EXPECT_EQ(codec.height, send_stream->GetLastHeight()); - channel_->SetSource(last_ssrc_, NULL); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, nullptr, nullptr)); } TEST_F(WebRtcVideoChannel2Test, EstimatesNtpStartTimeCorrectly) { @@ -2352,7 +2347,7 @@ TEST_F(WebRtcVideoChannel2Test, SetSendCodecsChangesExistingStreams) { FakeVideoSendStream* stream = AddSendStream(); cricket::FakeVideoCapturer capturer; - channel_->SetSource(last_ssrc_, &capturer); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, nullptr, &capturer)); EXPECT_EQ(cricket::CS_RUNNING, capturer.Start(capturer.GetSupportedFormats()->front())); EXPECT_TRUE(capturer.CaptureFrame()); @@ -2367,7 +2362,7 @@ TEST_F(WebRtcVideoChannel2Test, SetSendCodecsChangesExistingStreams) { streams = fake_call_->GetVideoSendStreams()[0]->GetVideoStreams(); EXPECT_EQ(kVp8Codec360p.width, streams[0].width); EXPECT_EQ(kVp8Codec360p.height, streams[0].height); - channel_->SetSource(last_ssrc_, NULL); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, nullptr, nullptr)); } TEST_F(WebRtcVideoChannel2Test, SetSendCodecsWithBitrates) { @@ -2468,7 +2463,7 @@ TEST_F(WebRtcVideoChannel2Test, SetMaxSendBitrateCanIncreaseSenderBitrate) { FakeVideoSendStream* stream = AddSendStream(); cricket::FakeVideoCapturer capturer; - channel_->SetSource(last_ssrc_, &capturer); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, nullptr, &capturer)); EXPECT_EQ(cricket::CS_RUNNING, capturer.Start(capturer.GetSupportedFormats()->front())); @@ -2482,7 +2477,7 @@ TEST_F(WebRtcVideoChannel2Test, SetMaxSendBitrateCanIncreaseSenderBitrate) { EXPECT_TRUE(capturer.CaptureFrame()); streams = stream->GetVideoStreams(); EXPECT_EQ(initial_max_bitrate_bps * 2, streams[0].max_bitrate_bps); - channel_->SetSource(last_ssrc_, nullptr); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, nullptr, nullptr)); } TEST_F(WebRtcVideoChannel2Test, @@ -2497,7 +2492,7 @@ TEST_F(WebRtcVideoChannel2Test, // Send a frame to make sure this scales up to >1 stream (simulcast). cricket::FakeVideoCapturer capturer; - channel_->SetSource(kSsrcs3[0], &capturer); + EXPECT_TRUE(channel_->SetVideoSend(kSsrcs3[0], true, nullptr, &capturer)); EXPECT_EQ(cricket::CS_RUNNING, capturer.Start(capturer.GetSupportedFormats()->front())); EXPECT_TRUE(capturer.CaptureFrame()); @@ -2516,7 +2511,7 @@ TEST_F(WebRtcVideoChannel2Test, int increased_max_bitrate_bps = GetTotalMaxBitrateBps(streams); EXPECT_EQ(initial_max_bitrate_bps * 2, increased_max_bitrate_bps); - channel_->SetSource(kSsrcs3[0], nullptr); + EXPECT_TRUE(channel_->SetVideoSend(kSsrcs3[0], true, nullptr, nullptr)); } TEST_F(WebRtcVideoChannel2Test, SetSendCodecsWithMaxQuantization) { @@ -2941,7 +2936,8 @@ TEST_F(WebRtcVideoChannel2Test, GetStatsTracksAdaptationStats) { video_capturer_vga.GetSupportedFormats(); cricket::VideoFormat capture_format_vga = (*formats)[1]; EXPECT_EQ(cricket::CS_RUNNING, video_capturer_vga.Start(capture_format_vga)); - channel_->SetSource(kSsrcs3[0], &video_capturer_vga); + EXPECT_TRUE( + channel_->SetVideoSend(kSsrcs3[0], true, nullptr, &video_capturer_vga)); EXPECT_TRUE(video_capturer_vga.CaptureFrame()); cricket::VideoCodec send_codec(100, "VP8", 640, 480, 30); @@ -2978,7 +2974,7 @@ TEST_F(WebRtcVideoChannel2Test, GetStatsTracksAdaptationStats) { info.senders[0].adapt_reason); // No capturer (no adapter). Adapt changes from old adapter should be kept. - channel_->SetSource(kSsrcs3[0], NULL); + EXPECT_TRUE(channel_->SetVideoSend(kSsrcs3[0], true, nullptr, nullptr)); info.Clear(); EXPECT_TRUE(channel_->GetStats(&info)); ASSERT_EQ(1U, info.senders.size()); @@ -2990,7 +2986,8 @@ TEST_F(WebRtcVideoChannel2Test, GetStatsTracksAdaptationStats) { cricket::FakeVideoCapturer video_capturer_hd; cricket::VideoFormat capture_format_hd = (*formats)[0]; EXPECT_EQ(cricket::CS_RUNNING, video_capturer_hd.Start(capture_format_hd)); - channel_->SetSource(kSsrcs3[0], &video_capturer_hd); + EXPECT_TRUE( + channel_->SetVideoSend(kSsrcs3[0], true, nullptr, &video_capturer_hd)); EXPECT_TRUE(video_capturer_hd.CaptureFrame()); // Trigger overuse, HD -> adapt (OnCpuResolutionRequest downgrade) -> HD/2. @@ -3002,7 +2999,7 @@ TEST_F(WebRtcVideoChannel2Test, GetStatsTracksAdaptationStats) { EXPECT_EQ(3, info.senders[0].adapt_changes); EXPECT_EQ(WebRtcVideoChannel2::ADAPTREASON_CPU, info.senders[0].adapt_reason); - channel_->SetSource(kSsrcs3[0], NULL); + EXPECT_TRUE(channel_->SetVideoSend(kSsrcs3[0], true, nullptr, nullptr)); } TEST_F(WebRtcVideoChannel2Test, GetStatsTracksAdaptationAndBandwidthStats) { @@ -3014,7 +3011,8 @@ TEST_F(WebRtcVideoChannel2Test, GetStatsTracksAdaptationAndBandwidthStats) { video_capturer_vga.GetSupportedFormats(); cricket::VideoFormat capture_format_vga = (*formats)[1]; EXPECT_EQ(cricket::CS_RUNNING, video_capturer_vga.Start(capture_format_vga)); - channel_->SetSource(kSsrcs3[0], &video_capturer_vga); + EXPECT_TRUE( + channel_->SetVideoSend(kSsrcs3[0], true, nullptr, &video_capturer_vga)); EXPECT_TRUE(video_capturer_vga.CaptureFrame()); cricket::VideoCodec send_codec(100, "VP8", 640, 480, 30); @@ -3066,7 +3064,7 @@ TEST_F(WebRtcVideoChannel2Test, GetStatsTracksAdaptationAndBandwidthStats) { EXPECT_EQ(WebRtcVideoChannel2::ADAPTREASON_NONE, info.senders[0].adapt_reason); - channel_->SetSource(kSsrcs3[0], NULL); + EXPECT_TRUE(channel_->SetVideoSend(kSsrcs3[0], true, nullptr, nullptr)); } TEST_F(WebRtcVideoChannel2Test, @@ -3432,7 +3430,7 @@ TEST_F(WebRtcVideoChannel2Test, CanSentMaxBitrateForExistingStream) { AddSendStream(); cricket::FakeVideoCapturer capturer; - channel_->SetSource(last_ssrc_, &capturer); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, nullptr, &capturer)); cricket::VideoFormat capture_format_hd = capturer.GetSupportedFormats()->front(); EXPECT_EQ(1280, capture_format_hd.width); @@ -3457,7 +3455,7 @@ TEST_F(WebRtcVideoChannel2Test, CanSentMaxBitrateForExistingStream) { SetAndExpectMaxBitrate(capturer, 0, 800, 800); SetAndExpectMaxBitrate(capturer, 0, 0, default_encoder_bitrate); - channel_->SetSource(last_ssrc_, NULL); + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, nullptr, nullptr)); } TEST_F(WebRtcVideoChannel2Test, CannotSetMaxBitrateForNonexistentStream) { @@ -3660,7 +3658,8 @@ class WebRtcVideoChannel2SimulcastTest : public testing::Test { // Send a full-size frame to trigger a stream reconfiguration to use all // expected simulcast layers. cricket::FakeVideoCapturer capturer; - channel_->SetSource(ssrcs.front(), &capturer); + EXPECT_TRUE( + channel_->SetVideoSend(ssrcs.front(), true, nullptr, &capturer)); EXPECT_EQ(cricket::CS_RUNNING, capturer.Start(cricket::VideoFormat( codec.width, codec.height, cricket::VideoFormat::FpsToInterval(30), @@ -3717,7 +3716,7 @@ class WebRtcVideoChannel2SimulcastTest : public testing::Test { ASSERT_EQ(1u, info.senders.size()); EXPECT_EQ(total_max_bitrate_bps, info.senders[0].preferred_bitrate); - channel_->SetSource(ssrcs.front(), NULL); + EXPECT_TRUE(channel_->SetVideoSend(ssrcs.front(), true, nullptr, nullptr)); } FakeVideoSendStream* AddSendStream() { diff --git a/webrtc/pc/channel.cc b/webrtc/pc/channel.cc index cac0ea2ea8..be476468c4 100644 --- a/webrtc/pc/channel.cc +++ b/webrtc/pc/channel.cc @@ -1871,18 +1871,13 @@ bool VideoChannel::SetSink(uint32_t ssrc, return true; } -void VideoChannel::SetSource( +bool VideoChannel::SetVideoSend( uint32_t ssrc, + bool mute, + const VideoOptions* options, rtc::VideoSourceInterface* source) { - worker_thread()->Invoke( - Bind(&VideoMediaChannel::SetSource, media_channel(), ssrc, source)); -} - -bool VideoChannel::SetVideoSend(uint32_t ssrc, - bool mute, - const VideoOptions* options) { return InvokeOnWorker(Bind(&VideoMediaChannel::SetVideoSend, media_channel(), - ssrc, mute, options)); + ssrc, mute, options, source)); } webrtc::RtpParameters VideoChannel::GetRtpSendParameters(uint32_t ssrc) const { diff --git a/webrtc/pc/channel.h b/webrtc/pc/channel.h index 3305d0e6fb..348dbe2996 100644 --- a/webrtc/pc/channel.h +++ b/webrtc/pc/channel.h @@ -497,8 +497,6 @@ class VideoChannel : public BaseChannel { } bool SetSink(uint32_t ssrc, rtc::VideoSinkInterface* sink); - // Register a source. The |ssrc| must correspond to a registered - // send stream. void SetSource(uint32_t ssrc, rtc::VideoSourceInterface* source); // Get statistics about the current media session. @@ -511,7 +509,12 @@ class VideoChannel : public BaseChannel { void StopMediaMonitor(); sigslot::signal2 SignalMediaMonitor; - bool SetVideoSend(uint32_t ssrc, bool enable, const VideoOptions* options); + // Register a source and set options. + // The |ssrc| must correspond to a registered send stream. + bool SetVideoSend(uint32_t ssrc, + bool enable, + const VideoOptions* options, + rtc::VideoSourceInterface* source); webrtc::RtpParameters GetRtpSendParameters(uint32_t ssrc) const; bool SetRtpSendParameters(uint32_t ssrc, const webrtc::RtpParameters& parameters); diff --git a/webrtc/pc/channel_unittest.cc b/webrtc/pc/channel_unittest.cc index 6c6a3c10e4..57a1d7504b 100644 --- a/webrtc/pc/channel_unittest.cc +++ b/webrtc/pc/channel_unittest.cc @@ -2796,18 +2796,18 @@ TEST_F(VideoChannelSingleThreadTest, TestMuteStream) { // Test that we can Mute the default channel even though the sending SSRC // is unknown. EXPECT_FALSE(media_channel1_->IsStreamMuted(0)); - EXPECT_TRUE(channel1_->SetVideoSend(0, false, nullptr)); + EXPECT_TRUE(channel1_->SetVideoSend(0, false, nullptr, nullptr)); EXPECT_TRUE(media_channel1_->IsStreamMuted(0)); - EXPECT_TRUE(channel1_->SetVideoSend(0, true, nullptr)); + EXPECT_TRUE(channel1_->SetVideoSend(0, true, nullptr, nullptr)); EXPECT_FALSE(media_channel1_->IsStreamMuted(0)); // Test that we can not mute an unknown SSRC. - EXPECT_FALSE(channel1_->SetVideoSend(kSsrc1, false, nullptr)); + EXPECT_FALSE(channel1_->SetVideoSend(kSsrc1, false, nullptr, nullptr)); SendInitiate(); // After the local session description has been set, we can mute a stream // with its SSRC. - EXPECT_TRUE(channel1_->SetVideoSend(kSsrc1, false, nullptr)); + EXPECT_TRUE(channel1_->SetVideoSend(kSsrc1, false, nullptr, nullptr)); EXPECT_TRUE(media_channel1_->IsStreamMuted(kSsrc1)); - EXPECT_TRUE(channel1_->SetVideoSend(kSsrc1, true, nullptr)); + EXPECT_TRUE(channel1_->SetVideoSend(kSsrc1, true, nullptr, nullptr)); EXPECT_FALSE(media_channel1_->IsStreamMuted(kSsrc1)); } @@ -3040,18 +3040,18 @@ TEST_F(VideoChannelDoubleThreadTest, TestMuteStream) { // Test that we can Mute the default channel even though the sending SSRC // is unknown. EXPECT_FALSE(media_channel1_->IsStreamMuted(0)); - EXPECT_TRUE(channel1_->SetVideoSend(0, false, nullptr)); + EXPECT_TRUE(channel1_->SetVideoSend(0, false, nullptr, nullptr)); EXPECT_TRUE(media_channel1_->IsStreamMuted(0)); - EXPECT_TRUE(channel1_->SetVideoSend(0, true, nullptr)); + EXPECT_TRUE(channel1_->SetVideoSend(0, true, nullptr, nullptr)); EXPECT_FALSE(media_channel1_->IsStreamMuted(0)); // Test that we can not mute an unknown SSRC. - EXPECT_FALSE(channel1_->SetVideoSend(kSsrc1, false, nullptr)); + EXPECT_FALSE(channel1_->SetVideoSend(kSsrc1, false, nullptr, nullptr)); SendInitiate(); // After the local session description has been set, we can mute a stream // with its SSRC. - EXPECT_TRUE(channel1_->SetVideoSend(kSsrc1, false, nullptr)); + EXPECT_TRUE(channel1_->SetVideoSend(kSsrc1, false, nullptr, nullptr)); EXPECT_TRUE(media_channel1_->IsStreamMuted(kSsrc1)); - EXPECT_TRUE(channel1_->SetVideoSend(kSsrc1, true, nullptr)); + EXPECT_TRUE(channel1_->SetVideoSend(kSsrc1, true, nullptr, nullptr)); EXPECT_FALSE(media_channel1_->IsStreamMuted(kSsrc1)); }