-
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
Composed RTCPeerConnection.gatheringState seems wrong in some cases #2914
Comments
"new" means you have not gathered candidates. Which is true for a very short period of time during an ice restart until new host candidates (which are unlikely to change) are gathered. |
But you have gathered candidates. The ICE restart has not been committed yet, and the old ICE transport is still being used. |
As for why we should care, having the gathering state sometimes transition through new if the timing is just right (wrong?) makes it really difficult to write tests. |
The relevant spec link is https://w3c.github.io/webrtc-pc/#rtcicegatheringstate-enum Question: Why are there two transports? Given that https://w3c.github.io/webrtc-pc/#rtcicetransport says "An ICE restart for an existing RTCRtpTransceiver will be represented by an existing RTCIceTransport object, whose state will be updated accordingly, as opposed to being represented by a new object." In the unbundled case, both transports would transition to "new", I guess. So what's the case when this can happen? |
Whoops, bad example. You can end up in this situation when adding a new transceiver (in sLD) that isn't bundled. The old transport would not transition back to new; adding a new unbundled transport has no effect on pre-existing transports. |
Of course, the spec saying "An ICE restart for an existing RTCRtpTransceiver will be represented by an existing RTCIceTransport" raises the question "What is the state of that transport while the ICE restart has not been committed yet?" There are two underlying transports that might (or might not) still be gathering; what are the state composition rules for that? Bear in mind that the newer transport might finish gathering first! |
Yeah, that situation is already suboptimal but then you can avoid it with max-bundle. |
What's the model that forces you to assume two underlying transports? To my mind, an RTCIceTransport manages a group of candidate pairs, some of which have data flowing on them; an ICE restart throws out the old ones (except the one that is in use, until changed), and tries to create new ones. None of this should be modeled as creating a new transport. |
If the transport does not change and is make-before-break we should see the iceConnectionState continue to be unchanged (i.e. connected in the somewhat abnormal case of a restart without a disconnect) while gatheringState transitions? I think the gathering state should never be "new" again. You can create an offer with iceRestart but that is only provisional and does not lead to the (re)creation of a transport (neither does creating the initial offer). So this only gets "committed" when you call SLD but that should set the new transport into "gathering" mode immediately. (this is confusing and we probably have subtly different mental models) |
A new set of candidate pairs, which ICE starts on from scratch, is a new transport in exactly the same way that a new non-bundled m-section is a new transport. Exactly the same state carries over in each of those cases (ie; the certs and basically nothing else). You still need to do a new DTLS handshake, you still need to establish new SRTP keys, you still need to carry out a new SCTP handshake (if you're using datachannel), etc. You can try to abstract that away (eg; with RTCIceTransport), but that's really what is going on under the hood. |
Agreed.
Yeah, I think we're using some terminology differently. When I say an ICE restart is "committed", I'm referring to the moment when we've decided that we're definitely going to try to use the new transport; when offer/answer concludes. Until that happens, we're kinda in transport limbo; we're gathering for a new underlying transport, but haven't started ICE for it yet, and we don't know whether we'll actually end up using it (note: this is the same sort of situation you get when reoffering an m-section unbundled). Once the answer is applied, then we start ICE on that new transport, while the old continues running for some unspecified duration to allow for continuity. |
One example:
transport1: new, pc: new
transport1: gathering, pc: gathering
sLD(ICE restart offer)
transport1: gathering, transport2: new, pc: gathering
transport1: complete, transport2: new, pc: new <--- Wrong!
transport1: complete, transport2: gathering, pc: gathering
I don't think it makes sense for RTCPeerConnection.gatheringState to be "new" unless we have never started gathering before. Once we transition out of "new", we should never return; the pc's gathering state should either be "complete" if all transports are complete or "gathering" otherwise.
The text was updated successfully, but these errors were encountered: