Introduce SelectSettings algorithm for MediaStream video tracks.

content/renderer/media/media_stream_constraints_util.h

Index: content/renderer/media/media_stream_constraints_util.h
diff --git a/content/renderer/media/media_stream_constraints_util.h b/content/renderer/media/media_stream_constraints_util.h
index a3f75a6e65bf3a19fd1fdeb5b6d9b99d141f7ae0..d92d82a444e632fc4fe7e1e042e55bb6204120f1 100644
--- a/content/renderer/media/media_stream_constraints_util.h
+++ b/content/renderer/media/media_stream_constraints_util.h
@@ -7,12 +7,109 @@
 
 #include <string>
 
+#include "base/logging.h"
 #include "content/common/content_export.h"
+#include "content/common/media/media_devices.mojom.h"
+#include "content/renderer/media/video_track_adapter.h"
+#include "media/capture/video_capture_types.h"
 #include "third_party/WebKit/public/platform/WebMediaConstraints.h"
 #include "third_party/webrtc/base/optional.h"
 
 namespace content {
 
+class ResolutionSet;
+template <typename T>
+class NumericRangeSet;
+
+class CONTENT_EXPORT VideoCaptureSettings {
+ public:
+  // Creates a result without value and with an empty failed constraint name.
+  VideoCaptureSettings();
+
+  // Creates a result without value and with the given |failed_constraint_name|.
+  // Does not take ownership of |failed_constraint_name|, so it must be null or
+  // point to a string that remains accessible.
+  explicit VideoCaptureSettings(const char* failed_constraint_name);
+
+  // Creates a result with the given values.
+  VideoCaptureSettings(std::string device_id,
+                       ::mojom::FacingMode facing_mode_,
+                       media::VideoCaptureParams capture_params_,
+                       base::Optional<bool> noise_reduction_,
+                       const VideoTrackAdapterSettings& track_adapter_settings,
+                       double min_frame_rate);
+
+  VideoCaptureSettings(const VideoCaptureSettings& other);
+  VideoCaptureSettings& operator=(const VideoCaptureSettings& other);
+  VideoCaptureSettings(VideoCaptureSettings&& other);
+  VideoCaptureSettings& operator=(VideoCaptureSettings&& other);
+  ~VideoCaptureSettings();
+
+  bool HasValue() const { return failed_constraint_name_ == nullptr; }
+
+  // Convenience accessors for fields embedded in |capture_params_|.
+  const media::VideoCaptureFormat& Format() const {
+    return capture_params_.requested_format;
+  }
+  int Width() const {
+    DCHECK(HasValue());
+    return capture_params_.requested_format.frame_size.width();
+  }
+  int Height() const {
+    DCHECK(HasValue());
+    return capture_params_.requested_format.frame_size.height();
+  }
+  float FrameRate() const {
+    DCHECK(HasValue());
+    return capture_params_.requested_format.frame_rate;
+  }
+  media::ResolutionChangePolicy ResolutionChangePolicy() const {
+    DCHECK(HasValue());
+    return capture_params_.resolution_change_policy;
+  }
+  media::PowerLineFrequency PowerLineFrequency() const {
+    DCHECK(HasValue());
+    return capture_params_.power_line_frequency;
+  }
+
+  // Other accessors.
+  const char* failed_constraint_name() const { return failed_constraint_name_; }
+
+  const std::string& device_id() const {
+    DCHECK(HasValue());
+    return device_id_;
+  }
+  ::mojom::FacingMode facing_mode() const {
+    DCHECK(HasValue());
+    return facing_mode_;
+  }
+  const media::VideoCaptureParams& capture_params() const {
+    DCHECK(HasValue());
+    return capture_params_;
+  }
+  const base::Optional<bool>& noise_reduction() const {
+    DCHECK(HasValue());
+    return noise_reduction_;
+  }
+  const VideoTrackAdapterSettings& track_adapter_settings() const {
+    DCHECK(HasValue());
+    return track_adapter_settings_;
+  }
+  double min_frame_rate() const {
+    DCHECK(HasValue());
+    return min_frame_rate_;
+  }
+
+ private:
+  const char* failed_constraint_name_;
+  std::string device_id_;
+  ::mojom::FacingMode facing_mode_;
+  media::VideoCaptureParams capture_params_;
+  base::Optional<bool> noise_reduction_;
+  VideoTrackAdapterSettings track_adapter_settings_;

[hbos_chromium, 2017/03/28 12:52:53]

VideoCaptureParams has w,h,fps.
VideoTrackAdapterSettings has max w,h,fps and min/max ratios.
Then there are ResolutionSets and stuff from other CLs.

I find it a little confusing to have several different classes for resolutions
etc.

Can you add a comment to say what this class (VideoCaptureSettings) is used for,
what its usage is compared to its component classes / why it has both a "value"
(capture_params_) and a "range" (track_adapter_settings_).

I also wonder if it makes sense to rename any of these classes or substitute
VideoTrackAdapterSettings with something more generic and descriptive like
"ResolutionRange" or something similar.

[Guido Urdaneta, 2017/03/28 18:35:43]

It can be quite confusing. Added a detailed comment that hopefully clarifies
things.
The general approach is that SelectSettings is a function that provides a
mapping from constraints to input values for other Chromium components.
Those input values are grouped in VideoCaptureSettings. VideoCaptureSettings is
not intended to make much sense on its own, but in the context of having values
that are inputs to other existing classes that are not (or should not be) aware
of constraints. For example, VideoTrackAdapterSettings is for VideoTrackAdapter,
which all tracks need,
VideoCaptureParams is an existing class to configure the video capturer, the
3-state noise_reduction is an input for MediaStreamVideoWebRtcSink, and so on.

Other classes related to SelectSettings such as ResolutionSet and similar are
just internal implementation details of SelectSettings.
Done. As explained above, VideoCaptureSettings is intended to make it easier to
configure other existing Chromium components.
I don't think it does.
Those values are intended as input to other classes, and their (sometimes
existing) naming reflects that.

BTW, removed |facing_mode| from VideoCaptureSettings. It was originally going to
be used for getSettings, but that was implemented in a different way already.

[hbos_chromium, 2017/03/29 14:54:53]

Acknowledged.

+  double min_frame_rate_;
+};
+
 // Method to get boolean value of constraint with |name| from constraints.
 // Returns true if the constraint is specified in either mandatory or optional
 // constraints.
@@ -93,6 +190,34 @@ auto ConstraintMin(const ConstraintType& constraint)
   return constraint.hasExact() ? constraint.exact() : constraint.min();
 }
 
