New resolution change policies for desktop and tab capture.

content/renderer/media/media_stream_video_capturer_source.cc

Index: content/renderer/media/media_stream_video_capturer_source.cc
diff --git a/content/renderer/media/media_stream_video_capturer_source.cc b/content/renderer/media/media_stream_video_capturer_source.cc
index 1c905ccc2468cd857ce8688fa915f82fa4663a26..482d72abf794f6f2d402de0b21cf8d9c667784ed 100644
--- a/content/renderer/media/media_stream_video_capturer_source.cc
+++ b/content/renderer/media/media_stream_video_capturer_source.cc
@@ -7,11 +7,15 @@
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/location.h"
+#include "content/public/common/media_stream_request.h"
+#include "content/renderer/media/media_stream_constraints_util.h"
 #include "content/renderer/media/video_capture_impl_manager.h"
 #include "content/renderer/render_thread_impl.h"
 #include "media/base/bind_to_current_loop.h"
 #include "media/base/video_frame.h"
 
+namespace content {
+
 namespace {
 
 // Resolutions used if the source doesn't support capability enumeration.
@@ -32,9 +36,136 @@ const int kVideoFrameRates[] = {30, 60};
 // Hard upper-bound frame rate for tab/desktop capture.
 const double kMaxScreenCastFrameRate = 120.0;
 
-}  // namespace
+// Returns true if the value for width or height is reasonable.
+bool DimensionValueIsValid(int x) {
+  return x >= 2 &&  // Lower-bound: Minimum valid I420 frame resolution.
+      x <= 16384;   // Upper-bound: Common maximum frame size in encoders, etc.

[mcasas, 2015/05/07 22:10:00]

Shouldn't we use GetArea(), for minimum size, and [1] for maximum?

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/media/base/limits....

[miu, 2015/05/08 06:43:34]

Done.  I thought about this again, and I think anything greater than zero should
be acceptable (since the coded size of an I420 frame would round up).  I used
media/base/limits for both the max constraint.  Good suggestion.

+}
 
-namespace content {
+// Returns true if the value for frame rate is reasonable.
+bool FrameRateValueIsValid(double frame_rate) {
+  return (frame_rate > (1.0 / 60.0)) &&  // Lower-bound: One frame per minute.
+      (frame_rate <= 120);               // Upper-bound: 120 frames per second.

[mcasas, 2015/05/07 22:10:00]

[1]?

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/media/base/limits....

[miu, 2015/05/08 06:43:34]

Done.  media/base/limits.h didn't have a minimum frame rate.  For practical
purposes, one frame per minute should be acceptable for screen casting.  (Yes,
we do want to allow less than 1 FPS.  There are reasonable use cases for this
that I can discuss offline, if you're interested.)

+}
+
+// Returns true if the aspect ratio of |a| and |b| are equivalent to two
+// significant digits.
+bool AreNearlyEquivalentInAspectRatio(const gfx::Size& a, const gfx::Size& b) {
+  DCHECK(!a.IsEmpty());
+  DCHECK(!b.IsEmpty());
+  const int aspect_ratio_a = (100 * a.width()) / a.height();
+  const int aspect_ratio_b = (100 * b.width()) / b.height();
+  return aspect_ratio_a == aspect_ratio_b;
+}
+
+// Interprets the properties in |constraints| to override values in |params| and
+// determine the resolution change policy.
+void SetScreenCastParamsFromConstraints(
+    const blink::WebMediaConstraints& constraints,
+    MediaStreamType type,
+    media::VideoCaptureParams* params) {
+  // The default resolution change policies for tab versus desktop capture are
+  // the way they are for legacy reasons.
+  if (type == MEDIA_TAB_VIDEO_CAPTURE) {
+    params->resolution_change_policy =
+        media::RESOLUTION_POLICY_FIXED_RESOLUTION;
+  } else if (type == MEDIA_DESKTOP_VIDEO_CAPTURE) {
+    params->resolution_change_policy =
+        media::RESOLUTION_POLICY_ANY_WITHIN_LIMIT;
+  } else {
+    NOTREACHED();
+  }
+
+  // If the maximum frame resolution was provided in the constraints, use it if
+  // either: 1) none has been set yet; or 2) the maximum specificed is smaller
+  // than the current setting.
+  int width = 0;
+  int height = 0;
+  gfx::Size desired_max_frame_size;
+  if (GetConstraintValueAsInteger(constraints,
+                                  MediaStreamVideoSource::kMaxWidth,
+                                  &width) &&
+      GetConstraintValueAsInteger(constraints,
+                                  MediaStreamVideoSource::kMaxHeight,
+                                  &height) &&
+      DimensionValueIsValid(width) &&
+      DimensionValueIsValid(height)) {
+    desired_max_frame_size.SetSize(width, height);
+    if (params->requested_format.frame_size.IsEmpty() ||
+        desired_max_frame_size.width() <
+            params->requested_format.frame_size.width() ||
+        desired_max_frame_size.height() <
+            params->requested_format.frame_size.height()) {
+      params->requested_format.frame_size = desired_max_frame_size;
+    }
+  }
+
+  // Set the default frame resolution if none was provided.
+  if (params->requested_format.frame_size.IsEmpty()) {
+    params->requested_format.frame_size.SetSize(
+        MediaStreamVideoSource::kDefaultWidth,
+        MediaStreamVideoSource::kDefaultHeight);
+  }
+
+  // If the maximum frame rate was provided, use it if either: 1) none has been
+  // set yet; or 2) the maximum specificed is smaller than the current setting.
+  double frame_rate = 0.0;
+  if (GetConstraintValueAsDouble(constraints,
+                                 MediaStreamVideoSource::kMaxFrameRate,
+                                 &frame_rate) &&
+      FrameRateValueIsValid(frame_rate)) {
+    if (params->requested_format.frame_rate <= 0.0f ||
+        frame_rate < params->requested_format.frame_rate) {
+      params->requested_format.frame_rate = frame_rate;
+    }
+  }
+
+  // Set the default frame rate if none was provided.
+  if (params->requested_format.frame_rate <= 0.0f) {
+    params->requested_format.frame_rate =
+        MediaStreamVideoSource::kDefaultFrameRate;
+  }
+
+  // If the minimum frame resolution was provided, compare it to the maximum
+  // frame resolution to determine the intended resolution change policy.
+  if (!desired_max_frame_size.IsEmpty() &&
+      GetConstraintValueAsInteger(constraints,
+                                  MediaStreamVideoSource::kMinWidth,
+                                  &width) &&
+      GetConstraintValueAsInteger(constraints,
+                                  MediaStreamVideoSource::kMinHeight,
+                                  &height) &&
+      width <= desired_max_frame_size.width() &&
+      height <= desired_max_frame_size.height()) {
+    if (width == desired_max_frame_size.width() &&
+        height == desired_max_frame_size.height()) {
+      // Constraints explicitly require a single frame resolution.
+      params->resolution_change_policy =
+          media::RESOLUTION_POLICY_FIXED_RESOLUTION;
+    } else if (DimensionValueIsValid(width) &&
+               DimensionValueIsValid(height) &&

[mcasas, 2015/05/07 22:10:00]

Ni: shouldn't l.146-147 be justified @8th column (i.e. four blanks right of the
} )?

[miu, 2015/05/08 06:43:34]

I don't quite understand, and the auto-indent feature in my editor didn't reveal
any issues.  Can you show me an example of how you think this should be
indented?

[mcasas, 2015/05/08 18:38:41]

I thought it should be
} else if (DimensionValueIsValid(width) &&
    DimensionValueIsValid(height) &&

but if the clang-format says it's fine it probably is.

+               AreNearlyEquivalentInAspectRatio(gfx::Size(width, height),
+                                                desired_max_frame_size)) {
+      // Constraints only mention a single aspect ratio.
+      params->resolution_change_policy =
+          media::RESOLUTION_POLICY_FIXED_ASPECT_RATIO;
+    } else {
+      // Constraints specify a minimum resolution that is smaller than the
+      // maximum resolution and has a different aspect ratio (possibly even
+      // 0x0). This indicates any frame resolution and aspect ratio is
+      // acceptable.
+      params->resolution_change_policy =
+          media::RESOLUTION_POLICY_ANY_WITHIN_LIMIT;
+    }
+  }
+
+  DVLOG(1) << "SetScreenCastParamsFromConstraints: "
+           << params->requested_format.ToString()
+           << " with resolution change policy "
+           << params->resolution_change_policy;
+}
+
+}  // namespace
 
 VideoCapturerDelegate::VideoCapturerDelegate(
     const StreamDeviceInfo& device_info)
@@ -246,14 +377,15 @@ void MediaStreamVideoCapturerSource::GetCurrentSupportedFormats(
 
 void MediaStreamVideoCapturerSource::StartSourceImpl(
     const media::VideoCaptureFormat& format,
+    const blink::WebMediaConstraints& constraints,
     const VideoCaptureDeliverFrameCB& frame_callback) {
   media::VideoCaptureParams new_params;
   new_params.requested_format = format;
   if (device_info().device.type == MEDIA_TAB_VIDEO_CAPTURE ||
       device_info().device.type == MEDIA_DESKTOP_VIDEO_CAPTURE) {
-    new_params.resolution_change_policy =
-        media::RESOLUTION_POLICY_DYNAMIC_WITHIN_LIMIT;
-  }
+    SetScreenCastParamsFromConstraints(
+        constraints, device_info().device.type, &new_params);
+    }

[mcasas, 2015/05/07 22:10:00]

Justiy two columns leftwards.

[miu, 2015/05/08 06:43:34]

Done.  Good catch.

   delegate_->StartCapture(
       new_params,
       frame_callback,