Returns RTCError for setting unimplemented RtpParameters.
We have a number of RtpParameters that aren't implemented. If a client
is setting these values it creates unexpected results when the value
doesn't do anything for them. This change incorporates returning the
correct error if the parameter is unimplemented.
It also changes the scale_resolution_down_by and scale_framerate_down_by
RtpEncodingParameters to rtc::Optionals because they aren't implemented.
This change is part of the effort to ship get/setParameters in Chrome.
Bug: webrtc:8772
Change-Id: I9797695e5116e6aeb3c02afddbf460b2a0d7d5ab
Reviewed-on: https://webrtc-review.googlesource.com/75421
Commit-Queue: Seth Hampson <shampson@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23314}diff --git a/pc/rtpsender.cc b/pc/rtpsender.cc
index 3b0bbf8..b7e48cd 100644
--- a/pc/rtpsender.cc
+++ b/pc/rtpsender.cc
@@ -30,6 +30,62 @@
return ++g_unique_id;
}
+// Returns an true if any RtpEncodingParameters member that isn't implemented
+// contains a value.
+bool UnimplementedRtpEncodingParameterHasValue(
+ const RtpEncodingParameters& encoding_params) {
+ if (encoding_params.codec_payload_type.has_value() ||
+ encoding_params.fec.has_value() || encoding_params.rtx.has_value() ||
+ encoding_params.dtx.has_value() || encoding_params.ptime.has_value() ||
+ encoding_params.max_framerate.has_value() ||
+ !encoding_params.rid.empty() ||
+ encoding_params.scale_resolution_down_by.has_value() ||
+ encoding_params.scale_framerate_down_by.has_value() ||
+ !encoding_params.dependency_rids.empty()) {
+ return true;
+ }
+ return false;
+}
+
+// Returns true if a "per-sender" encoding parameter contains a value that isn't
+// its default. Currently max_bitrate_bps and bitrate_priority both are
+// implemented "per-sender," meaning that these encoding parameters
+// are used for the RtpSender as a whole, not for a specific encoding layer.
+// This is done by setting these encoding parameters at index 0 of
+// RtpParameters.encodings. This function can be used to check if these
+// parameters are set at any index other than 0 of RtpParameters.encodings,
+// because they are currently unimplemented to be used for a specific encoding
+// layer.
+bool PerSenderRtpEncodingParameterHasValue(
+ const RtpEncodingParameters& encoding_params) {
+ if (encoding_params.max_bitrate_bps.has_value() ||
+ encoding_params.bitrate_priority != kDefaultBitratePriority) {
+ return true;
+ }
+ return false;
+}
+
+// Returns true if any RtpParameters member that isn't implemented contains a
+// value.
+bool UnimplementedRtpParameterHasValue(const RtpParameters& parameters) {
+ if (!parameters.mid.empty() || !parameters.header_extensions.empty() ||
+ parameters.degradation_preference != DegradationPreference::BALANCED) {
+ return true;
+ }
+ for (size_t i = 0; i < parameters.encodings.size(); ++i) {
+ if (UnimplementedRtpEncodingParameterHasValue(parameters.encodings[i])) {
+ return true;
+ }
+ // Encoding parameters that are per-sender should only contain value at
+ // index 0.
+ if (i != 0 &&
+ PerSenderRtpEncodingParameterHasValue(parameters.encodings[i])) {
+ return true;
+ }
+ }
+ return false;
+}
+
} // namespace
LocalAudioSinkAdapter::LocalAudioSinkAdapter() : sink_(nullptr) {}
@@ -217,6 +273,11 @@
" the last value returned from getParameters()");
}
+ if (UnimplementedRtpParameterHasValue(parameters)) {
+ LOG_AND_RETURN_ERROR(
+ RTCErrorType::UNSUPPORTED_PARAMETER,
+ "Attempted to set an unimplemented parameter of RtpParameters.");
+ }
return worker_thread_->Invoke<RTCError>(RTC_FROM_HERE, [&] {
RTCError result = media_channel_->SetRtpSendParameters(ssrc_, parameters);
last_transaction_id_.reset();
@@ -421,6 +482,11 @@
" the last value returned from getParameters()");
}
+ if (UnimplementedRtpParameterHasValue(parameters)) {
+ LOG_AND_RETURN_ERROR(
+ RTCErrorType::UNSUPPORTED_PARAMETER,
+ "Attempted to set an unimplemented parameter of RtpParameters.");
+ }
return worker_thread_->Invoke<RTCError>(RTC_FROM_HERE, [&] {
RTCError result = media_channel_->SetRtpSendParameters(ssrc_, parameters);
last_transaction_id_.reset();
diff --git a/pc/rtpsenderreceiver_unittest.cc b/pc/rtpsenderreceiver_unittest.cc
index b084b01..be0e975 100644
--- a/pc/rtpsenderreceiver_unittest.cc
+++ b/pc/rtpsenderreceiver_unittest.cc
@@ -15,6 +15,7 @@
#include "api/rtpparameters.h"
#include "media/base/fakemediaengine.h"
#include "media/base/rtpdataengine.h"
+#include "media/base/testutils.h"
#include "media/engine/fakewebrtccall.h"
#include "p2p/base/fakedtlstransport.h"
#include "pc/audiotrack.h"
@@ -162,16 +163,20 @@
void OnAudioSenderDestroyed() { audio_sender_destroyed_signal_fired_ = true; }
+ void CreateVideoRtpSender(uint32_t ssrc) {
+ CreateVideoRtpSender(false, ssrc);
+ }
+
void CreateVideoRtpSender() { CreateVideoRtpSender(false); }
- void CreateVideoRtpSender(bool is_screencast) {
+ void CreateVideoRtpSender(bool is_screencast, uint32_t ssrc = kVideoSsrc) {
AddVideoTrack(is_screencast);
video_rtp_sender_ =
new VideoRtpSender(worker_thread_, local_stream_->GetVideoTracks()[0],
{local_stream_->id()});
video_rtp_sender_->SetVideoMediaChannel(video_media_channel_);
- video_rtp_sender_->SetSsrc(kVideoSsrc);
- VerifyVideoChannelInput();
+ video_rtp_sender_->SetSsrc(ssrc);
+ VerifyVideoChannelInput(ssrc);
}
void CreateVideoRtpSenderWithNoTrack() {
@@ -661,6 +666,86 @@
RTCError result = audio_rtp_sender_->SetParameters(params);
EXPECT_EQ(RTCErrorType::INVALID_MODIFICATION, result.type());
+ DestroyAudioRtpSender();
+}
+
+TEST_F(RtpSenderReceiverTest, AudioSenderCantSetUnimplementedRtpParameters) {
+ CreateAudioRtpSender();
+ RtpParameters params = audio_rtp_sender_->GetParameters();
+ EXPECT_EQ(1u, params.encodings.size());
+
+ // Unimplemented RtpParameters: mid, header_extensions,
+ // degredation_preference.
+ params.mid = "dummy_mid";
+ EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
+ audio_rtp_sender_->SetParameters(params).type());
+ params = audio_rtp_sender_->GetParameters();
+
+ params.header_extensions.emplace_back();
+ 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();
+}
+
+TEST_F(RtpSenderReceiverTest,
+ AudioSenderCantSetUnimplementedRtpEncodingParameters) {
+ CreateAudioRtpSender();
+ RtpParameters params = audio_rtp_sender_->GetParameters();
+ EXPECT_EQ(1u, params.encodings.size());
+
+ // Unimplemented RtpParameters: codec_payload_type, fec, rtx, dtx, ptime,
+ // max_framerate, scale_resolution_down_by, scale_framerate_down_by, rid,
+ // dependency_rids.
+ params.encodings[0].codec_payload_type = 1;
+ EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
+ audio_rtp_sender_->SetParameters(params).type());
+ params = audio_rtp_sender_->GetParameters();
+
+ params.encodings[0].fec = RtpFecParameters();
+ EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
+ audio_rtp_sender_->SetParameters(params).type());
+ params = audio_rtp_sender_->GetParameters();
+
+ params.encodings[0].rtx = RtpRtxParameters();
+ EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
+ audio_rtp_sender_->SetParameters(params).type());
+ params = audio_rtp_sender_->GetParameters();
+
+ params.encodings[0].dtx = DtxStatus::ENABLED;
+ EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
+ audio_rtp_sender_->SetParameters(params).type());
+ params = audio_rtp_sender_->GetParameters();
+
+ params.encodings[0].ptime = 1;
+ EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
+ audio_rtp_sender_->SetParameters(params).type());
+ params = audio_rtp_sender_->GetParameters();
+
+ params.encodings[0].max_framerate = 1;
+ EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
+ audio_rtp_sender_->SetParameters(params).type());
+ params = audio_rtp_sender_->GetParameters();
+
+ params.encodings[0].scale_resolution_down_by = 2.0;
+ EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
+ audio_rtp_sender_->SetParameters(params).type());
+ params = audio_rtp_sender_->GetParameters();
+
+ params.encodings[0].rid = "dummy_rid";
+ EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
+ audio_rtp_sender_->SetParameters(params).type());
+ params = audio_rtp_sender_->GetParameters();
+
+ params.encodings[0].dependency_rids.push_back("dummy_rid");
+ EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
+ audio_rtp_sender_->SetParameters(params).type());
DestroyAudioRtpSender();
}
@@ -782,6 +867,115 @@
DestroyVideoRtpSender();
}
+TEST_F(RtpSenderReceiverTest, VideoSenderCantSetUnimplementedRtpParameters) {
+ CreateVideoRtpSender();
+ RtpParameters params = video_rtp_sender_->GetParameters();
+ EXPECT_EQ(1u, params.encodings.size());
+
+ // Unimplemented RtpParameters: mid, header_extensions,
+ // degredation_preference.
+ params.mid = "dummy_mid";
+ EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
+ video_rtp_sender_->SetParameters(params).type());
+ params = video_rtp_sender_->GetParameters();
+
+ params.header_extensions.emplace_back();
+ 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();
+}
+
+TEST_F(RtpSenderReceiverTest,
+ VideoSenderCantSetUnimplementedEncodingParameters) {
+ CreateVideoRtpSender();
+ RtpParameters params = video_rtp_sender_->GetParameters();
+ EXPECT_EQ(1u, params.encodings.size());
+
+ // Unimplemented RtpParameters: codec_payload_type, fec, rtx, dtx, ptime,
+ // max_framerate, scale_resolution_down_by, scale_framerate_down_by, rid,
+ // dependency_rids.
+ params.encodings[0].codec_payload_type = 1;
+ EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
+ video_rtp_sender_->SetParameters(params).type());
+ params = video_rtp_sender_->GetParameters();
+
+ params.encodings[0].fec = RtpFecParameters();
+ EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
+ video_rtp_sender_->SetParameters(params).type());
+ params = video_rtp_sender_->GetParameters();
+
+ params.encodings[0].rtx = RtpRtxParameters();
+ EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
+ video_rtp_sender_->SetParameters(params).type());
+ params = video_rtp_sender_->GetParameters();
+
+ params.encodings[0].dtx = DtxStatus::ENABLED;
+ EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
+ video_rtp_sender_->SetParameters(params).type());
+ params = video_rtp_sender_->GetParameters();
+
+ params.encodings[0].ptime = 1;
+ EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
+ video_rtp_sender_->SetParameters(params).type());
+ params = video_rtp_sender_->GetParameters();
+
+ params.encodings[0].max_framerate = 1;
+ EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
+ video_rtp_sender_->SetParameters(params).type());
+ params = video_rtp_sender_->GetParameters();
+
+ params.encodings[0].scale_resolution_down_by = 2.0;
+ EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
+ video_rtp_sender_->SetParameters(params).type());
+ params = video_rtp_sender_->GetParameters();
+
+ params.encodings[0].rid = "dummy_rid";
+ EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
+ video_rtp_sender_->SetParameters(params).type());
+ params = video_rtp_sender_->GetParameters();
+
+ params.encodings[0].dependency_rids.push_back("dummy_rid");
+ EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
+ video_rtp_sender_->SetParameters(params).type());
+
+ DestroyVideoRtpSender();
+}
+
+// A video sender can have multiple simulcast layers, in which case it will
+// contain multiple RtpEncodingParameters. This tests that if this is the case
+// (simulcast), then we can't set the bitrate_priority, or max_bitrate_bps
+// for any encodings besides at index 0, because these are both implemented
+// "per-sender."
+TEST_F(RtpSenderReceiverTest, VideoSenderCantSetPerSenderEncodingParameters) {
+ // Add a simulcast specific send stream that contains 2 encoding parameters.
+ std::vector<uint32_t> ssrcs({1, 2});
+ cricket::StreamParams stream_params =
+ cricket::CreateSimStreamParams("cname", ssrcs);
+ video_media_channel_->AddSendStream(stream_params);
+ uint32_t primary_ssrc = stream_params.first_ssrc();
+ CreateVideoRtpSender(primary_ssrc);
+ RtpParameters params = video_rtp_sender_->GetParameters();
+ EXPECT_EQ(ssrcs.size(), params.encodings.size());
+
+ params.encodings[1].bitrate_priority = 2.0;
+ EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
+ video_rtp_sender_->SetParameters(params).type());
+ params = video_rtp_sender_->GetParameters();
+
+ params.encodings[1].max_bitrate_bps = 200000;
+ EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
+ video_rtp_sender_->SetParameters(params).type());
+
+ DestroyVideoRtpSender();
+}
+
TEST_F(RtpSenderReceiverTest, SetVideoMaxSendBitrate) {
CreateVideoRtpSender();
