[Mirroring Service] Update capture resolution selection algorithm.
This change updates the capture resolution algorithm to reflect the changes made
in the component extension Javascript. It also refactors the code for
testability and adds the same test cases that were added to the extension.
(cherry picked from commit 9481c8a22142182d10d4a8cb771e7175379a5ed2)
Bug: 969873
Change-Id: I6040c5cf837fb20478d6d1294fe224b505097464
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1652214
Reviewed-by: Takumi Fujimoto <takumif@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Commit-Queue: mark a. foltz <mfoltz@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#668123}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1660923
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Cr-Commit-Position: refs/branch-heads/3809@{#320}
Cr-Branched-From: d82dec1a818f378c464ba307ddd9c92133eac355-refs/heads/master@{#665002}
diff --git a/chrome/browser/media/cast_mirroring_service_host.cc b/chrome/browser/media/cast_mirroring_service_host.cc
index d310bf3..163a518 100644
--- a/chrome/browser/media/cast_mirroring_service_host.cc
+++ b/chrome/browser/media/cast_mirroring_service_host.cc
@@ -10,6 +10,7 @@
#include "base/bind.h"
#include "base/logging.h"
#include "base/memory/ref_counted.h"
+#include "base/optional.h"
#include "base/single_thread_task_runner.h"
#include "base/task/post_task.h"
#include "chrome/browser/browser_process.h"
@@ -42,6 +43,9 @@
namespace mirroring {
+// Default resolution constraint.
+constexpr gfx::Size kMaxResolution(1920, 1080);
+
namespace {
void CreateVideoCaptureHostOnIO(const std::string& device_id,
@@ -99,35 +103,15 @@
return media_id;
}
-// Clamped resolution constraint to the screen size.
-gfx::Size GetCaptureResolutionConstraint() {
- // Default resolution constraint.
- constexpr gfx::Size kMaxResolution(1920, 1080);
+// Returns the size of the primary display in pixels, or base::nullopt if it
+// cannot be determined.
+base::Optional<gfx::Size> GetScreenResolution() {
display::Screen* screen = display::Screen::GetScreen();
if (!screen) {
DVLOG(1) << "Cannot get the Screen object.";
- return kMaxResolution;
+ return base::nullopt;
}
- const gfx::Size screen_resolution = screen->GetPrimaryDisplay().size();
- const int width_step = 160;
- const int height_step = 90;
- int clamped_width = 0;
- int clamped_height = 0;
- if (kMaxResolution.height() * screen_resolution.width() <
- kMaxResolution.width() * screen_resolution.height()) {
- clamped_width = std::min(kMaxResolution.width(), screen_resolution.width());
- clamped_width = clamped_width - (clamped_width % width_step);
- clamped_height = clamped_width * height_step / width_step;
- } else {
- clamped_height =
- std::min(kMaxResolution.height(), screen_resolution.height());
- clamped_height = clamped_height - (clamped_height % height_step);
- clamped_width = clamped_height * width_step / height_step;
- }
-
- clamped_width = std::max(clamped_width, width_step);
- clamped_height = std::max(clamped_height, height_step);
- return gfx::Size(clamped_width, clamped_height);
+ return screen->GetPrimaryDisplay().GetSizeInPixel();
}
} // namespace
@@ -216,6 +200,45 @@
ShowCaptureIndicator();
}
+// static
+gfx::Size CastMirroringServiceHost::GetCaptureResolutionConstraint() {
+ base::Optional<gfx::Size> screen_resolution = GetScreenResolution();
+ if (screen_resolution) {
+ return GetClampedResolution(screen_resolution.value());
+ } else {
+ return kMaxResolution;
+ }
+}
+
+// static
+gfx::Size CastMirroringServiceHost::GetClampedResolution(
+ gfx::Size screen_resolution) {
+ // Use landscape mode dimensions for screens in portrait mode.
+ if (screen_resolution.height() > screen_resolution.width()) {
+ screen_resolution =
+ gfx::Size(screen_resolution.height(), screen_resolution.width());
+ }
+ const int width_step = 160;
+ const int height_step = 90;
+ int clamped_width = 0;
+ int clamped_height = 0;
+ if (kMaxResolution.height() * screen_resolution.width() <
+ kMaxResolution.width() * screen_resolution.height()) {
+ clamped_width = std::min(kMaxResolution.width(), screen_resolution.width());
+ clamped_width = clamped_width - (clamped_width % width_step);
+ clamped_height = clamped_width * height_step / width_step;
+ } else {
+ clamped_height =
+ std::min(kMaxResolution.height(), screen_resolution.height());
+ clamped_height = clamped_height - (clamped_height % height_step);
+ clamped_width = clamped_height * width_step / height_step;
+ }
+
+ clamped_width = std::max(clamped_width, width_step);
+ clamped_height = std::max(clamped_height, height_step);
+ return gfx::Size(clamped_width, clamped_height);
+}
+
void CastMirroringServiceHost::GetVideoCaptureHost(
media::mojom::VideoCaptureHostRequest request) {
base::PostTaskWithTraits(
diff --git a/chrome/browser/media/cast_mirroring_service_host.h b/chrome/browser/media/cast_mirroring_service_host.h
index bfd0a0a..981df711 100644
--- a/chrome/browser/media/cast_mirroring_service_host.h
+++ b/chrome/browser/media/cast_mirroring_service_host.h
@@ -8,6 +8,7 @@
#include <memory>
#include <string>
+#include "base/gtest_prod_util.h"
#include "base/macros.h"
#include "components/mirroring/mojom/mirroring_service.mojom.h"
#include "components/mirroring/mojom/mirroring_service_host.mojom.h"
@@ -17,6 +18,7 @@
#include "content/public/browser/web_contents_observer.h"
#include "extensions/buildflags/buildflags.h"
#include "mojo/public/cpp/bindings/binding.h"
+#include "ui/gfx/geometry/size.h"
// TODO(https://crbug.com/879012): Remove the build flag. OffscreenTab should
// not only be defined when extension is enabled.
@@ -66,6 +68,12 @@
private:
friend class CastMirroringServiceHostBrowserTest;
+ FRIEND_TEST_ALL_PREFIXES(CastMirroringServiceHostTest,
+ TestGetClampedResolution);
+
+ static gfx::Size GetCaptureResolutionConstraint();
+ // Clamp resolution constraint to the screen size.
+ static gfx::Size GetClampedResolution(gfx::Size screen_resolution);
// ResourceProvider implementation.
void GetVideoCaptureHost(
diff --git a/chrome/browser/media/cast_mirroring_service_host_unittest.cc b/chrome/browser/media/cast_mirroring_service_host_unittest.cc
new file mode 100644
index 0000000..db93c63
--- /dev/null
+++ b/chrome/browser/media/cast_mirroring_service_host_unittest.cc
@@ -0,0 +1,30 @@
+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/media/cast_mirroring_service_host.h"
+
+#include "base/logging.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace mirroring {
+
+TEST(CastMirroringServiceHostTest, TestGetClampedResolution) {
+ // {screen width, screen height, capture width, capture height}
+ constexpr int test_cases[][4] = {
+ {1280, 800, 1280, 720}, {1366, 768, 1280, 720},
+ {768, 1366, 1280, 720}, {1920, 1080, 1920, 1080},
+ {1546, 2048, 1920, 1080}, {2399, 1598, 1920, 1080},
+ {2560, 1080, 1920, 1080}, {2560, 1600, 1920, 1080},
+ {3840, 2160, 1920, 1080}};
+
+ for (int i = 0; i < 9; i++) {
+ DVLOG(1) << "Testing resolution " << test_cases[i][0] << " x "
+ << test_cases[i][1];
+ EXPECT_EQ(CastMirroringServiceHost::GetClampedResolution(
+ gfx::Size(test_cases[i][0], test_cases[i][1])),
+ gfx::Size(test_cases[i][2], test_cases[i][3]));
+ }
+}
+
+} // namespace mirroring
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn
index 6081b0f9..6b59b7c 100644
--- a/chrome/test/BUILD.gn
+++ b/chrome/test/BUILD.gn
@@ -2754,6 +2754,7 @@
"../browser/mac/exception_processor_unittest.mm",
"../browser/mac/keystone_glue_unittest.mm",
"../browser/media/android/router/media_router_android_unittest.cc",
+ "../browser/media/cast_mirroring_service_host_unittest.cc",
"../browser/media/media_engagement_contents_observer_unittest.cc",
"../browser/media/media_engagement_preloaded_list_unittest.cc",
"../browser/media/media_engagement_score_unittest.cc",