Implement full ICE #56
Conversation
Pull Request Test Coverage Report for Build 397
💛 - Coveralls |
pkg/ice/candidate.go
Outdated
|
|
||
| // String for CandidateHost | ||
| func (c *CandidateHost) String(component int) string { | ||
| return fmt.Sprintf("udpcandidate %d udp %d %s %d typ host generation 0", |
There was a problem hiding this comment.
I have mixed feelings about this approach. It seems rather clean but this way we are leaking SDP details into the ICE package. E.g.: If we ever want to support the ORTC API this string format doesn't apply anymore.
There was a problem hiding this comment.
I agree!
Are you ok with the SDP package importing ICE? I can have it case off the Candidate type in there.
Then it can generate the string exactly the same way (and then in the future we can add ORTC support)
There was a problem hiding this comment.
That does seem more appropriate. ICE does seem like a pretty big dependency for the SDP package but we can worry about that another time.
Sean-Der
force-pushed
the
full-ice
branch
3 times, most recently
from
faf22b3
to
52ffc29
Compare
Candidates are now generated with real priority, and actually pay attention to the inbound messages. Next we just need to pluck use-candidate + ice-controlling from the messages and we will have a full-ice implementation (but we are not controlling) We then need to add timers to alert when the peer goes away OR when we need to send again.
All the examples need to be updated, SDP should only be emitted after gathering is done (until we support trickle) Currently we can't be controlling-ICE, but that is probably a days worth of work. After that is done pion-WebRTC should be able to talk to itself.
All our examples will need to be updated, now that pion-WebRTC properly implements full ICE it will discard messages from candidates that don't appear in the remote offer/answer
pion-WebRTC passes through every step, but messages get discared because MI check is failing. We are able to Offer (or answer) to Chrome after this is done
Examples need to be expanded to demo this, and lints need to be fixed but no additional code/features are needed
| } | ||
|
|
||
| // Start allocates DTLS/ICE state that is dependent on if we are offering or answering | ||
| func (m *Manager) Start(isOffer bool, remoteUfrag, remotePwd string) error { |
There was a problem hiding this comment.
Is using IsOffer as the DTLS and ICE role a deliberate shortcut for now? My current understanding of the specs:
There was a problem hiding this comment.
Yep! I am going to cut a ticket for support ice-lite and going to be testing against mediasoup. So will changed to isControllingICE and dtlsRole maybe?
DTLS does work right now using just offer/answer. The setup attribute just allows someone to fast start (which we should do in the future) but wasn't going to do yet.
The is the overall architecture.
ICEAgentis a member ofnetwork/manager. (All of the state innetwork/managercould go directly on RTCPeerConnection. I think we should keep the code in the top-level as terse as possible. If it isn't relevant for a user to browse/read it should be in internal or pkg.)Each port push/pulls from
ICEAgentthe STUN traffic for send/recv. The ICEAgent itself has all the timers/selected-pair.When data comes from the RTCPeerConnection it queries the ICEAgent for the selected pair, and if it has one it will admin the proper src/dest. Otherwise it will return nothing and the traffic will be dropped/queued.
All tests and lints pass, and the examples still work! After this lands IMO this is what we need to knock out.