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, sendonly codecs and dummy codecs #2937

Closed
henbos opened this issue Feb 13, 2024 · 22 comments
Closed

setCodecPreferences, sendonly codecs and dummy codecs #2937

henbos opened this issue Feb 13, 2024 · 22 comments

Comments

@henbos
Copy link
Contributor

henbos commented Feb 13, 2024

As of recent changes, we force you to have at least one codec capable of receiving, regardless of direction, or else setCodecPreferences throws.
But we also allow codecs of any direction to be in the SDP as well as filter what we do end up preferring based on direction.

This means that we still allow the JSEP use case of setting default preferences on the sender side in case the receiver has no preferences on their own, in this case the answer should be the same preference list as was in the offer.

This illustrates controlling sender codec from the offerer assuming answerer justs acks it:

// Does not reject because RecvCodec is a receiver codec. (Edit: actually it does, see discussion later.)
setCodecPreferences([SendOnlyCodec, RecvOnlyCodec]);

// Ensures a sendonly filter is applied at createOffer
transceiver.direction = 'sendonly';

// Filter to [SendOnlyCodec], this is fine to offer since we're not
// receiving so we don't need to include RecvOnlyCodec
createOffer()

So it works! But it also shows something silly: we had to include RecvOnlyCodec in order to offer SendOnlyCodec. But it's not actually preferred in the SDP, this is just a "dummy codec". So why do we force the app to add it?

Dummy codecs might be fine to protect against footguns when the direction changes in the future, but in that case we should still throw since we have no sendrecv codec, so if the direction ever changes to sendrecv then the codec filtering will result in having no codecs in the SDP and the m= section will reject itself. Foot gun!

What should we do?

  1. Make foot gun protection more robust, so that we always can offer or answer, by throwing exceptions earlier.
  2. Make foot gun protection less robust, so that you can prefer sendonly codecs for sendonly transcievers without dummy codecs, but could shoot yourself in the foot.
  3. Allow any preferences but if filtering ever results in empty set, change to default codec preferences to avoid having to reject the m= section.
@fippo
Copy link
Contributor

fippo commented Feb 13, 2024

Still looking for a sendonly codec example. Note that H264 is typically covered by level-asymetry-allowed

// Does not reject because RecvCodec is a receiver codec.

This would reject in step 8 of https://w3c.github.io/webrtc-pc/#dom-rtcrtptransceiver-setcodecpreferences ?

@henbos
Copy link
Contributor Author

henbos commented Feb 13, 2024

Oh but then we've unshipped default preferences for a sender. What, if any, are the benefits of this?

@henbos
Copy link
Contributor Author

henbos commented Feb 13, 2024

Example WPT that needs to change: https://chromium-review.googlesource.com/c/chromium/src/+/5287360

@fippo
Copy link
Contributor

fippo commented Feb 13, 2024

We haven't unshipped default . The answerer will try to match the order unless it uses sCP to override it

@fippo
Copy link
Contributor

fippo commented Feb 13, 2024

I am on Windows so the send codecs are as follows (14), with clockRate removed for brevity:

{mimeType: 'video/VP8'}
{mimeType: 'video/rtx'}
{mimeType: 'video/H264', sdpFmtpLine: 'level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42001f'}
{mimeType: 'video/H264', sdpFmtpLine: 'level-asymmetry-allowed=1;packetization-mode=0;profile-level-id=42001f'}
{mimeType: 'video/H264', sdpFmtpLine: 'level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42e01f'}
{mimeType: 'video/H264', sdpFmtpLine: 'level-asymmetry-allowed=1;packetization-mode=0;profile-level-id=42e01f'}
{mimeType: 'video/H264', sdpFmtpLine: 'level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=4d001f'}
{mimeType: 'video/H264', sdpFmtpLine: 'level-asymmetry-allowed=1;packetization-mode=0;profile-level-id=4d001f'}
{mimeType: 'video/AV1'}
{mimeType: 'video/VP9', sdpFmtpLine: 'profile-id=0'}
{mimeType: 'video/VP9', sdpFmtpLine: 'profile-id=2'}
{mimeType: 'video/H264', sdpFmtpLine: 'level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=64001f'}
{mimeType: 'video/red'}
{mimeType: 'video/ulpfec'}

