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

Specify what should happen if filtering [[PreferredCodecs]] on direction results in empty list #2936

Closed
henbos opened this issue Feb 8, 2024 · 7 comments

Comments

@henbos
Copy link
Contributor

henbos commented Feb 8, 2024

If you have no preference, you O/A with all codecs.

But if you have a recvonly codec preferences, and the transceiver is sendonly, the createOffer() filtering would result in an empty list.

  • What happens?
@fippo
Copy link
Contributor

fippo commented Feb 8, 2024

The list of codecs is empty so the m-line is rejected in the offer (port 0) and calling setLocalDescription with it causes the transceiver to be stopped.

@henbos
Copy link
Contributor Author

henbos commented Feb 8, 2024

I would rather act as if you have no preference than to have the transceiver blow up. Also if we stop transceivers before offer in other contexts I'm not sure we offer it, or if we do what about offerer tagged m= section and the stopping vs stopped state.

Probably less error prone to simply act as if you have no preference

@fippo
Copy link
Contributor

fippo commented Feb 8, 2024

No preference would be against the users intent to set codec preferences. Rejecting and stopping the transceiver when there is nothing in common is more consistent with the same results when you get a remote answer with no codecs in comon

@henbos
Copy link
Contributor Author

henbos commented Feb 9, 2024

Why would me saying which codecs I prefer to receive make a sendonly transceiver not just incapable of sending, but make the transceiver permanently die?

If this was "just receiver preferences", then setCodecPreferences would be a NO-OP. But it's not:

transciever.direction = 'sendrecv';
transceiver.setCodecPreferences([recvOnlyCodec]);
O/A and everything works, other endpoint is asking for H264

transceiver.direction = 'sendonly';
O/A and transceiver is permanently stopped, other endpoint is not allowed to ask for H264 anymore

To safely change direction, the app needs to remember to clear their preferences or to always add dummy codecs just for the sake of having SOMETHING in the offer. Even if that something is not what the other endpoint is interested in.

It's a footgun. Possible workaround:

  • Setting direction or codec preference in a way that results in an empty preference could throw.
  • Sensible defaults could be used.
  • Setting direction to sendonly automatically clears any preferences.

But I think whether we fail hard or patch this issue with workarounds, I think it's clear that receiver preferences ARE relevant for senders. JSEP does not allow not offering anything, and what we offer - even in a sendonly use case - are the "receiving" codec preferences. Receiving in scare-quotes.

@henbos
Copy link
Contributor Author

henbos commented Feb 9, 2024

@alvestrand @aboba

@henbos
Copy link
Contributor Author

henbos commented Feb 9, 2024

Another workaround could be: if you end up with the empty set, make sure we always have something to offer by adding a sendrecv codec of the UA's choice.

@henbos
Copy link
Contributor Author

henbos commented Feb 13, 2024

Filed #2937 for a more concrete example

@henbos henbos closed this as completed Feb 13, 2024
ibaoger pushed a commit to ibaoger/webrtc that referenced this issue Feb 29, 2024
which avoids throwing an error when using setCodecPreferences
to set a recvonly codec on a sendonly transceiver. See
  w3c/webrtc-pc#2936

BUG=webrtc:15396

Change-Id: I435a98c944ed2eeef87d9b8a7f791d095ec25502
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/338642
Reviewed-by: Harald Alvestrand <[email protected]>
Reviewed-by: Florent Castelli <[email protected]>
Commit-Queue: Philipp Hancke <[email protected]>
Cr-Commit-Position: refs/heads/main@{#41843}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue May 15, 2024
Upstream commit: https://webrtc.googlesource.com/src/+/20a90295fcea910dc774df0bbd0e65e13606c2e8
    sdp: set content to rejected if the list of common codecs is empty

    which avoids throwing an error when using setCodecPreferences
    to set a recvonly codec on a sendonly transceiver. See
      w3c/webrtc-pc#2936

    BUG=webrtc:15396

    Change-Id: I435a98c944ed2eeef87d9b8a7f791d095ec25502
    Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/338642
    Reviewed-by: Harald Alvestrand <[email protected]>
    Reviewed-by: Florent Castelli <[email protected]>
    Commit-Queue: Philipp Hancke <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#41843}
jamienicol pushed a commit to jamienicol/gecko that referenced this issue May 17, 2024
Upstream commit: https://webrtc.googlesource.com/src/+/20a90295fcea910dc774df0bbd0e65e13606c2e8
    sdp: set content to rejected if the list of common codecs is empty

    which avoids throwing an error when using setCodecPreferences
    to set a recvonly codec on a sendonly transceiver. See
      w3c/webrtc-pc#2936

    BUG=webrtc:15396

    Change-Id: I435a98c944ed2eeef87d9b8a7f791d095ec25502
    Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/338642
    Reviewed-by: Harald Alvestrand <[email protected]>
    Reviewed-by: Florent Castelli <[email protected]>
    Commit-Queue: Philipp Hancke <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#41843}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants