diff --git a/api/video_codecs/video_encoder_config.cc b/api/video_codecs/video_encoder_config.cc index 66ff7c94eb..2b1adc021b 100644 --- a/api/video_codecs/video_encoder_config.cc +++ b/api/video_codecs/video_encoder_config.cc @@ -22,6 +22,7 @@ VideoStream::VideoStream() min_bitrate_bps(-1), target_bitrate_bps(-1), max_bitrate_bps(-1), + scale_resolution_down_by(-1.), max_qp(-1), active(true) {} VideoStream::VideoStream(const VideoStream& other) = default; diff --git a/api/video_codecs/video_encoder_config.h b/api/video_codecs/video_encoder_config.h index 78f2db4cd5..0c69b93288 100644 --- a/api/video_codecs/video_encoder_config.h +++ b/api/video_codecs/video_encoder_config.h @@ -36,13 +36,15 @@ struct VideoStream { int min_bitrate_bps; int target_bitrate_bps; int max_bitrate_bps; + // Scaling factor applied to the stream size. + // |width| and |height| values are already scaled down. + double scale_resolution_down_by; int max_qp; absl::optional num_temporal_layers; absl::optional bitrate_priority; - // TODO(bugs.webrtc.org/8653): Support active per-simulcast layer. bool active; }; diff --git a/media/base/fake_media_engine.h b/media/base/fake_media_engine.h index 564b0dae78..a41d4e4226 100644 --- a/media/base/fake_media_engine.h +++ b/media/base/fake_media_engine.h @@ -146,8 +146,8 @@ class RtpHelper : public Base { const webrtc::RtpParameters& parameters) { auto parameters_iterator = rtp_send_parameters_.find(ssrc); if (parameters_iterator != rtp_send_parameters_.end()) { - auto result = - ValidateRtpParameters(parameters_iterator->second, parameters); + auto result = CheckRtpParametersInvalidModificationAndValues( + parameters_iterator->second, parameters); if (!result.ok()) return result; diff --git a/media/base/media_engine.cc b/media/base/media_engine.cc index 91fcc1ea72..62c3cec076 100644 --- a/media/base/media_engine.cc +++ b/media/base/media_engine.cc @@ -46,39 +46,23 @@ webrtc::RtpParameters CreateRtpParametersWithEncodings(StreamParams sp) { return parameters; } -webrtc::RTCError ValidateRtpParameters( - const webrtc::RtpParameters& old_rtp_parameters, +webrtc::RTCError CheckRtpParametersValues( const webrtc::RtpParameters& rtp_parameters) { using webrtc::RTCErrorType; - if (rtp_parameters.encodings.size() != old_rtp_parameters.encodings.size()) { - LOG_AND_RETURN_ERROR( - RTCErrorType::INVALID_MODIFICATION, - "Attempted to set RtpParameters with different encoding count"); - } - if (rtp_parameters.rtcp != old_rtp_parameters.rtcp) { - LOG_AND_RETURN_ERROR( - RTCErrorType::INVALID_MODIFICATION, - "Attempted to set RtpParameters with modified RTCP parameters"); - } - if (rtp_parameters.header_extensions != - old_rtp_parameters.header_extensions) { - LOG_AND_RETURN_ERROR( - RTCErrorType::INVALID_MODIFICATION, - "Attempted to set RtpParameters with modified header extensions"); - } for (size_t i = 0; i < rtp_parameters.encodings.size(); ++i) { - if (rtp_parameters.encodings[i].ssrc != - old_rtp_parameters.encodings[i].ssrc) { - LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_MODIFICATION, - "Attempted to set RtpParameters with modified SSRC"); - } if (rtp_parameters.encodings[i].bitrate_priority <= 0) { LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_RANGE, "Attempted to set RtpParameters bitrate_priority to " "an invalid number. bitrate_priority must be > 0."); } - + if (rtp_parameters.encodings[i].scale_resolution_down_by && + *rtp_parameters.encodings[i].scale_resolution_down_by < 1.0) { + LOG_AND_RETURN_ERROR( + RTCErrorType::INVALID_RANGE, + "Attempted to set RtpParameters scale_resolution_down_by to an " + "invalid number. scale_resolution_down_by must be >= 1.0"); + } if (rtp_parameters.encodings[i].min_bitrate_bps && rtp_parameters.encodings[i].max_bitrate_bps) { if (*rtp_parameters.encodings[i].max_bitrate_bps < @@ -107,9 +91,42 @@ webrtc::RTCError ValidateRtpParameters( " to a different value than other encoding layers."); } } + return webrtc::RTCError::OK(); } +webrtc::RTCError CheckRtpParametersInvalidModificationAndValues( + const webrtc::RtpParameters& old_rtp_parameters, + const webrtc::RtpParameters& rtp_parameters) { + using webrtc::RTCErrorType; + if (rtp_parameters.encodings.size() != old_rtp_parameters.encodings.size()) { + LOG_AND_RETURN_ERROR( + RTCErrorType::INVALID_MODIFICATION, + "Attempted to set RtpParameters with different encoding count"); + } + if (rtp_parameters.rtcp != old_rtp_parameters.rtcp) { + LOG_AND_RETURN_ERROR( + RTCErrorType::INVALID_MODIFICATION, + "Attempted to set RtpParameters with modified RTCP parameters"); + } + if (rtp_parameters.header_extensions != + old_rtp_parameters.header_extensions) { + LOG_AND_RETURN_ERROR( + RTCErrorType::INVALID_MODIFICATION, + "Attempted to set RtpParameters with modified header extensions"); + } + + for (size_t i = 0; i < rtp_parameters.encodings.size(); ++i) { + if (rtp_parameters.encodings[i].ssrc != + old_rtp_parameters.encodings[i].ssrc) { + LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_MODIFICATION, + "Attempted to set RtpParameters with modified SSRC"); + } + } + + return CheckRtpParametersValues(rtp_parameters); +} + CompositeMediaEngine::CompositeMediaEngine( std::unique_ptr voice_engine, std::unique_ptr video_engine) diff --git a/media/base/media_engine.h b/media/base/media_engine.h index 40c9ecc62e..5867c9e181 100644 --- a/media/base/media_engine.h +++ b/media/base/media_engine.h @@ -38,7 +38,10 @@ class Call; namespace cricket { -webrtc::RTCError ValidateRtpParameters( +webrtc::RTCError CheckRtpParametersValues( + const webrtc::RtpParameters& new_parameters); + +webrtc::RTCError CheckRtpParametersInvalidModificationAndValues( const webrtc::RtpParameters& old_parameters, const webrtc::RtpParameters& new_parameters); diff --git a/media/engine/simulcast.h b/media/engine/simulcast.h index 97cb196de6..0e5afc2455 100644 --- a/media/engine/simulcast.h +++ b/media/engine/simulcast.h @@ -26,6 +26,9 @@ int GetTotalMaxBitrateBps(const std::vector& streams); void BoostMaxSimulcastLayer(int max_bitrate_bps, std::vector* layers); +// Round size to nearest simulcast-friendly size +int NormalizeSimulcastSize(int size, size_t simulcast_layers); + // Gets simulcast settings. // TODO(asapersson): Remove max_bitrate_bps and max_framerate. std::vector GetSimulcastConfig( diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 756bc561a0..fc74a38b88 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -38,6 +38,8 @@ namespace cricket { namespace { +const int kMinLayerSize = 16; + // If this field trial is enabled, we will enable sending FlexFEC and disable // sending ULPFEC whenever the former has been negotiated in the SDPs. bool IsFlexfecFieldTrialEnabled() { @@ -1794,8 +1796,8 @@ void WebRtcVideoChannel::WebRtcVideoSendStream::SetSendParameters( webrtc::RTCError WebRtcVideoChannel::WebRtcVideoSendStream::SetRtpParameters( const webrtc::RtpParameters& new_parameters) { RTC_DCHECK_RUN_ON(&thread_checker_); - webrtc::RTCError error = - ValidateRtpParameters(rtp_parameters_, new_parameters); + webrtc::RTCError error = CheckRtpParametersInvalidModificationAndValues( + rtp_parameters_, new_parameters); if (!error.ok()) { return error; } @@ -1808,6 +1810,8 @@ webrtc::RTCError WebRtcVideoChannel::WebRtcVideoSendStream::SetRtpParameters( rtp_parameters_.encodings[i].max_bitrate_bps) || (new_parameters.encodings[i].max_framerate != rtp_parameters_.encodings[i].max_framerate) || + (new_parameters.encodings[i].scale_resolution_down_by != + rtp_parameters_.encodings[i].scale_resolution_down_by) || (new_parameters.encodings[i].num_temporal_layers != rtp_parameters_.encodings[i].num_temporal_layers)) { new_param = true; @@ -1979,6 +1983,10 @@ WebRtcVideoChannel::WebRtcVideoSendStream::CreateVideoEncoderConfig( encoder_config.simulcast_layers[i].max_framerate = *rtp_parameters_.encodings[i].max_framerate; } + if (rtp_parameters_.encodings[i].scale_resolution_down_by) { + encoder_config.simulcast_layers[i].scale_resolution_down_by = + *rtp_parameters_.encodings[i].scale_resolution_down_by; + } if (rtp_parameters_.encodings[i].num_temporal_layers) { encoder_config.simulcast_layers[i].num_temporal_layers = *rtp_parameters_.encodings[i].num_temporal_layers; @@ -2694,6 +2702,12 @@ std::vector EncoderStreamFactory::CreateEncoderStreams( int max_framerate = GetMaxFramerate(encoder_config, layers.size()); // Update the active simulcast layers and configured bitrates. bool is_highest_layer_max_bitrate_configured = false; + bool has_scale_resolution_down_by = + std::any_of(encoder_config.simulcast_layers.begin(), + encoder_config.simulcast_layers.end(), + [](const webrtc::VideoStream& layer) { + return layer.scale_resolution_down_by != -1.; + }); for (size_t i = 0; i < layers.size(); ++i) { layers[i].active = encoder_config.simulcast_layers[i].active; if (!is_screenshare_) { @@ -2706,6 +2720,18 @@ std::vector EncoderStreamFactory::CreateEncoderStreams( *encoder_config.simulcast_layers[i].num_temporal_layers; } } + if (has_scale_resolution_down_by) { + double scale_resolution_down_by = std::max( + encoder_config.simulcast_layers[i].scale_resolution_down_by, 1.0); + layers[i].width = + std::max(NormalizeSimulcastSize(width / scale_resolution_down_by, + encoder_config.number_of_streams), + kMinLayerSize); + layers[i].height = + std::max(NormalizeSimulcastSize(height / scale_resolution_down_by, + encoder_config.number_of_streams), + kMinLayerSize); + } // Update simulcast bitrates with configured min and max bitrate. if (encoder_config.simulcast_layers[i].min_bitrate_bps > 0) { layers[i].min_bitrate_bps = @@ -2771,6 +2797,17 @@ std::vector EncoderStreamFactory::CreateEncoderStreams( layer.height = height; layer.max_framerate = max_framerate; + if (encoder_config.simulcast_layers[0].scale_resolution_down_by > 1.) { + layer.width = std::max( + layer.width / + encoder_config.simulcast_layers[0].scale_resolution_down_by, + kMinLayerSize); + layer.height = std::max( + layer.height / + encoder_config.simulcast_layers[0].scale_resolution_down_by, + kMinLayerSize); + } + // In the case that the application sets a max bitrate that's lower than the // min bitrate, we adjust it down (see bugs.webrtc.org/9141). layer.min_bitrate_bps = std::min(min_bitrate_bps, max_bitrate_bps); diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index 14b4232236..ac2bf4b6b3 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -5547,6 +5547,218 @@ TEST_F(WebRtcVideoChannelTest, SetRtpSendParametersInvalidNetworkPriority) { } } +TEST_F(WebRtcVideoChannelTest, + GetAndSetRtpSendParametersScaleResolutionDownByVP8) { + cricket::VideoSendParameters parameters; + parameters.codecs.push_back(cricket::VideoCodec("VP8")); + ASSERT_TRUE(channel_->SetSendParameters(parameters)); + FakeVideoSendStream* stream = SetUpSimulcast(true, false); + + webrtc::test::FrameForwarder frame_forwarder; + cricket::FakeFrameSource frame_source(1280, 720, + rtc::kNumMicrosecsPerSec / 30); + + VideoOptions options; + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, &options, &frame_forwarder)); + channel_->SetSend(true); + + // Try layers in natural order (smallest to largest). + { + auto rtp_parameters = channel_->GetRtpSendParameters(last_ssrc_); + ASSERT_EQ(3u, rtp_parameters.encodings.size()); + rtp_parameters.encodings[0].scale_resolution_down_by = 4.0; + rtp_parameters.encodings[1].scale_resolution_down_by = 2.0; + rtp_parameters.encodings[2].scale_resolution_down_by = 1.0; + auto result = channel_->SetRtpSendParameters(last_ssrc_, rtp_parameters); + ASSERT_TRUE(result.ok()); + + frame_forwarder.IncomingCapturedFrame(frame_source.GetFrame()); + + std::vector video_streams = stream->GetVideoStreams(); + ASSERT_EQ(3u, video_streams.size()); + EXPECT_EQ(320u, video_streams[0].width); + EXPECT_EQ(180u, video_streams[0].height); + EXPECT_EQ(640u, video_streams[1].width); + EXPECT_EQ(360u, video_streams[1].height); + EXPECT_EQ(1280u, video_streams[2].width); + EXPECT_EQ(720u, video_streams[2].height); + } + + // Try layers in reverse natural order (largest to smallest). + { + auto rtp_parameters = channel_->GetRtpSendParameters(last_ssrc_); + ASSERT_EQ(3u, rtp_parameters.encodings.size()); + rtp_parameters.encodings[0].scale_resolution_down_by = 1.0; + rtp_parameters.encodings[1].scale_resolution_down_by = 2.0; + rtp_parameters.encodings[2].scale_resolution_down_by = 4.0; + auto result = channel_->SetRtpSendParameters(last_ssrc_, rtp_parameters); + ASSERT_TRUE(result.ok()); + + frame_forwarder.IncomingCapturedFrame(frame_source.GetFrame()); + + std::vector video_streams = stream->GetVideoStreams(); + ASSERT_EQ(3u, video_streams.size()); + EXPECT_EQ(1280u, video_streams[0].width); + EXPECT_EQ(720u, video_streams[0].height); + EXPECT_EQ(640u, video_streams[1].width); + EXPECT_EQ(360u, video_streams[1].height); + EXPECT_EQ(320u, video_streams[2].width); + EXPECT_EQ(180u, video_streams[2].height); + } + + // Try layers in mixed order. + { + auto rtp_parameters = channel_->GetRtpSendParameters(last_ssrc_); + ASSERT_EQ(3u, rtp_parameters.encodings.size()); + rtp_parameters.encodings[0].scale_resolution_down_by = 10.0; + rtp_parameters.encodings[1].scale_resolution_down_by = 2.0; + rtp_parameters.encodings[2].scale_resolution_down_by = 4.0; + auto result = channel_->SetRtpSendParameters(last_ssrc_, rtp_parameters); + ASSERT_TRUE(result.ok()); + + frame_forwarder.IncomingCapturedFrame(frame_source.GetFrame()); + + std::vector video_streams = stream->GetVideoStreams(); + ASSERT_EQ(3u, video_streams.size()); + EXPECT_EQ(128u, video_streams[0].width); + EXPECT_EQ(72u, video_streams[0].height); + EXPECT_EQ(640u, video_streams[1].width); + EXPECT_EQ(360u, video_streams[1].height); + EXPECT_EQ(320u, video_streams[2].width); + EXPECT_EQ(180u, video_streams[2].height); + } + + // Try with a missing scale setting, defaults to 1.0 if any other is set. + { + auto rtp_parameters = channel_->GetRtpSendParameters(last_ssrc_); + ASSERT_EQ(3u, rtp_parameters.encodings.size()); + rtp_parameters.encodings[0].scale_resolution_down_by = 1.0; + rtp_parameters.encodings[1].scale_resolution_down_by.reset(); + rtp_parameters.encodings[2].scale_resolution_down_by = 4.0; + auto result = channel_->SetRtpSendParameters(last_ssrc_, rtp_parameters); + ASSERT_TRUE(result.ok()); + + frame_forwarder.IncomingCapturedFrame(frame_source.GetFrame()); + + std::vector video_streams = stream->GetVideoStreams(); + ASSERT_EQ(3u, video_streams.size()); + EXPECT_EQ(1280u, video_streams[0].width); + EXPECT_EQ(720u, video_streams[0].height); + EXPECT_EQ(1280u, video_streams[1].width); + EXPECT_EQ(720u, video_streams[1].height); + EXPECT_EQ(320u, video_streams[2].width); + EXPECT_EQ(180u, video_streams[2].height); + } + + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, nullptr, nullptr)); +} + +TEST_F(WebRtcVideoChannelTest, + GetAndSetRtpSendParametersScaleResolutionDownByH264) { + encoder_factory_->AddSupportedVideoCodecType("H264"); + cricket::VideoSendParameters parameters; + parameters.codecs.push_back(cricket::VideoCodec("H264")); + ASSERT_TRUE(channel_->SetSendParameters(parameters)); + FakeVideoSendStream* stream = SetUpSimulcast(true, false); + + webrtc::test::FrameForwarder frame_forwarder; + cricket::FakeFrameSource frame_source(1280, 720, + rtc::kNumMicrosecsPerSec / 30); + + VideoOptions options; + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, &options, &frame_forwarder)); + channel_->SetSend(true); + + // Try layers in natural order (smallest to largest). + { + auto rtp_parameters = channel_->GetRtpSendParameters(last_ssrc_); + ASSERT_EQ(3u, rtp_parameters.encodings.size()); + rtp_parameters.encodings[0].scale_resolution_down_by = 4.0; + rtp_parameters.encodings[1].scale_resolution_down_by = 2.0; + rtp_parameters.encodings[2].scale_resolution_down_by = 1.0; + auto result = channel_->SetRtpSendParameters(last_ssrc_, rtp_parameters); + ASSERT_TRUE(result.ok()); + + frame_forwarder.IncomingCapturedFrame(frame_source.GetFrame()); + + std::vector video_streams = stream->GetVideoStreams(); + ASSERT_EQ(3u, video_streams.size()); + EXPECT_EQ(320u, video_streams[0].width); + EXPECT_EQ(180u, video_streams[0].height); + EXPECT_EQ(640u, video_streams[1].width); + EXPECT_EQ(360u, video_streams[1].height); + EXPECT_EQ(1280u, video_streams[2].width); + EXPECT_EQ(720u, video_streams[2].height); + } + + // Try layers in reverse natural order (largest to smallest). + { + auto rtp_parameters = channel_->GetRtpSendParameters(last_ssrc_); + ASSERT_EQ(3u, rtp_parameters.encodings.size()); + rtp_parameters.encodings[0].scale_resolution_down_by = 1.0; + rtp_parameters.encodings[1].scale_resolution_down_by = 2.0; + rtp_parameters.encodings[2].scale_resolution_down_by = 4.0; + auto result = channel_->SetRtpSendParameters(last_ssrc_, rtp_parameters); + ASSERT_TRUE(result.ok()); + + frame_forwarder.IncomingCapturedFrame(frame_source.GetFrame()); + + std::vector video_streams = stream->GetVideoStreams(); + ASSERT_EQ(3u, video_streams.size()); + EXPECT_EQ(1280u, video_streams[0].width); + EXPECT_EQ(720u, video_streams[0].height); + EXPECT_EQ(640u, video_streams[1].width); + EXPECT_EQ(360u, video_streams[1].height); + EXPECT_EQ(320u, video_streams[2].width); + EXPECT_EQ(180u, video_streams[2].height); + } + + // Try layers in mixed order. + { + auto rtp_parameters = channel_->GetRtpSendParameters(last_ssrc_); + ASSERT_EQ(3u, rtp_parameters.encodings.size()); + rtp_parameters.encodings[0].scale_resolution_down_by = 10.0; + rtp_parameters.encodings[1].scale_resolution_down_by = 2.0; + rtp_parameters.encodings[2].scale_resolution_down_by = 4.0; + auto result = channel_->SetRtpSendParameters(last_ssrc_, rtp_parameters); + ASSERT_TRUE(result.ok()); + + frame_forwarder.IncomingCapturedFrame(frame_source.GetFrame()); + + std::vector video_streams = stream->GetVideoStreams(); + ASSERT_EQ(3u, video_streams.size()); + EXPECT_EQ(128u, video_streams[0].width); + EXPECT_EQ(72u, video_streams[0].height); + EXPECT_EQ(640u, video_streams[1].width); + EXPECT_EQ(360u, video_streams[1].height); + EXPECT_EQ(320u, video_streams[2].width); + EXPECT_EQ(180u, video_streams[2].height); + } + + // Try with a missing scale setting, defaults to 1.0 if any other is set. + { + auto rtp_parameters = channel_->GetRtpSendParameters(last_ssrc_); + ASSERT_EQ(3u, rtp_parameters.encodings.size()); + rtp_parameters.encodings[0].scale_resolution_down_by = 1.0; + rtp_parameters.encodings[1].scale_resolution_down_by.reset(); + rtp_parameters.encodings[2].scale_resolution_down_by = 4.0; + auto result = channel_->SetRtpSendParameters(last_ssrc_, rtp_parameters); + ASSERT_TRUE(result.ok()); + + frame_forwarder.IncomingCapturedFrame(frame_source.GetFrame()); + + std::vector video_streams = stream->GetVideoStreams(); + ASSERT_EQ(3u, video_streams.size()); + EXPECT_EQ(1280u, video_streams[0].width); + EXPECT_EQ(720u, video_streams[0].height); + EXPECT_EQ(1280u, video_streams[1].width); + EXPECT_EQ(720u, video_streams[1].height); + EXPECT_EQ(320u, video_streams[2].width); + EXPECT_EQ(180u, video_streams[2].height); + } + EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, nullptr, nullptr)); +} + TEST_F(WebRtcVideoChannelTest, GetAndSetRtpSendParametersMaxFramerate) { const size_t kNumSimulcastStreams = 3; SetUpSimulcast(true, false); diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc index 67d89fdc29..4dec78e643 100644 --- a/media/engine/webrtc_voice_engine.cc +++ b/media/engine/webrtc_voice_engine.cc @@ -920,7 +920,8 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream } webrtc::RTCError SetRtpParameters(const webrtc::RtpParameters& parameters) { - webrtc::RTCError error = ValidateRtpParameters(rtp_parameters_, parameters); + webrtc::RTCError error = CheckRtpParametersInvalidModificationAndValues( + rtp_parameters_, parameters); if (!error.ok()) { return error; } diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 22cd60a4de..10901496f0 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -1485,6 +1485,11 @@ PeerConnection::AddTransceiver( "Attempted to set an unimplemented parameter of RtpParameters."); } + auto result = cricket::CheckRtpParametersValues(parameters); + if (!result.ok()) { + LOG_AND_RETURN_ERROR(result.type(), result.message()); + } + RTC_LOG(LS_INFO) << "Adding " << cricket::MediaTypeToString(media_type) << " transceiver in response to a call to AddTransceiver."; // Set the sender ID equal to the track ID if the track is specified unless diff --git a/pc/peer_connection_rtp_unittest.cc b/pc/peer_connection_rtp_unittest.cc index 2321ada86f..e80c95c7e8 100644 --- a/pc/peer_connection_rtp_unittest.cc +++ b/pc/peer_connection_rtp_unittest.cc @@ -1452,8 +1452,7 @@ TEST_F(PeerConnectionRtpTestUnifiedPlan, auto default_send_encodings = init.send_encodings; // Unimplemented RtpParameters: ssrc, codec_payload_type, fec, rtx, dtx, - // ptime, scale_resolution_down_by, scale_framerate_down_by, rid, - // dependency_rids. + // ptime, scale_framerate_down_by, rid, dependency_rids. init.send_encodings[0].ssrc = 1; EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, caller->pc() @@ -1502,14 +1501,6 @@ TEST_F(PeerConnectionRtpTestUnifiedPlan, .type()); init.send_encodings = default_send_encodings; - init.send_encodings[0].scale_resolution_down_by = 2.0; - EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, - caller->pc() - ->AddTransceiver(cricket::MEDIA_TYPE_AUDIO, init) - .error() - .type()); - init.send_encodings = default_send_encodings; - init.send_encodings[0].rid = "dummy_rid"; EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, caller->pc() @@ -1526,6 +1517,58 @@ TEST_F(PeerConnectionRtpTestUnifiedPlan, .type()); } +// Test that AddTransceiver fails if trying to use invalid RTP encoding +// parameters with the send_encodings parameters. +TEST_F(PeerConnectionRtpTestUnifiedPlan, CheckForInvalidEncodingParameters) { + auto caller = CreatePeerConnection(); + + RtpTransceiverInit init; + init.send_encodings.emplace_back(); + + auto default_send_encodings = init.send_encodings; + + init.send_encodings[0].scale_resolution_down_by = 0.5; + EXPECT_EQ(RTCErrorType::INVALID_RANGE, + caller->pc() + ->AddTransceiver(cricket::MEDIA_TYPE_VIDEO, init) + .error() + .type()); + init.send_encodings = default_send_encodings; + + init.send_encodings[0].bitrate_priority = 0; + EXPECT_EQ(RTCErrorType::INVALID_RANGE, + caller->pc() + ->AddTransceiver(cricket::MEDIA_TYPE_VIDEO, init) + .error() + .type()); + init.send_encodings = default_send_encodings; + + init.send_encodings[0].min_bitrate_bps = 200000; + init.send_encodings[0].max_bitrate_bps = 100000; + EXPECT_EQ(RTCErrorType::INVALID_RANGE, + caller->pc() + ->AddTransceiver(cricket::MEDIA_TYPE_VIDEO, init) + .error() + .type()); + init.send_encodings = default_send_encodings; + + init.send_encodings[0].num_temporal_layers = 0; + EXPECT_EQ(RTCErrorType::INVALID_RANGE, + caller->pc() + ->AddTransceiver(cricket::MEDIA_TYPE_VIDEO, init) + .error() + .type()); + init.send_encodings = default_send_encodings; + + init.send_encodings[0].num_temporal_layers = 5; + EXPECT_EQ(RTCErrorType::INVALID_RANGE, + caller->pc() + ->AddTransceiver(cricket::MEDIA_TYPE_VIDEO, init) + .error() + .type()); + init.send_encodings = default_send_encodings; +} + // Test that AddTransceiver transfers the send_encodings to the sender and they // are retained after SetLocalDescription(). TEST_F(PeerConnectionRtpTestUnifiedPlan, SendEncodingsPassedToSender) { diff --git a/pc/rtp_sender.cc b/pc/rtp_sender.cc index b97b876f3d..d88b1fc5a0 100644 --- a/pc/rtp_sender.cc +++ b/pc/rtp_sender.cc @@ -42,7 +42,6 @@ bool UnimplementedRtpEncodingParameterHasValue( encoding_params.fec.has_value() || encoding_params.rtx.has_value() || encoding_params.dtx.has_value() || encoding_params.ptime.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; @@ -290,7 +289,8 @@ RTCError AudioRtpSender::SetParameters(const RtpParameters& parameters) { "Attempted to set an unimplemented parameter of RtpParameters."); } if (!media_channel_) { - auto result = cricket::ValidateRtpParameters(init_parameters_, parameters); + auto result = cricket::CheckRtpParametersInvalidModificationAndValues( + init_parameters_, parameters); if (result.ok()) { init_parameters_ = parameters; } @@ -544,7 +544,8 @@ RTCError VideoRtpSender::SetParameters(const RtpParameters& parameters) { "Attempted to set an unimplemented parameter of RtpParameters."); } if (!media_channel_) { - auto result = cricket::ValidateRtpParameters(init_parameters_, parameters); + auto result = cricket::CheckRtpParametersInvalidModificationAndValues( + init_parameters_, parameters); if (result.ok()) { init_parameters_ = parameters; } diff --git a/pc/rtp_sender_receiver_unittest.cc b/pc/rtp_sender_receiver_unittest.cc index 69d0c74e8d..83f06c6449 100644 --- a/pc/rtp_sender_receiver_unittest.cc +++ b/pc/rtp_sender_receiver_unittest.cc @@ -810,7 +810,7 @@ TEST_F(RtpSenderReceiverTest, EXPECT_EQ(1u, params.encodings.size()); // Unimplemented RtpParameters: codec_payload_type, fec, rtx, dtx, ptime, - // scale_resolution_down_by, scale_framerate_down_by, rid, dependency_rids. + // 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()); @@ -836,11 +836,6 @@ TEST_F(RtpSenderReceiverTest, 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()); @@ -1087,7 +1082,7 @@ TEST_F(RtpSenderReceiverTest, EXPECT_EQ(1u, params.encodings.size()); // Unimplemented RtpParameters: codec_payload_type, fec, rtx, dtx, ptime, - // scale_resolution_down_by, scale_framerate_down_by, rid, dependency_rids. + // 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()); @@ -1113,11 +1108,6 @@ TEST_F(RtpSenderReceiverTest, 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()); @@ -1130,6 +1120,30 @@ TEST_F(RtpSenderReceiverTest, DestroyVideoRtpSender(); } +TEST_F(RtpSenderReceiverTest, VideoSenderCanSetScaleResolutionDownBy) { + CreateVideoRtpSender(); + + RtpParameters params = video_rtp_sender_->GetParameters(); + params.encodings[0].scale_resolution_down_by = 2; + + EXPECT_TRUE(video_rtp_sender_->SetParameters(params).ok()); + params = video_rtp_sender_->GetParameters(); + EXPECT_EQ(2, params.encodings[0].scale_resolution_down_by); + + DestroyVideoRtpSender(); +} + +TEST_F(RtpSenderReceiverTest, VideoSenderDetectInvalidScaleResolutionDownBy) { + CreateVideoRtpSender(); + + RtpParameters params = video_rtp_sender_->GetParameters(); + params.encodings[0].scale_resolution_down_by = 0.5; + RTCError result = video_rtp_sender_->SetParameters(params); + EXPECT_EQ(RTCErrorType::INVALID_RANGE, result.type()); + + DestroyVideoRtpSender(); +} + TEST_F(RtpSenderReceiverTest, VideoSenderCantSetUnimplementedEncodingParametersWithSimulcast) { CreateVideoRtpSenderWithSimulcast(); @@ -1137,7 +1151,7 @@ TEST_F(RtpSenderReceiverTest, EXPECT_EQ(kVideoSimulcastLayerCount, params.encodings.size()); // Unimplemented RtpParameters: codec_payload_type, fec, rtx, dtx, ptime, - // scale_resolution_down_by, scale_framerate_down_by, rid, dependency_rids. + // scale_framerate_down_by, rid, dependency_rids. for (size_t i = 0; i < params.encodings.size(); i++) { params.encodings[i].codec_payload_type = 1; EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, @@ -1164,11 +1178,6 @@ TEST_F(RtpSenderReceiverTest, video_rtp_sender_->SetParameters(params).type()); params = video_rtp_sender_->GetParameters(); - params.encodings[i].scale_resolution_down_by = 2.0; - EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, - video_rtp_sender_->SetParameters(params).type()); - params = video_rtp_sender_->GetParameters(); - params.encodings[i].rid = "dummy_rid"; EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, video_rtp_sender_->SetParameters(params).type());