u459114 Reporter
	Description • 11 years ago

ImageCapture is a W3C spec to enable web client to take a photo and set camera setting.
With getUserMedia/ MediaRecording/ MediaCapture support, we can remove mozCamera API.

Spec:
https://dvcs.w3.org/hg/dap/raw-file/tip/media-stream-capture/ImageCapture.html

	Shelly Lin [:shelly]
	Comment 1 • 11 years ago

Implemented the webidl of ImageCapture, but excluded the part of PhotoSettingsOptions for now.

	Shelly Lin [:shelly]
	Comment 3 • 11 years ago

Update:

-From w3c-
Giri will put up a new version soon.

- From Rob -
http://lists.w3.org/Archives/Public/public-media-capture/2013Mar/0118.html

	Shelly Lin [:shelly]
	Comment 4 • 11 years ago

A actually workable patch for the function takePhoto(), but the webidl is not yet updated to the latest version of ImageCapture.

	u459114 Reporter
	Comment 5 • 11 years ago

Comment on attachment 803497 [details] [diff] [review]
[WIP] WebIDL and function takePhoto()

Review of attachment 803497 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/ImageCapture.cpp
@@ +46,5 @@
> +    {
> +      MOZ_ASSERT(NS_IsMainThread());
> +      mCapture->mWorkerThread->Shutdown();
> +      mCapture->mWorkerThread = nullptr;
> +

Why don't just keep use the same mCaptureThread to execute TakePhotoTask::Run?

@@ +180,5 @@
> +      aResult.Throw(rv);
> +      return;
> +    }
> +    nsRefPtr<TakePhotoTask> event = new TakePhotoTask(this);
> +    mWorkerThread->Dispatch(event, NS_DISPATCH_NORMAL);

Move these two lines out of if (!mWorkerThread) statement?

@@ +204,5 @@
> +  nsTArray<uint8_t> argbImgBuf;
> +  gfxIntSize size;
> +  int stride;
> +  mVideoFrameAdapter->GetARGBImage(&argbImgBuf, &size, &stride);
> +

RGB or ARGB? I don't think the image from camera composed alpha channel

@@ +206,5 @@
> +  int stride;
> +  mVideoFrameAdapter->GetARGBImage(&argbImgBuf, &size, &stride);
> +
> +  nsCOMPtr<imgIEncoder> encoder =
> +      do_CreateInstance("@mozilla.org/image/encoder;2?type=image/png");

Make encoder as a data member of ImageCapture take more memory, but good for performance.

@@ +223,5 @@
> +  uint32_t outputBufLen;
> +  encoder->GetImageBufferUsed(&outputBufLen);
> +  outputBuf.SetLength(outputBufLen);
> +
> +  char* encodedImgBuf;

char* encodedImgBuf = nullptr

	u459114 Reporter
	Comment 6 • 11 years ago

Encode image from camera preview or record callback can not get high quality image.

	Florian Bender
	Comment 7 • 10 years ago

Is there a bug to enable this in unpriviliged content by default?

	Alfredo Yang (:alfredo)
	Comment 8 • 10 years ago

