Re: Mediastream Image Capture published as FPWD

Hi,

thanks for all the effort Giri. This API will be very useful 8)

Here's some constructive feedback on the Image Capture FPWD broken up 
into 3 main categories:

- Minor grammatical issues
- Linguistic issues
- Technical issues

Here's the detailed feedback.

Minor grammatical issues
------------------------
- replace "a" with "an"
In 8 places where it proceed a noun starting with "I" e.g. in order
   "to get a ImageCaptureError object"
   "fire a ImageCaptureErrorEvent event"
   "fire a ImageCaptureErrorEvent event"
   "into a ImageData object"
   "fire a ImageCaptureErrorEvent at"
   "fire a ImageCaptureErrorEvent event at"
   "fire a ImageCaptureErrorEvent event"
   "Returns a ImageCaptureError object"

- replace "then then" with "then"
In 2.2 Methods, setOptions e.g.
   "invoked, then then a valid"


Linguistic issues
-----------------
- naming symmetry
The names of the methods, event handlers and error codes are not 
symmetrical. It would be nice to tidy them up.  For example they are 
currently:
---------------------------------------------------------
method          event handler            error code
---------------------------------------------------------
getFrame()      onframegrab              FRAME_GRAB_ERROR
takePhoto()     onphoto                  PHOTO_ERROR
setOptions()    onphotosettingschange    SETTINGS_ERROR

I think a more symmetrical approach could be something like:
---------------------------------------------------------
method          event handler            error code
---------------------------------------------------------
grabFrame()     onframe                  FRAME_ERROR
takePhoto()     onphoto                  PHOTO_ERROR
setOptions()    onoptions                OPTIONS_ERROR

Or if people preferred to stick with get/set language then:
---------------------------------------------------------
method          event handler            error code
---------------------------------------------------------
getFrame()      onframe                  FRAME_ERROR
getPhoto()      onphoto                  PHOTO_ERROR
setOptions()    onoptions                OPTIONS_ERROR

- replace pictureDevice with captureDevice
It's just a small point but using the variable name "pictureDevice" in 
13.2 Grabbing a Frame for Post-Processing makes it obvious that this 
example was re-worked based on the code in 13.1. It would be nice to use 
the variable name "captureDevice" in both 13.1 and 13.2 to match the 
Image Capture name of the API.


Technical issues
----------------
- replace ImageData "length" with "width" e.g.
3.1 Attributes, imageData ..."object whose length and height"

- show looping in frame grabbing example
In 13.2 Grabbing a Frame for Post-Processing it would be good to show 
the recommended setTimeout() or requestAnimationFrame() based looping 
approach to call getFrame() repeatedly as it's only mentioned in one 
place that getFrame() is a single shot method - see {Note:...} at end of 
2.2 Methods, getFrame.

NOTE: Personally I still think this is a technical flaw in the API and 
it would be good to be able to set getFrame() as an auto-firing/looping 
method and then this code could be optimised by the implementors. 
Perhaps this could be passed in as an optional config dictionary 
element?  I know the WG members don't agree with this approach but this 
really is not the most efficient way to do this. It's very likely that 
the same frame data will be captured multiple times - when in the ideal 
world the callback would only be fired once the frames TypedArray was 
filled with a unique frame. This is just adding yet another inefficient 
layer to the web platforms already cumbersome image processing pipeline 
and makes timestamping of frames a complete hack 8(

- optional *EventInit Dictionaries
Are these really required? They make this document longer and I'm not 
sure how they would really be used? Since they're optional and all of 
them only accept the primary payload then they really seem superfluous. 
Happy to be proven wrong if I've misunderstood their meaning, but they 
just seem like an optional hook to inject the primary payload that would 
normally be generated by the *Event anyway. e.g.
   "3.2 FrameGrabEventInit Dictionary"
   "4.2 ImageCaptureErrorEventInit Dictionary"
   "5.2 BlobEventInit Dictionary"
   "6.2 SettingsChangeEventInit Dictionary"


I hope this feedback is clear, constructive and helpful.

roBman


On 10/07/13 01:06, Dominique Hazael-Massieux wrote:
> Hi,
>
> As agreed previously, our document “Mediastream Image Capture” has just
> been published as a First Public Working Draft:
> http://www.w3.org/TR/2013/WD-image-capture-20130709/
>
> Congratulations to all those involved, in particular Giri!
>
> Dom
>
>
>
>

Received on Thursday, 18 July 2013 07:09:32 UTC