Image Capture: cache constraints and append them to MediaStreamTrack.getConstraints()

third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.cpp

Index: third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.cpp
diff --git a/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.cpp b/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.cpp
index a93e097575f7b153eadf2a430bede2cdb5fa05ec..495918c8147950cc8b939c48e51e1e17a5bbdacc 100644
--- a/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.cpp
+++ b/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.cpp
@@ -216,10 +216,6 @@ MediaStreamTrack* MediaStreamTrack::clone(ScriptState* scriptState) {
   return clonedTrack;
 }
 
-void MediaStreamTrack::getConstraints(MediaTrackConstraints& constraints) {
-  MediaConstraintsImpl::convertConstraints(m_constraints, constraints);
-}
-
 void MediaStreamTrack::setConstraints(const WebMediaConstraints& constraints) {
   m_constraints = constraints;
 }
@@ -229,18 +225,42 @@ void MediaStreamTrack::getCapabilities(MediaTrackCapabilities& capabilities) {
     capabilities = m_imageCapture->getMediaTrackCapabilities();
 }
 
+void MediaStreamTrack::getConstraints(MediaTrackConstraints& constraints) {
+  MediaConstraintsImpl::convertConstraints(m_constraints, constraints);
+
+  if (!m_imageCapture)
+    return;
+  HeapVector<MediaTrackConstraintSet> vector;
+  if (constraints.hasAdvanced())

[Guido Urdaneta, 2017/03/19 13:53:12]

Shouldn't you support basic constraints as well?
If you support advanced, why not multiple advanced?
If this is correct, comment why.

What if you update MediaConstraintsImpl::convertConstraints to support the new
imageCaptureConstraints?

[mcasas, 2017/03/19 20:37:40]

IIUC all ImageCapture constraints go under the 
advanced section; happy to add a comment here
as to why if this assumption is correct.
That method translates a WebMediaConstraints into
MediaConstraints, and it's used to interact with the
content/renderer implementation of the constraints;
image capture goes directly to content/browser and
its devices, and doesn't use WebX versions.

[Guido Urdaneta, 2017/03/20 09:17:35]

I see that the code in the previous CL just passes advanced[0] to be processed.
I think this is unusual and not necessarily what the users would expect, but
since it's still behind a flag and we're still working on making spec-compliant
constraints work I guess it's OK for now. Add a comment explaining that.
content/renderer uses WebMediaConstraints, but the whole implementation is in
Blink. You can add this conversion there, but since for now you are supporting
only the first advanced set, it's OK to leave as is for now, with a TODO to move
the code to MediaConstraintsImpl.

[mcasas, 2017/03/20 16:45:56]

Correct, but note that the use of advanced[0] in applyConstraints()
and the hasAdvanced() here have nothing to do: in applyConstraints()
it's a limitation, whereas here it simply is a way to append 
 |imageCaptureConstraints| to |constraints|.
Added TODO. Usually WebX types are used for interchanges
to and from content/renderer, so if the idea of 
MediaConstraintsImpl is to centralise the constraints 
management using WebX-only naming would be inaccurate,
I'm assuming that class is still WIP and that's fine.

+    vector = constraints.advanced();
+  auto imageCaptureConstraints = m_imageCapture->getMediaTrackConstraints();
+  // TODO(mcasas): add |torch|, https://crbug.com/700607.
+  if (imageCaptureConstraints.hasWhiteBalanceMode() ||
+      imageCaptureConstraints.hasExposureMode() ||
+      imageCaptureConstraints.hasFocusMode() ||
+      imageCaptureConstraints.hasExposureCompensation() ||
+      imageCaptureConstraints.hasColorTemperature() ||
+      imageCaptureConstraints.hasIso() ||
+      imageCaptureConstraints.hasBrightness() ||
+      imageCaptureConstraints.hasContrast() ||
+      imageCaptureConstraints.hasSaturation() ||
+      imageCaptureConstraints.hasSharpness() ||
+      imageCaptureConstraints.hasZoom()) {
+    // Add image capture constraints, if any, as another entry to advanced().
+    vector.emplace_back(imageCaptureConstraints);
+    constraints.setAdvanced(vector);
+  }
+}
+
 void MediaStreamTrack::getSettings(MediaTrackSettings& settings) {
   WebMediaStreamTrack::Settings platformSettings;
   m_component->getSettings(platformSettings);
-  if (platformSettings.hasFrameRate()) {
+  if (platformSettings.hasFrameRate())
     settings.setFrameRate(platformSettings.frameRate);
-  }
-  if (platformSettings.hasWidth()) {
+  if (platformSettings.hasWidth())
     settings.setWidth(platformSettings.width);
-  }
-  if (platformSettings.hasHeight()) {
+  if (platformSettings.hasHeight())
     settings.setHeight(platformSettings.height);
-  }
   if (RuntimeEnabledFeatures::mediaCaptureDepthEnabled() &&
       m_component->source()->type() == MediaStreamSource::TypeVideo) {
     if (platformSettings.hasVideoKind())