(In reply to Florian Bender from comment #7)
> Is there a bug to enable this in unpriviliged content by default?

No.
ImageCapture spec could be changed a lot in W3C discussion; for example, using Promise instead of event callback. It'd be better to wait for spec is more mature.

	[:fabrice] Fabrice Desré
	Comment 9 • 9 years ago

Alfredo, is the new spec the one at https://w3c.github.io/mediacapture-image/ now?

	Alfredo Yang (:alfredo)
	Comment 10 • 9 years ago

Yes.

	Maire Reavy [:mreavy]
	Comment 11 • 9 years ago

alfredo - where do you think this spec stands, regarding being ready for implementation?

	Alfredo Yang (:alfredo)
	Comment 12 • 9 years ago

IMHO, the spec status is not good. Only mozilla has implemented it so far. Questions raised by us are unsolved. So the implementation is pending.

	ddascalescu+github
	Comment 13 • 8 years ago

Chrome on Android has implemented ImageCapture behind a flag, according to https://bugs.chromium.org/p/chromium/issues/detail?id=603328#c6

	ddascalescu+github
	Comment 14 • 8 years ago

grabFrame() works in the current Chrome Dev on Linux and Mac.

	beaufort.francois
	Comment 15 • 8 years ago

Here's a patch that renames videoStreamTrack to track in the ImageCapture API as discussed in https://github.com/w3c/mediacapture-image/issues/87

	Mounir Lamouri (:mounir)
	Comment 16 • 8 years ago

François, I think it would be better to create a new bug with this patch. Feel free to CC me and I can do a first round of review for you.

	Jan-Ivar Bruaroey [:jib] (needinfo? me)
	Comment 17 • 8 years ago

Comment on attachment 8800544 [details] [diff] [review]
s/videoStreamTrack/track/g in ImageCapture API

Review of attachment 8800544 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/webidl/ImageCapture.webidl
@@ +13,4 @@
>  [Pref="dom.imagecapture.enabled", Constructor(VideoStreamTrack track)]
>  interface ImageCapture : EventTarget {
>    // readonly attribute PhotoSettingsOptions photoSettingsOptions;
> +  readonly attribute VideoStreamTrack track;

Just a quick note, we should align with the spec and use MediaStreamTrack here, and remove VideoStreamTrack entirely [1].

[1] https://dxr.mozilla.org/mozilla-central/source/dom/webidl/VideoStreamTrack.webidl#14

	Bulk Bug Changes for mreavy's org
	Comment 18 • 7 years ago

Mass change P3->P4 to align with new Mozilla triage process.

	reda.housnialaoui
	Comment 19 • 7 years ago

Since Chrome 62, ImageCapture is active without the need of a flag.

Are you still waiting for the spec to stabilize?
I think that many of us want to use this feature even in the current state.

	Blake Wu [:bwu][:blakewu]
	Comment 20 • 7 years ago

Unassigned Alfredo since he is not working on this. If anyone is interested in fixing this bug, please feel free to take it. :-)

	Blake Wu [:bwu][:blakewu]
	Comment 21 • 7 years ago

(In reply to reda.housnialaoui from comment #19)
> Since Chrome 62, ImageCapture is active without the need of a flag.
> 
> Are you still waiting for the spec to stabilize?
> I think that many of us want to use this feature even in the current state.
Can you tell us what kind of use cases you like to apply this to? 
I believe if there are many strong requests/demands, people in Firefox would reconsider its priority.

	reda.housnialaoui
	Comment 22 • 7 years ago

Hello blakewu,

We have a computer vision use case requiring to capture a very high resolution photo on an end consumer Camera.
Many cameras are able to capture a video stream using a maximal resolution of 1920*1080.
But some of them are also able to capture a still image at much higher resolution (~3200*2400 for example).

Today in Firefox, we are only able to grab a frame from the video stream. 
Therefore we cannot capture very high quality image using Firefox.

I mentioned the resolution, but I think that abilities to control the zoom, the exposure, the light, etc ... are expected by those who work in computer vision.

Chrome is not yet ok on its windows media foundation implementation (which is the one that I am interested in).
I proposed a patch in https://bugs.chromium.org/p/chromium/issues/detail?id=730068 to fix it.

I didn't take a look at Firefox source code.
Do W3C image capture abstractions already exist in the code?

	Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018
	Comment 23 • 7 years ago

It has been implemented but hidden with ther pref dom.imagecapture.enabled
Unfortunately, current implementation only provides video resolution (the same as what you have via grabbing a frame from video)

	reda.housnialaoui
	Comment 24 • 7 years ago

I looked quickly at the source code.

From what I understand, the first thing to do is to implement takePhoto defined in MediaStreamTrack.h [1] into dom/media/MediaManager.cpp/LocalTrackSource [2].

[1] https://dxr.mozilla.org/mozilla-central/source/dom/media/MediaStreamTrack.h#117
[2] https://dxr.mozilla.org/mozilla-central/source/dom/media/MediaManager.cpp#1080

	reda.housnialaoui
	Comment 25 • 7 years ago

But I didn't find any abstraction allowing to:
- retrieve photo capabilities
- pass photo settings to the takePhoto

Am I missing something?

	Andreas Pehrson [:pehrsons]
	Comment 26 • 7 years ago

We used to support it better for B2G (Firefox OS), but it has since been removed from the tree. You can see the latest version at https://hg.mozilla.org/mozilla-central/file/0a5cf829f56e/dom/media/webrtc/MediaEngineGonkVideoSource.cpp.