-
Notifications
You must be signed in to change notification settings - Fork 20
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
Track transferability requirement adds complexity to some common use cases #116
Comments
How does it relate to #113? |
Some quick notes:
|
Transfer does not require cloning. It's the before/after use case showing the original camera track that needs it. Without that, no clone is needed (and one less track to stop).
This issue focuses on the web developer aspect only, phrased as a problem with the existing spec. Thanks @guidou!
Agree. 25 vs 40 lines doesn't seem enough to justify a bespoke API.
@youennf's tip in #117 (comment) lets us simplify further (32 lines): // main.html
function setUpWorker() {
const worker = new Worker("worker.js");
worker.onmessage = async ({data}) => {
processedVideo.srcObject = new MediaStream([data.track]);
await processedVideo.play();
}
}
async function startCapture() {
const stream = await navigator.mediaDevices.getUserMedia({video:true});
originalVideo.srcObject = stream.clone();
await originalVideo.play();
const [track] = stream.getVideoTracks();
worker.postMessage({track}, [track]);
}
function stopCapture() {
[originalVideo, processedVideo].forEach(v => v.srcObject.getTracks()[0].stop());
}
// worker.js
onmessage = async ({data: {track}}) => {
const processor = new MediaStreamTrackProcessor({track});
const generator = new VideoTrackGenerator();
postMessage({track: generator.track}, [generator.track]);
try {
await processor.readable.pipeThrough(GetTransform()).pipeTo(generator.writable);
} finally {
track.stop();
}
} |
This is a common use case, and there are many others that require or prefer the track on window and don't need or want a track in the worker. The current API shape forces a clone in such cases, which shouldn't be necessary.
You're including code that is common to both cases in your counts, which makes the relative difference look smaller. Also, the difference in complexity is not just line counts, but also the complexity of the abstractions involved. With the track split on two execution contexts, what used to be just a track for the application has to be modeled differently. Now you have to think in terms of an abstract track of which there are two replicas running on two different execution contexts, so you need to keep track of all those objects and define a consistency protocol. Some use cases where this additional complexity is necessary might exist, but today there are a lot use cases where this extra complexity isn't needed and it just makes life difficult for developers. |
Seems more common not to have a "before" preview: None of the big sites with video effects have it.
Which websites require the "before" video on window? How do you know what they "prefer"? As #117 clarified, having a track tucked away in the worker using it is neither a burden nor a liability. It's quite clean: it lets the worker stop the source once piping ends. This cleanly reacts to downstream JS closing the "after" track, regardless of what thread it's on.
A "before" preview element is a separate sink with potentially separate concerns. Using clone to separate those concerns should be encouraged. For instance, tying their lifespans together seems like a mistake. In contrast, your alternative API seems flawed:
IOW leaving the window to manage the source discourages encapsulation and encourages repurposing without cloning, creating tight coupling that may surprise developers. |
Let's compare without a "before" preview element, which seems more realistic. Current spec (22 lines): start.onclick = async () => {
const stream = await navigator.mediaDevices.getUserMedia({video: true});
const [before] = stream.getVideoTracks();
const worker = new Worker(`data:text/javascript,(${work.toString()})()`);
worker.postMessage({before}, [before]);
const {data: {after}} = await new Promise(r => worker.onmessage = r);
video.srcObject = new MediaStream([after]);
stop.onclick = () => after.stop();
};
function work() {
onmessage = async ({data: {before}}) => {
const {writable, track} = new VideoTrackGenerator();
self.postMessage({after: track}, [track]);
const {readable} = new MediaStreamTrackProcessor({track: before});
try {
await readable.pipeThrough(new TransformStream({transform})).pipeTo(writable);
} finally {
before.stop();
}
};
} vs. bespoke API (17 lines): start.onclick = async () => {
const stream = await navigator.mediaDevices.getUserMedia({video: true});
const [before] = stream.getVideoTracks();
const worker = new Worker(`data:text/javascript,(${work.toString()})()`);
const after = navigator.mediaDevices.createVideoTrackGeneratorAndProcessor(worker, before);
video.srcObject = new MediaStream([after]);
stop.onclick = () => {
before.stop();
after.stop();
}
};
function work() {
oncapturetransform = async ({processor, generator}) => {
await processor.readable.pipeThrough({transform}).pipeTo(generator.writable);
}
} Two new redundant APIs ( |
Are you insinuating this use case shouldn't be supported?
All VC sites I know of allow you to switch between the processed and unprocessed track when configuring which effects you want to apply. They don't necessarily show both at the same time, but they allow you to switch between effects/no effects interactively and both tracks are on Window at the same time. Demos and tools for developing effects are more likely to show the before and after views simultaneously. Also, all Web sites that do video processing for MediaStreamTracks do it with the track on Window. They keep both the processed and unprocessed tracks on Window, regardless of whether they show the unprocessed one or not. Can you mention one that is doing it with the track on the worker, without at least having a clone on Window?
That's at best an opinion unsupported by any evidence.
For the use case when the application wants to stop the gUM track from the Window, this is a better design. This covers all applications I'm aware of. Also, the proposal does not prevent a hypothetical application that wants to stop the gUM track from the Worker from transferring the track to the Worker and managing its lifetime here. I am just not aware of any such application.
Why is managing one track on Window and one on Worker better, especially if all the application logic is on Window?
If that's what the application wants I don't see why it should be forbidden. My proposal does not prevent cloning and transferring. It just doesn't force it. Your argument seems to be that forcing it is a good idea.
That contradicts real world applications, where all the track lifetime management and other logic is on Window and the worker is used only for processing logic. |
No, I'm saying it's one of several use cases, is already supported, and therefore doesn't need its own redundant API. The argument for a dedicated API seems to rest solely on the existing API creating significant hardship. I believe I've shown with multiple working demos and shims in this issue and #113 that the difference in behavior is quite small and manageable. |
In many use cases it is better to keep all tracks on Window and use the worker just for the processing.
Currently, mediacapture-transform requires transferring the track to be processed to the worker, which adds complexity to these use cases.
Consider the use case of a before/after preview window where the original camera track is shown together with the processed track. With the spec as currently written, something like this is needed (omitting some variable declarations, for simplicity):
Compare to how it would be without forcing track transferability:
Even though it is a minimal demo, the difference in complexity is considerable. Forcing track transferability requires cloning the camera track and a communication protocol between Worker and Window to keep the state consistent between them, all of which is unnecessary if the tracks can be kept on Window.
Note that this issue is not about advocating for removing track transferability. Use cases where transferring tracks to a worker makes sense can continue to be supported without problem even if this issue is addressed.
A runnable implementation of this demo (spec version) is available at https://guidou.github.io/mct/spec-main.html
Source code for the spec and proposed variants is available at https://github.com/guidou/guidou.github.io/tree/master/mct
The text was updated successfully, but these errors were encountered: