-
Notifications
You must be signed in to change notification settings - Fork 115
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 vs unidirectional codecs #2888
Comments
I like failing fast. I.e. I'd rather have So I think What if you want to change from |
Do the h264 'mismatches' actually fail to decode? So I think perhaps the correct 'fix' here is relaxing the matching rules. |
@steely-glint this is probably a different bug - that browsers use a library codec without actually investigating what it's capable of decoding, or how to represent that as a profile-level. (Chrome, for instance, uses ffmpeg for decode, which is capable of decoding a lot of stuff, but uses openh264 for encode, which is only capable of encoding constrained baseline. I don't think we ever tell ffmpeg what promises we've made on its behalf.) |
setCodecPreferences() is setting the list of PTs at the end of the m= line, which indicates which codecs you prefer to receive. Send-only codecs should either be silently dropped from the list (and an empty list should be an error), or they should cause the setCodecPreferences() call to fail. |
Harald:
The spec says this:
and a little below
which led me to believe that the input also comes from the sender capabilities. JSEP makes it quite clear what the original intent was:
but webrtc-pc does not? The solution seems to be to align with JSEP. |
The order in the codec list is purely a receive preference that can be ignored. But the set of codecs - which ones are present in the negotiation - does affect what is allowed to be sent and/or received. You can't send something that doesn't have a PT or which the other side does not recognize. The sentence that says You can always get around it by adding another codec to the list as a dummy codec ensuring that there always is something that could be used for sending or receiving, but that is a bad API design when we want to support uni-direction. Fixing this would not be a backwards compat issue since it would only expand the set of possibilities beyond what they are today. |
Negotiating that you can receive something that you don't know how to receive though is a problem. JSEP doesn't seem to support that which would lead to dropping packets? (It's a problem, but it is a "soft" problem not a "hard" problem) |
Trying to summarize the current state of discussion (from this code change). JSEP is pretty clear:
So sCP (setCodecPreferences ) is not something you use to pick the send codec. sCP should not take send codecs into account which webrtc-pc does here. Answer may include codecs not offered, see https://www.rfc-editor.org/rfc/rfc8829.html#section-5.3.1
No browsers do that, but behave as intended (or can be tricked?) sCP should typically be done in ontrack, see updated samples PR, e.g. This hopefully is a somewhat web-compatible change. Woes with unidirectional codecs were not discovered for years The proposed fix is threefold:
Open question: what does setCodecPreferences do for a sendonly m-line? |
I support making setCodecPreferences only control receiver side preferences, which shouldn't be controversial considering JSEP says they are receiver preferences. More motivation based on the JSEP rule about order below.
This JSEP quote has implications. If I want to send H264 but receive VP8 I can do this:
Based on 2) breaking down, I think 1) is the only way to go, i.e. SDP says this is receiver preferences so let's treat it as such. Send codecs are irrelevant and don't need to be on the list. So going with 1), how would we deal with a JSEP-respecting endpoint that responds "VP8, H264" instead of "H264, VP8"? I think the answer is @Orphis' setParameters(codec:H264) which allows sending a codec regardless of its placement on the list, as long as it was negotiated.
Not much, it sounds like. But what does JSEP say, is it allowed to have an empty list of codecs in the SDP or are we forced to add dummy codecs? The current API rule that says "treat empty list as using browser defaults" seems to not work here... backwards-compat issues incoming? |
But it might still be controversial if there are backwards compat issues... setCodecPreferences() currently controls the m= line, regardless of direction or if offerer or answerer, so I don't fully understand the implications. E.g. if we get an offer that says "VP8, H264" and because we want to send VP8 (respect the offer) but only want to receive H264, so we setCodecPreferneces(H264) before the answer and then complete the negotiation... will our sender still be configured to do VP8? Or does the fact that I removed it for our answer also remove it from my sender configurations? This might not work today... |
I suspect a lot of the more complicated scenarios are talking to non-WebRTC endpoints. And I hope that most of those use only sendonly/recvonly transceivers and not sendrecv. https://jsfiddle.net/fippo/n8upqxbL/1/ shows how sCP does handle asymmetry today which seems surprising but is ok? |
Unfortunately I fear that a majority of the non-webrtc (legacy) endpoints are using only sendrecv..... |
@fippo's fiddle gives me hope though, it shows that we support sending one codec and receiving another, and it does so using only What is "weird" is that However I think we still have one problem, if I set I suspect the "bug" here is that we only include codecs that are in both offer and answer? Both send and receive? But if we take JSEP seriously, then there should be no problem to offer ONLY VP8 and answer ONLY VP9. (It breaks even before reaching the follow up offer in the other direction which is what I wanted to test originally) |
My (current) thinking is that the list of codecs in a=rtpmap lines is the list of codecs we want to have a PT assigned for - no matter if they're receiving or sending. The list set in setCodecPreferences() is the list that we want to receive. By implication, anything that's in the a=rtpmap lines and not in the setCodecPreferences()-generated list is something the responder should treat as a send-only codec. At the moment there's no API to control the generation of a=rtpmap lines. If setCodecPreferences() affects that, then we're holding it wrong. (I think.) |
That's clever (having a=rtpmap contain all codecs you support, but the preferences could be a subset of the PTs). This lets the answering endpoint prefer to receive the codec in any order it likes, even if the offerer does not prefer to receive it at all. We still have the issue that the SDP answerer has no way of knowing the directional support of a codec that is listed in the a=rtpmap line, but it sounds like this is a limitation of SDP rather than a limitation of webrtc-pc APIs. So I think we can live with it: the app can easily check codec directionality support using |
At a higher level of abstraction, I think we should add a "direction" attribute to the codec description - with a default value of "sendrecv" for backwards compatibility. This would expose the possible usages of the codec to JS in an unambiguous way. Mapping this to representation in SDP is still unclear, but at least we would have the information to build on. |
Is this related?: I'm looking at a case where the offerer offers sendonly video. I struggle on how to enable the receiving application to apply codec preferences (re-order and prune). I think you can setParameters (including codec data) on an RTPSender, but is there any similar for an RTPReceiver, or how should this be done? |
if your receive side is the answerer, you can apply your codec preferences in between the |
Thanks @Orphis. I guess that is what I want to do, but I struggle on how to apply those preferences. Edit, it should be between the |
Dumping what I originally had in the slides which are now a bit more condensed:
@stefhak you may find the samples PR useful. It seemed awkward at first but is actually easier to work with since you have one ontrack handler but many ways to add a transceiver |
@fippo thanks for a good summary! For my case (unidirectional medai), things now make sense as the receiving (also being SDP answerer) end can use transceiver.setCodecPreferences to indicate preferred receive codec(s). Exactly what I was looking for. One slightly confusing thing for me is that webrtc-extensions adds the possibility to define a codec with setParameters on the RTCRTPsender. Can't this be a problem if used before negotiation? In that case any of the implemented (send) codecs can be used, so the promise probably resolves, but the receiver may not support it, which only becomes apparent after SDP o/a. |
There's also (from RFC3264):
So while we can add codecs we want to receive that were not present in the offer, it appears that what is offered cannot be entirely distinct either, as at least one codec needs to either be present in the offer or that is sendrecv-capable? |
Section 7.2.3 handles that, and if the codec is not in the |
This one turned out to be interesting. For setCodecPreferences this needs to be an exact match, i.e. input needs to come from getCapabilities. Also things like level-asymetry-allowed must not be taken into account. |
which is what is noted in JSEP: https://www.rfc-editor.org/rfc/rfc8829.html#name-setcodecpreferences Some W3C spec modifications are required since the W3C specification currently takes into account send codecs as well. Spec issue: w3c/webrtc-pc#2888 Spec PR: w3c/webrtc-pc#2926 setCodecPreferences continues to modify the codecs in an offer. Also rename RtpSender::SetCodecPreferences to RtpSender::SetSendCodecs for consistent semantics. BUG=webrtc:15396 Change-Id: I1e8fbe77cb2670575578a777ed1336567a1e4031 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/328780 Reviewed-by: Henrik Boström <[email protected]> Commit-Queue: Philipp Hancke <[email protected]> Reviewed-by: Harald Alvestrand <[email protected]> Cr-Commit-Position: refs/heads/main@{#41719}
This reverts commit 1cce1d7. Reason for revert: Breaks WPTs Original change's description: > Make setCodecPreferences only look at receive codecs > > which is what is noted in JSEP: > https://www.rfc-editor.org/rfc/rfc8829.html#name-setcodecpreferences > > Some W3C spec modifications are required since the W3C specification > currently takes into account send codecs as well. > > Spec issue: > w3c/webrtc-pc#2888 > Spec PR: > w3c/webrtc-pc#2926 > > setCodecPreferences continues to modify the codecs in an offer. > > Also rename RtpSender::SetCodecPreferences to RtpSender::SetSendCodecs for consistent semantics. > > BUG=webrtc:15396 > > Change-Id: I1e8fbe77cb2670575578a777ed1336567a1e4031 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/328780 > Reviewed-by: Henrik Boström <[email protected]> > Commit-Queue: Philipp Hancke <[email protected]> > Reviewed-by: Harald Alvestrand <[email protected]> > Cr-Commit-Position: refs/heads/main@{#41719} Bug: webrtc:15396 Change-Id: I7b545e91f820c3affc39841c6e93939eac75c363 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/339520 Reviewed-by: Harald Alvestrand <[email protected]> Commit-Queue: Harald Alvestrand <[email protected]> Owners-Override: Henrik Boström <[email protected]> Reviewed-by: Henrik Boström <[email protected]> Auto-Submit: Henrik Boström <[email protected]> Bot-Commit: [email protected] <[email protected]> Cr-Commit-Position: refs/heads/main@{#41725}
This is a reland of commit 1cce1d7 after updating the WPT that broke on Mac. Original change's description: > Make setCodecPreferences only look at receive codecs > > which is what is noted in JSEP: > https://www.rfc-editor.org/rfc/rfc8829.html#name-setcodecpreferences > > Some W3C spec modifications are required since the W3C specification > currently takes into account send codecs as well. > > Spec issue: > w3c/webrtc-pc#2888 > Spec PR: > w3c/webrtc-pc#2926 > > setCodecPreferences continues to modify the codecs in an offer. > > Also rename RtpSender::SetCodecPreferences to RtpSender::SetSendCodecs for consistent semantics. > > BUG=webrtc:15396 > > Change-Id: I1e8fbe77cb2670575578a777ed1336567a1e4031 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/328780 > Reviewed-by: Henrik Boström <[email protected]> > Commit-Queue: Philipp Hancke <[email protected]> > Reviewed-by: Harald Alvestrand <[email protected]> > Cr-Commit-Position: refs/heads/main@{#41719} Bug: webrtc:15396 Change-Id: I0c7b17f00de02286f176b500460e17980b83b35b Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/339541 Commit-Queue: Philipp Hancke <[email protected]> Reviewed-by: Harald Alvestrand <[email protected]> Cr-Commit-Position: refs/heads/main@{#41807}
Essentially a no-op since we're going to see this change reverted when we vendor in 1e7a6f3b6a. Upstream commit: https://webrtc.googlesource.com/src/+/1cce1d7ddcbde3a3648007b5a131bd0c2638724b Make setCodecPreferences only look at receive codecs which is what is noted in JSEP: https://www.rfc-editor.org/rfc/rfc8829.html#name-setcodecpreferences Some W3C spec modifications are required since the W3C specification currently takes into account send codecs as well. Spec issue: w3c/webrtc-pc#2888 Spec PR: w3c/webrtc-pc#2926 setCodecPreferences continues to modify the codecs in an offer. Also rename RtpSender::SetCodecPreferences to RtpSender::SetSendCodecs for consistent semantics. BUG=webrtc:15396 Change-Id: I1e8fbe77cb2670575578a777ed1336567a1e4031 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/328780 Reviewed-by: Henrik Boström <[email protected]> Commit-Queue: Philipp Hancke <[email protected]> Reviewed-by: Harald Alvestrand <[email protected]> Cr-Commit-Position: refs/heads/main@{#41719}
We already cherry-picked this when we vendored 1cce1d7ddc. Upstream commit: https://webrtc.googlesource.com/src/+/1e7a6f3b6a8eee7efcb129eec10fe734d718ebc8 Revert "Make setCodecPreferences only look at receive codecs" This reverts commit 1cce1d7ddcbde3a3648007b5a131bd0c2638724b. Reason for revert: Breaks WPTs Original change's description: > Make setCodecPreferences only look at receive codecs > > which is what is noted in JSEP: > https://www.rfc-editor.org/rfc/rfc8829.html#name-setcodecpreferences > > Some W3C spec modifications are required since the W3C specification > currently takes into account send codecs as well. > > Spec issue: > w3c/webrtc-pc#2888 > Spec PR: > w3c/webrtc-pc#2926 > > setCodecPreferences continues to modify the codecs in an offer. > > Also rename RtpSender::SetCodecPreferences to RtpSender::SetSendCodecs for consistent semantics. > > BUG=webrtc:15396 > > Change-Id: I1e8fbe77cb2670575578a777ed1336567a1e4031 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/328780 > Reviewed-by: Henrik Boström <[email protected]> > Commit-Queue: Philipp Hancke <[email protected]> > Reviewed-by: Harald Alvestrand <[email protected]> > Cr-Commit-Position: refs/heads/main@{#41719} Bug: webrtc:15396 Change-Id: I7b545e91f820c3affc39841c6e93939eac75c363 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/339520 Reviewed-by: Harald Alvestrand <[email protected]> Commit-Queue: Harald Alvestrand <[email protected]> Owners-Override: Henrik Boström <[email protected]> Reviewed-by: Henrik Boström <[email protected]> Auto-Submit: Henrik Boström <[email protected]> Bot-Commit: [email protected] <[email protected]> Cr-Commit-Position: refs/heads/main@{#41725}
Essentially a no-op since we're going to see this change reverted when we vendor in 1e7a6f3b6a. Upstream commit: https://webrtc.googlesource.com/src/+/1cce1d7ddcbde3a3648007b5a131bd0c2638724b Make setCodecPreferences only look at receive codecs which is what is noted in JSEP: https://www.rfc-editor.org/rfc/rfc8829.html#name-setcodecpreferences Some W3C spec modifications are required since the W3C specification currently takes into account send codecs as well. Spec issue: w3c/webrtc-pc#2888 Spec PR: w3c/webrtc-pc#2926 setCodecPreferences continues to modify the codecs in an offer. Also rename RtpSender::SetCodecPreferences to RtpSender::SetSendCodecs for consistent semantics. BUG=webrtc:15396 Change-Id: I1e8fbe77cb2670575578a777ed1336567a1e4031 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/328780 Reviewed-by: Henrik Boström <[email protected]> Commit-Queue: Philipp Hancke <[email protected]> Reviewed-by: Harald Alvestrand <[email protected]> Cr-Commit-Position: refs/heads/main@{#41719}
We already cherry-picked this when we vendored 1cce1d7ddc. Upstream commit: https://webrtc.googlesource.com/src/+/1e7a6f3b6a8eee7efcb129eec10fe734d718ebc8 Revert "Make setCodecPreferences only look at receive codecs" This reverts commit 1cce1d7ddcbde3a3648007b5a131bd0c2638724b. Reason for revert: Breaks WPTs Original change's description: > Make setCodecPreferences only look at receive codecs > > which is what is noted in JSEP: > https://www.rfc-editor.org/rfc/rfc8829.html#name-setcodecpreferences > > Some W3C spec modifications are required since the W3C specification > currently takes into account send codecs as well. > > Spec issue: > w3c/webrtc-pc#2888 > Spec PR: > w3c/webrtc-pc#2926 > > setCodecPreferences continues to modify the codecs in an offer. > > Also rename RtpSender::SetCodecPreferences to RtpSender::SetSendCodecs for consistent semantics. > > BUG=webrtc:15396 > > Change-Id: I1e8fbe77cb2670575578a777ed1336567a1e4031 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/328780 > Reviewed-by: Henrik Boström <[email protected]> > Commit-Queue: Philipp Hancke <[email protected]> > Reviewed-by: Harald Alvestrand <[email protected]> > Cr-Commit-Position: refs/heads/main@{#41719} Bug: webrtc:15396 Change-Id: I7b545e91f820c3affc39841c6e93939eac75c363 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/339520 Reviewed-by: Harald Alvestrand <[email protected]> Commit-Queue: Harald Alvestrand <[email protected]> Owners-Override: Henrik Boström <[email protected]> Reviewed-by: Henrik Boström <[email protected]> Auto-Submit: Henrik Boström <[email protected]> Bot-Commit: [email protected] <[email protected]> Cr-Commit-Position: refs/heads/main@{#41725}
This reverts commit 1cce1d7. Reason for revert: Breaks WPTs Original change's description: > Make setCodecPreferences only look at receive codecs > > which is what is noted in JSEP: > https://www.rfc-editor.org/rfc/rfc8829.html#name-setcodecpreferences > > Some W3C spec modifications are required since the W3C specification > currently takes into account send codecs as well. > > Spec issue: > w3c/webrtc-pc#2888 > Spec PR: > w3c/webrtc-pc#2926 > > setCodecPreferences continues to modify the codecs in an offer. > > Also rename RtpSender::SetCodecPreferences to RtpSender::SetSendCodecs for consistent semantics. > > BUG=webrtc:15396 > > Change-Id: I1e8fbe77cb2670575578a777ed1336567a1e4031 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/328780 > Reviewed-by: Henrik Boström <[email protected]> > Commit-Queue: Philipp Hancke <[email protected]> > Reviewed-by: Harald Alvestrand <[email protected]> > Cr-Commit-Position: refs/heads/main@{#41719} Bug: webrtc:15396 Change-Id: I7b545e91f820c3affc39841c6e93939eac75c363 No-Presubmit: true No-Tree-Checks: true No-Try: true
Upstream commit: https://webrtc.googlesource.com/src/+/db2f52ba88cf9f98211df2dabb3f8aca9251c4a2 Reland "Make setCodecPreferences only look at receive codecs" This is a reland of commit 1cce1d7ddcbde3a3648007b5a131bd0c2638724b after updating the WPT that broke on Mac. Original change's description: > Make setCodecPreferences only look at receive codecs > > which is what is noted in JSEP: > https://www.rfc-editor.org/rfc/rfc8829.html#name-setcodecpreferences > > Some W3C spec modifications are required since the W3C specification > currently takes into account send codecs as well. > > Spec issue: > w3c/webrtc-pc#2888 > Spec PR: > w3c/webrtc-pc#2926 > > setCodecPreferences continues to modify the codecs in an offer. > > Also rename RtpSender::SetCodecPreferences to RtpSender::SetSendCodecs for consistent semantics. > > BUG=webrtc:15396 > > Change-Id: I1e8fbe77cb2670575578a777ed1336567a1e4031 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/328780 > Reviewed-by: Henrik Boström <[email protected]> > Commit-Queue: Philipp Hancke <[email protected]> > Reviewed-by: Harald Alvestrand <[email protected]> > Cr-Commit-Position: refs/heads/main@{#41719} Bug: webrtc:15396 Change-Id: I0c7b17f00de02286f176b500460e17980b83b35b Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/339541 Commit-Queue: Philipp Hancke <[email protected]> Reviewed-by: Harald Alvestrand <[email protected]> Cr-Commit-Position: refs/heads/main@{#41807}
Upstream commit: https://webrtc.googlesource.com/src/+/db2f52ba88cf9f98211df2dabb3f8aca9251c4a2 Reland "Make setCodecPreferences only look at receive codecs" This is a reland of commit 1cce1d7ddcbde3a3648007b5a131bd0c2638724b after updating the WPT that broke on Mac. Original change's description: > Make setCodecPreferences only look at receive codecs > > which is what is noted in JSEP: > https://www.rfc-editor.org/rfc/rfc8829.html#name-setcodecpreferences > > Some W3C spec modifications are required since the W3C specification > currently takes into account send codecs as well. > > Spec issue: > w3c/webrtc-pc#2888 > Spec PR: > w3c/webrtc-pc#2926 > > setCodecPreferences continues to modify the codecs in an offer. > > Also rename RtpSender::SetCodecPreferences to RtpSender::SetSendCodecs for consistent semantics. > > BUG=webrtc:15396 > > Change-Id: I1e8fbe77cb2670575578a777ed1336567a1e4031 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/328780 > Reviewed-by: Henrik Boström <[email protected]> > Commit-Queue: Philipp Hancke <[email protected]> > Reviewed-by: Harald Alvestrand <[email protected]> > Cr-Commit-Position: refs/heads/main@{#41719} Bug: webrtc:15396 Change-Id: I0c7b17f00de02286f176b500460e17980b83b35b Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/339541 Commit-Queue: Philipp Hancke <[email protected]> Reviewed-by: Harald Alvestrand <[email protected]> Cr-Commit-Position: refs/heads/main@{#41807}
Closing this issue in favor of the more recent #3006 that reflects what we discussed at the latest TPAC. However while we settled on the path forward (Proposal A of the slides) we didn't discuss how to handle errors (e.g. changing direction and not having any codecs applicable to that direction anymore) so we'll have to continue at next meeting |
(for after TPAC, this is a minor detail and we have more important things to discuss)
https://w3c.github.io/webrtc-pc/#dom-rtcrtptransceiver-setcodecpreferences
Step 6 says that
This means that you must negotiate at least one codec that you can send and receive.
While this seems harmless, it causes trouble with asymmetric codec support and that is a fairly common thing. E.g. in Chrome on Windows we have 14 send codecs and 21 receive codecs, with the asymmetric ones ranging from H264 over VP9 to AV1.
The worst example we found was Android where the H264 send and receive profiles are not an exact string match and we have support for sending 42e032, 4d0032 and 640032 while being able to receive 42001f, 4d001f and 64001f. This lead to errors in a fairly innocent attempt to restrict the codecs to H264 variants:
The sentence "this ensures that we always have something to offer" is there to avoid the subsequent problem, not being able to negotiate a codec when the transceiver direction changes like this:
If this was taken to createOffer and setLocalDescription it might lead to an m= line that is rejected which would cause the transceiver to be stopped which would be quite surprising.
setCodecPreferences might check the direction in that case but that just leads to the following scenario:
One approach to solving this is that an attempt to set the direction to 'recvonly' might throw. This also requires a compability check in setCodecPreferences in case the direction was set before setCodecPreferences.
The other alternative would be to let createOffer (which knows about send+recv codes as well as the direction) throw an error when it detects this mismatch. It gets a bit finicky to figure out which transceiver was causing this, the mid might not be available.
Overall I think this is an edge case but the first example is a fairly common flow that is currently broken (depending on the codec support)
Also: how is the "intersection of codecs" defined? It seems that for H264 this must take into account whether level-asymmetry is allowed, no?
The text was updated successfully, but these errors were encountered: