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

Clarify a=rtpmap codec mappings should be offered even if the codec is not a preferred one #2932

Closed
henbos opened this issue Feb 2, 2024 · 20 comments
Labels
Needs Test Needs a WPT test

Comments

@henbos
Copy link
Contributor

henbos commented Feb 2, 2024

My reading of RFC3264, section 6.1 leads me to believe that in sendrecv and recvonly use cases, the answerer has no choice but to have at least one receive codec that was in the offer. So even though it can add its own codecs, there has to be some codec in common between offerer and answerer.

In order to support unidirectional use cases (pc1 prefers X, pc2 prefers Y), pc1 has to list codec Y in the offer even though it is not in its preference list and has no intention of ever receiving it. In other words the a=rtpmap list of codecs is a separate list of codecs than the a=audio/video preference list.

Proposal: Add a NOTE under setCodecPreferences explaining that the preference list only affects the a=audio/video line, not which codecs get a PT. And we need test coverage that removing a codec from the preference list does not prevent it getting a PT.

IIUC...

@fippo
Copy link
Contributor

fippo commented Feb 7, 2024

https://www.rfc-editor.org/rfc/rfc8829.html#section-5.3.1 updates this in the bullet points following

Otherwise, each "m=" section in the answer

I do not think those "unidirectional use-cases" are well supported or tested currently and would not push this further without a clear need.

@henbos
Copy link
Contributor Author

henbos commented Feb 7, 2024

pc1 should be able to ask for VP8 and pc2 ask for H264 and it should work, but on Chromium the following fiddle results in VP8 in both directions: https://jsfiddle.net/henbos/1vzjybs3/

When we setCodecPreferences, we don't just filter out the preferences, we filter out the codecs, and it doesn't seem to add new codecs either IDK

@fippo
Copy link
Contributor

fippo commented Feb 7, 2024

Which is expected and how things have been behaving like this for a decade.

And as https://wpt.fyi/results/webrtc/protocol/additional-codecs.html?label=experimental&label=master&aligned shows adding H264 in the answer causes H264 to be sent by pc1 so we have forward compability for whenever implementations feel motivated to implement what JSEP suggested (as an optional thing I believe)

I'm open to the argument that setCodecPreferences should not restrict the codecs pc1 offers but this introduces a compability risk without clear benefits.

@henbos
Copy link
Contributor Author

henbos commented Feb 7, 2024

Which is expected and how things have been behaving like this for a decade.

It's good for compat that adding H264 via SDP munging works already. But how do I ask to receive VP8 and have the other endpoint ask to receive H264 using setCodecPreferences?

You could tweak it to list both [vp8, h264] and then swap order in answer: https://jsfiddle.net/henbos/jeLav7c3/
But I don't know that this makes sense if the codecs were unidirectional. I don't want to "prefer to receive" something I can't decode if they actually listened to me. (Also the fiddle currently does not work)

@henbos
Copy link
Contributor Author

henbos commented Feb 7, 2024

There was a bug in the fiddle (by awaiting replaceTrack I did setCodecPreferences too late at pc2 so it missed the answer!).

Fixed and it works for the case of offer [vp8, h264] and answer [h264, vp8]: https://jsfiddle.net/henbos/v1rpmfqs/
But it does not work in the case of offering [vp8] and answering [h264]: https://jsfiddle.net/henbos/jcgwvf20/

This may be good or it may be bad depending on what we want to do with unidirectional codecs.

@fippo
Copy link
Contributor

fippo commented Feb 7, 2024

It's good for compat that adding H264 via SDP munging works already.

This isn't munging, this is signaling ;-)

But how do I ask to receive VP8 and have the other endpoint ask to receive H264 using setCodecPreferences?

That is not what setCodecPreferences does, it only controls receive preferences. We have answer handling for additional codecs but we do not have anything in offers. https://www.rfc-editor.org/rfc/rfc8829.html#section-5.2.1 is quite specific:

If codec preferences have been set for the associated transceiver, media formats MUST be generated in the
corresponding order and MUST exclude any codecs not present in the codec preferences.

and that is what we are doing?

@henbos
Copy link
Contributor Author

henbos commented Feb 8, 2024

That is not what setCodecPreferences does, it only controls receive preferences. [...] and that is what we are doing?

What we're seeing is not being allowed to prefer to receive H264 unless the offerer is preferring to receive it too. This is fine for codecs that work in both directions because you can always swap preference order and use the codec you don't want to use as a fallback, but it makes little sense for unidirectional codecs.

Whether we want to solve this or not is up to us, there are workarounds like modifying SDP (in a legal way) or preferring to receive something you can't receive knowing the other endpoint will swap orders. Just because you can shoot yourself in the foot doesn't mean you will.

@henbos
Copy link
Contributor Author

henbos commented Feb 8, 2024

Heeey, we're already dealing with unidirectionality with this filtering:

If transceiver.direction is "sendonly" or "sendrecv", exclude any codecs not included in the list of implemented send codecs for kind.

If transceiver.direction is "recvonly" or "sendrecv", exclude any codecs not included in the list of implemented receive codecs for kind.

This means that if I want to use a sendonly codec WITHOUT having to modify SDP I simply ensure the transceiver is sendonly. And if I want to do sendrecv, then I can always modify SDP, but nobody is forcing me to do this.

