Re: Image Capture Proposal, third version

On Mon, Mar 25, 2013 at 3:00 PM, Mandyam, Giridhar <mandyam@quicinc.com>wrote:

>  Yes, a VideoStreamTrack constructor makes sense, but simply having it
> will not obviate the need for validation.  This is because the sourceType
> of the VideoStreamTrack would have be verified as to whether it is ‘camera’
> or ‘photo-camera’ (see Sec. 4.3.3 of the latest Media Capture and Streams
> spec), and takePhoto() is prohibited on a ‘camera’ sourceType currently.
>

I don't see why distinguishing "photo-camera" from "camera" is useful for
authors. Video cameras could support the takePhoto() API just fine. So I
suggest simply allowing ImageCapture for any VideoStreamTrack.

I think to make life easy for authors the ImageCapture object should look
as much as possible like MediaRecorder. I guess we can't reuse
MediaRecorder (I know you just removed it from there, which I think was the
right thing to do) but I think it would make sense to make the API match
MediaRecorder as much as possible. For example, setPhotoSettings() should
just be setOptions().

What's the use-case for the frameGrabber stuff? If it's real-time video
processing, I think we need a completely different API that's based on Web
Workers. I'll ignore the frameGrabber stuff for now.

A lot needs to be clarified here. For example, are multiple takePhoto()
calls allowed? What happens if the author makes multiple takePhoto() calls?

"videoStreamTrack" should just be "track". "onphotoerror" can probably just
be "onerror".

There is no point in firing events synchronously to indicate the success or
failure of methods such as setPhotoSettings. You can raise an exception
instead if necessary.

Does setPhotoSettings/setOptions affect the contents returned in the video
track? Or just the data returned on the onphoto callback?

'whiteBalanceModeSetting' can just be "whiteBalance'. Instead of an enum
which simply maps to Kelvin, why not make it an unsigned long in Kelvin and
have the UA map that to the nearest value it supports?

In fact all members of PhotoSettings (which should probably be called
ImageCaptureOptions if we make MediaRecorder.setOptions take a
RecorderOptions) can lose their "Setting" suffix.

'autoExposureMode' should be declared ExposureModeEnum (which should be
called ExposureMode, no need for "Enum"). It could use a more precise
definition.

'contrast', 'brightness', 'exposureCompensation', 'iso', 'saturation', and
'sharpness' seem to be completely undefined. That's bad.

PhotoSettingsOptions seems over-abstracted. What's the point of
MediaSettingsItem anyway? Attributes like whiteBalanceMode can just be
declared as values of the correct type. The value of MediaSettingsRange is
also unclear. As in MediaRecorder, I think it would be much simpler to
examine these options on a case-by-case basis and either define the allowed
range in the spec, or if the supported range of an option must be device
dependent *and* exposed to apps, add individual attributes for the min
and/or max limit of each option.

imageHeight and imageWidth are especially problematic. Generally these
can't be set independently so instead of providing a min and max for each
one, you probably want to either export a list of allowed (width,height)
pairs, or better still just expose maxImageWidth and maxImageHeight and let
the app specify any smaller size when setting the options.

Firing events synchronously to indicate success or failure of API calls
seems unnecessary. If the call fails, throw an exception (or silently
ignore). If it succeeds, do nothing. That's the general rule for Web APIs.

Rob
-- 
q“qIqfq qyqoquq qlqoqvqeq qtqhqoqsqeq qwqhqoq qlqoqvqeq qyqoquq,q qwqhqaqtq
qcqrqeqdqiqtq qiqsq qtqhqaqtq qtqoq qyqoquq?q qEqvqeqnq qsqiqnqnqeqrqsq
qlqoqvqeq qtqhqoqsqeq qwqhqoq qlqoqvqeq qtqhqeqmq.q qAqnqdq qiqfq qyqoquq
qdqoq qgqoqoqdq qtqoq qtqhqoqsqeq qwqhqoq qaqrqeq qgqoqoqdq qtqoq qyqoquq,q
qwqhqaqtq qcqrqeqdqiqtq qiqsq qtqhqaqtq qtqoq qyqoquq?q qEqvqeqnq
qsqiqnqnqeqrqsq qdqoq qtqhqaqtq.q"