commit 843a3173f26a823927ab4163f581ad2aeaf6b244
author Henrik Boström <hbos@webrtc.org> Tue Sep 03 07:55:03 2024
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue Sep 03 10:38:40 2024
tree 8a7469ef475383e4a8b4a49293aec7f7da78cf3b
parent d34f3b8e043a202bb16dcc4a83231a0bf41e2e0e

Fix requested_resolution orientation assumption in OnSinkWants().

The VideoAdapter is used to configure encoding resolutions based on
requested_resolution in an orientation agnostic way[1]. This means that
if you request 1280x720 and the input frame is 720x1280, there is no
downscale happening.

However in the same file there is one instance of
VideoAdapter::OnSinkWants() where requested_resolution is assumed to be
expressed in landscape mode. This breaks the case where the 720x1280 is
requested but the frame is 1280x720 which causes inconsistent behavior
and breaks symmetry. This would also break simulcast since this code
path is only applied with the top layer's requested resolution while the
lower layers are still scaled in an agnostic way.

A new test is added to verify the fix. Prior to the fix, the first half
of the test was passing, after the fix both parts of the test pass.

[1] https://source.chromium.org/chromium/chromium/src/+/main:third_party/webrtc/media/base/video_adapter.h;l=76;drc=02b5b024b66755a851a752b7851b124ba03f6cb6

Bug: webrtc:363019836
Change-Id: I564068e98c93cab89eb38a10b0f8378899438e5b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/361160
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42923}

media/base/video_adapter.cc

diff --git a/media/base/video_adapter.cc b/media/base/video_adapter.cc
index 1d0beb5..487845d 100644
--- a/media/base/video_adapter.cc
+++ b/media/base/video_adapter.cc
@@ -379,6 +379,11 @@
   }
 
   auto res = *sink_wants.requested_resolution;
+  if (res.width < res.height) {
+    // Adjust `res` to landscape mode.
+    res.width = sink_wants.requested_resolution->height;
+    res.height = sink_wants.requested_resolution->width;
+  }
   auto pixel_count = res.width * res.height;
   output_format_request_.target_landscape_aspect_ratio =
       std::make_pair(res.width, res.height);

media/base/video_adapter_unittest.cc

diff --git a/media/base/video_adapter_unittest.cc b/media/base/video_adapter_unittest.cc
index aab96dc..5011438 100644
--- a/media/base/video_adapter_unittest.cc
+++ b/media/base/video_adapter_unittest.cc
@@ -1281,6 +1281,31 @@
   }
 }
 
+TEST_P(VideoAdapterTest, RequestedResolutionIsOrientationAgnostic) {
+  // Request 1280x720 when frame is 720x1280.
+  {
+    adapter_.OnSinkWants(
+        BuildSinkWants(Resolution{.width = 1280, .height = 720},
+                       /* any_active_without_requested_resolution= */ false));
+
+    EXPECT_THAT(
+        AdaptFrameResolution(/* input frame */ {.width = 720, .height = 1280})
+            .first,
+        Eq(Resolution{.width = 720, .height = 1280}));
+  }
+  // Request 720x1280 when frame is 1280x720.
+  {
+    adapter_.OnSinkWants(
+        BuildSinkWants(Resolution{.width = 720, .height = 1280},
+                       /* any_active_without_requested_resolution= */ false));
+
+    EXPECT_THAT(
+        AdaptFrameResolution(/* input frame */ {.width = 1280, .height = 720})
+            .first,
+        Eq(Resolution{.width = 1280, .height = 720}));
+  }
+}
+
 class VideoAdapterWithSourceAlignmentTest : public VideoAdapterTest {
  protected:
   static constexpr int kSourceResolutionAlignment = 7;