From dbe2b8744f77b630af36f4ffc436b55f90e121f0 Mon Sep 17 00:00:00 2001 From: deadbeef Date: Tue, 22 Mar 2016 15:42:00 -0700 Subject: [PATCH] Adding support for RTCRtpEncodingParameters.active flag. This will allow a sender to stop/start sending media on the application's demand. Among other things, this can allow an application to set a track on a sender while the encoding(s) are inactive, allowing the encoder to be initialized for that track, then later set the encodings to "active" to instantly start sending the track. Review URL: https://codereview.webrtc.org/1822923002 Cr-Commit-Position: refs/heads/master@{#12094} --- webrtc/api/rtpparameters.h | 1 + webrtc/media/engine/webrtcvideoengine2.cc | 67 +++++++++---------- webrtc/media/engine/webrtcvideoengine2.h | 15 ++--- .../engine/webrtcvideoengine2_unittest.cc | 25 ++++++- 4 files changed, 63 insertions(+), 45 deletions(-) diff --git a/webrtc/api/rtpparameters.h b/webrtc/api/rtpparameters.h index e71a1aebba..2c29d9843d 100644 --- a/webrtc/api/rtpparameters.h +++ b/webrtc/api/rtpparameters.h @@ -18,6 +18,7 @@ namespace webrtc { // These structures are defined as part of the RtpSender interface. // See http://w3c.github.io/webrtc-pc/#rtcrtpsender-interface for details. struct RtpEncodingParameters { + bool active = true; int max_bitrate_bps = -1; }; diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc index 9dff1ee197..fe5c610d08 100644 --- a/webrtc/media/engine/webrtcvideoengine2.cc +++ b/webrtc/media/engine/webrtcvideoengine2.cc @@ -877,7 +877,7 @@ webrtc::RtpParameters WebRtcVideoChannel2::GetRtpParameters( return webrtc::RtpParameters(); } - return it->second->rtp_parameters(); + return it->second->GetRtpParameters(); } bool WebRtcVideoChannel2::SetRtpParameters( @@ -990,10 +990,11 @@ bool WebRtcVideoChannel2::SetSend(bool send) { LOG(LS_ERROR) << "SetSend(true) called before setting codec."; return false; } - if (send) { - StartAllSendStreams(); - } else { - StopAllSendStreams(); + { + rtc::CritScope stream_lock(&stream_crit_); + for (const auto& kv : send_streams_) { + kv.second->SetSend(send); + } } sending_ = send; return true; @@ -1076,7 +1077,7 @@ bool WebRtcVideoChannel2::AddSendStream(const StreamParams& sp) { default_send_ssrc_ = ssrc; } if (sending_) { - stream->Start(); + stream->SetSend(true); } return true; @@ -1484,24 +1485,6 @@ bool WebRtcVideoChannel2::SendRtcp(const uint8_t* data, size_t len) { return MediaChannel::SendRtcp(&packet, rtc::PacketOptions()); } -void WebRtcVideoChannel2::StartAllSendStreams() { - rtc::CritScope stream_lock(&stream_crit_); - for (std::map::iterator it = - send_streams_.begin(); - it != send_streams_.end(); ++it) { - it->second->Start(); - } -} - -void WebRtcVideoChannel2::StopAllSendStreams() { - rtc::CritScope stream_lock(&stream_crit_); - for (std::map::iterator it = - send_streams_.begin(); - it != send_streams_.end(); ++it) { - it->second->Stop(); - } -} - WebRtcVideoChannel2::WebRtcVideoSendStream::VideoSendStreamParameters:: VideoSendStreamParameters( const webrtc::VideoSendStream::Config& config, @@ -1884,9 +1867,17 @@ bool WebRtcVideoChannel2::WebRtcVideoSendStream::SetRtpParameters( pending_encoder_reconfiguration_ = true; } rtp_parameters_ = new_parameters; + // Encoding may have been activated/deactivated. + UpdateSendState(); return true; } +webrtc::RtpParameters +WebRtcVideoChannel2::WebRtcVideoSendStream::GetRtpParameters() const { + rtc::CritScope cs(&lock_); + return rtp_parameters_; +} + bool WebRtcVideoChannel2::WebRtcVideoSendStream::ValidateRtpParameters( const webrtc::RtpParameters& rtp_parameters) { if (rtp_parameters.encodings.size() != 1) { @@ -1897,6 +1888,19 @@ bool WebRtcVideoChannel2::WebRtcVideoSendStream::ValidateRtpParameters( return true; } +void WebRtcVideoChannel2::WebRtcVideoSendStream::UpdateSendState() { + // TODO(deadbeef): Need to handle more than one encoding in the future. + RTC_DCHECK(rtp_parameters_.encodings.size() == 1u); + if (sending_ && rtp_parameters_.encodings[0].active) { + RTC_DCHECK(stream_ != nullptr); + stream_->Start(); + } else { + if (stream_ != nullptr) { + stream_->Stop(); + } + } +} + webrtc::VideoEncoderConfig WebRtcVideoChannel2::WebRtcVideoSendStream::CreateVideoEncoderConfig( const Dimensions& dimensions, @@ -1996,19 +2000,10 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::SetDimensions( parameters_.encoder_config = encoder_config; } -void WebRtcVideoChannel2::WebRtcVideoSendStream::Start() { +void WebRtcVideoChannel2::WebRtcVideoSendStream::SetSend(bool send) { rtc::CritScope cs(&lock_); - RTC_DCHECK(stream_ != NULL); - stream_->Start(); - sending_ = true; -} - -void WebRtcVideoChannel2::WebRtcVideoSendStream::Stop() { - rtc::CritScope cs(&lock_); - if (stream_ != NULL) { - stream_->Stop(); - } - sending_ = false; + sending_ = send; + UpdateSendState(); } void WebRtcVideoChannel2::WebRtcVideoSendStream::OnLoadUpdate(Load load) { diff --git a/webrtc/media/engine/webrtcvideoengine2.h b/webrtc/media/engine/webrtcvideoengine2.h index 27eadb299a..3668a18e4b 100644 --- a/webrtc/media/engine/webrtcvideoengine2.h +++ b/webrtc/media/engine/webrtcvideoengine2.h @@ -248,16 +248,14 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport { // 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; bool SetCapturer(VideoCapturer* capturer); void MuteStream(bool mute); bool DisconnectCapturer(); - void Start(); - void Stop(); - - webrtc::RtpParameters rtp_parameters() const { return rtp_parameters_; } + void SetSend(bool send); // Implements webrtc::LoadObserver. void OnLoadUpdate(Load load) override; @@ -345,6 +343,10 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport { EXCLUSIVE_LOCKS_REQUIRED(lock_); bool ValidateRtpParameters(const webrtc::RtpParameters& parameters); + // Calls Start or Stop according to whether or not |sending_| is true, + // and whether or not the encoding in |rtp_parameters_| is active. + void UpdateSendState() EXCLUSIVE_LOCKS_REQUIRED(lock_); + rtc::ThreadChecker thread_checker_; rtc::AsyncInvoker invoker_; rtc::Thread* worker_thread_; @@ -372,7 +374,7 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport { // TODO(skvlad): Move ssrcs_ and ssrc_groups_ into rtp_parameters_. // TODO(skvlad): Combine parameters_ and rtp_parameters_ once we have only // one stream per MediaChannel. - webrtc::RtpParameters rtp_parameters_; + webrtc::RtpParameters rtp_parameters_ GUARDED_BY(lock_); bool pending_encoder_reconfiguration_ GUARDED_BY(lock_); VideoEncoderSettings encoder_settings_ GUARDED_BY(lock_); AllocatedEncoder allocated_encoder_ GUARDED_BY(lock_); @@ -482,9 +484,6 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport { const webrtc::PacketOptions& options) override; bool SendRtcp(const uint8_t* data, size_t len) override; - void StartAllSendStreams(); - void StopAllSendStreams(); - static std::vector MapCodecs( const std::vector& codecs); std::vector FilterSupportedCodecs( diff --git a/webrtc/media/engine/webrtcvideoengine2_unittest.cc b/webrtc/media/engine/webrtcvideoengine2_unittest.cc index b0145b472b..649f905400 100644 --- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc +++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc @@ -3269,7 +3269,7 @@ TEST_F(WebRtcVideoChannel2Test, CannotSetRtpParametersWithIncorrectNumberOfEncodings) { // This test verifies that setting RtpParameters succeeds only if // the structure contains exactly one encoding. - // TODO(skvlad): Update this test when we strat supporting setting parameters + // TODO(skvlad): Update this test when we start supporting setting parameters // for each encoding individually. AddSendStream(); @@ -3284,6 +3284,29 @@ TEST_F(WebRtcVideoChannel2Test, EXPECT_FALSE(channel_->SetRtpParameters(last_ssrc_, parameters)); } +// Test that a stream will not be sending if its encoding is made +// inactive through SetRtpParameters. +// TODO(deadbeef): Update this test when we start supporting setting parameters +// for each encoding individually. +TEST_F(WebRtcVideoChannel2Test, SetRtpParametersEncodingsActive) { + FakeVideoSendStream* stream = AddSendStream(); + EXPECT_TRUE(channel_->SetSend(true)); + EXPECT_TRUE(stream->IsSending()); + + // Get current parameters and change "active" to false. + webrtc::RtpParameters parameters = channel_->GetRtpParameters(last_ssrc_); + ASSERT_EQ(1u, parameters.encodings.size()); + ASSERT_TRUE(parameters.encodings[0].active); + parameters.encodings[0].active = false; + EXPECT_TRUE(channel_->SetRtpParameters(last_ssrc_, parameters)); + EXPECT_FALSE(stream->IsSending()); + + // Now change it back to active and verify we resume sending. + parameters.encodings[0].active = true; + EXPECT_TRUE(channel_->SetRtpParameters(last_ssrc_, parameters)); + EXPECT_TRUE(stream->IsSending()); +} + void WebRtcVideoChannel2Test::TestReceiverLocalSsrcConfiguration( bool receiver_first) { EXPECT_TRUE(channel_->SetSendParameters(send_parameters_));