+// This function selects track settings from a set of candidate resolutions and
+// frame rates, given the source video-capture format and ideal values.
+// The output are settings for a VideoTrackAdapter, which can adjust the
+// resolution and frame rate of the source, and consist of maximum (or
+// target) width, height and frame rate, and minimum and maximum aspect ratio.
+// * Minimum and maximum aspect ratios are taken from |resolution_set| and are
+//   not affected by ideal values.
+// * The selected frame rate is always the value within the |frame_rate_set|
+//   range that is closest to the ideal frame rate (or the source frame rate
+//   if no ideal is supplied). If the chosen frame rate is greater than or equal
+//   to the source's frame rate, a value of 0.0 is returned, which means that
+//   there will be no frame-rate adjustment.

[hbos_chromium, 2017/03/28 12:52:53]

Should Optional be used instead of 0.0?

[Guido Urdaneta, 2017/03/28 18:35:43]

No. It is the way it is because that is how VideoTrackAdapter interprets its
input.
Otherwise we would need a conversion layer.

[hbos_chromium, 2017/03/29 14:54:53]

Acknowledged.

+// * Width and height are selected using the
+//   ResolutionSet::SelectClosestPointToIdeal function, using ideal values from
+//   |basic_constraint_set| and using the source's width and height as the
+//   default resolution. The width and height returned by
+//   SelectClosestPointToIdeal are rounded to the nearest int. For more details,
+//   see the documentation for ResolutionSet::SelectClosestPointToIdeal.
+// Note that this function ignores the min/max/exact values from
+// |basic_constraint_set|. Only its ideal values are used.
+// This function has undefined behavior if any of |resolution_set| or
+// |frame_rate_set| are empty.

[hbos_chromium, 2017/03/28 12:52:53]

Undefined behavior? Why not DCHECK that they aren't empty?

[Guido Urdaneta, 2017/03/28 18:35:43]

They are DCHECKed, and this is a way to document it.
DCHECK is defined in debug mode (crash), but since it doesn't run in release
mode, all bets are off as to what this function does in that case.

[hbos_chromium, 2017/03/29 14:54:53]

Acknowledged.

+VideoTrackAdapterSettings CONTENT_EXPORT SelectVideoTrackAdapterSettings(

[hbos_chromium, 2017/03/28 12:52:53]

Why do we express the desired result settings as a maximum and not an actual
value?

Can multiple VideoTrackAdapterSettings be applied on the same track to give it
the s.max_blah = min(s1.max_blah, s2.max_blah)?

[Guido Urdaneta, 2017/03/28 18:35:43]

Because that's the way VideoTrackAdapter works. 
I general, for this work, I prefer to minimize changes to VideoTrackAdapter and
other classes that have nothing to do with constraints.
VTA interpretation of its max resolution has nothing to do with the max value in
constraints.

VTA, once configured, works by transforming input frames into frames that are no
larger than its (max_width, max_height). If the input frames are smaller than
its max, VTA leaves them alone.

We configure the source to not provide smaller input frames, but VTA doesn't
know that, and IMO that's OK.

[hbos_chromium, 2017/03/29 14:54:53]

Acknowledged.

+    const blink::WebMediaTrackConstraintSet& basic_constraint_set,
+    const ResolutionSet& resolution_set,
+    const NumericRangeSet<double>& frame_rate_set,
+    const media::VideoCaptureFormat& source_format);
+
 }  // namespace content
 
 #endif  // CONTENT_RENDERER_MEDIA_MEDIA_STREAM_CONSTRAINTS_UTIL_H_