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

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

 /*
  * Copyright (C) 2011 Google Inc. All rights reserved.
  * Copyright (C) 2011 Ericsson AB. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
@@ skipping 198 matching lines @@
 MediaStreamTrack* MediaStreamTrack::clone(ScriptState* scriptState) {
   // TODO(pbos): Make sure m_readyState and m_stopped carries over on cloned
   // tracks.
   MediaStreamComponent* clonedComponent = component()->clone();
   MediaStreamTrack* clonedTrack = MediaStreamTrack::create(
       scriptState->getExecutionContext(), clonedComponent);
   MediaStreamCenter::instance().didCreateMediaStreamTrack(clonedComponent);
   return clonedTrack;
 }
 
-void MediaStreamTrack::getConstraints(MediaTrackConstraints& constraints) {
-  MediaConstraintsImpl::convertConstraints(m_constraints, constraints);
-}
-
 void MediaStreamTrack::setConstraints(const WebMediaConstraints& constraints) {
   m_constraints = constraints;
 }
 
 void MediaStreamTrack::getCapabilities(MediaTrackCapabilities& capabilities) {
   if (m_imageCapture)
     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())
       settings.setVideoKind(platformSettings.videoKind);
     if (platformSettings.hasDepthNear())
       settings.setDepthNear(platformSettings.depthNear);
     if (platformSettings.hasDepthFar())
       settings.setDepthFar(platformSettings.depthFar);
     if (platformSettings.hasFocalLengthX())
       settings.setFocalLengthX(platformSettings.focalLengthX);
@@ skipping 130 matching lines @@
 
 DEFINE_TRACE(MediaStreamTrack) {
   visitor->trace(m_registeredMediaStreams);
   visitor->trace(m_component);
   visitor->trace(m_imageCapture);
   EventTargetWithInlineData::trace(visitor);
   ContextLifecycleObserver::trace(visitor);
 }
 
 }  // namespace blink