Bug 30972

If I understand correctly, when the streaming implementation calls NewActiveCandidatePairWithInfo(ni, nt, ri, rt), that gives us up to five things:

A. tells us that the active candidate pair is (ni, ri), exactly equivalent to NewActiveCandidatePair(ni, ri)

B. potentially tells us a new native candidate that we didn't already know about, which is equivalent to getting NewNativeCandidate(ni, [nt]) just before NewActiveCandidatePair

C. potentially tells us a new remote candidate that wasn't in the signalling (is that even meaningful? in the current API, the CM is assumed to know all remote candidates from the signalling, and feed them to the streaming implementation)

D. tells us which one of the signalled native candidates the streaming implementation has actually chosen to use (is this meaningful?)

E. tells us which one of the remote candidates the streaming implementation has actually chosen to use (is this meaningful?)

Which of those five things are actually needed here? Obviously (A) is.

Note that NativeCandidatesPrepared just means that all native candidates have been discovered *for the moment* (the spec specifically says so), so I think CMs should be prepared to deal with NewNativeCandidate at any time.

Sorry, that comment made no sense, because I mixed up candidates and transports. Let me try again.

If I understand correctly, when the streaming implementation calls
NewActiveCandidatePairWithInfo(ni, nt, ri, rt), that gives us up to five
things:

A. tells us that the active candidate pair is (ni, ri), exactly equivalent to
NewActiveCandidatePair(ni, ri)

B. potentially tells us a new native candidate (which is restricted to having exactly one transport) that we didn't already know about, which is equivalent to getting NewNativeCandidate(ni, [nt]) just before NewActiveCandidatePair

C. potentially tells us a new remote candidate, restricted to having exactly one transport, that wasn't in the signalling (is that even meaningful? in the current API, the CM is assumed to know all remote candidates from the signalling, and feed them to the streaming implementation)

D. tells us which one of the native candidate's transports the streaming
implementation has actually chosen to use (is this meaningful?)

E. tells us which one of the remote candidate's transports the streaming implementation has actually chosen to use (is this meaningful?)

Which of those five things are actually needed here? Obviously (A) is.

Note that NativeCandidatesPrepared just means that all native candidates have
been discovered *for the moment* (the spec specifically says so), so I think
CMs should be prepared to deal with NewNativeCandidate at any time.

(In reply to comment #2)

Ok, so first, let me explain an important detail:

 1- The signal is emitted each time a new pair is selected for a component. So it's a "Candidate pair" in [farsight, ice] world but in Telepathy world, it should be called a "Transport pair". I'm using the wrong terminology, but so is current NewActiveCandidatePair that is emitted for each component as well (and it doesn't even tell us which component).


Why do I need the selected candidate pair details so badly ?

 2- According to the ICE spec (RFC 5245), once ICE discovery process is completed (doesn't mean you have a valid pair for each component), we need to send an updated offer (re-INVITE). See also "B.9. Why Send an Updated Offer?" of the ICE spec.

   2a- The SDP body of that re-invite need to contains the "default candidate" encoded in the m and c lines. We need the selected local candidate.  The bottom line is that some gateway devices use these SDP informations.

   2b- That re-invite contains a "a=remote-candidate" line listing selected remote candidates.


Can't we just use the ID to find candidate details on CM side ?

 3- Peer-reflexive candidates are not known by the connection manager. 

   3a- We could in theory know about local ones, but right now, tp-farsight only emits NewNativeCandidate when Prepared is called. Also, to emit the new candidate (after Prepared), we would need to wait for all transports to be discovered and it's not sure that would happens. (according to Olivier, we only really need the RTP component)

   3b- There is absolutely no way to find out about remote ones.


> A. tells us that the active candidate pair is (ni, ri), exactly equivalent to
> NewActiveCandidatePair(ni, ri)

Yes

> B. potentially tells us a new native candidate (which is restricted to having
> exactly one transport) that we didn't already know about, which is equivalent
> to getting NewNativeCandidate(ni, [nt]) just before NewActiveCandidatePair

Not restricted to one transport (See [1])
Not really equivalent...it's equivalent to (fictional) NewNativeTransport just before NewActiveCandidatePair (that doesn't tell us which component is affected)

> C. potentially tells us a new remote candidate, restricted to having exactly
> one transport, that wasn't in the signalling (is that even meaningful? in the
> current API, the CM is assumed to know all remote candidates from the
> signalling, and feed them to the streaming implementation)

Not restricted to one transport (See [1])
CM doesn't know about all remote candidates (See [3b])

> D. tells us which one of the native candidate's transports the streaming
> implementation has actually chosen to use (is this meaningful?)

Yes, see [2a]

> E. tells us which one of the remote candidate's transports the streaming
> implementation has actually chosen to use (is this meaningful?)

Yes, see [2b]

> Which of those five things are actually needed here? Obviously (A) is.

All of them

> Note that NativeCandidatesPrepared just means that all native candidates have
> been discovered *for the moment* (the spec specifically says so), so I think
> CMs should be prepared to deal with NewNativeCandidate at any time.

tp-butterfly could be prepared to deal with it, but that's not gonna happen (See [3a])

My branch has been updated with these changes:

 - Renamed to NewActiveTransportPair.
 - Replaced "candidate" with "transport" where needed to avoid confusion.
 - Add constraint about component ID being the same for both transports and specify that the pair is only valid for that component.
 - Add a section about what CMs should implement this method.

> +        <p>Only connection managers that want to inform the peer about
> +          selected transports SHOULD implement this method.</p>

I'd prefer that worded something like:

Connection managers SHOULD NOT implement this method unless they need to inform the peer about selected transports. As a result, streaming implementations MUST NOT treat errors raised by this method as fatal.

++, with or without that text.

Pushed