commit 1c262bf5a4c2f2aaf226b24f11195ba8cae772f1
author Henrik Boström <hbos@webrtc.org> Wed Oct 23 11:14:40 2024
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Oct 23 12:37:14 2024
tree 2eb41bb9e6a3324cf7655c01f3b8d0f467a78ef5
parent 5f324db7fa83174f312019ba5c74c1b49039eaf1

Allow not specifying `requested_resolution` on inactive encodings.

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 <hbos@webrtc.org>
Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43292}

media/base/media_engine.cc

diff --git a/media/base/media_engine.cc b/media/base/media_engine.cc
index 9aeac42..0b4b930 100644
--- a/media/base/media_engine.cc
+++ b/media/base/media_engine.cc
@@ -221,7 +221,8 @@
   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 "

pc/peer_connection_encodings_integrationtest.cc

diff --git a/pc/peer_connection_encodings_integrationtest.cc b/pc/peer_connection_encodings_integrationtest.cc
index d97e06b..079dbbe 100644
--- a/pc/peer_connection_encodings_integrationtest.cc
+++ b/pc/peer_connection_encodings_integrationtest.cc
@@ -2152,8 +2152,8 @@
       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 @@
   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());