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

Align spec /w codec direction decision #3006

Open
henbos opened this issue Oct 2, 2024 · 16 comments
Open

Align spec /w codec direction decision #3006

henbos opened this issue Oct 2, 2024 · 16 comments

Comments

@henbos
Copy link
Contributor

henbos commented Oct 2, 2024

How to deal with codecs and directionality was discussed at TPAC 2024:

There was a strong preference for Proposal A, i.e. to "keep it simple": sendonly transceivers can do all send codecs, recvonly transcievers can do all receive codecs, and sendrecv transceivers can only do codecs and profiles that you can BOTH send AND receive with. So if you want to do fancy unidirectional codecs or profiles you use multiple m= sections.

Let this issue track making sure the spec aligns with Proposal A and allow us to merge other codec related issues as fixed by this issue

@henbos
Copy link
Contributor Author

henbos commented Oct 2, 2024

The TPAC decision made it clear which direction to go, but we never touched on what happens if the transceiver direction makes it such that none of your codec preferences are applicable anymore (e.g. transceiver is sendrecv but preferences only include recvonly codecs). This issue was pointed out in #2939

@henbos
Copy link
Contributor Author

henbos commented Oct 2, 2024

Adding "Discuss at next meeting" label to reflect we need to discuss error handling as per previous comment

@henbos
Copy link
Contributor Author

henbos commented Oct 2, 2024

Laundry list:

  • Update the normative text here to clarify this is not just "default receive codec preferences" (e.g. can also be used to set send codec preferences on a sendonly transceiver and so on).
  • Update algorithm to allow both send codecs and receive codecs (currently it says to throw InvalidModificationError in the sendonly use case).
  • The final steps to create an offer and answer's directionality filtering is actually correct already, however we need to clarify/decide what to do when preferences become empty due to filtering.
  • The setParameters algorithm is as far as I can tell correct, no action needed.

@henbos
Copy link
Contributor Author

henbos commented Oct 2, 2024

The spec used to say a preference is a send codec OR receive codec, this was actually changed in an amendment. So I wonder what our implementation does, if it still does both send and receive or if it was updated as per PR, do you know @fippo @alvestrand ?

@henbos
Copy link
Contributor Author

henbos commented Oct 2, 2024

Filing crbug to align with any spec changes happening here: https://crbug.com/370792782

@henbos
Copy link
Contributor Author

henbos commented Oct 2, 2024

To be discussed at October Virtual Interim (slides)

@fippo
Copy link
Contributor

fippo commented Oct 2, 2024

I do not think there needs to be more alignment, we are already doing what RFC 3550 says:

  • VP9 (Firefox is not supporting more than the default profile=0)
  • H264 (Firefox is only supporting CBP so not testable)

As usual we do need tests though.

@henbos
Copy link
Contributor Author

henbos commented Oct 2, 2024

We still need to align the spec with the decision such that setCodecPreferences does not throw if you set a sendonly codec (e.g. sendonly transceiver use case)

@henbos henbos changed the title Align codec preference and direction Align spec with codec preference and direction decision Oct 2, 2024
@henbos henbos changed the title Align spec with codec preference and direction decision Align spec /w codec direction decision Oct 2, 2024
@fippo
Copy link
Contributor

fippo commented Oct 2, 2024

I thought we had been discussing this before and agreed no changes are needed? Do you have a fiddle or test showing your problem?

https://www.rfc-editor.org/rfc/rfc9429#section-4.2.6

It only affects which codecs the implementation indicates that it prefers to receive, via the offer or answer

If you use sCP on a sendonly transceiver it does not matter much in practice but would only be observable in future O/A after changing the direction.

@henbos
Copy link
Contributor Author

henbos commented Oct 2, 2024

We discussed this at TPAC and there was a preference for Proposal A, see slides and recording. This issue is about doing what we decided to do.

@aboba
Copy link
Contributor

aboba commented Oct 2, 2024

@fippo IETF RTCWEB and AVTCORE WGs discussed RFC 9429 Section 4.2.6 - and the conclusion was that it was being interpretted by WEBRTC WG in a way that was inconsistent with RFC 3264. The decision at TPAC 2024 fixes that.

@henbos
Copy link
Contributor Author

henbos commented Oct 3, 2024

Yep, and to really spell this out, the following should work:

const sendOnlyTransceiver = pc1.addTransceiver('video', {direction: 'sendonly'});
sendOnlyTransceiver.setCodecPreferences([sendOnlyCodec]);

const recvOnlyTransceiver = pc1.addTransceiver('video', {direction: 'recvonly'});
recvOnlyTransceiver.setCodecPreferences([recvOnlyCodec]);

const sendRecvTransceiver = pc1.addTransceiver('video', {direction: 'sendrecv'});
sendRecvTransceiver.setCodecPreferences([sendRecvCodec]);

await pc1.setLocalDescription();
await pc2.setRemoteDescription(pc1.localDescription);
await pc2.setLocalDescription();  // Notice how pc2 does not need to set any preference of their own
await pc1.setRemoteDescription(pc2.localDescription);

But the following should NOT work:

const sendRecvTransceiver = pc1.addTransceiver('video', {direction: 'sendrecv'});
sendRecvTransceiver.setCodecPreferences([sendOnlyCodec, recvOnlyCodec]);
... negotiate ....

In the next meeting I will discuss how to handle errors (e.g. if the bad example should result in exceptions or if it should be interpreted as "no preference" after filtering). TBD

@fippo
Copy link
Contributor

fippo commented Oct 3, 2024

https://www.rfc-editor.org/rfc/rfc8829.html#section-4.2.6-1
remains clear on what setCodecPreferences is intended for:

It only affects which codecs the implementation indicates that it prefers to receive, via the offer or answer

I keep looking for a sendonly codec that is not covered by special codec rules like level-asymmetry-allowed which covered the weird (and likely buggy) H264 instance we found.

@henbos
Copy link
Contributor Author

henbos commented Oct 3, 2024

It does not affect it directly, but it affects it indirectly. This JSEP reading is what led to this regression in the first place, we decided to fix this.

@henbos
Copy link
Contributor Author

henbos commented Oct 16, 2024

Decision for the remaining error handling was Proposal A of this slide. This issue is now ready for PR.

@henbos
Copy link
Contributor Author

henbos commented Oct 17, 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

3 participants