w3c / webrtc-pc

WebRTC 1.0 API
https://w3c.github.io/webrtc-pc/
Other
438 stars 115 forks source link

Align spec /w codec direction decision #3006

Open henbos opened 1 month ago

henbos commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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

henbos commented 1 month ago

Laundry list:

henbos commented 1 month ago

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 commented 1 month ago

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

henbos commented 1 month ago

To be discussed at October Virtual Interim (slides)

fippo commented 1 month ago

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

As usual we do need tests though.

henbos commented 1 month ago

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)

fippo commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

@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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 2 weeks ago

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

henbos commented 2 weeks ago

https://www.w3.org/2024/10/15-webrtc-minutes.html

fippo commented 5 days ago

Let me try to clarify this after cappucino and water with hbos:

That part is đź‘Ť

I do not see a need to change those rules based on direction so disagree with the second code block of this comment but that might have been overtaken by events?

I question the conclusion to do "Proposal A" of the slide above. The Working Group seems to make decisions based on opinions while lacking tests which, like this fiddle (took five minutes to write... for the second time, I had shown this in February already) which shows that "deployed code" is doing proposal C. Transceivers can get stopped for a number of reasons, applications must check what was negotiated anyway.

henbos commented 4 days ago

You’re able to recognize the the past sendrecv limitation was a problem for your recvonly use case, but you can’t see that the exact same problem exists for the sendonly usecase? Your questioning of Proposal A has downsides (not fixing sendonly) without upside (what would thay be?). Your opposition also requires not caring what the RFCs referenced in the slides say. I stand by the WG decision.

fippo commented 4 days ago

How does Proposal C (m-line is rejected; this has a downside of having a stopped transceiver after SLD), consistent with "incompatible choices result in a stopped transceiver" not fix sendonly? Transceivers get stopped, they are cheap.

See also #2936 (it seems we are going in circles)