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

generateKeyFrame algorithm makes wrong assumptions about number of encoders #145

Open
fippo opened this issue Aug 30, 2022 · 1 comment
Open
Labels
TPAC 2022 For discussion at TPAC

Comments

@fippo
Copy link
Collaborator

fippo commented Aug 30, 2022

https://w3c.github.io/webrtc-encoded-transform/#KeyFrame-algorithms

  1. Gather a list of video encoders, named videoEncoders from encoder, ordered according negotiated RIDs if any.

VP8 in libvpx based encoders use a single encoder to encode multiple spatial layers.
In libwebrtc H264 simulcast is done by a triplet of separate encoders but there seem to be encoders such as OpenH264 with built-in support for multiple spatial layers.

  1. [...] videoEncoders is expected to be empty if the corresponding RTCRtpSender is not active

I don't think that is how things behave.

6 [...] If rid is undefined, set rid to the RID value corresponding to videoEncoder.

The video encoder might not have a rid in non-simulcast cases.

Overall the algorithm seems overly specific and should not attempt to venture into such details.
Something along the lines of "ask the encoder to generate a keyframe for the given set of rids or all available layers if the set is empty"

@youennf
Copy link
Collaborator

youennf commented Sep 1, 2022

The spec uses an abstract representation which does not have to match how UA are implemented.
I am opened to readability/clarification changes, but I prefer being specific about the behaviour, for instance:

  • If sender is not sending, reject with NotFoundError.
  • If rid is not matching any negotiated rid, reject with NotFoundError.
  • If rid is not specified, use the first rid negotiated rid value.
  • If there is an ongoing request for the provided rid, do nothing but resolve at the same time as the previous request.

@aboba aboba added the TPAC 2022 For discussion at TPAC label Sep 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TPAC 2022 For discussion at TPAC
Projects
None yet
Development

No branches or pull requests

3 participants