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 <[email protected]> Reviewed-by: Steve Anton <[email protected]> Commit-Queue: Florent Castelli <[email protected]> Cr-Commit-Position: refs/heads/master@{#24024}
luncang · Jul 18, 2018 · 87b3c51 · 87b3c51
1 parent a61f7db
commit 87b3c51
Show file tree

Hide file tree

Showing 6 changed files with 54 additions and 29 deletions.
diff --git 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
@@ -288,7 +288,6 @@ void FakeVideoSendStream::Stop() {
 void FakeVideoSendStream::SetSource(
     rtc::VideoSourceInterface<webrtc::VideoFrame>* 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
@@ -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
@@ -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
@@ -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
@@ -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();
 }