pion / webrtc

Pure Go implementation of the WebRTC API
https://pion.ly
MIT License
13.68k stars 1.65k forks source link

Implement ICECandidatePoolSize (gathering before SetLocalDescription/SetRemoteDescription) #2892

Open om26er opened 1 month ago

om26er commented 1 month ago

Here is the code

    // Prepare the configuration
    config := webrtc.Configuration{
        ICEServers:           r.iceServers,
        ICECandidatePoolSize: 10,
    }

    // Create a new RTCPeerConnection
    connection, err := webrtc.NewPeerConnection(config)
    if err != nil {
        panic(err)
    }

    connection.OnICECandidate(func(candidate *webrtc.ICECandidate) {
        log.Println(candidate)
    })

It doesn't do anything unless description is set

om26er commented 1 month ago

What I expect is the candidates gathering to start even before setting the description.

nils-ohlmeier commented 1 month ago

I'm not sure if Pion has a special mode for your expectation. But your expectation is certainly not true for browser implementations. The reason is that before you set any description the implementation doesn't know for how many streams it needs to provide ICE candidates and if bundle is going to be used or not.

I think the more reasonable expectation would be to say that with pools activated there should be a lot less delay in gathering candidates after setting a description, compared to without using an ICE candidate pool.

Sean-Der commented 1 week ago

This could be a great optimization! All for adding this @nils-ohlmeier @om26er

To keep things simple we could treate ICECandidatePoolSize as a bool. If != 0 then gather before signaling.

Great first issue for someone to work on!

nils-ohlmeier commented 1 week ago

I didn't realize that Pion doesn't have support for ICE candidate pool yet.

Yes having the default pool size be 0 and meaning no gathering before set(Local|Remote)Description makes perfect sense to me. And then an API to set it to some positive number if pooling is desired.