Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add WebCodecs VideoFrame as CanvasImageSource. #6589

Merged
merged 8 commits into from
Jul 29, 2021

Conversation

dalecurtis
Copy link
Contributor

@dalecurtis dalecurtis commented Apr 16, 2021

Adding VideoFrame as a CanvasImageSource allows drawImage(),
createImageBitmap(), and texImage() to interoperate with VideoFrames.
VideoFrames are effectively the same as the existing point-in-time
capture done for HTMLVideoElement on each of these interfaces.

This adds a non-normative reference on the WebCodecs spec for the
VideoFrame interface and associated properties to accomplish this.

Bug: w3c/webcodecs#158


/canvas.html ( diff )
/imagebitmap-and-animations.html ( diff )
/infrastructure.html ( diff )
/references.html ( diff )

source Outdated

<ol>
<li><p>Resolve <var>p</var> with <var>imageBitmap</var>.</p></li>
</ol>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this based on existing language? You cannot really resolve something without queuing a task if you're in parallel. It's also unclear why this needs to go in parallel.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just based on the existing language. All the others said this in some form, so I cloned it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@annevk did you want any changes here? Since this is existing language it seems like a separate cleanup CL would be a better place for this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's okay.

@annevk
Copy link
Member

annevk commented Apr 21, 2021

By the way, does a VideoFrame object only ever contain same-origin information?

@dalecurtis
Copy link
Contributor Author

By the way, does a VideoFrame object only ever contain same-origin information?

Yes, VideoFrames are required to be same origin.

@annevk annevk added addition/proposal New features or enhancements topic: canvas labels Apr 28, 2021
@annevk
Copy link
Member

annevk commented Apr 28, 2021

cc @whatwg/canvas

@kenrussell
Copy link
Member

cc @whatwg/canvas

Excited to see this being added to the spec! It'll be a more performant code path than drawing HTMLVideoElement to a canvas.

Copy link
Contributor

@fserb fserb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Adding VideoFrame as a CanvasImageSource allows drawImage(),
createImageBitmap(), and texImage() to interoperate with VideoFrames.
VideoFrames are effectively the same as the existing point-in-time
capture done for HTMLVideoElement on each of these interfaces.

This adds a non-normative reference on the WebCodecs spec for the
VideoFrame interface and associated properties to accomplish this.

Bug: w3c/webcodecs#158
@annevk
Copy link
Member

annevk commented Jul 21, 2021

I pushed some nits and thought this was ready to land, but https://w3c.github.io/webcodecs/#dom-videoframe-display-width-slot does not match the name in this PR (same for the other slot). This is also the first time I'm seeing slots with spaces in them.

@dalecurtis
Copy link
Contributor Author

Whoops, fixed up the names, hopefully the spaces work out okay. If not @chcunningham in case that's a style issue.

@annevk annevk requested a review from domenic July 23, 2021 11:22
@annevk
Copy link
Member

annevk commented Jul 23, 2021

I'll let @domenic make a call on that. @dalecurtis you need to join the googlers organization and make your membership public to satisfy the IPR bot.

@domenic
Copy link
Member

domenic commented Jul 23, 2021

I have no problem with spaces in internal slots. (Besides my eternal slight regret for introducing [[internal slots]] into the web spec ecosystem in the first place, heh.)

@dalecurtis
Copy link
Contributor Author

@annevk the membership should be public now. Thanks!

@annevk
Copy link
Member

annevk commented Jul 29, 2021

@dalecurtis when I go to your profile I see google, but not googlers. Only the latter works for WHATWG IPR.

@dalecurtis
Copy link
Contributor Author

Huh, TIL, that's confusing. I've figured out what that is and have joined accordingly.

@domenic domenic merged commit 57f3bb0 into whatwg:main Jul 29, 2021
@dalecurtis
Copy link
Contributor Author

Thanks everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements topic: canvas
Development

Successfully merging this pull request may close these issues.

6 participants