Re: [mediacapture-transform] MSTP's VideoFrame lifetime management model assumes a single consumer (#56)

> 1. Assumes a single consumer per VideoFrame

I think it's fair to say the current design is not optimized for the tee scenario you outlined. But I wouldn't go so far as to say "assumes a single consumer", since you can always clone() the frame as @aboba points out.

You could workaround the tee() limitation by implementing a custom FrameTee that reads from the original stream, and writes to two output streams, cloning the frames as needed for the second stream. 

> 2. Creates a footgun where consumers may forget to call this method, with potentially devastating OS-level side-effects

It's definitely critical that folks close() their frames. The WC spec attempts to highlight this and Chrome's impl actually console warns if we observe GC for a VideoFrame that isn't already closed. 

I'm not sure it's as bad as OS-level side-effects. Chrome's pool of camera frames is currently a fixed size thing, so it is technically possible for one bad tab to ruin the experience for other tabs. We have some basic protections in place, but the long term plan is to simply make the pool dynamic and limit each tab's number of frames such that a tab may harm itself by leaking frames, but it can't harm other tabs or the larger OS. This would follow the same pattern as OOM for general JS... tab is to OOM itself, but it doesn't bring down the system.

> Good observation. This could be solved if the MSTP owned the VideoFrames and was responsible for their cleanup.

How would MSTP know when it was safe to close() the frame. Is the idea that you allow a single pass of the main thread to use the frame after its read from the ReadableStream? I'm not immediately sure if that is architecturally (from streams spec pov) feasible. Such a design would also have footguns. 

> [BA] MSTP can't be the only owner if there are multiple references (e.g. if VideoFrame.clone() was called). In that case, even an implicit close() won't reclaim the VideoFrame.

Agreed. Authors are still on the hook to close() their own clones. 


-- 
GitHub Notification of comment by chcunningham
Please view or discuss this issue at https://github.com/w3c/mediacapture-transform/issues/56#issuecomment-897250321 using your GitHub account


-- 
Sent via github-notify-ml as configured in https://github.com/w3c/github-notify-ml-config

Received on Thursday, 12 August 2021 00:20:53 UTC