aiortc / aioice

asyncio-based Interactive Connectivity Establishment (RFC 5245)
BSD 3-Clause "New" or "Revised" License
106 stars 52 forks source link

Add support for ICE trickle #4

Open jlaine opened 6 years ago

jlaine commented 6 years ago

Currently aioice:

Adding ICE trickle support is highly desirable, but:

ggsato commented 5 years ago

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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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

ivelin commented 4 years ago

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

jjsenay commented 4 years ago

@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 commented 2 years ago

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 commented 2 years ago

@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:

rprata commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

Follow my branch (I create a WIP PR to check difference while developing): https://github.com/aiortc/aioice/pull/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 commented 1 year ago

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

guidocalvano commented 1 year ago

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?