while receive codecs are as follows (21):

{mimeType: 'video/VP8'}
{mimeType: 'video/rtx'}
{mimeType: 'video/VP9', sdpFmtpLine: 'profile-id=0'}
{mimeType: 'video/VP9', sdpFmtpLine: 'profile-id=2'}
{mimeType: 'video/VP9', sdpFmtpLine: 'profile-id=1'}
{mimeType: 'video/VP9', sdpFmtpLine: 'profile-id=3'}
{mimeType: 'video/H264', sdpFmtpLine: 'level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42001f'}
{mimeType: 'video/H264', sdpFmtpLine: 'level-asymmetry-allowed=1;packetization-mode=0;profile-level-id=42001f'}
{mimeType: 'video/H264', sdpFmtpLine: 'level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42e01f'}
{mimeType: 'video/H264', sdpFmtpLine: 'level-asymmetry-allowed=1;packetization-mode=0;profile-level-id=42e01f'}
{mimeType: 'video/H264', sdpFmtpLine: 'level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=4d001f'}
{mimeType: 'video/H264', sdpFmtpLine: 'level-asymmetry-allowed=1;packetization-mode=0;profile-level-id=4d001f'}
{mimeType: 'video/H264', sdpFmtpLine: 'level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=f4001f'}
{mimeType: 'video/H264', sdpFmtpLine: 'level-asymmetry-allowed=1;packetization-mode=0;profile-level-id=f4001f'}
{mimeType: 'video/AV1'}
{mimeType: 'video/AV1', sdpFmtpLine: 'profile=1'} 
{mimeType: 'video/H264', sdpFmtpLine: 'level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=64001f'} 
{mimeType: 'video/H264', sdpFmtpLine: 'level-asymmetry-allowed=1;packetization-mode=0;profile-level-id=64001f'}
{mimeType: 'video/red'}
{mimeType: 'video/ulpfec'}
{mimeType: 'video/flexfec-03', sdpFmtpLine: 'repair-window=10000000'}

Prior to making sCP only look at recv codecs we would not have been able to set a few of those codecs (e.g. VP9 profile 1) using sCP. Nobody tried either ;-)
Note that we use an exact match in sCP (i.e. sdpFmtpLine must be the same) while SDP O/A is more codec-specific in the comparison, e.g. level-asymmetry-allowed from https://www.rfc-editor.org/rfc/rfc6184#section-8.1. This is ok since we want folks to use the input from getCapabilities and do not want them to hardcode.

I am still looking for a sendonly codec (and thought I had one but ... nope). But if you look at the differences you'll notice that so far we are just extending profiles (in their different spellings) of already supported codec families, some of which define rules for dealing with this kind of stuff.

@henbos
Copy link
Contributor Author

henbos commented Feb 13, 2024

We haven't unshipped default . The answerer will try to match the order unless it uses sCP to override it

I mean we only support "offer send codecs" if the send codecs are sendrecv codeces, we're making this not work for the sendonly codec use case.

I am still looking for a sendonly codec (and thought I had one but ... nope).

We might want to be future proof. Besides, custom codecs might become a thing? Should they have to be sendrecv?

But if you look at the differences you'll notice that so far we are just extending profiles (in their different spellings) of already supported codec families, some of which define rules for dealing with this kind of stuff.

So we're only changing things for certain profiles. I'm having trouble understanding what this means in practice?
Is there anything we could previously do that we can no longer do?

@fippo
Copy link
Contributor

fippo commented Feb 13, 2024

We finally found a sendonly codec (or is that only called co?) 🎉:
Chrome on Mac offers H264 with profile 640034 (which is level 5.2) but claims to only be able to receive 64001f (level 3.1).
It seems Safari on the same machine restricts itself to level 3.1 too?

@henbos
Copy link
Contributor Author

henbos commented Feb 13, 2024

Imagine a server supports decoding 5.2.

What's the harm with allowing a sendonly transceiver to offer 5.2?

It's very unintuitive to me that we say that the sendonly transceiver must offer something it knows how to decode to avoid empty offer and then signal, outside the SDP, to "please ask me to send 5.2", and then that the server does pc.ontrack = e => e.transceiver.setCodecPreferences(5.2)

@fippo
Copy link
Contributor

fippo commented Feb 13, 2024

from some discussion in other places:
Prior to the last sCP change, any codec in sCP needed to be both in send and receive codecs.
Now it has to be in receive codecs only.

And you are arguing that it should be ok if we pass a codec in send codecs to sCP, so the new condition would be that
all codecs must be in send codecs or receive codecs and the intersection between sCP and receive codecs must not be empty?

@henbos
Copy link
Contributor Author

henbos commented Feb 13, 2024

Prior to the last sCP change, any codec in sCP needed to be both in send and receive codecs.
Now it has to be in receive codecs only.

I think a codec being required to be sendrecv was wrong, so I definitely like the direction of being able to set recvonly codecs in SCP! But I think it makes sense to also allow sendonly codecs for the sendonly use case, considering what you offer are the default preferences used unless the receiver has their own preferences.

So I'm not trying to revert the progress, I'm trying to go one step further.

  • Can we say SCP accepts both sending codecs and receiving codecs as input? Making both senders and receivers happy.

the new condition would be that all codecs must be in send codecs or receive codecs

Correct. A codec is "send OR recv", not "send AND recv".

and the intersection between sCP and receive codecs must not be empty?

Or rather the SCP codec filtered by the transceiver direction must not be empty. Either that or saying "any codec is fine, but at least one of them has to be sendrecv" as means to avoid direction = 'sendrecv' footgun. I don't have a strong preference

@henbos
Copy link
Contributor Author

henbos commented Feb 14, 2024

I don't expect everyone to follow along with my ramblings, so I turned this last comment into a concrete proposal: #2939.
If we go with that then this issue becomes obsolete and we can close it.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 15, 2024
to supplement the discussion in
  w3c/webrtc-pc#2937

BUG=324930413

Change-Id: Iebf02aade64030e11590af211fa7bc90f976c592
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 15, 2024
to supplement the discussion in
  w3c/webrtc-pc#2937

BUG=324930413

Change-Id: Iebf02aade64030e11590af211fa7bc90f976c592
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 16, 2024
to supplement the discussion in
  w3c/webrtc-pc#2937

BUG=324930413

Change-Id: Iebf02aade64030e11590af211fa7bc90f976c592
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 16, 2024
to supplement the discussion in
  w3c/webrtc-pc#2937

BUG=324930413

Change-Id: Iebf02aade64030e11590af211fa7bc90f976c592
aarongable pushed a commit to chromium/chromium that referenced this issue Feb 19, 2024
to supplement the discussion in
  w3c/webrtc-pc#2937

BUG=324930413

Change-Id: Iebf02aade64030e11590af211fa7bc90f976c592
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5293562
Commit-Queue: Philipp Hancke <[email protected]>
Reviewed-by: Harald Alvestrand <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1262367}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 19, 2024
to supplement the discussion in
  w3c/webrtc-pc#2937

BUG=324930413

Change-Id: Iebf02aade64030e11590af211fa7bc90f976c592
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5293562
Commit-Queue: Philipp Hancke <[email protected]>
Reviewed-by: Harald Alvestrand <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1262367}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 20, 2024
to supplement the discussion in
  w3c/webrtc-pc#2937

BUG=324930413

Change-Id: Iebf02aade64030e11590af211fa7bc90f976c592
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5293562
Commit-Queue: Philipp Hancke <[email protected]>
Reviewed-by: Harald Alvestrand <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1262367}
@dontcallmedom-bot
Copy link

This issue was mentioned in WEBRTCWG-2024-02-20 (Page 18)

marcoscaceres pushed a commit to web-platform-tests/wpt that referenced this issue Feb 23, 2024
to supplement the discussion in
  w3c/webrtc-pc#2937

BUG=324930413

Change-Id: Iebf02aade64030e11590af211fa7bc90f976c592
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5293562
Commit-Queue: Philipp Hancke <[email protected]>
Reviewed-by: Harald Alvestrand <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1262367}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Feb 28, 2024
… filtering of codecs, a=testonly

Automatic update from web-platform-tests
webrtc wpt: add test for direction-based filtering of codecs

to supplement the discussion in
  w3c/webrtc-pc#2937

BUG=324930413

Change-Id: Iebf02aade64030e11590af211fa7bc90f976c592
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5293562
Commit-Queue: Philipp Hancke <[email protected]>
Reviewed-by: Harald Alvestrand <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1262367}

--

wpt-commits: 8d72cdeb97859e01b8bdad99a7b627c13455ac72
wpt-pr: 44604
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this issue Feb 28, 2024
… filtering of codecs, a=testonly

Automatic update from web-platform-tests
webrtc wpt: add test for direction-based filtering of codecs

to supplement the discussion in
  w3c/webrtc-pc#2937

BUG=324930413

Change-Id: Iebf02aade64030e11590af211fa7bc90f976c592
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5293562
Commit-Queue: Philipp Hancke <[email protected]>
Reviewed-by: Harald Alvestrand <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1262367}

--

wpt-commits: 8d72cdeb97859e01b8bdad99a7b627c13455ac72
wpt-pr: 44604
@aboba
Copy link
Contributor

aboba commented Apr 19, 2024

Looking at receive codecs for sendrecv or recvonly m-lines makes some sense, even though it violates a SHOULD in RFC 3264 Section 5.1:

For a sendonly stream, the offer SHOULD indicate those formats the offerer is willing to send for this stream. For a recvonly stream, the offer SHOULD indicate those formats the offerer is willing to receive for this stream. For a sendrecv stream, the offer SHOULD indicate those codecs that the offerer is willing to send and receive with.

It's ok because including a codec in an Offer on a recvonly or sendrecv m-line only requires the Offerer to be able to receive the codec; the Offerer is not committing to sending an Offered codec. Including a sendonly codec in an Offer on a recvonly or sendrecv m-line wouldn't make sense, because that could mislead the Answerer into sending a codec to the Offerer that it was not capable of receiving.

In an Offer on a sendonly m-line, adding recvonly codecs could be problematic if the Answerer prunes the Offered codecs to those that the Offer can only receive but not send. In that case, communication will fail.

@rshpount
Copy link

It's ok because including a codec in an Offer on a recvonly or sendrecv m-line only requires the Offerer to be able to receive the codec;

Including a receive-only codec in the sendrecv line is not ok. You need to, somehow, tell the answerer that the receive-only codec should not be removed from the answer and that another codec should present with a higher preference. If the answerer includes only this codec in the answer, that will force the offerer to send it, which it cannot do. If the answerer does not include the receive-only codec in the answer, then this codec cannot be used in the session since, according to RFC 3264, it should be released by the offerer. So, this is not a negotiation. It is going through signaling motions that only work if we already know the remote capabilities.

@aboba
Copy link
Contributor

aboba commented Apr 23, 2024

@rshpount As we've discussed on the RTCWEB mailing list, JSEPbis (now RFC 9429) Section 4.2.6 says:

"Note that setCodecPreferences does not directly affect which codec the implementation decides to send. It only affects which codecs the implementation indicates that it prefers to receive, via the offer or answer."

This is being interpreted to imply that on recvonly and sendrecv m-lines, sCP should only require support for receiving but not sending codecs. That is what allows a receive-only codec to be provided in an Offer on a sendrecv m-line.

As you point out, that is not consistent with the SHOULDs in RFC 3264 Section 5.1. So the question is:

  1. Is RFC 9429 Section 4.2.6 in error, or at least requiring some additional context to avoid misinterpretation?
  2. If so, should we file an errata?

@rshpount
Copy link

@aboba RFC 9429 is not an error. An additional context might help, but it would not be normative. RFC 3264 will still be a normative document that controls how the offer/answer works. RFC 9429 Section 4.2.6 is being misinterpreted since it is considered independent from RFC 3264. All it says is that setCodecPreferences will control the order of codecs for the m= line associated with the transceiver for the next offer or answer. It will specify in which order the codecs will be offered and in which order they will be considered when generating the answer (overwriting the offer). It does not define how certain things are implemented, such as how to return two codecs in the answer, which would be required for asymmetric codecs used for a sendrecv line. It does not control which codecs are actually used; it just adjusts the preferences for the negotiation.

We might add context to this section, but I don't know if errata is the proper process. It might be enough to fix the webrtc-pc document.

@stefhak
Copy link
Contributor

stefhak commented Apr 23, 2024

@rshpount what you are basically saying IIUC is that between RFC 3264 and RFC 9429 it is not possible to have asymmetric codecs for sendrecv m-lines. In other words, to have different codecs for send and receive you would have to resort to using two transceivers, one for sending and one for receiving. This does not seem ideal to me.

@rshpount
Copy link

@stefhak I am saying that the procedure for asymmetric codecs in sendrecv line is not defined in RFC 9429.

Implementing asymmetric codecs on a sendrecv line using RFC 3264 is possible. To do this, the offerer will send codecs "A B" in the offer, and the answerer will send codecs "B A" in the answer. This will imply that the offerer prefers to receive A, but B is also supported, and that the answerer prefers to receive B, but A is also supported. This might need an additional nudge outside of the offer/answer to force using different codecs in both directions and to signal that the offerer cannot receive B or that the answerer cannot receive A.

Based on the above, what is missing is an API that specifies that two codecs should be left in the answer. sCP can be used to control their order and signal which codec the answerer prefers to receive, but the mechanism that specifies that the codecs should be present in the answer is missing. Furthermore, per RFC 3264, it is possible to have codecs in the answer, which were not present in the offer. I am not sure of the API to add codecs in the answer which were not present in the offer (apart from SDP manipulation).

@stefhak
Copy link
Contributor

stefhak commented Apr 24, 2024

Thanks for elaborating @rshpount. One thing that is not clear to me is: your inputs indicate that the answer should only list one codec usually. I think current implementations have a quite extensive list of codecs both in offer and answer (unless sCP is used to trim), and that this is as it should be. In other words. the O/A exchange establishes a set of codecs (rather than a codec) that (with preference order) might be used for actual media. Is this wrong?

@rshpount
Copy link

@stefhak

Thanks for elaborating @rshpount. One thing that is not clear to me is: your inputs indicate that the answer should only list one codec usually. I think current implementations have a quite extensive list of codecs both in offer and answer (unless sCP is used to trim), and that this is as it should be. In other words. the O/A exchange establishes a set of codecs (rather than a codec) that (with preference order) might be used for actual media. Is this wrong?

The best practice is only to include the codecs that will be used. Not including a codec in the answer guarantees both parties that this codec will never be used and allows the associated resources to be freed. For instance, if the offerer allocates hardware resources for a specific video codec, these hardware resources can be freed if the codec is not present in the answer. See RFC 3264 Section 7.

@stefhak
Copy link
Contributor

stefhak commented Apr 25, 2024

@rshpount to me it seems better to have several options for the sender - the encoding is more complex, so if one of the codecs that are acceptable for the receiving endpoint is HW accelerated at the sender, it might be better to use that one. The way current implementations seem to behave enables this IIUC.

Anyway, we still miss a way for the offerer to indicate (via SDP O/A) codecs it can only send (but not receive).

@henbos
Copy link
Contributor Author

henbos commented Oct 2, 2024

At TPAC we decided to avoid footguns and keep things simple (Proposal A), see #3006. Closing this issue

@henbos henbos closed this as completed Oct 2, 2024
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

6 participants