From 1c262bf5a4c2f2aaf226b24f11195ba8cae772f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Bostr=C3=B6m?= Date: Wed, 23 Oct 2024 13:14:40 +0200 Subject: [PATCH] Allow not specifying `requested_resolution` on inactive encodings. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixes the bug where scaleResolutionDownTo must be specified even on inactive encodings (scaleResolutionDownTo is the JavaScript name for what is called requested_resolution inside WebRTC). Bug: chromium:375048792 Change-Id: I3206ef7de09eaba24a5b4305d888ec4904617e58 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/366522 Auto-Submit: Henrik Boström Commit-Queue: Ilya Nikolaevskiy Reviewed-by: Ilya Nikolaevskiy Commit-Queue: Henrik Boström Cr-Commit-Position: refs/heads/main@{#43292} --- media/base/media_engine.cc | 3 ++- pc/peer_connection_encodings_integrationtest.cc | 12 ++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/media/base/media_engine.cc b/media/base/media_engine.cc index 9aeac42b64..0b4b9302f5 100644 --- a/media/base/media_engine.cc +++ b/media/base/media_engine.cc @@ -221,7 +221,8 @@ webrtc::RTCError CheckRtpParametersValues( if (has_requested_resolution && absl::c_any_of(rtp_parameters.encodings, [](const webrtc::RtpEncodingParameters& encoding) { - return !encoding.requested_resolution.has_value(); + return encoding.active && + !encoding.requested_resolution.has_value(); })) { LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_MODIFICATION, "If a resolution is specified on any encoding then " diff --git a/pc/peer_connection_encodings_integrationtest.cc b/pc/peer_connection_encodings_integrationtest.cc index d97e06b856..079dbbec73 100644 --- a/pc/peer_connection_encodings_integrationtest.cc +++ b/pc/peer_connection_encodings_integrationtest.cc @@ -2152,8 +2152,8 @@ TEST_F(PeerConnectionEncodingsIntegrationTest, 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. + // SetParameters: If `requested_resolution` is specified on any active + // encoding it must be specified on all active encodings. auto sender = transceiver_or_error.value()->sender(); auto parameters = sender->GetParameters(); parameters.encodings[0].requested_resolution = {.width = 640, .height = 480}; @@ -2161,11 +2161,19 @@ TEST_F(PeerConnectionEncodingsIntegrationTest, auto error = sender->SetParameters(parameters); EXPECT_FALSE(error.ok()); EXPECT_EQ(error.type(), RTCErrorType::INVALID_MODIFICATION); + // But it's OK not to specify `requested_resolution` on an inactive encoding. + parameters = sender->GetParameters(); + parameters.encodings[0].requested_resolution = {.width = 640, .height = 480}; + parameters.encodings[1].active = false; + parameters.encodings[1].requested_resolution = std::nullopt; + error = sender->SetParameters(parameters); + EXPECT_TRUE(error.ok()); // SetParameters: Width and height must not be zero. sender = transceiver_or_error.value()->sender(); parameters = sender->GetParameters(); parameters.encodings[0].requested_resolution = {.width = 1280, .height = 0}; + parameters.encodings[1].active = true; parameters.encodings[1].requested_resolution = {.width = 0, .height = 720}; error = sender->SetParameters(parameters); EXPECT_FALSE(error.ok());