Fix fitness distance in video capture constraints processing

The old algorithm computed fitness for width, height and aspect ratio
using the best possible width, height and aspect ratio supported
by a constrained format. However, the actual selected resolution does not
necessarily have the width, height and aspect ratio that are
simultaneously closest to the ideal values.

In practice, this results in differences only in marginal cases such as:
* Extreme ideal aspect ratios, where rounding a selected resolution
in floating-point format might lead to a significant variation in
aspect ratio. For example (1000x1.25 has aspect ratio 800 before rounding
but aspect ratio 1000 after rounding to 1000x1). This is not an issue
for resolutions that are used in practice and the spec allows some
deviation in the optimization of the fitness distance anyway.
* The ideal aspect ratio significantly contradicts the ratio between
the ideal width and the ideal height. In this case, the selected point
will be close to ideal width and ideal height, but might be far from
the ideal aspect ratio (which is ignored), thus it might have a higher
fitness distance than the one computed by the old algorithm.

This minor fix has more importance when extra complexity is added to
the computation of fitness distance to support the new resizeMode
constraint in a follow-up CL.

Bug: 854980
Change-Id: I0cadb28553816357ce84bbf38b5686cc48b427e7
Reviewed-on: https://chromium-review.googlesource.com/c/1329788Reviewed-by: Henrik Boström <hbos@chromium.org>
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610015}

content/renderer/media/stream/media_stream_constraints_util_video_device.cc

