diff --git a/api/rtpparameters.h b/api/rtpparameters.h index a026c5ae0f..ba02fd24d6 100644 --- a/api/rtpparameters.h +++ b/api/rtpparameters.h @@ -426,11 +426,11 @@ struct RtpEncodingParameters { // For video, scale the resolution down by this factor. // TODO(deadbeef): Not implemented. - double scale_resolution_down_by = 1.0; + rtc::Optional scale_resolution_down_by; // Scale the framerate down by this factor. // TODO(deadbeef): Not implemented. - double scale_framerate_down_by = 1.0; + rtc::Optional scale_framerate_down_by; // For an RtpSender, set to true to cause this encoding to be encoded and // sent, and false for it not to be encoded and sent. This allows control diff --git a/media/base/fakemediaengine.h b/media/base/fakemediaengine.h index e9a6bd6f40..50ef0a3502 100644 --- a/media/base/fakemediaengine.h +++ b/media/base/fakemediaengine.h @@ -110,7 +110,7 @@ template class RtpHelper : public Base { } send_streams_.push_back(sp); rtp_send_parameters_[sp.first_ssrc()] = - CreateRtpParametersWithOneEncoding(); + CreateRtpParametersWithEncodings(sp); return true; } virtual bool RemoveSendStream(uint32_t ssrc) { diff --git a/pc/rtpsender.cc b/pc/rtpsender.cc index 3b0bbf886a..b7e48cd33b 100644 --- a/pc/rtpsender.cc +++ b/pc/rtpsender.cc @@ -30,6 +30,62 @@ int GenerateUniqueId() { 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 @@ RTCError AudioRtpSender::SetParameters(const RtpParameters& parameters) { " 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(RTC_FROM_HERE, [&] { RTCError result = media_channel_->SetRtpSendParameters(ssrc_, parameters); last_transaction_id_.reset(); @@ -421,6 +482,11 @@ RTCError VideoRtpSender::SetParameters(const RtpParameters& parameters) { " 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(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 b084b01e1a..be0e975cce 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 @@ class RtpSenderReceiverTest : public testing::Test, 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 @@ TEST_F(RtpSenderReceiverTest, AudioSenderSetParametersOldValueFail) { 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 @@ TEST_F(RtpSenderReceiverTest, VideoSenderSetParametersOldValueFail) { 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 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();