diff --git a/media/base/media_engine.cc b/media/base/media_engine.cc index acb95d11c0..dff4de0b14 100644 --- a/media/base/media_engine.cc +++ b/media/base/media_engine.cc @@ -145,6 +145,7 @@ webrtc::RTCError CheckRtpParametersValues( const webrtc::FieldTrialsView& field_trials) { using webrtc::RTCErrorType; + bool has_requested_resolution = false; for (size_t i = 0; i < rtp_parameters.encodings.size(); ++i) { if (rtp_parameters.encodings[i].bitrate_priority <= 0) { LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_RANGE, @@ -183,11 +184,8 @@ webrtc::RTCError CheckRtpParametersValues( } } - if (rtp_parameters.encodings[i].requested_resolution && - rtp_parameters.encodings[i].scale_resolution_down_by) { - LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_RANGE, - "Attempted to set scale_resolution_down_by and " - "requested_resolution simultaniously."); + if (rtp_parameters.encodings[i].requested_resolution.has_value()) { + has_requested_resolution = true; } if (!field_trials.IsEnabled("WebRTC-MixedCodecSimulcast")) { @@ -200,6 +198,16 @@ webrtc::RTCError CheckRtpParametersValues( } } + if (has_requested_resolution && + absl::c_any_of(rtp_parameters.encodings, + [](const webrtc::RtpEncodingParameters& encoding) { + return !encoding.requested_resolution.has_value(); + })) { + LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_MODIFICATION, + "If a resolution is specified on any encoding then " + "it must be specified on all encodings."); + } + return CheckScalabilityModeValues(rtp_parameters, send_codecs, send_codec); } diff --git a/pc/peer_connection_encodings_integrationtest.cc b/pc/peer_connection_encodings_integrationtest.cc index bce27fcfb5..552df132a1 100644 --- a/pc/peer_connection_encodings_integrationtest.cc +++ b/pc/peer_connection_encodings_integrationtest.cc @@ -187,6 +187,24 @@ class PeerConnectionEncodingsIntegrationTest : public ::testing::Test { local_pc_wrapper.get(), &PeerConnectionTestWrapper::AddIceCandidate); } + // Negotiate without any tweaks (does not work for simulcast loopback). + void Negotiate( + rtc::scoped_refptr local_pc_wrapper, + rtc::scoped_refptr remote_pc_wrapper) { + std::unique_ptr offer = + CreateOffer(local_pc_wrapper); + rtc::scoped_refptr p1 = + SetLocalDescription(local_pc_wrapper, offer.get()); + rtc::scoped_refptr p2 = + SetRemoteDescription(remote_pc_wrapper, offer.get()); + EXPECT_TRUE(Await({p1, p2})); + std::unique_ptr answer = + CreateAnswer(remote_pc_wrapper); + p1 = SetLocalDescription(remote_pc_wrapper, answer.get()); + p2 = SetRemoteDescription(local_pc_wrapper, answer.get()); + EXPECT_TRUE(Await({p1, p2})); + } + void NegotiateWithSimulcastTweaks( rtc::scoped_refptr local_pc_wrapper, rtc::scoped_refptr remote_pc_wrapper) { @@ -2098,6 +2116,86 @@ TEST_F(PeerConnectionEncodingsIntegrationTest, ASSERT_TRUE(transceiver_or_error.ok()); } +TEST_F(PeerConnectionEncodingsIntegrationTest, + RequestedResolutionParameterChecking) { + rtc::scoped_refptr pc_wrapper = CreatePc(); + + // AddTransceiver: If `requested_resolution` is specified on any encoding it + // must be specified on all encodings. + RtpTransceiverInit init; + RtpEncodingParameters encoding; + encoding.requested_resolution = std::nullopt; + init.send_encodings.push_back(encoding); + encoding.requested_resolution = {.width = 1280, .height = 720}; + init.send_encodings.push_back(encoding); + auto transceiver_or_error = + pc_wrapper->pc()->AddTransceiver(cricket::MEDIA_TYPE_VIDEO, init); + EXPECT_FALSE(transceiver_or_error.ok()); + EXPECT_EQ(transceiver_or_error.error().type(), + RTCErrorType::UNSUPPORTED_OPERATION); + + // AddTransceiver: Specifying both `requested_resolution` and + // `scale_resolution_down_by` is allowed (the latter is ignored). + init.send_encodings[0].requested_resolution = {.width = 640, .height = 480}; + init.send_encodings[0].scale_resolution_down_by = 1.0; + init.send_encodings[1].requested_resolution = {.width = 1280, .height = 720}; + init.send_encodings[1].scale_resolution_down_by = 2.0; + transceiver_or_error = + pc_wrapper->pc()->AddTransceiver(cricket::MEDIA_TYPE_VIDEO, init); + ASSERT_TRUE(transceiver_or_error.ok()); + + // SetParameters: If `requested_resolution` is specified on any encoding it + // must be specified on all encodings. + auto sender = transceiver_or_error.value()->sender(); + auto parameters = sender->GetParameters(); + parameters.encodings[0].requested_resolution = {.width = 640, .height = 480}; + parameters.encodings[1].requested_resolution = std::nullopt; + auto error = sender->SetParameters(parameters); + EXPECT_FALSE(error.ok()); + EXPECT_EQ(error.type(), RTCErrorType::INVALID_MODIFICATION); + + // SetParameters: Specifying both `requested_resolution` and + // `scale_resolution_down_by` is allowed (the latter is ignored). + parameters = sender->GetParameters(); + parameters.encodings[0].requested_resolution = {.width = 640, .height = 480}; + parameters.encodings[0].scale_resolution_down_by = 2.0; + parameters.encodings[1].requested_resolution = {.width = 1280, .height = 720}; + parameters.encodings[1].scale_resolution_down_by = 1.0; + error = sender->SetParameters(parameters); + EXPECT_TRUE(error.ok()); +} + +TEST_F(PeerConnectionEncodingsIntegrationTest, + ScaleResolutionDownByIsIgnoredWhenRequestedResolutionIsSpecified) { + rtc::scoped_refptr local_pc_wrapper = CreatePc(); + rtc::scoped_refptr remote_pc_wrapper = CreatePc(); + + rtc::scoped_refptr stream = + local_pc_wrapper->GetUserMedia( + /*audio=*/false, {}, /*video=*/true, {.width = 640, .height = 360}); + rtc::scoped_refptr track = stream->GetVideoTracks()[0]; + + // Configure contradicting scaling factors (180p vs 360p). + RtpTransceiverInit init; + RtpEncodingParameters encoding; + encoding.scale_resolution_down_by = 2.0; + encoding.requested_resolution = {.width = 640, .height = 360}; + init.send_encodings.push_back(encoding); + auto transceiver_or_error = + local_pc_wrapper->pc()->AddTransceiver(track, init); + + // Negotiate singlecast. + ExchangeIceCandidates(local_pc_wrapper, remote_pc_wrapper); + Negotiate(local_pc_wrapper, remote_pc_wrapper); + + // Confirm 640x360 is sent. + // If `scale_resolution_down_by` was not ignored we would never ramp up to + // full resolution. + ASSERT_TRUE_WAIT(GetEncodingResolution(local_pc_wrapper) == + (Resolution{.width = 640, .height = 360}), + kLongTimeoutForRampingUp.ms()); +} + // Tests that use the standard path (specifying both `scalability_mode` and // `scale_resolution_down_by` or `requested_resolution`) should pass for all // codecs.