WebGPU: Add fromPixelsAsync for tfjs webgpu #4139
Conversation
|
@qjia7 Please take a look if you have time |
| const outShape = [pixels.height, pixels.width, numChannels]; | ||
| const size = util.sizeFromShape(outShape); | ||
| const uniformData: [number, number] = [size, numChannels]; | ||
| //let imageData = (pixels as ImageData | backend_util.PixelData).data; |
There was a problem hiding this comment.
If above line is not needed, remove it.
There was a problem hiding this comment.
Sry. Force update last CL and it has been covered. This comments has been resolved.
tfjs-core/src/ops/browser.ts
Outdated
| return await kernel.kernelFunc({inputs, attrs, backend: ENGINE.backend}) as Tensor3D; | ||
|
|
||
| } else { | ||
| throw new Error( |
There was a problem hiding this comment.
How about calling the sync FromPixels here instead of throwing an error? Also you can put this checking at the beginning of this function.
tfjs-core/src/ops/browser.ts
Outdated
| [pixels.width, pixels.height]; | ||
| let vals: Uint8ClampedArray|Uint8Array; | ||
| [ | ||
| (pixels as HTMLVideoElement).videoWidth, |
There was a problem hiding this comment.
Why you need to change the original fromPixels_ function?
| return; | ||
| } | ||
|
|
||
| // TODO(tfjs-optimization): using new mapAsync API to update the buffer. |
There was a problem hiding this comment.
I suppose this comment should be added in line 99. Just curious, why do you think mapAsync is better than writeBuffer?
There was a problem hiding this comment.
This is due to the staging buffer. But actually for the samll sized uniform data. I think they have similar perf. (BTW, this is the legancy comments from last related patch :P) I'll remove it.
This patch add fromPixelsAsync for tfjs webgpu to handle: 1. Convert inputs to imageBitmap in a proper way 2. Using CopyImageBitmapToTexture + shader to handle inputs.
shaoboyan
force-pushed
the
fromPixelsAsync
branch
from
0256d09
to
bb0585c
Compare
tfjs-backend-webgpu/src/kernels/FromPixels_utils/from_pixels_webgpu.ts
Outdated
Show resolved
Hide resolved
|
Add @annxingyuan @kainino0x take a look, thanks. |
| } | ||
|
|
||
| const uniformBuffer = device.createBuffer({ | ||
| size: 4 * 2, |
There was a problem hiding this comment.
size: uniformData.byteLength? Please avoid to use the raw data.
There was a problem hiding this comment.
uniformData is a normal array and it doesn't have byteLength. So I have to use uniformData.length * 4. Maybe @annxingyuan and @kainino0x have some suggestions.
| import {BROWSER_ENVS, describeWithFlags} from '../jasmine_util'; | ||
| import {expectArraysClose, expectArraysEqual} from '../test_util'; | ||
|
|
||
| describeWithFlags('fromPixelsAsync', BROWSER_ENVS, () => { |
There was a problem hiding this comment.
One issue here is that I don't know how to add test case to check the wrong input and catch the exception. Like https://github.com/tensorflow/tfjs/blob/master/tfjs-core/src/ops/from_pixels_test.ts#L248. @qjia7 @annxingyuan @kainino0x Do you have any suggestions?
|
Ping @annxingyuan @kainino0x . Do you have any concern for current change? |
There was a problem hiding this comment.
I have not closely reviewed, but I trust @qjia7 's LGTM :) Thank you @shaoboyan !
This patch add fromPixelsAsync for tfjs webgpu to handle:
This change is