Does juice_add_remote_candidate have to be called p2p in on_candidate or on_gathering_done callback to be effective?

[fengshangren]

I have been unsuccessful in p2p today. I have tested many times and found that juice_add_remote_candidate must be called in the on_candidate or on_gathering_done callback for p2p to succeed. Is this true?

[paullouisageneau]

Please prefer discussions to ask questions rather than opening issues in the future.

I have tested many times and found that juice_add_remote_candidate must be called in the on_candidate or on_gathering_done callback for p2p to succeed. Is this true?

juice_add_remote_candidate must be called when receiving a candidate from the remote side: when one side obtains a candidate in on_candidate, it sends it to the other side, which receives it and pass it to juice_add_remote_candidate.

However, if you choose to send the descriptions with candidates inside, by sending the local description in on_gathering_done rather than immediately, juice_add_remote_candidate should not be used and on_candidate should be ignored (see the notrickle test).

[paullouisageneau]

What do you mean by P2P always fail? This example will fail if connecting through NATs as you left the check that the connection is local:

if ((!strstr(local, "typ host") && !strstr(local, "typ prflx")) ||
            (!strstr(remote, "typ host") && !strstr(remote, "typ prflx")))
            success = false; // local connection should be possible

[paullouisageneau]

You want to test on two computers with a NAT traversable environment, but this example will never succeed in a direct P2P connection

I spent time doing so, and it succeeds.

Output for peer 1:

State 1: gathering
State 1: connecting
Gathering done 1

LocalSdp:
a=ice-ufrag:Aic6
a=ice-pwd:hwn5CxuyHuXG3AoHThyiO9
a=candidate:1 1 UDP 2122317823 192.168.139.114 34709 typ host
a=candidate:2 1 UDP 1686109951 77.63.X.X 36413 typ srflx raddr 0.0.0.0 rport 0
a=end-of-candidates
a=ice-options:ice2

RemoteSdp: 
a=ice-ufrag:P9rE
a=ice-pwd:Iaoo/UE52DodM+fXTpuGYy
a=candidate:3 1 UDP 2130705919 2a02:X:X:X:X:X:X:X 53396 typ host
a=candidate:1 1 UDP 2122317823 10.9.0.101 53396 typ host
a=candidate:3 1 UDP 1686108927 77.249.X.X 53396 typ srflx raddr 0.0.0.0 rport 0
a=end-of-candidates
a=ice-options:ice2

State 1: connected
Received 1: Hello from 1
State 1: completed
Local candidate  1: a=candidate:2 1 UDP 1686109951 77.63.X.X 36413 typ srflx raddr 0.0.0.0 rport 0
Remote candidate 1: a=candidate:6 1 UDP 1686108927 77.249.X.X 53396 typ srflx raddr 0.0.0.0 rport 0
Success

Output for peer 2:

State 1: gathering
State 1: connecting
Gathering done 1

LocalSdp:
a=ice-ufrag:P9rE
a=ice-pwd:Iaoo/UE52DodM+fXTpuGYy
a=candidate:3 1 UDP 2130705919 2a02:X:X:X:X:X:X:X 53396 typ host
a=candidate:1 1 UDP 2122317823 10.9.0.101 53396 typ host
a=candidate:3 1 UDP 1686108927 77.249.X.X 53396 typ srflx raddr 0.0.0.0 rport 0
a=end-of-candidates
a=ice-options:ice2

RemoteSdp: 
a=ice-ufrag:Aic6
a=ice-pwd:hwn5CxuyHuXG3AoHThyiO9
a=candidate:1 1 UDP 2122317823 192.168.139.114 34709 typ host
a=candidate:2 1 UDP 1686109951 77.63.X.X 36413 typ srflx raddr 0.0.0.0 rport 0
a=end-of-candidates
a=ice-options:ice2

Received 1: Hello from 1
State 1: connected
State 1: completed
Local candidate  1: a=candidate:6 1 UDP 1686108927 77.249.X.X 53396 typ srflx raddr 0.0.0.0 rport 0
Remote candidate 1: a=candidate:2 1 UDP 1686109951 77.63.X.X 36413 typ srflx raddr 0.0.0.0 rport 0
Success

You should fix the extremely short sleep(4) with sleep(30) in your code. As it is, the first peer to get the remote description will wait for only 4 seconds, then destroy the agent and print success or failure. This obviously makes manual testing very difficult.

Then, your code to read the remote SDP is bad, it mixes C and C++, doesn't check bounds and will break if gathering is not finished or if you forget to press enter at the end. You should rewrite it correctly, for instance:

cout << "RemoteSdp: " << endl;
string remotesdp, remotesdpline;
while (getline(cin, remotesdpline))
{
    if (remotesdpline.empty())
        break;

    remotesdp += remotesdpline + "\r\n";
}

juice_set_remote_description(agent, remotesdp.c_str());
cout << "OK" << endl;
sleep(30);

Finally, please develop a proper signaling channel to automatically forward descriptions and candidates between agents rather than relying on clumsy manual copy-pasting between machines. It's unreliable since you might not be able to do so manually before the first peer times out.

[paullouisageneau]

I'm closing this as it doesn't look like an issue with libjuice.