← 输入框|github.com|刷新
Skip to content

Add HTMLVideoElement to copyExternalImageToTexture - #3410

Merged
kainino0x merged 1 commit into
gpuweb:mainfrom
kainino0x:from-video
Sep 8, 2022
Merged

Add HTMLVideoElement to copyExternalImageToTexture#3410
kainino0x merged 1 commit into
gpuweb:mainfrom
kainino0x:from-video

Conversation

@kainino0x

@kainino0x kainino0x commented Sep 2, 2022

Copy link
Copy Markdown
Contributor

No description provided.

@kainino0x kainino0x added this to the V1.0 milestone Sep 2, 2022
@github-actions

github-actions Bot commented Sep 2, 2022 •
edited by mehmetoguzderin
Loading

Copy link
Copy Markdown
Contributor

Previews, as seen when this build job started (1ce664f):
WebGPU webgpu.idl | Explainer | Correspondence Reference
WGSL grammar.js | wgsl.lalr.txt

@kainino0x kainino0x added the needs-cts-issue This change requires tests (or would need tests if accepted), but may not have a CTS issue filed yet label Sep 8, 2022
@kainino0x
kainino0x merged commit 71c2f07 into gpuweb:main Sep 8, 2022
@kainino0x
kainino0x deleted the from-video branch September 8, 2022 00:25
@kdashg

kdashg commented Sep 9, 2022

Copy link
Copy Markdown
Contributor
GPU Web 2022-09-07/08 APAC-timed
  • KN: proposal meant to make for a long time. Right now, two ways to use a video. importExternalTexture, or to async turn it to ImageBitmap and copy that in. The latter's a weird path - even weirder than I originally thought. Looks more like WebGL, think it's good to add this, for people who need the one-copy path for various reasons, it'll be better than the ImageBitmap path.
  • KG: main reason I see we might not want this - if we want to push people away from the one-copy solution and toward zero-copy solution. Think people will make their own choices and let them do what they want. Polymorphic solution.
  • KN: worth adding that we don't have HTMLImageElement here because it has a good ImageBitmap path, and it's better to turn Blob into ImageBitmap.
  • KG: interesting. I'd rather put HTMLImageElement here too.
  • KN: not much reason to do image element.
  • MM: general for this proposal - we should add HTMLVideoElement to all the things, sounds great.
  • BJ: to understand - don't have a problem with this - wouldn't remove the need for the video -> ImageBitmap -> texture paths, right? Won't reject ImageBitmaps because they came from video?
  • KN: right. It's an efficiency thing. You tell us exactly what you need.
  • KG: in FF in WebGL it's better to preserve fast path from video elements. ImageBitmaps don't know their destination, coming from video directly makes it easier. +1 to directly listing source of truth.
  • KG: I'd like to add HTMLImageElement to this…not difficult.
  • KN: since we have consensus on videos - I don't have a problem with HTMLImageElements specifically. Think there's value in pushing people toward Blob -> ImageBitmap; keeps your image decode entirely asynchronous. Images are async anyway since getting something from the network, so not painful to add another async - this is why we have Canvases here.
  • MM: if app is passing static image data from network to WebGPU - you're saying that's better if it doesn't go through HTMLImageElement?
  • KN: it's better regardless of whether it comes from network, because decode is async. There is some argument to be said - why are we doing the decode here? Want to know what we'll use it for. Some value for adding images here, but not as much.
  • MM: there's an attribute on images to decode async.
  • KG: don't want to remove ImageBitmap, but having other inputs here is useful. We have consensus on this.
  • KN: if you'd like to put up PR for HTMLImageElement please do so.
  • KG: I will.
  • Resolved: merge this, KG to make a follow-on PR.

@kainino0x kainino0x mentioned this pull request Sep 10, 2022
Merged
juj added a commit to juj/wasm_webgpu that referenced this pull request Nov 2, 2022
@juj
Spec update: GPUImageCopyExternalImage can take a HTMLVideoElement: g…
5897327 
…puweb/gpuweb#3410
@lokokung

lokokung commented Dec 6, 2022

Copy link
Copy Markdown
Contributor

Removing 'needs-cts-label' with filing of draft CTS issue named: web_platform: copyToTexture,video:*.

@lokokung lokokung removed the needs-cts-issue This change requires tests (or would need tests if accepted), but may not have a CTS issue filed yet label Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

proposal

Projects

None yet

Milestone

Milestone 0

Development

Successfully merging this pull request may close these issues.

3 participants

@kainino0x @kdashg @lokokung