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

Expose flow control information to transform #206

Closed
alvestrand opened this issue Sep 29, 2023 · 7 comments · Fixed by #207
Closed

Expose flow control information to transform #206

alvestrand opened this issue Sep 29, 2023 · 7 comments · Fixed by #207
Labels
enhancement New feature or request

Comments

@alvestrand
Copy link
Contributor

Discucssion of the late fanout use case at TPAC 2023 led to the realization that for this use case, and also for others such as "additional frame-level metadata", we need to expose congestion control signals from the transport via the packetizer to the transform, and allow them to be modified before (possibly) passing them on to the encoder (or, in the late fanout use case, to the incoming stream that it is doing fanout for).

Similarly, there may be signals that a decoder pipeline wishes to pass on to the depacketizer and to the transport, such as a keyframe request - these signals should also be exposed at the Javascript API.

A first suggestion would be to expose a series of events (one per feedback type identified) on the downstream interface of the transform, with a corresponding series of methods on the upstream interface of the transform that would take the same parameters as the content of the incoming events; this would make "default handling" easy by connecting the event directly to the handler if the transform did not desire to do any processing of them.

@alvestrand
Copy link
Contributor Author

This resembles the proposal outlined in https://github.com/alvestrand/hackathon-encoded-media

@youennf
Copy link
Collaborator

youennf commented Sep 29, 2023

RTCRtpScriptTransformer seems like an appropriate place for such API. If using events, Event.preventDefault() could be used to disable the default handling.

For instance, a keyFrameRequested event could be defined and fired when receiving PLI/FIR. Default handling would be to call the generate key frame algorithm. Web app could decide to be smarter, by calling preventDefault() on the event and then call itself generateKeyFrame() whenever appropriate.

@steely-glint
Copy link

Side note here on keyFrameRequested :

We found that you can get repeated keyFrameRequests which you need to ignore when when you know a keyframe is still inflight. Likewise you probably want to remove any frames in an output queue if they predate the keyFrame. So keyFrameRequested handling is by no means stateless.
Which might make it an awkward match for an event.

@youennf
Copy link
Collaborator

youennf commented Sep 29, 2023

By default, it is up to the UA to not trigger too many key frames.
If the web app wants to handle it itself, it could be something like:

let isKeyFrameInflight = false;
const reader = transformer.readable.getReader();
const writer = transformer.writable.getWriter();
transformer.onKeyFrameRequest = async (e) => {
    e.preventDefault();
    // Do not ask for multiple key frames.
    if (isKeyFrameInflight)
        return;
    isKeyFrameInflight = true;
    await transformer.generateKeyFrame();
    isKeyFrameInflight = false;
}

And skipping of frames could be done like:

async function doTransform()
{
    const chunk = await reader.read();
    if (chunk.done)
        return;
    // Do not write chunks if we are waiting for a key frame.
    if (!isKeyFrameInflight) {
        await transformChunk(chunk.value);
        await writer.write(chunk.value);
    }
    return doTransform();
}

Does this look ok?

@steely-glint
Copy link

Yep, looks good.

However you now have to require that transformer.generateKeyFrame() returns and resets isKeyFrameInflight before the first keyframe chunk is given to doTransform() otherwise you'll drop the keyframe.

@youennf
Copy link
Collaborator

youennf commented Sep 29, 2023

However you now have to require that transformer.generateKeyFrame() returns and resets isKeyFrameInflight before the first keyframe chunk is given to doTransform() otherwise you'll drop the keyframe.

Right!
And this is the current intent of the spec and WebKit implementation:

By resolving the promises just before enqueuing the corresponding key frame in a [RTCRtpScriptTransformer](https://w3c.github.io/webrtc-encoded-transform/#rtcrtpscripttransformer)'s readable, the resolution callbacks of the promises are always executed just before the corresponding key frame is exposed.

The intent is also for generateKeyFrame to be a no-op if a previous generateKeyFrame call did not yet resolve (see [[pendingKeyFrameTasks]] management).

@alvestrand
Copy link
Contributor Author

Sounds good. I'll prepare a PR that adds events and corresponding listeners / action functions, and we can use that as a basis for discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants