From 87b3c510b4eb156575f30c6ff2e100727e54b10f Mon Sep 17 00:00:00 2001 From: Florent Castelli Date: Wed, 18 Jul 2018 16:00:28 +0200 Subject: [PATCH] Implement changing degradation preference with setParameters() The current default behavior is unchanged and points to MAINTAIN_FRAMERATE, meaning there is no way to currently use BALANCED as we can't detect when the value as been set or not. Updating this is an API change that should be done in another CL and properly communicated first. Bug: webrtc:7607 Change-Id: Ic3877ad8dd7bc418296f21a04bc37f59ec55934a Reviewed-on: https://webrtc-review.googlesource.com/88766 Reviewed-by: Stefan Holmer Reviewed-by: Steve Anton Commit-Queue: Florent Castelli Cr-Commit-Position: refs/heads/master@{#24024} --- api/rtpparameters.h | 4 ++- media/engine/fakewebrtccall.cc | 1 - media/engine/webrtcvideoengine.cc | 35 +++++++++++++++------- media/engine/webrtcvideoengine_unittest.cc | 24 +++++++++++++++ pc/rtpsender.cc | 3 +- pc/rtpsenderreceiver_unittest.cc | 16 ++-------- 6 files changed, 54 insertions(+), 29 deletions(-) diff --git a/api/rtpparameters.h b/api/rtpparameters.h index ba318ca040..b7560f1b34 100644 --- a/api/rtpparameters.h +++ b/api/rtpparameters.h @@ -602,7 +602,9 @@ struct RtpParameters { // abstraction on which RTCP parameters are set. RtcpParameters rtcp; - // TODO(deadbeef): Not implemented. + // When bandwidth is constrained and the RtpSender needs to choose between + // degrading resolution or degrading framerate, degradationPreference + // indicates which is preferred. Only for video tracks. DegradationPreference degradation_preference = DegradationPreference::BALANCED; diff --git a/media/engine/fakewebrtccall.cc b/media/engine/fakewebrtccall.cc index 70353fe45e..84623f17a2 100644 --- a/media/engine/fakewebrtccall.cc +++ b/media/engine/fakewebrtccall.cc @@ -288,7 +288,6 @@ void FakeVideoSendStream::Stop() { void FakeVideoSendStream::SetSource( rtc::VideoSourceInterface* source, const webrtc::DegradationPreference& degradation_preference) { - RTC_DCHECK(source != source_); if (source_) source_->RemoveSink(this); source_ = source; diff --git a/media/engine/webrtcvideoengine.cc b/media/engine/webrtcvideoengine.cc index 134fa40489..d75b48b00c 100644 --- a/media/engine/webrtcvideoengine.cc +++ b/media/engine/webrtcvideoengine.cc @@ -1693,19 +1693,23 @@ WebRtcVideoChannel::WebRtcVideoSendStream::GetDegradationPreference() const { // |this| acts like a VideoSource to make sure SinkWants are handled on the // correct thread. webrtc::DegradationPreference degradation_preference; - if (!enable_cpu_overuse_detection_) { + if (rtp_parameters_.degradation_preference != + webrtc::DegradationPreference::BALANCED) { + // If the degradationPreference is different from the default value, assume + // it is what we want, regardless of trials or other internal settings. + degradation_preference = rtp_parameters_.degradation_preference; + } else if (!enable_cpu_overuse_detection_) { degradation_preference = webrtc::DegradationPreference::DISABLED; + } else if (parameters_.options.is_screencast.value_or(false)) { + degradation_preference = webrtc::DegradationPreference::MAINTAIN_RESOLUTION; + } else if (webrtc::field_trial::IsEnabled( + "WebRTC-Video-BalancedDegradation")) { + degradation_preference = webrtc::DegradationPreference::BALANCED; } else { - if (parameters_.options.is_screencast.value_or(false)) { - degradation_preference = - webrtc::DegradationPreference::MAINTAIN_RESOLUTION; - } else if (webrtc::field_trial::IsEnabled( - "WebRTC-Video-BalancedDegradation")) { - degradation_preference = webrtc::DegradationPreference::BALANCED; - } else { - degradation_preference = - webrtc::DegradationPreference::MAINTAIN_FRAMERATE; - } + // TODO(orphis): The default should be BALANCED as the standard mandates. + // Right now, there is no way to set it to BALANCED as it would change + // the behavior for any project expecting MAINTAIN_FRAMERATE by default. + degradation_preference = webrtc::DegradationPreference::MAINTAIN_FRAMERATE; } return degradation_preference; } @@ -1812,6 +1816,12 @@ webrtc::RTCError WebRtcVideoChannel::WebRtcVideoSendStream::SetRtpParameters( } } + bool new_degradation_preference = false; + if (new_parameters.degradation_preference != + rtp_parameters_.degradation_preference) { + new_degradation_preference = true; + } + // TODO(bugs.webrtc.org/8807): The bitrate priority really doesn't require an // entire encoder reconfiguration, it just needs to update the bitrate // allocator. @@ -1838,6 +1848,9 @@ webrtc::RTCError WebRtcVideoChannel::WebRtcVideoSendStream::SetRtpParameters( if (new_send_state) { UpdateSendState(); } + if (new_degradation_preference) { + stream_->SetSource(this, GetDegradationPreference()); + } return webrtc::RTCError::OK(); } diff --git a/media/engine/webrtcvideoengine_unittest.cc b/media/engine/webrtcvideoengine_unittest.cc index 1d56bffa27..e42e3a5f19 100644 --- a/media/engine/webrtcvideoengine_unittest.cc +++ b/media/engine/webrtcvideoengine_unittest.cc @@ -6035,6 +6035,30 @@ TEST_F(WebRtcVideoChannelTest, DetectRtpSendParameterHeaderExtensionsChange) { EXPECT_EQ(webrtc::RTCErrorType::INVALID_MODIFICATION, result.type()); } +TEST_F(WebRtcVideoChannelTest, GetRtpSendParametersDegradationPreference) { + AddSendStream(); + + FakeVideoCapturerWithTaskQueue capturer; + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, nullptr, &capturer)); + + webrtc::RtpParameters rtp_parameters = + channel_->GetRtpSendParameters(last_ssrc_); + EXPECT_EQ(rtp_parameters.degradation_preference, + webrtc::DegradationPreference::BALANCED); + rtp_parameters.degradation_preference = + webrtc::DegradationPreference::MAINTAIN_FRAMERATE; + + EXPECT_TRUE(channel_->SetRtpSendParameters(last_ssrc_, rtp_parameters).ok()); + + webrtc::RtpParameters updated_rtp_parameters = + channel_->GetRtpSendParameters(last_ssrc_); + EXPECT_EQ(updated_rtp_parameters.degradation_preference, + webrtc::DegradationPreference::MAINTAIN_FRAMERATE); + + // Remove the source since it will be destroyed before the channel + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, nullptr, nullptr)); +} + // Test that if we set/get parameters multiple times, we get the same results. TEST_F(WebRtcVideoChannelTest, SetAndGetRtpSendParameters) { AddSendStream(); diff --git a/pc/rtpsender.cc b/pc/rtpsender.cc index 927379d588..91708fa225 100644 --- a/pc/rtpsender.cc +++ b/pc/rtpsender.cc @@ -67,8 +67,7 @@ bool PerSenderRtpEncodingParameterHasValue( // Returns true if any RtpParameters member that isn't implemented contains a // value. bool UnimplementedRtpParameterHasValue(const RtpParameters& parameters) { - if (!parameters.mid.empty() || - parameters.degradation_preference != DegradationPreference::BALANCED) { + if (!parameters.mid.empty()) { return true; } for (size_t i = 0; i < parameters.encodings.size(); ++i) { diff --git a/pc/rtpsenderreceiver_unittest.cc b/pc/rtpsenderreceiver_unittest.cc index 3e0488dcb4..4014df54cb 100644 --- a/pc/rtpsenderreceiver_unittest.cc +++ b/pc/rtpsenderreceiver_unittest.cc @@ -675,18 +675,12 @@ TEST_F(RtpSenderReceiverTest, AudioSenderCantSetUnimplementedRtpParameters) { RtpParameters params = audio_rtp_sender_->GetParameters(); EXPECT_EQ(1u, params.encodings.size()); - // Unimplemented RtpParameters: mid, header_extensions, - // degredation_preference. + // Unimplemented RtpParameters: mid params.mid = "dummy_mid"; EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, audio_rtp_sender_->SetParameters(params).type()); params = audio_rtp_sender_->GetParameters(); - ASSERT_EQ(DegradationPreference::BALANCED, params.degradation_preference); - params.degradation_preference = DegradationPreference::MAINTAIN_FRAMERATE; - EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, - audio_rtp_sender_->SetParameters(params).type()); - DestroyAudioRtpSender(); } @@ -868,18 +862,12 @@ TEST_F(RtpSenderReceiverTest, VideoSenderCantSetUnimplementedRtpParameters) { RtpParameters params = video_rtp_sender_->GetParameters(); EXPECT_EQ(1u, params.encodings.size()); - // Unimplemented RtpParameters: mid, header_extensions, - // degredation_preference. + // Unimplemented RtpParameters: mid params.mid = "dummy_mid"; EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, video_rtp_sender_->SetParameters(params).type()); params = video_rtp_sender_->GetParameters(); - ASSERT_EQ(DegradationPreference::BALANCED, params.degradation_preference); - params.degradation_preference = DegradationPreference::MAINTAIN_FRAMERATE; - EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, - video_rtp_sender_->SetParameters(params).type()); - DestroyVideoRtpSender(); }