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

Issue 630040 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

ImageCapture: use mojo typemappings to identify parameter(s) that should be moved (and not copied).

Project Member Reported by mcasas@chromium.org, Jul 20 2016

Issue description

In the new mojo world where generated bindings use stl string/vector
(or WTF variants),  https://crbug.com/624136 , a mojom such as the 
image_capture one saying [1]:

    TakePhoto(string source_id)
        => (string mime_type, array<uint8> data);

generates a callback with a const std::vector<uint8_t>& for
second parameter, and that prevents std::move()ing things 
around and into a base::Callback() (this came up during 
https://crrev.com/2166713002/).

Instead of using an evil const_cast, yzshen@ proposed the
following alternative:

<quote>

(1) define a mojom struct
// in mojom:
struct SomeData {
  array<uint8> data;
};

(2) create a custom typemapping between SomeData and
std::unique_ptr<std::vector<uint8_t>>, in the typemap config annotate that this
type is move-only:

type_mappings = [ "SomeData=std::unique_ptr<std::vector<uint8_t>>[move_only]" ]

</quote>

[1] https://cs.chromium.org/chromium/src/media/mojo/interfaces/image_capture.mojom?q=image_capture.mojom&sq=package:chromium&dr&l=51
 

Comment 1 by mcasas@chromium.org, Jul 20 2016

Components: Blink>MediaStream>ImageCapture
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 23 2016

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

commit 431ae31e66ce1251570828f5a2c2773696774d58
Author: mcasas <mcasas@chromium.org>
Date: Sat Jul 23 03:30:16 2016

ImageCapture: introduce image_capture Blob to allow move-only semantics

https://crrev.com/2166713002 introduce usage of STL/WTF
types for image_capture. But that translated bindings of
 array<> to const std::vector<>& which forced strange
const_cast<>s to avoid copies.

This CL bundles mime_type and data into a mojo::Struct
that is move-only, hence avoiding the copies of, potentially
large, photo frames.

BUG= 630040 

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

[modify] https://crrev.com/431ae31e66ce1251570828f5a2c2773696774d58/content/browser/media/capture/image_capture_impl.cc
[modify] https://crrev.com/431ae31e66ce1251570828f5a2c2773696774d58/media/capture/video/android/video_capture_device_android.cc
[modify] https://crrev.com/431ae31e66ce1251570828f5a2c2773696774d58/media/capture/video/fake_video_capture_device.cc
[modify] https://crrev.com/431ae31e66ce1251570828f5a2c2773696774d58/media/capture/video/fake_video_capture_device_unittest.cc
[modify] https://crrev.com/431ae31e66ce1251570828f5a2c2773696774d58/media/capture/video/mac/video_capture_device_mac.mm
[modify] https://crrev.com/431ae31e66ce1251570828f5a2c2773696774d58/media/capture/video/video_capture_device.h
[modify] https://crrev.com/431ae31e66ce1251570828f5a2c2773696774d58/media/capture/video/video_capture_device_unittest.cc
[modify] https://crrev.com/431ae31e66ce1251570828f5a2c2773696774d58/media/mojo/interfaces/image_capture.mojom
[modify] https://crrev.com/431ae31e66ce1251570828f5a2c2773696774d58/third_party/WebKit/LayoutTests/imagecapture/resources/mock-imagecapture.js
[modify] https://crrev.com/431ae31e66ce1251570828f5a2c2773696774d58/third_party/WebKit/LayoutTests/imagecapture/takephoto.html
[modify] https://crrev.com/431ae31e66ce1251570828f5a2c2773696774d58/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp
[modify] https://crrev.com/431ae31e66ce1251570828f5a2c2773696774d58/third_party/WebKit/Source/modules/imagecapture/ImageCapture.h
[modify] https://crrev.com/431ae31e66ce1251570828f5a2c2773696774d58/third_party/WebKit/Source/modules/imagecapture/PhotoCapabilities.idl

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

Components: -Blink>MediaStream>ImageCapture Blink>ImageCapture

Comment 4 by scheib@chromium.org, Apr 20 2017

Labels: Pri-3
Issues not modified in last 50 days aren't on track to ship in next release.

Comment 5 by mcasas@chromium.org, May 11 2017

Labels: -518807

Comment 6 by mcasas@chromium.org, Jul 18 2017

Status: WontFix (was: Assigned)
I guess we can live with Blob, anyway we needs its Mime Type.

https://cs.chromium.org/chromium/src/media/capture/mojo/image_capture.mojom?q=imagecapture+mojom&sq=package:chromium&dr=C&l=116

Sign in to add a comment