New issue
Advanced search Search tips

Issue 700607 link

Starred by 3 users
Status: Fixed
Owner:
mcasas@chromium.org
Closed: Apr 2017
Cc:
guidou@chromium.org
reillyg@chromium.org
mcasas@chromium.org
haraken@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Feature

Blocked on:
issue 543997
Blocking:
issue 708723
issue 518807



Sign in to add a comment

Migrate existing ImageCapture implementation to use Constrainable Pattern

Project Member Reported by mcasas@chromium.org, Mar 11 2017 
The use of Constrainable Pattern API in the Spec landed in [1].


[1] https://github.com/w3c/mediacapture-image/pull/150
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 15 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/30a67395e8cfccfd5954b9244440a7906d065563

commit 30a67395e8cfccfd5954b9244440a7906d065563
Author: mcasas <mcasas@chromium.org>
Date: Wed Mar 15 18:50:11 2017

Image Capture: MediaStreamTrack::getCapabilities()

This CL adds MediaTrackCapabilities.idl and
MediaStreamTrack::getCapabilities(). The latter function is wired
to grab the capabilities from image capture, since these are
the only ones available ATM.  These capabilities should only
be collected for Video tracks from devices (i.e. not for
tab/window/cast/remote etc).

MediaStreamTrack.getCapabilities() is synchronous, whereas
image_capture mojom::getPhotoCapabities() is not -- the former
probably doesn't make a lot of sense Spec-wise but, regardless,
I don't think is a good idea to make the Web thread wait for
these capabilities to be collected, so what this CL does is
to trigger such collection upon MediaStreamTrack ctor, without
waiting for this process to be finished. (IOW I tried alternatives
and were much worse).

Added LayoutTests for starters.

This bug is part of a transition from
 ImageCapture.getPhotoCapabilities() to MediaStreamTrack.getCapabilities()
but, since Image Capture is already in Origin Trials, I'm
planning on landing the new infra in parallel, then send a PSA
and switch over (that's why there's a few TODOs) in the code.

BUG= 700607 

Incidentally, it improves the state of
external/wpt/mediacapture-streams/MediaStreamTrack-init.https.html
by allowing the test-for-getCapabilities() to PASS.

