diff --git a/webrtc/api/peerconnection.cc b/webrtc/api/peerconnection.cc index ee359271c2..125300a3b5 100644 --- a/webrtc/api/peerconnection.cc +++ b/webrtc/api/peerconnection.cc @@ -615,15 +615,20 @@ bool PeerConnection::Initialize( // We rely on default values when constraints aren't found. cricket::MediaConfig media_config; - media_config.disable_prerenderer_smoothing = + media_config.video.disable_prerenderer_smoothing = configuration.disable_prerenderer_smoothing; // Find DSCP constraint. FindConstraint(constraints, MediaConstraintsInterface::kEnableDscp, &media_config.enable_dscp, NULL); - // Find constraints for cpu overuse detection. + // Find constraint for cpu overuse detection. FindConstraint(constraints, MediaConstraintsInterface::kCpuOveruseDetection, - &media_config.enable_cpu_overuse_detection, NULL); + &media_config.video.enable_cpu_overuse_detection, NULL); + + // Find Suspend Below Min Bitrate constraint. + FindConstraint(constraints, + MediaConstraintsInterface::kEnableVideoSuspendBelowMinBitrate, + &media_config.video.suspend_below_min_bitrate, NULL); media_controller_.reset(factory_->CreateMediaController(media_config)); diff --git a/webrtc/api/peerconnectioninterface_unittest.cc b/webrtc/api/peerconnectioninterface_unittest.cc index 701ab3c001..8fad21e139 100644 --- a/webrtc/api/peerconnectioninterface_unittest.cc +++ b/webrtc/api/peerconnectioninterface_unittest.cc @@ -2383,8 +2383,9 @@ TEST_F(PeerConnectionMediaConfigTest, TestDefaults) { TestCreatePeerConnection(config, &constraints); EXPECT_FALSE(media_config.enable_dscp); - EXPECT_TRUE(media_config.enable_cpu_overuse_detection); - EXPECT_FALSE(media_config.disable_prerenderer_smoothing); + EXPECT_TRUE(media_config.video.enable_cpu_overuse_detection); + EXPECT_FALSE(media_config.video.disable_prerenderer_smoothing); + EXPECT_FALSE(media_config.video.suspend_below_min_bitrate); } // This test verifies the DSCP constraint is recognized and passed to @@ -2411,7 +2412,7 @@ TEST_F(PeerConnectionMediaConfigTest, TestCpuOveruseConstraintFalse) { const cricket::MediaConfig media_config = TestCreatePeerConnection(config, &constraints); - EXPECT_FALSE(media_config.enable_cpu_overuse_detection); + EXPECT_FALSE(media_config.video.enable_cpu_overuse_detection); } // This test verifies that the disable_prerenderer_smoothing flag is @@ -2424,7 +2425,23 @@ TEST_F(PeerConnectionMediaConfigTest, TestDisablePrerendererSmoothingTrue) { const cricket::MediaConfig& media_config = TestCreatePeerConnection(config, &constraints); - EXPECT_TRUE(media_config.disable_prerenderer_smoothing); + EXPECT_TRUE(media_config.video.disable_prerenderer_smoothing); +} + +// This test verifies the suspend below min bitrate constraint is +// recognized and passed to the CreateMediaController call. +TEST_F(PeerConnectionMediaConfigTest, + TestSuspendBelowMinBitrateConstraintTrue) { + PeerConnectionInterface::RTCConfiguration config; + FakeConstraints constraints; + + constraints.AddOptional( + webrtc::MediaConstraintsInterface::kEnableVideoSuspendBelowMinBitrate, + true); + const cricket::MediaConfig media_config = + TestCreatePeerConnection(config, &constraints); + + EXPECT_TRUE(media_config.video.suspend_below_min_bitrate); } // The following tests verify that session options are created correctly. diff --git a/webrtc/api/webrtcsession.cc b/webrtc/api/webrtcsession.cc index e5cea14439..6bfa8a5eb6 100644 --- a/webrtc/api/webrtcsession.cc +++ b/webrtc/api/webrtcsession.cc @@ -597,15 +597,6 @@ bool WebRtcSession::Initialize( } } - // Find Suspend Below Min Bitrate constraint. - if (FindConstraint( - constraints, - MediaConstraintsInterface::kEnableVideoSuspendBelowMinBitrate, - &value, - NULL)) { - video_options_.suspend_below_min_bitrate = rtc::Optional(value); - } - SetOptionFromOptionalConstraint(constraints, MediaConstraintsInterface::kScreencastMinBitrate, &video_options_.screencast_min_bitrate_kbps); diff --git a/webrtc/api/webrtcsession_unittest.cc b/webrtc/api/webrtcsession_unittest.cc index 8062fe5db7..fc0d0cc7ed 100644 --- a/webrtc/api/webrtcsession_unittest.cc +++ b/webrtc/api/webrtcsession_unittest.cc @@ -4116,24 +4116,6 @@ TEST_F(WebRtcSessionTest, TestSetRemoteOfferFailIfDtlsDisabledAndNoCrypto) { offer); } -TEST_F(WebRtcSessionTest, TestSuspendBelowMinBitrateConstraint) { - constraints_.reset(new FakeConstraints()); - constraints_->AddOptional( - webrtc::MediaConstraintsInterface::kEnableVideoSuspendBelowMinBitrate, - true); - Init(); - SendAudioVideoStream1(); - SessionDescriptionInterface* offer = CreateOffer(); - - SetLocalDescriptionWithoutError(offer); - - video_channel_ = media_engine_->GetVideoChannel(0); - - ASSERT_TRUE(video_channel_ != NULL); - const cricket::VideoOptions& video_options = video_channel_->options(); - EXPECT_EQ(rtc::Optional(true), video_options.suspend_below_min_bitrate); -} - TEST_F(WebRtcSessionTest, TestCombinedAudioVideoBweConstraint) { constraints_.reset(new FakeConstraints()); constraints_->AddOptional( diff --git a/webrtc/media/base/mediachannel.h b/webrtc/media/base/mediachannel.h index badec464cd..0d3cf3fa30 100644 --- a/webrtc/media/base/mediachannel.h +++ b/webrtc/media/base/mediachannel.h @@ -87,30 +87,38 @@ struct MediaConfig { // PeerConnection constraint 'googDscp'. bool enable_dscp = false; - // Video-specific config + // Video-specific config. + struct Video { + // Enable WebRTC CPU Overuse Detection. This flag comes from the + // PeerConnection constraint 'googCpuOveruseDetection' and is + // checked in WebRtcVideoChannel2::OnLoadUpdate, where it's passed + // to VideoCapturer::video_adapter()->OnCpuResolutionRequest. + bool enable_cpu_overuse_detection = true; - // Enable WebRTC CPU Overuse Detection. This flag comes from the - // PeerConnection constraint 'googCpuOveruseDetection' and is - // checked in WebRtcVideoChannel2::OnLoadUpdate, where it's passed - // to VideoCapturer::video_adapter()->OnCpuResolutionRequest. - bool enable_cpu_overuse_detection = true; + // Enable WebRTC suspension of video. No video frames will be sent + // when the bitrate is below the configured minimum bitrate. This + // flag comes from the PeerConnection constraint + // 'googSuspendBelowMinBitrate', and WebRtcVideoChannel2 copies it + // to VideoSendStream::Config::suspend_below_min_bitrate. + bool suspend_below_min_bitrate = false; - // Set to true if the renderer has an algorithm of frame selection. - // If the value is true, then WebRTC will hand over a frame as soon as - // possible without delay, and rendering smoothness is completely the duty - // of the renderer; - // If the value is false, then WebRTC is responsible to delay frame release - // in order to increase rendering smoothness. - // - // This flag comes from PeerConnection's RtcConfiguration, but is - // currently only set by the command line flag - // 'disable-rtc-smoothness-algorithm'. - // WebRtcVideoChannel2::AddRecvStream copies it to the created - // WebRtcVideoReceiveStream, where it is returned by the - // SmoothsRenderedFrames method. This method is used by the - // VideoReceiveStream, where the value is passed on to the - // IncomingVideoStream constructor. - bool disable_prerenderer_smoothing = false; + // Set to true if the renderer has an algorithm of frame selection. + // If the value is true, then WebRTC will hand over a frame as soon as + // possible without delay, and rendering smoothness is completely the duty + // of the renderer; + // If the value is false, then WebRTC is responsible to delay frame release + // in order to increase rendering smoothness. + // + // This flag comes from PeerConnection's RtcConfiguration, but is + // currently only set by the command line flag + // 'disable-rtc-smoothness-algorithm'. + // WebRtcVideoChannel2::AddRecvStream copies it to the created + // WebRtcVideoReceiveStream, where it is returned by the + // SmoothsRenderedFrames method. This method is used by the + // VideoReceiveStream, where the value is passed on to the + // IncomingVideoStream constructor. + bool disable_prerenderer_smoothing = false; + } video; }; // Options that can be applied to a VoiceMediaChannel or a VoiceMediaEngine. @@ -249,13 +257,11 @@ struct AudioOptions { struct VideoOptions { void SetAll(const VideoOptions& change) { SetFrom(&video_noise_reduction, change.video_noise_reduction); - SetFrom(&suspend_below_min_bitrate, change.suspend_below_min_bitrate); SetFrom(&screencast_min_bitrate_kbps, change.screencast_min_bitrate_kbps); } bool operator==(const VideoOptions& o) const { return video_noise_reduction == o.video_noise_reduction && - suspend_below_min_bitrate == o.suspend_below_min_bitrate && screencast_min_bitrate_kbps == o.screencast_min_bitrate_kbps; } @@ -263,8 +269,6 @@ struct VideoOptions { std::ostringstream ost; ost << "VideoOptions {"; ost << ToStringIfSet("noise reduction", video_noise_reduction); - ost << ToStringIfSet("suspend below min bitrate", - suspend_below_min_bitrate); ost << ToStringIfSet("screencast min bitrate kbps", screencast_min_bitrate_kbps); ost << "}"; @@ -275,12 +279,6 @@ struct VideoOptions { // constraint 'googNoiseReduction', and WebRtcVideoEngine2 passes it // on to the codec options. Disabled by default. rtc::Optional video_noise_reduction; - // Enable WebRTC suspension of video. No video frames will be sent - // when the bitrate is below the configured minimum bitrate. This - // flag comes from the PeerConnection constraint - // 'googSuspendBelowMinBitrate', and WebRtcVideoChannel2 copies it - // to VideoSendStream::Config::suspend_below_min_bitrate. - rtc::Optional suspend_below_min_bitrate; // Force screencast to use a minimum bitrate. This flag comes from // the PeerConnection constraint 'googScreencastMinBitrate'. It is // copied to the encoder config by WebRtcVideoChannel2. diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc index 0e9866064d..38d2e5670b 100644 --- a/webrtc/media/engine/webrtcvideoengine2.cc +++ b/webrtc/media/engine/webrtcvideoengine2.cc @@ -612,8 +612,7 @@ WebRtcVideoChannel2::WebRtcVideoChannel2( : VideoMediaChannel(config), call_(call), unsignalled_ssrc_handler_(&default_unsignalled_ssrc_handler_), - signal_cpu_adaptation_(config.enable_cpu_overuse_detection), - disable_prerenderer_smoothing_(config.disable_prerenderer_smoothing), + video_config_(config.video), external_encoder_factory_(external_encoder_factory), external_decoder_factory_(external_decoder_factory) { RTC_DCHECK(thread_checker_.CalledOnValidThread()); @@ -1001,10 +1000,12 @@ bool WebRtcVideoChannel2::AddSendStream(const StreamParams& sp) { send_ssrcs_.insert(used_ssrc); webrtc::VideoSendStream::Config config(this); - WebRtcVideoSendStream* stream = new WebRtcVideoSendStream( - call_, sp, config, external_encoder_factory_, signal_cpu_adaptation_, - bitrate_config_.max_bitrate_bps, send_codec_, send_rtp_extensions_, - send_params_); + config.suspend_below_min_bitrate = video_config_.suspend_below_min_bitrate; + WebRtcVideoSendStream* stream = + new WebRtcVideoSendStream(call_, sp, config, external_encoder_factory_, + video_config_.enable_cpu_overuse_detection, + bitrate_config_.max_bitrate_bps, send_codec_, + send_rtp_extensions_, send_params_); uint32_t ssrc = sp.first_ssrc(); RTC_DCHECK(ssrc != 0); @@ -1132,7 +1133,7 @@ bool WebRtcVideoChannel2::AddRecvStream(const StreamParams& sp, receive_streams_[ssrc] = new WebRtcVideoReceiveStream( call_, sp, config, external_decoder_factory_, default_stream, - recv_codecs_, disable_prerenderer_smoothing_); + recv_codecs_, video_config_.disable_prerenderer_smoothing); return true; } @@ -1509,7 +1510,7 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::WebRtcVideoSendStream( enable_cpu_overuse_detection ? this : nullptr; if (codec_settings) { - SetCodecAndOptions(*codec_settings, parameters_.options); + SetCodec(*codec_settings); } } @@ -1648,13 +1649,10 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::GetSsrcs() const { void WebRtcVideoChannel2::WebRtcVideoSendStream::SetOptions( const VideoOptions& options) { rtc::CritScope cs(&lock_); - if (parameters_.codec_settings) { - LOG(LS_INFO) << "SetCodecAndOptions because of SetOptions; options=" - << options.ToString(); - SetCodecAndOptions(*parameters_.codec_settings, options); - } else { - parameters_.options = options; - } + parameters_.options.SetAll(options); + // Reconfigure encoder settings on the next frame or stream + // recreation. + pending_encoder_reconfiguration_ = true; } webrtc::VideoCodecType CodecTypeFromName(const std::string& name) { @@ -1711,9 +1709,8 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::DestroyVideoEncoder( delete encoder->encoder; } -void WebRtcVideoChannel2::WebRtcVideoSendStream::SetCodecAndOptions( - const VideoCodecSettings& codec_settings, - const VideoOptions& options) { +void WebRtcVideoChannel2::WebRtcVideoSendStream::SetCodec( + const VideoCodecSettings& codec_settings) { parameters_.encoder_config = CreateVideoEncoderConfig(last_dimensions_, codec_settings.codec); RTC_DCHECK(!parameters_.encoder_config.streams.empty()); @@ -1744,16 +1741,8 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::SetCodecAndOptions( parameters_.config.rtp.nack.rtp_history_ms = HasNack(codec_settings.codec) ? kNackHistoryMs : 0; - parameters_.config.suspend_below_min_bitrate = - options.suspend_below_min_bitrate.value_or(false); - parameters_.codec_settings = rtc::Optional(codec_settings); - parameters_.options = options; - - LOG(LS_INFO) - << "RecreateWebRtcStream (send) because of SetCodecAndOptions; options=" - << options.ToString(); RecreateWebRtcStream(); if (allocated_encoder_.encoder != new_encoder.encoder) { DestroyVideoEncoder(&allocated_encoder_); @@ -1789,21 +1778,15 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::SetSendParameters( if (params.conference_mode) { parameters_.conference_mode = *params.conference_mode; } + if (params.options) + SetOptions(*params.options); + // Set codecs and options. if (params.codec) { - SetCodecAndOptions(*params.codec, - params.options ? *params.options : parameters_.options); + SetCodec(*params.codec); return; - } else if (params.options) { - // Reconfigure if codecs are already set. - if (parameters_.codec_settings) { - SetCodecAndOptions(*parameters_.codec_settings, *params.options); - return; - } else { - parameters_.options = *params.options; - } } else if (params.conference_mode && parameters_.codec_settings) { - SetCodecAndOptions(*parameters_.codec_settings, parameters_.options); + SetCodec(*parameters_.codec_settings); return; } if (recreate_stream) { diff --git a/webrtc/media/engine/webrtcvideoengine2.h b/webrtc/media/engine/webrtcvideoengine2.h index 3dd05f2400..76c2fc0c18 100644 --- a/webrtc/media/engine/webrtcvideoengine2.h +++ b/webrtc/media/engine/webrtcvideoengine2.h @@ -333,8 +333,7 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport { EXCLUSIVE_LOCKS_REQUIRED(lock_); void DestroyVideoEncoder(AllocatedEncoder* encoder) EXCLUSIVE_LOCKS_REQUIRED(lock_); - void SetCodecAndOptions(const VideoCodecSettings& codec, - const VideoOptions& options) + void SetCodec(const VideoCodecSettings& codec) EXCLUSIVE_LOCKS_REQUIRED(lock_); void RecreateWebRtcStream() EXCLUSIVE_LOCKS_REQUIRED(lock_); webrtc::VideoEncoderConfig CreateVideoEncoderConfig( @@ -499,8 +498,7 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport { DefaultUnsignalledSsrcHandler default_unsignalled_ssrc_handler_; UnsignalledSsrcHandler* const unsignalled_ssrc_handler_; - const bool signal_cpu_adaptation_; - const bool disable_prerenderer_smoothing_; + const MediaConfig::Video video_config_; rtc::CriticalSection stream_crit_; // Using primary-ssrc (first ssrc) as key. diff --git a/webrtc/media/engine/webrtcvideoengine2_unittest.cc b/webrtc/media/engine/webrtcvideoengine2_unittest.cc index 529d77b14b..ea8a300395 100644 --- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc +++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc @@ -1017,10 +1017,16 @@ class WebRtcVideoChannel2Test : public WebRtcVideoEngine2Test { bool expect_created_receive_stream); FakeVideoSendStream* SetDenoisingOption( - const cricket::VideoSendParameters& parameters, bool enabled) { + const cricket::VideoSendParameters& parameters, + cricket::FakeVideoCapturer* capturer, + bool enabled) { cricket::VideoSendParameters params = parameters; params.options.video_noise_reduction = rtc::Optional(enabled); + // TODO(nisse): Switch to using SetOptions? channel_->SetSendParameters(params); + // Options only take effect on the next frame. + EXPECT_TRUE(capturer->CaptureFrame()); + return fake_call_->GetVideoSendStreams().back(); } @@ -1554,19 +1560,25 @@ TEST_F(WebRtcVideoChannel2Test, SuspendBelowMinBitrateDisabledByDefault) { EXPECT_FALSE(stream->GetConfig().suspend_below_min_bitrate); } -TEST_F(WebRtcVideoChannel2Test, SetOptionsWithSuspendBelowMinBitrate) { - send_parameters_.options.suspend_below_min_bitrate = - rtc::Optional(true); +TEST_F(WebRtcVideoChannel2Test, SetMediaConfigSuspendBelowMinBitrate) { + MediaConfig media_config = MediaConfig(); + media_config.video.suspend_below_min_bitrate = true; + + channel_.reset( + engine_.CreateChannel(fake_call_.get(), media_config, VideoOptions())); + channel_->SetSendParameters(send_parameters_); FakeVideoSendStream* stream = AddSendStream(); EXPECT_TRUE(stream->GetConfig().suspend_below_min_bitrate); - send_parameters_.options.suspend_below_min_bitrate = - rtc::Optional(false); + media_config.video.suspend_below_min_bitrate = false; + channel_.reset( + engine_.CreateChannel(fake_call_.get(), media_config, VideoOptions())); + channel_->SetSendParameters(send_parameters_); - stream = fake_call_->GetVideoSendStreams()[0]; + stream = AddSendStream(); EXPECT_FALSE(stream->GetConfig().suspend_below_min_bitrate); } @@ -1601,14 +1613,14 @@ TEST_F(WebRtcVideoChannel2Test, VerifyVp8SpecificSettings) { EXPECT_TRUE(vp8_settings.denoisingOn) << "VP8 denoising should be on by default."; - stream = SetDenoisingOption(parameters, false); + stream = SetDenoisingOption(parameters, &capturer, false); ASSERT_TRUE(stream->GetVp8Settings(&vp8_settings)) << "No VP8 config set."; EXPECT_FALSE(vp8_settings.denoisingOn); EXPECT_TRUE(vp8_settings.automaticResizeOn); EXPECT_TRUE(vp8_settings.frameDroppingOn); - stream = SetDenoisingOption(parameters, true); + stream = SetDenoisingOption(parameters, &capturer, true); ASSERT_TRUE(stream->GetVp8Settings(&vp8_settings)) << "No VP8 config set."; EXPECT_TRUE(vp8_settings.denoisingOn); @@ -1632,7 +1644,7 @@ TEST_F(WebRtcVideoChannel2Test, VerifyVp8SpecificSettings) { capturer.SetScreencast(true); EXPECT_TRUE(channel_->SetCapturer(last_ssrc_, &capturer)); EXPECT_TRUE(capturer.CaptureFrame()); - stream = SetDenoisingOption(parameters, false); + stream = SetDenoisingOption(parameters, &capturer, false); EXPECT_EQ(1, stream->GetVideoStreams().size()); ASSERT_TRUE(stream->GetVp8Settings(&vp8_settings)) << "No VP8 config set."; @@ -1641,7 +1653,7 @@ TEST_F(WebRtcVideoChannel2Test, VerifyVp8SpecificSettings) { EXPECT_FALSE(vp8_settings.automaticResizeOn); EXPECT_FALSE(vp8_settings.frameDroppingOn); - stream = SetDenoisingOption(parameters, true); + stream = SetDenoisingOption(parameters, &capturer, true); ASSERT_TRUE(stream->GetVp8Settings(&vp8_settings)) << "No VP8 config set."; EXPECT_FALSE(vp8_settings.denoisingOn); @@ -1695,14 +1707,14 @@ TEST_F(Vp9SettingsTest, VerifyVp9SpecificSettings) { EXPECT_FALSE(vp9_settings.denoisingOn) << "VP9 denoising should be off by default."; - stream = SetDenoisingOption(parameters, false); + stream = SetDenoisingOption(parameters, &capturer, false); ASSERT_TRUE(stream->GetVp9Settings(&vp9_settings)) << "No VP9 config set."; EXPECT_FALSE(vp9_settings.denoisingOn); // Frame dropping always on for real time video. EXPECT_TRUE(vp9_settings.frameDroppingOn); - stream = SetDenoisingOption(parameters, true); + stream = SetDenoisingOption(parameters, &capturer, true); ASSERT_TRUE(stream->GetVp9Settings(&vp9_settings)) << "No VP9 config set."; EXPECT_TRUE(vp9_settings.denoisingOn); @@ -1714,14 +1726,14 @@ TEST_F(Vp9SettingsTest, VerifyVp9SpecificSettings) { EXPECT_TRUE(channel_->SetCapturer(last_ssrc_, &capturer)); EXPECT_TRUE(capturer.CaptureFrame()); - stream = SetDenoisingOption(parameters, false); + stream = SetDenoisingOption(parameters, &capturer, false); ASSERT_TRUE(stream->GetVp9Settings(&vp9_settings)) << "No VP9 config set."; EXPECT_FALSE(vp9_settings.denoisingOn); // Frame dropping always off for screen sharing. EXPECT_FALSE(vp9_settings.frameDroppingOn); - stream = SetDenoisingOption(parameters, false); + stream = SetDenoisingOption(parameters, &capturer, false); ASSERT_TRUE(stream->GetVp9Settings(&vp9_settings)) << "No VP9 config set."; EXPECT_FALSE(vp9_settings.denoisingOn); @@ -1817,7 +1829,7 @@ void WebRtcVideoChannel2Test::TestCpuAdaptation(bool enable_overuse, MediaConfig media_config = MediaConfig(); if (!enable_overuse) { - media_config.enable_cpu_overuse_detection = false; + media_config.video.enable_cpu_overuse_detection = false; } channel_.reset( engine_.CreateChannel(fake_call_.get(), media_config, VideoOptions()));