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

setCodecPreferences should trigger negotiationneeded #2964

Open
jan-ivar opened this issue Apr 19, 2024 · 3 comments
Open

setCodecPreferences should trigger negotiationneeded #2964

jan-ivar opened this issue Apr 19, 2024 · 3 comments

Comments

@jan-ivar
Copy link
Member

setCodecPreferences "overrides the default receive codec preferences used by the user agent", but negotiation is needed before changes affect transmission. Example:

Oddly, setCodecPreferences doesn't trigger negotiationneeded, so the fiddle has to call its own pc1.onnegotiationneeded() explicitly as a workaround. Unfortunately, this hack risks introducing intermittent timing issues with other negotiation needs.

Proposal: make setCodecPreferences trigger negotiationneeded as needed.

The "as needed" part might require some judgement: it's sometimes desirable to call setCodecPreferences in "have-remote-offer" just in time to affect an answer. Since answers can carry just as much weight as offers (arguably more), it might be undesirable to trigger what would be a reverse-role negotiation in this case (once it returns to "stable").

We'd need to work out these details in the check if negotiation is needed algorithm. Language similar to how we handle direction there might work (i.e. consider a match in the local description satisfactory regardless of whether it's an offer or an answer).


1. Works in Chrome/Edge and Safari right now. Landing soon in Firefox 127.

@jan-ivar
Copy link
Member Author

jan-ivar commented Apr 25, 2024

Concern was raised in yesterday's meeting over the web compatibility of introducing this change in behavior now.

Let's examine cases where existing websites might feature-detect and call setCodecPreferences in Chromium and Safari and their possibility of breakage.

No breakage:

  • calling it in "stable" alongside other methods like addTrack/addTransceiver/setStreams/direction/createDataChannel
  • calling it in "have-remote-offer" as part of answering
  • not using a negotiationneeded event listener to negotiate
  • using sLD() without arguments to negotiate

Breakage of intent:

  • calling it with the intent for it to take effect in a much later negotiation, or not at all
    • breakage = updates too soon (seems mild), or updates when it shouldn't (seems esoteric/rare)

Negotiation completes but logs an error:

Negotiation breaks:

  • I can't think of any cases, though it would require custom code in negotiationneeded event handler (or double code) that requires tight control of when it can be called (which seems unlikely to me)
  • Maybe some SFU cases I haven't thought about?
  • Applications explicitly written to terminate on any unexpected error logged (doctor it hurts when I do this)

Happy to field questions on any of these.

@alvestrand
Copy link
Contributor

alvestrand commented Apr 25, 2024

It's a stretch of the imagination, but one can imagine a setup like this (equivalent to your "breakage of intent" setup):

  • Negotiate an initial connection (DC only, for instance)
  • set up a negotiationneeded handler that does an immediate O/A
  • start preparing for the "real" negotiation
    • call SetCodecPreferences
    • ask for camera/microphone permission
    • add tracks when permission is given
  • based on the onnegotiationneeded from addTracks, negotiate the final config based on the negotiationneeded in "add tracks"

The change will then cause an extra negotiation without the media tracks, which may or may not have any bad effects.
The fix would be "move the handler setup to just before the addTrack". But it is a change in behavior.

@jan-ivar
Copy link
Member Author

    • call SetCodecPreferences
    • ask for camera/microphone permission
    • add tracks when permission is given

This order seems unlikely, since there's no transceiver to call setCodecPreferences on ahead of addTrack.

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

No branches or pull requests

2 participants