-
Notifications
You must be signed in to change notification settings - Fork 64
Create TextureStream.md #2743
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
base: main
Are you sure you want to change the base?
Create TextureStream.md #2743
Changes from 1 commit
514839e
d896d6b
36a9128
e277953
d28f0a3
99ab9c0
bc5d267
a28ec23
0392021
cf1d3c2
b2b6932
c86906b
37ca44d
a8889e2
d8ffcb0
f01f594
4327402
370337a
fb9f997
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,353 @@ | ||||||||||
|
|
||||||||||
sunggook marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||
| # Background | ||||||||||
| Many native apps use a native engine for real-time communication scenarios, which include video | ||||||||||
| capture, networking and video rendering. However, often, these apps still use WebView or | ||||||||||
| Electrion for UI rendering. The separation between real-time video rendering and UI rendering | ||||||||||
sunggook marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||
| prevents apps from rendering real-time video inside the web contents. This forces apps to | ||||||||||
| render the real-time video on top of the web contents, which is limiting. Rendering video on | ||||||||||
| top constrains the user experience and it may also cause performance problems. | ||||||||||
| We can ask the native apps to use web renderer for video handling because web standard already | ||||||||||
| provides these features through WebRTC APIs. The end developers, however, prefer to use | ||||||||||
| their existing engine such as capturing and composition, meanwhile using WebRTC API for rendering. | ||||||||||
|
|
||||||||||
| # Description | ||||||||||
| The proposed APIs will allow the end developers to stream the captured or composed video frame to | ||||||||||
| the WebView renderer where Javascript is able to insert the frame to the page through W3C standard | ||||||||||
| API of Video, MediaStream element for displaying it. | ||||||||||
| The API will use the shared GPU texture buffer so that it can minimize the overall cost with | ||||||||||
| regards to frame copy. | ||||||||||
|
|
||||||||||
| # Examples | ||||||||||
| Javascript | ||||||||||
sunggook marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||
| // User click the video capture button. | ||||||||||
| document.querySelector('#showVideo').addEventListener('click', | ||||||||||
| e => getStreamFromTheHost(e)); | ||||||||||
| async function getStreamFromTheHost(e) { | ||||||||||
| try { | ||||||||||
| // Request stream to the host with unique stream id. | ||||||||||
| const stream = await window.chrome.webview.getTextureStream('webview2-abcd1234'); | ||||||||||
|
||||||||||
| // The MediaStream object is returned and it gets video MediaStreamTrack element from it. | ||||||||||
| const video_tracks = stream.getVideoTracks(); | ||||||||||
| const videoTrack = video_tracks[0]; | ||||||||||
|
||||||||||
| // Show the video via Video Element. | ||||||||||
| document.getElementById(video_id).srcObject = stream; | ||||||||||
|
||||||||||
| } catch (error) { | ||||||||||
| console.log(error); | ||||||||||
| } | ||||||||||
| } | ||||||||||
sunggook marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||
| Win32 C++ | ||||||||||
|
||||||||||
| Win32 C++ | |
| ## Win32 C++ | |
| ```cpp |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many calls throughout here are missing error handling - aren't checking the HRESULT return value. In our sample code we have a CHECK_HRESULT (or something like that) macro we use.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually in WebView2, object creation happens on the CoreWebView2Environment as it acts sort of like a factory. Then the object may have additional initialization (like setting up event handlers) and then add or use the object with a CoreWebView2.
Here it looks like the Create method is on the CoreWebView2 and creating it also adds it (by the name parameter) to available texture streams for that CoreWebView2. Is the TextureStream tied to that CoreWebView2 in particular? Or can we move the TextureStream creation to the CoreWebView2Environment and have a separate method for 'adding' it to a CoreWebView2? If so this would have the benefits of:
- Clearer from API calls when the TextureStream has been added to a WebView2.
- No concern over races where the TextureStream has been created with a particular name, but the event handlers and such haven't been setup yet.
- Able to use the same one TextureStream object with different CoreWebView2s or the same CoreWebView2 but with different names.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| [hWnd](ICoreWebView2StagingTextureStream* webview, IUnknown* eventArgs) -> HRESULT { | |
| [hWnd](ICoreWebView2StagingTextureStream* textureStream, IUnknown* eventArgs) -> HRESULT { |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this method that we don't show what its doing, going to interact with the TextureStream by writing frames to it or something? If so, we should show that method or parts of that method. The sample should show you how to interact with the TextureStream.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly we should show what's happening in here. As is, I don't really know what I'm supposed to do with this event.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The goal is for this to be code that shows how to use this api end to end. We should have working error handling. How would we expect errors like this to be handled? Displayed to user in native UI or web UI or something else?
sunggook marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need ``` to end code sample block here
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| [v1_enum] | |
| ```c# (but really IDL) | |
| [v1_enum] |
sunggook marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
sunggook marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that only frames within this CoreWebView2? Or does it apply across any CoreWebView2 associated with that browser process?
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've had issues in the past with APIs intending to work with iframes that are a different origin than the top level document's origin not working in that case. Please make sure to test that
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please document the manner in which it will fail.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about naming this AddAllowedOrigin instead? (And the corresponding Remove method)
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if it takes longer than 10s? Is this a requirement by our code or just a suggested goal for end developers?
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does it mean to fulfill the stream request? Is the JS getTextureStream call waiting until the first Present call before it returns the MediaStream?
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly to above, Create* methods usually go on the CoreWebView2Environment. We should consider separating Create from the Add/Register like mentioned above. If we keep them together at the very least it should be named CreateAndAddBuffer or something like that. Just naming something Create doesn't suggest that there will be registration as well.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You call it a buffer in the parameter name and in the Create method name, but the type name is Texture. How about making the type name and Create name match
- TextureStreamBuffer?
- TextureStreamTexture?
- TextureStreamFrame?
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great note that this can be called on any thread. Please make sure all methods and properties that work on other threads are noted as working on other threads. Its stated generally that WebView2 methods only work on the WebView2 UI thread so we need to explicitly call out anything that doesn't work like that.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does GetAvailableBuffer do? Its documentation above needs to explain.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is SetBuffer and Present different methods? Why doesn't Present take the parameters of SetBuffer and present them?
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the point of the Stop method? Isn't the native code in charge of calling Present and CreateBuffer? Why does it call Stop versus just no longer streaming data to the TextureStream? Does this do something to the corresponding JavaScript objects?
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comments doesn't seem to apply to add_TextureError does it?
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Event names should be verb phrases like ErrorOccurred or ErrorDetected
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If these are general TextureStream errors then we don't need the 'Texture' name prefix.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does update mean in this context? Is it obvious to someone familiar with D3D what Update would mean here? Can you be more specific in this comment about what Update means?
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be a property instead right? [propget] HRESULT Buffer([out, retval] ICoreWebView2StagingTexture** value);
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And add the ``` to end the idl block here
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This document is missing the MIDL3 API definition and its missing the WinRT/.NET C# sample code
Uh oh!
There was an error while loading. Please reload this page.