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

Add support for ICE trickle #4

Open
jlaine opened this issue Jun 2, 2018 · 13 comments
Open

Add support for ICE trickle #4

jlaine opened this issue Jun 2, 2018 · 13 comments

Comments

@jlaine
Copy link
Collaborator

jlaine commented Jun 2, 2018

Currently aioice:

  • emits all local ICE candidates in one go
  • expects remote ICE candidates to be provided in one go

Adding ICE trickle support is highly desirable, but:

  • it adds significant complexity to the state machine
  • it requires rethinking the API through which candidates are provided / retrieved
@ggsato
Copy link

ggsato commented Mar 15, 2019

Thanks for your clarification.

So, is this kind of code the way to get all the candidates in the half trickle way?

        @self.pc.on('icegatheringstatechange')
        def on_icegatheringstatechange():
            logging.info('iceGatheringState changed to {}'.format(self.pc.iceGatheringState))
            if self.pc.iceGatheringState == 'complete':
                # candidates are ready
                candidates = self.pc.sctp.transport.transport.iceGatherer.getLocalCandidates()
                # add ice candidate iteratively

@ivelin
Copy link

ivelin commented May 18, 2020

@jlaine I wonder if trickle ICE support might be back on the roadmap this year?

Users report the 5-10s delay since their sensitivity to web page response time is in the 1-2s range nowadays.

FYI, pion recently added trickle ICE support.

Not sure if it is directly related, but I noticed when one of the STUN or TURN servers is down, the whole ICE setup process is delayed and eventually fails, even if some of the other STUN, TURN servers are online. I can open a separate issue on this.

@jlaine jlaine reopened this May 19, 2020
@jlaine
Copy link
Collaborator Author

jlaine commented May 19, 2020

Let's keep the discussion here. TURN or STUN slowing down the process is a symptom of the same limitation: candidates are collected in one go.

@ivelin
Copy link

ivelin commented May 19, 2020

Sounds good. Thank you for re-opening the issue. I will keep an eye for guidance on next steps and try to help.

@ivelin
Copy link

ivelin commented Oct 13, 2020

@jlaine just checking in on this issue. Would you be open to a PR that adds ICE trickle support to aiortc?

@jjsenay
Copy link

jjsenay commented Oct 14, 2020

@jlaine If Ambianic would put a bounty on the ICE trickle feature set would you know of anyone that would be interested in adding/implementing ICE trickle in aiortc ?

Thankss!!

@rprata
Copy link

rprata commented Apr 18, 2022

Hi,

I'm interested to help to add support for ice trickle in aioice. I'll check the code to understand the implementation.

@jlaine: How can I tested ice communication? There is an example here that I could start my analysis?

@jlaine
Copy link
Collaborator Author

jlaine commented Apr 18, 2022

@rprata I think the unit test are you best source of information TBH, they illustrate a number of scenarii.

To give you some starting points:

  • look at the gather() method which is in charge of gathering our local candidates
  • AFAICT, ICE trickle for host candidates makes zero sense, we get these instantly
  • the type of candidates which might benefit from ICE trickle are server reflexive (STUN) and relayed (TURN) candidates, as gathering these means getting a reply (which might never arrive) from a distant host

@rprata
Copy link

rprata commented Apr 21, 2022

I'm using this docs to support the implementation. Is it ok?

AFAICT, ICE trickle for host candidates makes zero sense, we get these instantly
Why?I don't understand this. According this article:

In other words, a Trickle ICE client would gather local host candidates and immediately send them to the remote party. It would then discover a TURN/STUN address and send that, then potentially UPnP and send that, etc.

look at the gather() method which is in charge of gathering our local candidates:
I'll start this work refactoring get_component_candidates() to twice new functions get_partial_candidates() and get_stun_turn_candidates(). I'll create a new branch to help us.

@jlaine
Copy link
Collaborator Author

jlaine commented Apr 21, 2022

Yes the RFC you linked seems to be the correct one. Right now aioice is a "half trickle" implementation if I'm not mistaken.

@rprata
Copy link

rprata commented Apr 22, 2022

Follow my branch (I create a WIP PR to check difference while developing):
#56

An interesting point is HEAD (main) failed in lint and test stages CI. Is it better fix it before to implement this or fix it in this PR.

@VaTupuri
Copy link

Is this implementation correct?
I am getting AttributeError: 'NoneType' object has no attribute 'transport'

@guidocalvano
Copy link

https://github.com/aiortc/aiortc/tree/main/examples/webcam

This example still doesn't work, but from this thread I get the impression that it should work. Is there still a problem?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants