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

Life-time management of VideoFrame when used with streams #1185

Open
youennf opened this issue Nov 16, 2021 · 6 comments
Open

Life-time management of VideoFrame when used with streams #1185

youennf opened this issue Nov 16, 2021 · 6 comments

Comments

@youennf
Copy link
Contributor

youennf commented Nov 16, 2021

VideoFrame objects require to call close so as to clean resources in advance of garbage collection.
This is important as VideoFrame may be big and scarce resources.
Streams currently rely on garbage collection to do the lifetime management, which is an important mismatch with VideoFrame model.

The first consequence is that it is unclear what the API contract is when dealing with streams of VideoFrame.
When reading a ReadableStream, should it be the source or the reader that call close.
Ditto for WritableStream.

It would be good to get a clear understanding of what is desired, document the ideal API contract and, if possible, enforce part or all of the contract in the API and algorithms.

The second issue is that, if a stream has VideoFramed being queued, and the stream gets aborted, we want the VideoFrames to be closed, or at least leave the opportunity for the web application to do that clean-up.

@dalecurtis
Copy link

dalecurtis commented Apr 4, 2023

https://github.com/whatwg/streams/blob/main/streams-for-raw-video-explainer.md#transferring-ownership-streams-explained was a proposal for addressing this. Did anything more come from these conversations? This was highlighted by @tidoust as one of the more painful issues he encountered: https://lists.w3.org/Archives/Public/public-media-wg/2023Mar/0010.html

@youennf
Copy link
Contributor Author

youennf commented Apr 4, 2023

I think the plan is pretty clear (as described in the raw video explainer).
It is now mostly a matter of prioritizing the editing and implementation work.
I can probably help with the editing work.

@dalecurtis
Copy link

Okay, I wasn't sure if we had consensus on the direction, but it seems reasonable to me. @ricea @domenic in case they have opinions.

@ricea
Copy link
Collaborator

ricea commented Apr 5, 2023

I think the plan is pretty clear (as described in the raw video explainer). It is now mostly a matter of prioritizing the editing and implementation work. I can probably help with the editing work.

I'm not certain whether we want a new kind of controller or a flag on the existing DefaultController that enables transfers. However, if you could draft something then that would be a big help in seeing what approach is simpler.

@youennf
Copy link
Contributor Author

youennf commented Apr 9, 2023

I was thinking of just adding a new flag on default controllers.
I did an initial pass (ReadableStream only for now) at #1271.
This approach looks ok to me. Thoughts?

@ricea
Copy link
Collaborator

ricea commented Apr 10, 2023

I made some comments over there but in general it looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants