New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 655107 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

imageCapture: takePhoto image is not as contrasted as grabFrame image

Project Member Reported by fbeaufort@chromium.org, Oct 12 2016

Issue description

Google Chrome	55.0.2878.0 (Official Build) dev (64-bit)
Platform	8861.0.0 (Official Build) dev-channel link

What steps will reproduce the problem?
(1) Go to https://beaufortfrancois.github.io/sandbox/image-capture/playground.html
(2) Click "grabFrame & takePhoto" button
(3) Two snapshots show up

What is the expected output?
They should be almost the same...

What do you see instead?
They're not the same as expected but what strikes me the most is that the "takePhoto" image is less contrasted than the "grabFrame" one.
By simply adding a contrast of 120% to the "takePhoto" one, I can see that the "grabFrame" and "takePhoto" are almost identical now.

May you explain what may happen there?
 
Status: Available (was: Untriaged)

Comment 2 by mcasas@chromium.org, Oct 12 2016

Cc: xianglu@chromium.org
Owner: mcasas@chromium.org
Status: Assigned (was: Available)

Comment 3 by sshru...@google.com, Nov 23 2016

Components: -Blink>MediaStream>ImageCapture Blink>ImageCapture

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

Cc: fbeaufort@chromium.org
I have no idea, and just tried with an Asus Chromebook and see them
pretty similar to the naked eye.

For reference, the Linux/CrOS implementation does not "take" pictures per
se but simply encodes the frame being capture as a jpeg/png and sends 
it. The code can be seen in [1], in a nutshell:
- if the capture format is MJPEG frame (and it will always be the 
  case if the resolution is >=720p), then the frame is simply sent as
 image/x-jpeg. Otherwise,
- the captured I420 frame is ConvertToARGB()d
- the ARGB/BGRA frame is encoded to PNG.

Not much room for the contrast to be changed.

[1] https://cs.chromium.org/chromium/src/media/capture/video/blob_utils.cc?type=cs&sq=package:chromium&l=15

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

fbeaufort@, is this still reproducible?  Perhaps with an
external camera?
I can still reproduce it with Google Chrome 59.0.3065.0 (Official Build) dev (64-bit) and a Logitech C270 external camera.
See grabFrame and takePhoto screenshots below. Open them in separate tabs and toggle between them to see the difference.
Screenshot 2017-04-14 at 9.35.11 AM - Display 1.png
634 KB View Download
Screenshot 2017-04-14 at 9.35.07 AM - Display 1.png
640 KB View Download
As requested, here's a screenshot when taking a picture of http://www.cr173.com/up/2010-4/201042594926.png

Left is grabFrame
Right is takePhoto

Using a color picker, I can tell that magenta in grabFrame is rgb(197, 56, 120) while magenta in takePhoto is rgb(189, 49, 115)


Screenshot_20170418-095835.png
713 KB View Download
Let me reconstruct the conversion steps each image goes through in CrOs/Linux;
to me, the almost-equal-to-the-naked-eye plots is symptomatic of too many
pixel format conversions/compressions :-)

a) Live video capture produces frames via V4L2CaptureDelegate::DoCapture() [1].
The original data (from the WebCam) comes in either YUY2 (a 4:2:2 format) or 
MJPEG, depending if the capture is smaller than 1280x720p or not, respectively.

b) This V4L2CaptureDelegate sends the capture frame to a conversion stage to 
I420 [2].  I420 is a 4:2:0 format, so it has lost some information 
irretrievably.  This I420 format is the one used for transporting video frames
to the rendered.

c) This I420 is the input to grabFrame(), which produces a JS ImageBitmap, 
unencoded, after converting the I420 into RGBA [3] of the appropriate endian-ness.


What happens to takePhoto()? It takes the data from the Webcam in a) and
either returns a JPEG Blob [4] or converts the YUY2 [5] and encodes it to PNG
using the default compression value (6 in a 0-10 scale IIRC) [6].



IOW:

- for smaller video resolutions:

OS -> YUY2 ---> I420 --> RGBA --> ImageBitmap     grabFrame()
           |
           +--> RGBA --> PNG ---> Blob            takePhoto()

- and for larger video resolutions:

OS -> MJPEG ---> I420 --> RGBA --> ImageBitmap    grabFrame()
            |
            +--> JPG ------------> Blob           takePhoto()


Where every conversion to-I420 loses information and so does the encoding to
PNG.  Even a conversion RGBA --> I420 --> RGBA would not produce the original
image.  (Plus, when you show ImageBitmap and/or Blob on an <img> or <canvas>
there are more stages of decoding and even colour correction involved!)

With all that, I'm not surprised at all that the images are not pixel 
accurate!  :-)



[1] https://cs.chromium.org/chromium/src/media/capture/video/linux/v4l2_capture_delegate.cc?q=v4l2_capture_dele&sq=package:chromium&dr=C&l=806
[2] https://cs.chromium.org/chromium/src/media/capture/video/video_capture_device_client.cc?sq=package:chromium&dr=C&l=297
[3] https://cs.chromium.org/chromium/src/content/renderer/image_capture/image_capture_frame_grabber.cc?q=frame_grabber&sq=package:chromium&dr=C&l=87
[4] https://cs.chromium.org/chromium/src/media/capture/video/blob_utils.cc?sq=package:chromium&dr=C&l=24
[5] https://cs.chromium.org/chromium/src/media/capture/video/blob_utils.cc?sq=package:chromium&dr=C&l=47
[6] https://cs.chromium.org/chromium/src/media/capture/video/blob_utils.cc?sq=package:chromium&dr=C&l=59
I'm working on adding a readme.md to WebKit/.../imagecapture
to explain this and a bunch of other stuff.
Project Member

Comment 10 by bugdroid1@chromium.org, May 12 2017

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

commit 6f6c6d53a9eecd08916e55545dd2e051505b9340
Author: mcasas <mcasas@chromium.org>
Date: Fri May 12 00:42:39 2017

Image Capture: Add README.md file to WebKit/ folder

This CL adds a README.md to modules/imagecapture;
it gathers some links and also addresses some points
that were not evident, as gathered from the experiences
of fbeaufort@.

BUG= 655107 

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

[add] https://crrev.com/6f6c6d53a9eecd08916e55545dd2e051505b9340/third_party/WebKit/Source/modules/imagecapture/README.md

Project Member

Comment 12 by bugdroid1@chromium.org, May 12 2017

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

commit 0ee47102dc41a4336c77ab1489ff5a2601117b86
Author: mcasas <mcasas@chromium.org>
Date: Fri May 12 18:37:48 2017

Image Capture README.md: fix table

This CL fixes the table in imagecapture/README.md: gitiles markdown renderer
likes to have pipes ('|') before the start of the line. ToT [1]  renders it incorrectly.

[1] https://chromium.googlesource.com/chromium/src/+/master/third_party/WebKit/Source/modules/imagecapture/README.md

BUG= 655107 
TBR=scheib@chromium.org

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

[modify] https://crrev.com/0ee47102dc41a4336c77ab1489ff5a2601117b86/third_party/WebKit/Source/modules/imagecapture/README.md

Project Member

Comment 13 by bugdroid1@chromium.org, May 12 2017

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

commit 57b0df585ca73e3c920515ad895a7e68ee0ef8db
Author: mcasas <mcasas@chromium.org>
Date: Fri May 12 20:20:46 2017

Image Capture: yet another README.md table fix

Seems like gitiles needs at least three hyphens to identify whatever as
a table.

BUG= 655107 
TBR=scheib@chromium.org

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

[modify] https://crrev.com/57b0df585ca73e3c920515ad895a7e68ee0ef8db/third_party/WebKit/Source/modules/imagecapture/README.md

Status: WontFix (was: Assigned)
fbeaufort@, please have a look at 
https://chromium.googlesource.com/chromium/src/+/master/third_party/WebKit/Source/modules/imagecapture/README.md


otherwise, I think this is a WontFix, thanks !!
Nit:

Instead of "retrieved by...", how about something like "code sample" or "example" so that we can add `theImageCapturer.takePhoto(photoSettings)` in PhotoSettings row.
#15: trying to convince Gerrit to add the suggestion in 
https://chromium-review.googlesource.com/c/506369/

Sign in to add a comment