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 ICE consent freshness feedback #517

Closed
nils-ohlmeier opened this Issue Feb 22, 2016 · 8 comments

Comments

Projects
None yet
4 participants
@nils-ohlmeier
Copy link

nils-ohlmeier commented Feb 22, 2016

When implementing RFC 7665 it is not very well defined how the different state transitions from that RFC get reported back through the PeerConnection APIs.

@nils-ohlmeier

This comment has been minimized.

Copy link

nils-ohlmeier commented Feb 22, 2016

I think it is kind of obvious to use the RTCIceConnectionState to communicate the events.

As RFC 7665 mandates that loosing consent after the 30s timer is permanent for a given pair translating loosing consent for a single component into the 'failed' state appears appropriate.
The open question for me is if each STUN transaction timeout from 7665 should get translated into 'disconnected', followed by potentially switching back to 'connected' or 'completed' once consent has been re-established.

@alvestrand

This comment has been minimized.

Copy link
Contributor

alvestrand commented Feb 23, 2016

@nils-ohlmeier can you check #457 to see if this is part of the same problem?
Also check the slides for the interim here:
https://docs.google.com/presentation/d/1R7lQeo4LNGu0KKW9GLdkwJ5x8FUmR4qrnhfPFZO3WS8/edit#slide=id.gd4bd98a32_0_387

@elagerway opinions?

@nils-ohlmeier

This comment has been minimized.

Copy link

nils-ohlmeier commented Feb 23, 2016

To some degree yes it is the same problem, as it also touches the same state diagram.

Whether [CONSENT] final 30s timeout leads to 'failed' or to 'disconnected' (or 'checking' as proposed on the slides for the checking state) does not really matter to me. It just needs to be defined and written down. That should be easy.
The open questions which I don't have a good answer to (yet) is if a timeout on a single STUN transaction from [CONSENT] should result in a (temporary) state change (and if so to which state). From reading through #457 that does not appear as question which has been raised. But I'll happily add it to the discussion in #457, because it makes sense to clean that up together.

@aboba

This comment has been minimized.

Copy link
Contributor

aboba commented Feb 24, 2016

The outcome of each individual consent check should not be reflected in either RTCIceTransportState or RTCIceConnectionState, because there is nothing a developer can or should do about the failure of individual consent checks. Consent request/response statistics is another matter.

My suggestion is that a consent failure in the "completed" state should cause a transition to "failed" for both RTCIceTransportState and RTCIceConnectionState.

@aboba

This comment has been minimized.

Copy link
Contributor

aboba commented Mar 3, 2016

@nils-ohlmeier @alvestrand I have submitted RTCIceTransportState definitions that at least partially takes consent check failures into account: PR #533

@aboba aboba added the PR exists label Mar 3, 2016

@aboba aboba removed the PR exists label Mar 17, 2016

@aboba

This comment has been minimized.

Copy link
Contributor

aboba commented Apr 14, 2016

Related to #457

@alvestrand

This comment has been minimized.

Copy link
Contributor

alvestrand commented Apr 28, 2016

Depends on #457

aboba added a commit that referenced this issue May 2, 2016

@aboba

This comment has been minimized.

Copy link
Contributor

aboba commented May 2, 2016

@nils-ohlmeier @taylor-b Can you review PR #611 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment