Implement all resolution change policies for desktop and tab capture.

content/browser/media/capture/capture_resolution_evaluator.cc

Index: content/browser/media/capture/capture_resolution_evaluator.cc
diff --git a/content/browser/media/capture/capture_resolution_evaluator.cc b/content/browser/media/capture/capture_resolution_evaluator.cc
new file mode 100644
index 0000000000000000000000000000000000000000..e082f6c98bc5f73b6b2014db6805530231a8d3a3
--- /dev/null
+++ b/content/browser/media/capture/capture_resolution_evaluator.cc
@@ -0,0 +1,123 @@
+// Copyright 2015 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 "content/browser/media/capture/capture_resolution_evaluator.h"
+
+#include "media/base/limits.h"
+#include "media/base/video_util.h"
+
+namespace content {
+
+namespace {
+
+// Compute the minimum frame size from the given |max_frame_size| and
+// |resolution_change_policy|.
+gfx::Size ComputeMinimumCaptureSize(
+    const gfx::Size& max_frame_size,
+    media::ResolutionChangePolicy resolution_change_policy) {
+  switch (resolution_change_policy) {
+    case media::RESOLUTION_POLICY_FIXED_RESOLUTION:
+      return max_frame_size;
+    case media::RESOLUTION_POLICY_FIXED_ASPECT_RATIO: {
+      // TODO(miu): This is a place-holder until "min constraints" are plumbed-
+      // in from the MediaStream framework.  http://crbug.com/473336
+      const int kMinLines = 180;
+      if (max_frame_size.height() <= kMinLines)
+        return max_frame_size;
+      const gfx::Size result(
+          kMinLines * max_frame_size.width() / max_frame_size.height(),
+          kMinLines);
+      if (result.width() <= 0 || result.width() > media::limits::kMaxDimension)
+        return max_frame_size;
+      return result;
+    }
+    case media::RESOLUTION_POLICY_ANY_WITHIN_LIMIT:
+      break;

[Wez, 2015/05/13 19:05:08]

nit: Suggest returning Size(1,1) explicitly here, for clarity.

[miu, 2015/05/14 01:21:32]

Done.

+    case media::RESOLUTION_POLICY_LAST:

[Wez, 2015/05/13 19:05:08]

Won't this raise compilation errors since you're handling both LAST and whatever
the other name for the last value is?

[miu, 2015/05/14 01:21:32]

This is the "meaningless" enum value that only exists for the IPC layer to check
for a valid enum range.  In other words, this switch statement has a case for
exactly one of each possible enum value.

[Wez, 2015/05/15 00:55:42]

Dang, hoisted by my own petard!

I thought the IPC "last" value was the same as the highest of the actual real
values (i.e. actually the highest value, not one-more-than-highest), hence the
comment.

+      NOTREACHED();
+  }
+  return gfx::Size(1, 1);

[Wez, 2015/05/13 19:05:08]

nit: Can you get rid of this if you add a return to the LAST case? Then all your
cases have clear, explicit results.

[miu, 2015/05/14 01:21:32]

MSVC++ compiler complains, unfortunately, even though it's obvious the code
point couldn't be reached.  Unless you're aware they've fixed this problem
recently?

I'll give it a shot and see what happens on the trybots.

[Wez, 2015/05/15 00:55:42]

I thought that VS2010 and above got this right, provided you take care to handle
all the enum values, but I couldn't swear to that.

[miu, 2015/05/15 21:14:00]

Nope.  The compile of patch set 4 (where I tried it our way) failed to compile
on the win8_chromium_rel trybot with:

FAILED: ninja -t msvc -e environment.x86 -- E:\b\build\goma/gomacc
"E:\b\depot_tools\win_toolchain\vs2013_files\VC\bin\amd64_x86\cl.exe" /nologo
/showIncludes /FC
@obj\content\browser\media\capture\content_browser.capture_resolution_chooser.obj.rsp
/c ..\..\content\browser\media\capture\capture_resolution_chooser.cc
/Foobj\content\browser\media\capture\content_browser.capture_resolution_chooser.obj
/Fdobj\content\content_browser.cc.pdb 
e:\b\build\slave\win\build\src\content\browser\media\capture\capture_resolution_chooser.cc(41)
: error C2220: warning treated as error - no 'object' file generated
e:\b\build\slave\win\build\src\content\browser\media\capture\capture_resolution_chooser.cc(41)
: warning C4715: 'content::`anonymous namespace'::ComputeMinimumCaptureSize' :
not all control paths return a value
ninja: build stopped: subcommand failed.

+}
+
+// Returns |size|, unless it exceeds |max_size| or is under |min_size|.  When
+// the bounds are exceeded, a uniformly scaled |size| is returned that is within

[Wez, 2015/05/13 19:05:08]

nit: Is "uniformly scaled" sufficiently well-understood to be clear here?

[miu, 2015/05/14 01:21:32]

Done.  Clarified.

+// the bounds.
+gfx::Size ComputeBoundedCaptureSize(const gfx::Size& size,
+                                    const gfx::Size& min_size,
+                                    const gfx::Size& max_size) {
+  if (size.width() > max_size.width() || size.height() > max_size.height()) {
+    gfx::Size result = media::ScaleSizeToFitWithinTarget(size, max_size);
+    result.SetToMax(min_size);
+    return result;
+  } else if (size.width() < min_size.width() ||
+             size.height() < min_size.height()) {
+    gfx::Size result = media::ScaleSizeToEncompassTarget(size, min_size);
+    result.SetToMin(max_size);
+    return result;
+  } else {
+    return size;
+  }
+}
+
+}  // nampspace
+
+CaptureResolutionEvaluator::CaptureResolutionEvaluator(
+    const gfx::Size& max_frame_size,
+    media::ResolutionChangePolicy resolution_change_policy)
+    : max_frame_size_(max_frame_size),
+      min_frame_size_(ComputeMinimumCaptureSize(max_frame_size,
+                                                resolution_change_policy)),
+      resolution_change_policy_(resolution_change_policy),
+      ideal_capture_size_(max_frame_size) {
+  DCHECK_LT(0, max_frame_size_.width());
+  DCHECK_LT(0, max_frame_size_.height());
+  DCHECK_LE(min_frame_size_.width(), max_frame_size_.width());
+  DCHECK_LE(min_frame_size_.height(), max_frame_size_.height());
+
+  UpdateCaptureSize();
+}
+
+CaptureResolutionEvaluator::~CaptureResolutionEvaluator() {}
+
+void CaptureResolutionEvaluator::UpdateForNewSourceSize(
+    const gfx::Size& source_size) {
+  if (source_size.IsEmpty())
+    return;
+
+  switch (resolution_change_policy_) {
+    case media::RESOLUTION_POLICY_FIXED_RESOLUTION:
+      // Source size changes do not affect the frame resolution.  Frame
+      // resolution is always fixed to |max_frame_size_|.
+      break;
+
+    case media::RESOLUTION_POLICY_FIXED_ASPECT_RATIO:
+      ideal_capture_size_ = ComputeBoundedCaptureSize(
+          media::PadToMatchAspectRatio(source_size, max_frame_size_),
+          min_frame_size_,
+          max_frame_size_);
+      UpdateCaptureSize();
+      break;
+
+    case media::RESOLUTION_POLICY_ANY_WITHIN_LIMIT:
+      ideal_capture_size_ = ComputeBoundedCaptureSize(
+          source_size, min_frame_size_, max_frame_size_);
+      UpdateCaptureSize();
+      break;
+
+    case media::RESOLUTION_POLICY_LAST:
+      NOTREACHED();
+  }
+}
+
+void CaptureResolutionEvaluator::UpdateForNewCapabilityLimit(int num_pixels) {

[Wez, 2015/05/13 19:05:08]

Is the rationale for including this in this CL rather than in the follow up CL
just that the calling code wants to call it even though it's not yet
implemented?

[miu, 2015/05/14 01:21:32]

No.  I decided to add it in as a skeleton method so that it's clear in this
change why I'm going to all the trouble of adding a whole separate class to
encapsulate the resolution computation logic.

[Wez, 2015/05/15 00:55:42]

OK; in that case I'd remove it and just add it in the CL where it gets
implemented - your CL description can explain that breaking out this class is
ground-work.

[miu, 2015/05/15 21:14:00]

Done.

+  NOTIMPLEMENTED();
+}
+
+void CaptureResolutionEvaluator::UpdateCaptureSize() {
+  // TODO(miu): An upcoming change will introduce the ability to find the best
+  // capture resolution, given the current capabilities of the system.

[Wez, 2015/05/13 19:05:08]

Bug # for that change? :)

[miu, 2015/05/14 01:21:31]

Done.

+  capture_size_ = ideal_capture_size_;
+}
+
+}  // namespace content