Review-Url: https://codereview.chromium.org/2747573002
Cr-Commit-Position: refs/heads/master@{#457150}

[modify] https://crrev.com/30a67395e8cfccfd5954b9244440a7906d065563/content/browser/renderer_host/media/media_stream_manager.cc
[modify] https://crrev.com/30a67395e8cfccfd5954b9244440a7906d065563/third_party/WebKit/LayoutTests/external/wpt/mediacapture-streams/MediaStreamTrack-init.https-expected.txt
[add] https://crrev.com/30a67395e8cfccfd5954b9244440a7906d065563/third_party/WebKit/LayoutTests/fast/imagecapture/MediaStreamTrack-getCapabilities.html
[add] https://crrev.com/30a67395e8cfccfd5954b9244440a7906d065563/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-getCapabilities.html
[modify] https://crrev.com/30a67395e8cfccfd5954b9244440a7906d065563/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/30a67395e8cfccfd5954b9244440a7906d065563/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp
[modify] https://crrev.com/30a67395e8cfccfd5954b9244440a7906d065563/third_party/WebKit/Source/modules/imagecapture/ImageCapture.h
[modify] https://crrev.com/30a67395e8cfccfd5954b9244440a7906d065563/third_party/WebKit/Source/modules/imagecapture/MediaSettingsRange.h
[modify] https://crrev.com/30a67395e8cfccfd5954b9244440a7906d065563/third_party/WebKit/Source/modules/mediastream/DEPS
[modify] https://crrev.com/30a67395e8cfccfd5954b9244440a7906d065563/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.cpp
[modify] https://crrev.com/30a67395e8cfccfd5954b9244440a7906d065563/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.h
[modify] https://crrev.com/30a67395e8cfccfd5954b9244440a7906d065563/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.idl
[add] https://crrev.com/30a67395e8cfccfd5954b9244440a7906d065563/third_party/WebKit/Source/modules/mediastream/MediaTrackCapabilities.idl
[modify] https://crrev.com/30a67395e8cfccfd5954b9244440a7906d065563/third_party/WebKit/Source/modules/modules_idl_files.gni

Comment 3 by mcasas@chromium.org, Mar 16 2017 

https://codereview.chromium.org/2753513007/ was landed against this
bug as well -- it didn't show up here :?

Comment 4 by mcasas@chromium.org, Mar 16 2017

Cc: haraken@chromium.org reillyg@chromium.org
Before it gets lost, I'd like to bring here haraken@s comment in [1]
about the constructions like [2]

  caps->setIso(MediaSettingsRange::create(std::move(capabilities->iso)));

|caps| is a PhotoCapabilities (idl interface [3]) that has a
bunch of MediaSettingsRanges (another idl interface [4]) inside,
so the creation of a garbage-collected MSRange to be plugged
in a garbage-collected PhotoCapabilities is needed and cannot
be avoided either by mojo-mapping MediaSettingsRange to a similar
move-only mojo:: struct called Range [5] or by mojo-mapping the
latter to another struct inside MediaSettingsRange.  (I've been 
trying to do that as well).

I'm afraid the legit concern of "we're creating too many
garbage collected objects" in the PhotoCapabilities-MediaSettingsRange
is due to the idl definition.  Any ideas?




[1] https://codereview.chromium.org/2747573002#msg20
[2] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp?q=imagecapture.cpp&sq=package:chromium&dr&l=324
[3] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/imagecapture/PhotoCapabilities.idl?sq=package:chromium&dr
[4] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/imagecapture/MediaSettingsRange.idl?sq=package:chromium&dr
[5] https://cs.chromium.org/chromium/src/media/capture/mojo/image_capture.mojom?sq=package:chromium&dr&q=image_capture&l=10

Comment 5 by haraken@chromium.org, Mar 17 2017 

Can we do something like this?

class MediaSettingsRangeData {  // This is just a struct. You can pass it around.
  STATIC_ONLY();
  double m_max;
  ...;
};

class MediaSettingsRange : public GarbageCollected<MediaSettingsRange> {
  MediaSettingsRangeData m_data;
};
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 17 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ef23b14712be35baab5b30b0176808911f9bb638

commit ef23b14712be35baab5b30b0176808911f9bb638
Author: mcasas <mcasas@chromium.org>
Date: Fri Mar 17 15:19:44 2017

Image Capture: MediaStreamTrack::applyConstraints()

This CL adds MediaStreamTrack.applyConstraints()
method behind a flag (that is implied if ImageCapture
is on).  The method essentially derives to
ImageCapture::setMediaTrackConstraints(); this one
prepares a mojo::PhotoSettings and applies it via
mojo interface.

Only simple Constrains are supported now, i.e.,
{zoom : 3.14} is supported, but {zoom : { ideal : 100}}
or {zoom : { exact : 27}} are not (TBD later).

LayoutTests are added where appropriate.

Also a cleanup: methods that return Promises are not
supposed to throw exceptions; so I removed an unused
ExceptionState from some ImageCapture methods.

applyConstraints() when called without the optional
argument |constraints|, and is supposed to do... nothing?
Filed Spec bug:
https://github.com/w3c/mediacapture-main/issues/438

BUG= 700607 

Review-Url: https://codereview.chromium.org/2757673005
Cr-Commit-Position: refs/heads/master@{#457771}

[modify] https://crrev.com/ef23b14712be35baab5b30b0176808911f9bb638/third_party/WebKit/LayoutTests/external/wpt/mediacapture-streams/MediaStreamTrack-init.https-expected.txt
[add] https://crrev.com/ef23b14712be35baab5b30b0176808911f9bb638/third_party/WebKit/LayoutTests/fast/imagecapture/MediaStreamTrack-applyConstraints.html
[add] https://crrev.com/ef23b14712be35baab5b30b0176808911f9bb638/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-applyConstraints.html
[modify] https://crrev.com/ef23b14712be35baab5b30b0176808911f9bb638/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/ef23b14712be35baab5b30b0176808911f9bb638/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp
[modify] https://crrev.com/ef23b14712be35baab5b30b0176808911f9bb638/third_party/WebKit/Source/modules/imagecapture/ImageCapture.h
[modify] https://crrev.com/ef23b14712be35baab5b30b0176808911f9bb638/third_party/WebKit/Source/modules/imagecapture/ImageCapture.idl
[modify] https://crrev.com/ef23b14712be35baab5b30b0176808911f9bb638/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.cpp
[modify] https://crrev.com/ef23b14712be35baab5b30b0176808911f9bb638/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.h
[modify] https://crrev.com/ef23b14712be35baab5b30b0176808911f9bb638/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.idl
[modify] https://crrev.com/ef23b14712be35baab5b30b0176808911f9bb638/third_party/WebKit/Source/modules/mediastream/MediaTrackConstraintSet.idl
[modify] https://crrev.com/ef23b14712be35baab5b30b0176808911f9bb638/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5
Project Member

Comment 7 by bugdroid1@chromium.org, Mar 20 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/726f6720d94c103683515f9887082a4c9376d133

commit 726f6720d94c103683515f9887082a4c9376d133
Author: mcasas <mcasas@chromium.org>
Date: Mon Mar 20 21:10:02 2017

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

This CL teaches Image Capture to cache the constraints passed via
applyConstraints() and wires MediaStreamTrack::getConstraints() to
append those upon request.

Note that constraints are accumulative, i.e. two calls to
applyConstraints() with different constraints will essentially
be understood by the platform as a single applyConstraints()
call with the constraint dictionaries merged.

(Also moved MediaStreamTrack::getConstraints() to follow the
declaration order in MediaStreamTrack.h).

Added LayoutTests.

BUG= 700607 

Review-Url: https://codereview.chromium.org/2758063002
Cr-Commit-Position: refs/heads/master@{#458183}

[add] https://crrev.com/726f6720d94c103683515f9887082a4c9376d133/third_party/WebKit/LayoutTests/fast/imagecapture/MediaStreamTrack-getConstraints.html
[modify] https://crrev.com/726f6720d94c103683515f9887082a4c9376d133/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp
[modify] https://crrev.com/726f6720d94c103683515f9887082a4c9376d133/third_party/WebKit/Source/modules/imagecapture/ImageCapture.h
[modify] https://crrev.com/726f6720d94c103683515f9887082a4c9376d133/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.cpp
Project Member

Comment 9 by bugdroid1@chromium.org, Mar 21 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/863711a1f0b27b5aa30ed18af6932eae19ccf5fc

commit 863711a1f0b27b5aa30ed18af6932eae19ccf5fc
Author: mcasas <mcasas@chromium.org>
Date: Tue Mar 21 18:56:41 2017

Image Capture: wire getSettings() from MediaStreamTrack

This CL wires the fourth and last Constrainable Pattern
method, MediaStreamTrack.getSettings(), appending
Image-Capture specific Settings to what is returned
now.

ImageCapture::getMediaTrackSettings simply extends the
parameter |settings| with whatever the current |m_capabilities|
holds.  Perhaps confusingly, ImageCapture::m_capabilities
includes _both_ working ranges and current values so, in a way,
it encompasses the capabilities and the settings; this is
due to the previous API (and mojo) design, and will be
untangled in subsequent CLs.

Also, this CL forces an update of |m_capabilities| when
configuring or reading the device.

A few LayoutTests are added, including one where the
applied constraints are retrieved with getSettings and
compared (the mojo mock is extended to latch the former
blindly).

BUG= 700607 

Review-Url: https://codereview.chromium.org/2766473002
Cr-Commit-Position: refs/heads/master@{#458495}

[add] https://crrev.com/863711a1f0b27b5aa30ed18af6932eae19ccf5fc/third_party/WebKit/LayoutTests/fast/imagecapture/MediaStreamTrack-getSettings.html
[add] https://crrev.com/863711a1f0b27b5aa30ed18af6932eae19ccf5fc/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-applyConstraints-getSettings.html
[add] https://crrev.com/863711a1f0b27b5aa30ed18af6932eae19ccf5fc/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-getSettings.html
[modify] https://crrev.com/863711a1f0b27b5aa30ed18af6932eae19ccf5fc/third_party/WebKit/LayoutTests/imagecapture/resources/mock-imagecapture.js
[modify] https://crrev.com/863711a1f0b27b5aa30ed18af6932eae19ccf5fc/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp
[modify] https://crrev.com/863711a1f0b27b5aa30ed18af6932eae19ccf5fc/third_party/WebKit/Source/modules/imagecapture/ImageCapture.h
[modify] https://crrev.com/863711a1f0b27b5aa30ed18af6932eae19ccf5fc/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.cpp
[modify] https://crrev.com/863711a1f0b27b5aa30ed18af6932eae19ccf5fc/third_party/WebKit/Source/modules/mediastream/MediaTrackSettings.idl
Project Member

Comment 10 by bugdroid1@chromium.org, Mar 24 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7f9e2de323e9265aee33ba9683ba98b09fec6769

commit 7f9e2de323e9265aee33ba9683ba98b09fec6769
Author: mcasas <mcasas@chromium.org>
Date: Fri Mar 24 05:47:27 2017

Image Capture: prune Photo{Capabilities/Settings} and add MediaTrackConstraintSet.pointsOfInterest

- This CL removes from PhotoCapabilities.idl the fields that are
not there anymore according to the spec [1], leaving just four
of them.  The corresponding code and LayoutTests are updated.
Same for PhotoSettings.idl [2].

- This CL also adds ConstrainPoint2D data type [3] and uses it
for MediaTrackConstraintSet.pointsOfInterest, wiring it down
to the mojo interface and in the mock.

- This is the one and only important change visible to the users.
(There is an Origin Trial going on for Image Capture in
M56-57-58, but these changes will land in 59.x.y.z, so shouldn't
affect them, in any case an update to blink-dev@ is being
brewed and there's a migration guide:
  https://tinyurl.com/image-capture-migration-guide
)

- No more MediaStreamTrack idl changes are expected after
this CL \o/

TEST= Layouttests etc updated.
BUG= 700607 

[1] https://w3c.github.io/mediacapture-image/##photocapabilities-section
[2] https://w3c.github.io/mediacapture-image/##photosettings-section
[3] https://w3c.github.io/mediacapture-image/#additional-constrainable-props

Review-Url: https://codereview.chromium.org/2773593004
Cr-Commit-Position: refs/heads/master@{#459367}

[modify] https://crrev.com/7f9e2de323e9265aee33ba9683ba98b09fec6769/third_party/WebKit/LayoutTests/fast/imagecapture/ImageCapture-MediaTrackSupportedConstraints.html
[modify] https://crrev.com/7f9e2de323e9265aee33ba9683ba98b09fec6769/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-applyConstraints-getSettings.html
[modify] https://crrev.com/7f9e2de323e9265aee33ba9683ba98b09fec6769/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-applyConstraints.html
[modify] https://crrev.com/7f9e2de323e9265aee33ba9683ba98b09fec6769/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-getCapabilities.html
[modify] https://crrev.com/7f9e2de323e9265aee33ba9683ba98b09fec6769/third_party/WebKit/LayoutTests/imagecapture/getphotocapabilities.html
[modify] https://crrev.com/7f9e2de323e9265aee33ba9683ba98b09fec6769/third_party/WebKit/LayoutTests/imagecapture/resources/mock-imagecapture.js
[modify] https://crrev.com/7f9e2de323e9265aee33ba9683ba98b09fec6769/third_party/WebKit/LayoutTests/imagecapture/setoptions.html
[modify] https://crrev.com/7f9e2de323e9265aee33ba9683ba98b09fec6769/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/7f9e2de323e9265aee33ba9683ba98b09fec6769/third_party/WebKit/Source/bindings/modules/v8/generated.gni
[add] https://crrev.com/7f9e2de323e9265aee33ba9683ba98b09fec6769/third_party/WebKit/Source/modules/imagecapture/ConstrainPoint2DParameters.idl
[modify] https://crrev.com/7f9e2de323e9265aee33ba9683ba98b09fec6769/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp
[modify] https://crrev.com/7f9e2de323e9265aee33ba9683ba98b09fec6769/third_party/WebKit/Source/modules/imagecapture/ImageCapture.h
[modify] https://crrev.com/7f9e2de323e9265aee33ba9683ba98b09fec6769/third_party/WebKit/Source/modules/imagecapture/PhotoCapabilities.cpp
[modify] https://crrev.com/7f9e2de323e9265aee33ba9683ba98b09fec6769/third_party/WebKit/Source/modules/imagecapture/PhotoCapabilities.h
[modify] https://crrev.com/7f9e2de323e9265aee33ba9683ba98b09fec6769/third_party/WebKit/Source/modules/imagecapture/PhotoCapabilities.idl
[modify] https://crrev.com/7f9e2de323e9265aee33ba9683ba98b09fec6769/third_party/WebKit/Source/modules/imagecapture/PhotoSettings.idl
[modify] https://crrev.com/7f9e2de323e9265aee33ba9683ba98b09fec6769/third_party/WebKit/Source/modules/mediastream/MediaTrackConstraintSet.idl
[modify] https://crrev.com/7f9e2de323e9265aee33ba9683ba98b09fec6769/third_party/WebKit/Source/modules/modules_idl_files.gni
Project Member

Comment 11 by bugdroid1@chromium.org, Apr 1 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ca371a65982c6c43fc81dbb997271ed2aab6c9da

commit ca371a65982c6c43fc81dbb997271ed2aab6c9da
Author: mcasas <mcasas@chromium.org>
Date: Sat Apr 01 00:57:38 2017

ImageCapture: separate fillLightMode, redEyeReduction and Torch

Implementation catch-up to some idl changes (already landed):

- RedEyeReduction capability-reading-wise is changed from being a
  boolean to an enum ("never", "always", "controllable")

- FillLightMode enum capability-reading-wise is changed to
  returning an array of supported capabilities (vs. on ToT
  where the _actual_ setting that was returned).
  It also loses the enum entries "none" and "torch": "torch"
  is separated, and the condition of "no fill light mode
  supported" is indicated via an empty supported mode array.

- MediaTrackCapabilities/ConstraintSet's |torch| is wired all
 the way down to the implementations.

Here and there I reordered the capabilities/settings
manipulation to follow the idl definitions.

BUG= 700607 
TEST=Update LayoutTests, capture_unittests and manual
testing using e.g.
https://rawgit.com/Miguelao/demos/master/imagecapture.html

Review-Url: https://codereview.chromium.org/2787933002
Cr-Commit-Position: refs/heads/master@{#461293}

[modify] https://crrev.com/ca371a65982c6c43fc81dbb997271ed2aab6c9da/media/capture/mojo/image_capture.mojom
[modify] https://crrev.com/ca371a65982c6c43fc81dbb997271ed2aab6c9da/media/capture/video/android/java/src/org/chromium/media/PhotoCapabilities.java
[modify] https://crrev.com/ca371a65982c6c43fc81dbb997271ed2aab6c9da/media/capture/video/android/java/src/org/chromium/media/VideoCapture.java
[modify] https://crrev.com/ca371a65982c6c43fc81dbb997271ed2aab6c9da/media/capture/video/android/java/src/org/chromium/media/VideoCaptureCamera.java
[modify] https://crrev.com/ca371a65982c6c43fc81dbb997271ed2aab6c9da/media/capture/video/android/java/src/org/chromium/media/VideoCaptureCamera2.java
[modify] https://crrev.com/ca371a65982c6c43fc81dbb997271ed2aab6c9da/media/capture/video/android/photo_capabilities.cc
[modify] https://crrev.com/ca371a65982c6c43fc81dbb997271ed2aab6c9da/media/capture/video/android/photo_capabilities.h
[modify] https://crrev.com/ca371a65982c6c43fc81dbb997271ed2aab6c9da/media/capture/video/android/video_capture_device_android.cc
[modify] https://crrev.com/ca371a65982c6c43fc81dbb997271ed2aab6c9da/media/capture/video/fake_video_capture_device.cc
[modify] https://crrev.com/ca371a65982c6c43fc81dbb997271ed2aab6c9da/media/capture/video/fake_video_capture_device_unittest.cc
[modify] https://crrev.com/ca371a65982c6c43fc81dbb997271ed2aab6c9da/media/capture/video/linux/v4l2_capture_delegate.cc
[modify] https://crrev.com/ca371a65982c6c43fc81dbb997271ed2aab6c9da/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-applyConstraints-getSettings.html
[modify] https://crrev.com/ca371a65982c6c43fc81dbb997271ed2aab6c9da/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-applyConstraints.html
[modify] https://crrev.com/ca371a65982c6c43fc81dbb997271ed2aab6c9da/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-getCapabilities.html
[modify] https://crrev.com/ca371a65982c6c43fc81dbb997271ed2aab6c9da/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-getSettings.html
[modify] https://crrev.com/ca371a65982c6c43fc81dbb997271ed2aab6c9da/third_party/WebKit/LayoutTests/imagecapture/getphotocapabilities.html
[modify] https://crrev.com/ca371a65982c6c43fc81dbb997271ed2aab6c9da/third_party/WebKit/LayoutTests/imagecapture/resources/mock-imagecapture.js
[modify] https://crrev.com/ca371a65982c6c43fc81dbb997271ed2aab6c9da/third_party/WebKit/LayoutTests/imagecapture/setoptions.html
[modify] https://crrev.com/ca371a65982c6c43fc81dbb997271ed2aab6c9da/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp
[modify] https://crrev.com/ca371a65982c6c43fc81dbb997271ed2aab6c9da/third_party/WebKit/Source/modules/imagecapture/PhotoCapabilities.cpp
[modify] https://crrev.com/ca371a65982c6c43fc81dbb997271ed2aab6c9da/third_party/WebKit/Source/modules/imagecapture/PhotoCapabilities.h
Project Member

Comment 12 by bugdroid1@chromium.org, Apr 4 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6f7ac2ab3f093655cddec3e000578e625dcd6c31

commit 6f7ac2ab3f093655cddec3e000578e625dcd6c31
Author: mcasas <mcasas@chromium.org>
Date: Tue Apr 04 04:25:56 2017

Image Capture: add |points_of_interest| to mojo and wire it to IDL layer

This CL adds an array<Point2D> |points_of_interest| member
to image_capture.mojom and wires the use of the equivalent
members from idl, i.e. points of interest are:

- cached (as a constraint) when being set via applyConstraints(),
 and produced upon getConstraints();

- cached (as status) when being read back from the mojo pipe,
 together with other Capabilities, and produced upon request
 on getSettings().

LayoutTests updated. The only platform supporting points
of interest currently is Android -- will be updated
in a subsequent CL.

BUG= 700607 

Review-Url: https://codereview.chromium.org/2795693004
Cr-Commit-Position: refs/heads/master@{#461629}

[modify] https://crrev.com/6f7ac2ab3f093655cddec3e000578e625dcd6c31/media/capture/mojo/image_capture.mojom
[modify] https://crrev.com/6f7ac2ab3f093655cddec3e000578e625dcd6c31/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-applyConstraints-getSettings.html
[modify] https://crrev.com/6f7ac2ab3f093655cddec3e000578e625dcd6c31/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-applyConstraints.html
[modify] https://crrev.com/6f7ac2ab3f093655cddec3e000578e625dcd6c31/third_party/WebKit/LayoutTests/imagecapture/resources/mock-imagecapture.js
[modify] https://crrev.com/6f7ac2ab3f093655cddec3e000578e625dcd6c31/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp
[modify] https://crrev.com/6f7ac2ab3f093655cddec3e000578e625dcd6c31/third_party/WebKit/Source/modules/imagecapture/ImageCapture.h
[modify] https://crrev.com/6f7ac2ab3f093655cddec3e000578e625dcd6c31/third_party/WebKit/Source/modules/imagecapture/MediaSettingsRange.h

Comment 13 by mcasas@chromium.org, Apr 4 2017

Cc: mcasas@chromium.org
Issue 703533 has been merged into this issue.
Project Member

Comment 14 by bugdroid1@chromium.org, Apr 4 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/68035060ecbaeefc9c49fdd7731b262570dcd4da

commit 68035060ecbaeefc9c49fdd7731b262570dcd4da
Author: mcasas <mcasas@chromium.org>
Date: Tue Apr 04 19:07:11 2017

Image Capture: remove unnecessary .get() (followup)

Remove unnecessary .get() in unique_ptr<> following up comment [1]

[1] https://codereview.chromium.org/2795693004/#msg18

BUG= 700607 

Review-Url: https://codereview.chromium.org/2795293002
Cr-Commit-Position: refs/heads/master@{#461794}

[modify] https://crrev.com/68035060ecbaeefc9c49fdd7731b262570dcd4da/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp
Project Member

Comment 15 by bugdroid1@chromium.org, Apr 5 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a1052a48bc85cc5d68c0f7dc0782b577e92cc9c7

commit a1052a48bc85cc5d68c0f7dc0782b577e92cc9c7
Author: mcasas <mcasas@chromium.org>
Date: Wed Apr 05 04:08:44 2017

Image Capture: remove MediaSettingsRange.current

This CL removes MediaSettingsRange.current member, which
was never in the Spec to start with. "Current" feature values
still need to be kept around for getMediaTrackSettings(): with
the |current| member gone, we aneed to dd a new member
|m_settings| to keep them (and it encompasses the previous
points of interest vector).

LayoutTests updated accordingly.

BUG= 700607 ,  707262 

Review-Url: https://codereview.chromium.org/2795923003
Cr-Commit-Position: refs/heads/master@{#461974}

[modify] https://crrev.com/a1052a48bc85cc5d68c0f7dc0782b577e92cc9c7/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-applyConstraints-getSettings.html
[modify] https://crrev.com/a1052a48bc85cc5d68c0f7dc0782b577e92cc9c7/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-getCapabilities.html
[modify] https://crrev.com/a1052a48bc85cc5d68c0f7dc0782b577e92cc9c7/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-getSettings.html
[modify] https://crrev.com/a1052a48bc85cc5d68c0f7dc0782b577e92cc9c7/third_party/WebKit/LayoutTests/imagecapture/getphotocapabilities.html
[modify] https://crrev.com/a1052a48bc85cc5d68c0f7dc0782b577e92cc9c7/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/a1052a48bc85cc5d68c0f7dc0782b577e92cc9c7/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp
[modify] https://crrev.com/a1052a48bc85cc5d68c0f7dc0782b577e92cc9c7/third_party/WebKit/Source/modules/imagecapture/ImageCapture.h
[modify] https://crrev.com/a1052a48bc85cc5d68c0f7dc0782b577e92cc9c7/third_party/WebKit/Source/modules/imagecapture/MediaSettingsRange.h
[modify] https://crrev.com/a1052a48bc85cc5d68c0f7dc0782b577e92cc9c7/third_party/WebKit/Source/modules/imagecapture/MediaSettingsRange.idl

Comment 16 by mcasas@chromium.org, Apr 5 2017 

I filed https://crbug.com/708723 to track finishing up the advanced
parts of the MediaTrackConstraintSet (i.e. sophisticated constraints,
clearing of constraints etc).

Comment 17 by mcasas@chromium.org, Apr 5 2017

Blocking: 708723
Project Member

Comment 18 by bugdroid1@chromium.org, Apr 6 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e3f5c08d551b9a818dfa63874b5db137694b4889

commit e3f5c08d551b9a818dfa63874b5db137694b4889
Author: Philip Jägenstedt <foolip@chromium.org>
Date: Thu Apr 06 10:48:43 2017

Allow shapedetection/detection-security-test.html to timeout on Mac

BUG=703533, 700607 
TBR=mcasas@chromium.org

Review-Url: https://codereview.chromium.org/2801963002 .
Cr-Commit-Position: refs/heads/master@{#462418}

[modify] https://crrev.com/e3f5c08d551b9a818dfa63874b5db137694b4889/third_party/WebKit/LayoutTests/TestExpectations
Project Member

Comment 19 by bugdroid1@chromium.org, Apr 6 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1d5b6a1cd0258c12363b53f0d382471141463abe

commit 1d5b6a1cd0258c12363b53f0d382471141463abe
Author: mcasas <mcasas@chromium.org>
Date: Thu Apr 06 17:31:29 2017

Image Capture: make sure applyConstraints() only works for ImageCapture constraints

This CL:
 - makes sure MediaStreamTrack::applyConstraints() is rejected if non
 image-capture related constraints are passed; (this follows the
 Spec e.g. in [1], " If the constraints cannot be applied, the promise is
 rejected").

- generally cleans up  MediaStreamTrack::applyConstraints() moving
 part of the TODO()s to Image Capture so that they are in the
 same place (facilitates later reviews).

 - derives to clearMediaTrackConstraints() if the constraint set is
 empty, after spec issue somehow clarified it

LayoutTests updated/extended.

[1] https://developer.mozilla.org/en-US/docs/Web/API/MediaStreamTrack/applyConstraints#Return_value

BUG= 700607 , 708723

Review-Url: https://codereview.chromium.org/2804653003
Cr-Commit-Position: refs/heads/master@{#462531}

[modify] https://crrev.com/1d5b6a1cd0258c12363b53f0d382471141463abe/third_party/WebKit/LayoutTests/fast/imagecapture/MediaStreamTrack-applyConstraints.html
[modify] https://crrev.com/1d5b6a1cd0258c12363b53f0d382471141463abe/third_party/WebKit/LayoutTests/fast/imagecapture/MediaStreamTrack-getConstraints.html
[modify] https://crrev.com/1d5b6a1cd0258c12363b53f0d382471141463abe/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp
[modify] https://crrev.com/1d5b6a1cd0258c12363b53f0d382471141463abe/third_party/WebKit/Source/modules/imagecapture/ImageCapture.h
[modify] https://crrev.com/1d5b6a1cd0258c12363b53f0d382471141463abe/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.cpp
Project Member

Comment 20 by bugdroid1@chromium.org, Apr 7 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/15f5b51d19bd375b45ca8f9f8377b0522f261395

commit 15f5b51d19bd375b45ca8f9f8377b0522f261395
Author: mcasas <mcasas@chromium.org>
Date: Fri Apr 07 01:22:39 2017

Image Capture: add |supports_torch| field to mojom

This CL adds |supports_torch| to mojom.PhotoCapabilities, to differentiate
the capability from the actual value of |torch|, and wires it
for the Android implementations and for the LayoutTests.

Tested via unittests and manual test using
https://rawgit.com/Miguelao/demos/master/imagecapture.html

BUG= 700607 

Review-Url: https://codereview.chromium.org/2798263003
Cr-Commit-Position: refs/heads/master@{#462731}

[modify] https://crrev.com/15f5b51d19bd375b45ca8f9f8377b0522f261395/media/capture/mojo/image_capture.mojom
[modify] https://crrev.com/15f5b51d19bd375b45ca8f9f8377b0522f261395/media/capture/video/android/java/src/org/chromium/media/PhotoCapabilities.java
[modify] https://crrev.com/15f5b51d19bd375b45ca8f9f8377b0522f261395/media/capture/video/android/java/src/org/chromium/media/VideoCaptureCamera.java
[modify] https://crrev.com/15f5b51d19bd375b45ca8f9f8377b0522f261395/media/capture/video/android/java/src/org/chromium/media/VideoCaptureCamera2.java
[modify] https://crrev.com/15f5b51d19bd375b45ca8f9f8377b0522f261395/media/capture/video/android/photo_capabilities.cc
[modify] https://crrev.com/15f5b51d19bd375b45ca8f9f8377b0522f261395/media/capture/video/android/photo_capabilities.h
[modify] https://crrev.com/15f5b51d19bd375b45ca8f9f8377b0522f261395/media/capture/video/android/video_capture_device_android.cc
[modify] https://crrev.com/15f5b51d19bd375b45ca8f9f8377b0522f261395/media/capture/video/fake_video_capture_device.cc
[modify] https://crrev.com/15f5b51d19bd375b45ca8f9f8377b0522f261395/media/capture/video/fake_video_capture_device_unittest.cc
[modify] https://crrev.com/15f5b51d19bd375b45ca8f9f8377b0522f261395/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-applyConstraints-getSettings.html
[modify] https://crrev.com/15f5b51d19bd375b45ca8f9f8377b0522f261395/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-getCapabilities.html
[modify] https://crrev.com/15f5b51d19bd375b45ca8f9f8377b0522f261395/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-getSettings.html
[modify] https://crrev.com/15f5b51d19bd375b45ca8f9f8377b0522f261395/third_party/WebKit/LayoutTests/imagecapture/resources/mock-imagecapture.js
[modify] https://crrev.com/15f5b51d19bd375b45ca8f9f8377b0522f261395/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp
Project Member

Comment 21 by bugdroid1@chromium.org, Apr 8 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7aafd1384fd8c6dc46d85d0cec98e85c480bd464

commit 7aafd1384fd8c6dc46d85d0cec98e85c480bd464
Author: mcasas <mcasas@chromium.org>
Date: Sat Apr 08 03:03:26 2017

Image Capture: split {white_balance,exposure,focus}_mode into current_ and supported_

|This CL splits mojo |{white_balance,exposure,focus}_mode|
into |current_{white_balance,exposure,focus}_mode| and an
array of |supported_{white_balance,exposure,focus}_modes|.

The |current_...| versions are essentially the same as the
previous variables, with a twist: if there are no
|supported_...| entries, then there can be no |current_...|.
(This prevents the capabilities dictionary from having
superfluous entries).

LayoutTests, Fake capture device and V4L2 implementations are
updated. (Will do the Android parts in another CL to ease
review).

Also I reordered the fields of mock-imagecapture.js |state_|
to match .mojom definition order because it was driving me
nuts to compare both :-)

BUG= 700607 

Review-Url: https://codereview.chromium.org/2806743003
Cr-Commit-Position: refs/heads/master@{#463116}

[modify] https://crrev.com/7aafd1384fd8c6dc46d85d0cec98e85c480bd464/media/capture/mojo/image_capture.mojom
[modify] https://crrev.com/7aafd1384fd8c6dc46d85d0cec98e85c480bd464/media/capture/video/android/video_capture_device_android.cc
[modify] https://crrev.com/7aafd1384fd8c6dc46d85d0cec98e85c480bd464/media/capture/video/fake_video_capture_device.cc
[modify] https://crrev.com/7aafd1384fd8c6dc46d85d0cec98e85c480bd464/media/capture/video/fake_video_capture_device_unittest.cc
[modify] https://crrev.com/7aafd1384fd8c6dc46d85d0cec98e85c480bd464/media/capture/video/linux/v4l2_capture_delegate.cc
[modify] https://crrev.com/7aafd1384fd8c6dc46d85d0cec98e85c480bd464/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-getCapabilities.html
[modify] https://crrev.com/7aafd1384fd8c6dc46d85d0cec98e85c480bd464/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-getSettings.html
[modify] https://crrev.com/7aafd1384fd8c6dc46d85d0cec98e85c480bd464/third_party/WebKit/LayoutTests/imagecapture/resources/mock-imagecapture.js
[modify] https://crrev.com/7aafd1384fd8c6dc46d85d0cec98e85c480bd464/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp
Project Member

Comment 22 by bugdroid1@chromium.org, Apr 10 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c1c7111870c820080a0bc5953e3b6505058f07b5

commit c1c7111870c820080a0bc5953e3b6505058f07b5
Author: mcasas <mcasas@chromium.org>
Date: Mon Apr 10 22:44:01 2017

Image Capture: wire supported exposure/focus/white balance modes Android

This CL follows on the steps of [1] and wires the supported
auto white balance/focus/exposure modes for the Android
implementations -- whereas on ToT only the 'current' state
of these features was collected.

Manually tested using N7 and N6P and
https://rawgit.com/Miguelao/demos/master/imagecapture.html

[1] https://codereview.chromium.org/2806743003/

BUG= 700607 

Review-Url: https://codereview.chromium.org/2808073003
Cr-Commit-Position: refs/heads/master@{#463426}

[modify] https://crrev.com/c1c7111870c820080a0bc5953e3b6505058f07b5/media/capture/video/android/java/src/org/chromium/media/PhotoCapabilities.java
[modify] https://crrev.com/c1c7111870c820080a0bc5953e3b6505058f07b5/media/capture/video/android/java/src/org/chromium/media/VideoCapture.java
[modify] https://crrev.com/c1c7111870c820080a0bc5953e3b6505058f07b5/media/capture/video/android/java/src/org/chromium/media/VideoCaptureCamera.java
[modify] https://crrev.com/c1c7111870c820080a0bc5953e3b6505058f07b5/media/capture/video/android/java/src/org/chromium/media/VideoCaptureCamera2.java
[modify] https://crrev.com/c1c7111870c820080a0bc5953e3b6505058f07b5/media/capture/video/android/photo_capabilities.cc
[modify] https://crrev.com/c1c7111870c820080a0bc5953e3b6505058f07b5/media/capture/video/android/photo_capabilities.h
[modify] https://crrev.com/c1c7111870c820080a0bc5953e3b6505058f07b5/media/capture/video/android/video_capture_device_android.cc

Comment 23 by mcasas@chromium.org, Apr 12 2017

Labels: M-59
Status: Fixed (was: Started)
Marking this issue as Fixed -- the rest of the job of
landing the sophisticated Constrain syntax will be tracked
in https://crbug.com/708723
►

Sign in to add a comment