@@ -51,18 +51,11 @@ blink::WebString ToWebString(media::VideoFacingMode facing_mode) {
 // closest value to it in the range [min, max].
 // Based on https://w3c.github.io/mediacapture-main/#dfn-fitness-distance.
 template <typename NumericConstraint>
-double NumericRangeFitness(const NumericConstraint& constraint,
-                           decltype(constraint.Min()) min,
-                           decltype(constraint.Min()) max) {
-  if (!constraint.HasIdeal())
-    return 0.0;
-
-  if (constraint.Ideal() > max)
-    return NumericConstraintFitnessDistance(max, constraint.Ideal());
-  if (constraint.Ideal() < min)
-    return NumericConstraintFitnessDistance(min, constraint.Ideal());
-
-  return 0.0;
+double NumericValueFitness(const NumericConstraint& constraint,
+                           decltype(constraint.Min()) value) {
+  return constraint.HasIdeal()
+             ? NumericConstraintFitnessDistance(value, constraint.Ideal())
+             : 0.0;
 }
 
 // Returns a custom distance between |native_value| and the ideal value and
@@ -223,22 +216,34 @@ class CandidateFormat {
     return true;
   }
 
-  // Returns the fitness distance for this candidate format.
-  // Since a format can support multiple track settings, this function returns
-  // the best fitness that can be achieved with this format.
-  // The fitness function is based on
+  // Returns the best fitness distance that can be achieved with this candidate
+  // format based on distance from the ideal values in |basic_constraint_set|.
+  // The track settings that correspond to this fitness are returned on the
+  // |track_settings| output parameter. The fitness function is based on
   // https://w3c.github.io/mediacapture-main/#dfn-fitness-distance.
-  double Fitness(
-      const blink::WebMediaTrackConstraintSet& constraint_set) const {
-    return NumericRangeFitness(constraint_set.aspect_ratio, MinAspectRatio(),
-                               MaxAspectRatio()) +
-           NumericRangeFitness(constraint_set.height, MinHeight(),
-                               MaxHeight()) +
-           NumericRangeFitness(constraint_set.width, MinWidth(), MaxWidth()) +
-           NumericRangeFitness(constraint_set.frame_rate, MinFrameRate(),
-                               MaxFrameRate()) +
+  double Fitness(const blink::WebMediaTrackConstraintSet& basic_constraint_set,
+                 VideoTrackAdapterSettings* track_settings) const {
+    *track_settings = SelectVideoTrackAdapterSettings(
+        basic_constraint_set, resolution_set(), constrained_frame_rate(),
+        format(), true /* enable_rescale */);
+
+    double target_aspect_ratio =
+        static_cast<double>(track_settings->target_width()) /
+        track_settings->target_height();
+    double target_frame_rate = track_settings->max_frame_rate();
+    if (target_frame_rate == 0.0)
+      target_frame_rate = NativeFrameRate();
+    DCHECK(!std::isnan(target_aspect_ratio));
+    return NumericValueFitness(basic_constraint_set.aspect_ratio,
+                               target_aspect_ratio) +
+           NumericValueFitness(basic_constraint_set.height,
+                               track_settings->target_height()) +
+           NumericValueFitness(basic_constraint_set.width,
+                               track_settings->target_width()) +
+           NumericValueFitness(basic_constraint_set.frame_rate,
+                               target_frame_rate) +
            StringConstraintFitnessDistance(VideoKind(),
-                                           constraint_set.video_kind);
+                                           basic_constraint_set.video_kind);
   }
 
   // Returns a custom "native" fitness distance that expresses how close the
@@ -367,13 +372,15 @@ double DeviceFitness(const DeviceInfo& device,
 // Returns the fitness distance between |constraint_set| and |candidate| given
 // that the configuration is already constrained by |candidate_format|.
 // Based on https://w3c.github.io/mediacapture-main/#dfn-fitness-distance.
-double CandidateFitness(
-    const DeviceInfo& device,
+// The track settings for |candidate| that correspond to the returned fitness
+// are returned in |track_settings|.
+double CandidateFitness(const DeviceInfo& device,
                         const CandidateFormat& candidate_format,
                         const base::Optional<bool>& noise_reduction,
-    const blink::WebMediaTrackConstraintSet& constraint_set) {
+                        const blink::WebMediaTrackConstraintSet& constraint_set,
+                        VideoTrackAdapterSettings* track_settings) {
   return DeviceFitness(device, constraint_set) +
-         candidate_format.Fitness(constraint_set) +
+         candidate_format.Fitness(constraint_set, track_settings) +
          OptionalBoolFitness(noise_reduction,
                              constraint_set.goog_noise_reduction);
 }
@@ -419,24 +426,6 @@ void AppendDistancesFromDefault(
   distance_vector->push_back(frame_rate_distance);
 }
 
-VideoCaptureSettings ComputeVideoDeviceCaptureSettings(
-    const DeviceInfo& device,
-    const base::Optional<bool>& noise_reduction,
-    const CandidateFormat& candidate_format,
-    const blink::WebMediaTrackConstraintSet& basic_constraint_set) {
-  media::VideoCaptureParams capture_params;
-  capture_params.requested_format = candidate_format.format();
-  auto track_adapter_settings = SelectVideoTrackAdapterSettings(
-      basic_constraint_set, candidate_format.resolution_set(),
-      candidate_format.constrained_frame_rate(),
-      capture_params.requested_format, true /* enable_rescale */);
-
-  return VideoCaptureSettings(device->device_id, capture_params,
-                              noise_reduction, track_adapter_settings,
-                              candidate_format.constrained_frame_rate().Min(),
-                              candidate_format.constrained_frame_rate().Max());
-}
-
 }  // namespace
 
 blink::WebString GetVideoKindForFormat(
@@ -565,9 +554,11 @@ VideoCaptureSettings SelectSettingsVideoDeviceCapture(
               satisfies_advanced_set ? 0 : HUGE_VAL);
         }
 
+        VideoTrackAdapterSettings track_settings;
         // Second criterion is fitness distance.
-        candidate_distance_vector.push_back(CandidateFitness(
-            device, candidate_format, noise_reduction, constraints.Basic()));
+        candidate_distance_vector.push_back(
+            CandidateFitness(device, candidate_format, noise_reduction,
+                             constraints.Basic(), &track_settings));
 
         // Third criterion is native fitness distance.
         candidate_distance_vector.push_back(
@@ -582,8 +573,13 @@ VideoCaptureSettings SelectSettingsVideoDeviceCapture(
         DCHECK_EQ(best_distance.size(), candidate_distance_vector.size());
         if (candidate_distance_vector < best_distance) {
           best_distance = candidate_distance_vector;
-          result = ComputeVideoDeviceCaptureSettings(
-              device, noise_reduction, candidate_format, constraints.Basic());
+
+          media::VideoCaptureParams capture_params;
+          capture_params.requested_format = candidate_format.format();
+          result = VideoCaptureSettings(
+              device->device_id, capture_params, noise_reduction,
+              track_settings, candidate_format.constrained_frame_rate().Min(),
+              candidate_format.constrained_frame_rate().Max());
         }
       }
     }

content/renderer/media/stream/media_stream_constraints_util_video_device_unittest.cc

@@ -1578,13 +1578,16 @@ TEST_F(MediaStreamConstraintsUtilVideoDeviceTest, IdealAspectRatio) {
     // The only device that supports the ideal aspect ratio is the high-res
     // device.
     EXPECT_EQ(high_res_device_->device_id, result.device_id());
-    EXPECT_EQ(1920, result.Width());
-    EXPECT_EQ(1080, result.Height());
+    EXPECT_EQ(1280, result.Width());
+    EXPECT_EQ(720, result.Height());
     // The most exact way to support the ideal aspect ratio would be to crop to
-    // 1500x1. However, the algorithm tries to crop to 1920x1.28 and rounds.
+    // 1920x1080 to 1500x1. However, with 1920x1080 the algorithm tries to crop
+    // to 1920x1.28 and rounds to 1920x1. Since the aspect ratio of 1280x1 is
+    // closer to ideal than 1920x1, 1280x1 is selected instead.
     // In this case, the effect of rounding is noticeable because of the
-    // resulting low value for height. For more typical resolution values,
-    // the at-most 1-pixel error caused by rounding is not an issue.
+    // resulting low value for height. For more typical aspect-ratio values,
+    // the 1-pixel error caused by rounding one dimension does not translate to
+    // a absolute error on the other dimension.
     EXPECT_EQ(std::round(result.Width() / kIdealAspectRatio),
               result.track_adapter_settings().target_height());
     EXPECT_EQ(result.Width(), result.track_adapter_settings().target_width());
@@ -1600,10 +1603,14 @@ TEST_F(MediaStreamConstraintsUtilVideoDeviceTest, IdealAspectRatio) {
     constraint_factory_.basic().aspect_ratio.SetIdeal(kIdealAspectRatio);
     auto result = SelectSettings();
     EXPECT_TRUE(result.HasValue());
-    // The only device that supports the ideal aspect ratio is the high-res
-    // device with its highest resolution.
+    // The best way to support this ideal aspect ratio would be to rescale
+    // 2304x1536 to 2000x1, but the algorithm would try to rescale to 2304x1.15
+    // and then round. Since 1920x1 has an aspect ratio closer to 2000, it is
+    // selected over 2304x1. The only device that supports this resolution is
+    // the high-res device open at 1920x1080.
     EXPECT_EQ(high_res_device_->device_id, result.device_id());
-    EXPECT_EQ(*high_res_highest_format_, result.Format());
+    EXPECT_EQ(1920, result.Width());
+    EXPECT_EQ(1080, result.Height());
     EXPECT_EQ(std::round(result.Width() / kIdealAspectRatio),
               result.track_adapter_settings().target_height());
     EXPECT_EQ(result.Width(), result.track_adapter_settings().target_width());