I think no changes are needed but we should add more WPTs. Yay.

@henbos henbos closed this as completed Feb 8, 2024
@henbos
Copy link
Contributor Author

henbos commented Feb 8, 2024

Oh but the plot thickens, it's not possible to setCodecPreferences([send-only codec]) if #2926 lands

@alvestrand
Copy link
Contributor

preferring to receive on a send-only codec is kind of silly, so that would be a feature not a bug....

@henbos
Copy link
Contributor Author

henbos commented Feb 8, 2024

preferring to receive on a send-only codec is kind of silly, so that would be a feature not a bug....

If transciever.direction is sendrecv or recvonly, the filtering we have already makes sure you wouldn't prefer to receive a send-only codec, protecting against that which makes no sense.

But if the direction is sendonly, then we have to list the sendonly codec in order to allow pc2 to answer with this codec.

  • Either that or we say that the way to do unidirection is by modifying SDP. It's not illegal, it's just ugly.

@henbos
Copy link
Contributor Author

henbos commented Feb 8, 2024

@fippo Explained to me that the fact that we have to SDP munge to add H264 in the setCodecPreferences([h264]) fiddle is an implementation bug, not a spec issue. In that case this mess resolves itself. I'll re-approve fippo's PR

@henbos henbos closed this as completed Feb 8, 2024
@fippo
Copy link
Contributor

fippo commented Feb 8, 2024

It is a missing feature in the implementation, in particular the

Any currently available media formats that are not present in the current remote description MUST be added after all existing formats.

from https://www.rfc-editor.org/rfc/rfc8829.html#name-initial-answers

I did tinker with a recv-only codec and setting the transceiver direction to sendonly:
https://jsfiddle.net/fippo/r63bp8d2/1/
Works fine with and without this change. This is fine but the logic is weird... you can only determine the send codec once you have the other side's answer. But the other side will not know what codecs you support...

@henbos
Copy link
Contributor Author

henbos commented Feb 8, 2024

But the other side will not know what codecs you support...

Yeah this to me is an argument for allowing to "prefer to receive send codecs" and let the transciever.direction filtering take care of whether or not you're doing nonsense, but still base the a=rtpmap list based on what you provided to setCodecPreferences, so that the answerer can always know whether or not the codec is supported.

But while this helps sendonly transceiver with sendonly codec use case, it also adds complexity and confusion, so probably best to keep it simple: setting preference is the same thing as setting receive codecs.

@henbos
Copy link
Contributor Author

henbos commented Feb 8, 2024

[[PreferredCodecs]] should be the single-element list with VP9 profile=2. Since that is not in the list of implemented send codecs it should be excluded. But it is not as the generated offer still contains that codec.

This is the sentence that I proposed to delete in #2926 because it doesn't make sense when we consider setCodecPreferences just receiver preferences.

If we delete this sentence, then the fact that we don't exclude it is a feature, but if we don't delete it, it is a bug.

@henbos henbos reopened this Feb 8, 2024
@fippo
Copy link
Contributor

fippo commented Feb 8, 2024

Pilot error previously, deleted that comment. So there is a change of behavior in what I am proposing:
https://jsfiddle.net/fippo/r63bp8d2/2/
currently bails out in sCP because VP9 profile-id=1 is not a codec that can be sent.

With my change it bails out in setLocalDescription. Further splitting this up into createOffer + SLD shows that the offer is created with a rejected m-line. This gets rejected by Chromium because

Failed to set local offer sdp: Failed to set local video description recv parameters for m-section with mid='0'

which I believe to be a bogus error since it is attempting to set receive parameters on a m-line marked as rejected.

@fippo
Copy link
Contributor

fippo commented Feb 8, 2024

This is the sentence that I proposed to delete in #2926 because it doesn't make sense when we consider setCodecPreferences just receiver preferences.

In general this needs to stay. The current behavior is not to include recvonly codecs on a sendrecv m-line. We can not and should not change that.

The consequence of doing something foolish is the transceiver being stopped after SLD (I hope... we currently don't get there)

@henbos
Copy link
Contributor Author

henbos commented Feb 8, 2024

I can argue either way on that one. The sad consequence is it that the receiving endpoint does not know which profiles the sending endpoint supports. So we're designing the API in a way where you have to signal codec capabilities outside of the SDP instead of simply including them in the a=rtpmap but without including them in the preferences.

This problem goes away if we allow a=rtpmap to not be filtered out while preferences is filtered out. Doesn't that make more sense somehow?

@fippo
Copy link
Contributor

fippo commented Feb 8, 2024

Doesn't that make more sense somehow?

JSEP says we MUST NOT do that. I assume the reason is that the answerer could interpret such codecs as codecs we are willing to receive.

I've addressed the SLD failure from https://jsfiddle.net/fippo/r63bp8d2/2/ with my change, turned out to be a fairly trivial implementation issue. The new behavior if you set a receive-only codec via sCP and set the direction to sendonly is that the transceiver is stopped by SLD.

@henbos
Copy link
Contributor Author

henbos commented Feb 8, 2024

Perfect, if JSEP forbids, we don't even need to discuss this. sCP is just received codecs, the app picking codecs that works is the app's problem

@henbos henbos closed this as completed Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Test Needs a WPT test
Projects
None yet
Development

No branches or pull requests

3 participants