-
Notifications
You must be signed in to change notification settings - Fork 451
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
ICE and dtls_srtp fails with dtls already accepted #925
Comments
|
Maybe related: creytiv/re#238 Update: Sadly this was not enough to fix the error. Maybe there is still more todo. |
|
thanks for the report, I will have a look .. |
|
do you have a test account on your TURN server, that I can use for testing? |
|
Sure I sent you a description to get login data. For testing you can use also: |
|
I noticed this: the first 2 pairs have inconsistent state; "Failed" and on the Valid list. |
|
Hm. Looks like a race condition? Any idea how I can debug? One client is behind a normal NAT and the other behind an LTE carrier-grade NAT. Would it be possible to define, like in WEBRTC, that only relay candidates are used? At first look I did not find an easy way to disable other candidates. |
|
I made a test-call from local baresip (behind NAT) to echo@studio.link we should first find this error: it is a matter of checking all pairs, and decide if a pair should be valid or not. in order to reduce the complexity, you can try to use STUN instead of TURN. |
|
Ok i had to switch the echo account to production "turn" only. But we can use now: echo1@studio.link - ICE with turn |
|
I just want to add that the ICE stack in libre has many problems. One of the problems is that the embedded STUN servers https://github.com/creytiv/re/blob/master/src/ice/stunsrv.c#L161 I think this is wrong. You can try to disable this line and see if it makes a difference. Note: we should fix all these ICE errors, but it requires quite a lot of testing ... :) |
|
Is it useful to optimize/repair the ICE in re, or should I take a look at Librew and help/test to merge this work? Is this roadmap https://github.com/creytiv/re/wiki/ICE-Stacks-merge still up-to-date? |
|
it is difficult to say .. I think we have to do it step by step, and make alot |
|
@sreimers I sent you a patch for testing .. |
|
I did some more testing of ICE between Chrome and Baresip-Webrtc. |
|
@alfredh, could you share the patch ? I would like to help on this. |
|
here is the patch: https://gist.github.com/alfredh/78b4adb09ed796e0a2b972dc51b2e0e1 thanks for helping :) |
|
Love it, the patch is simple and make ICE connection working for me. Before the patch I couldn't use baresip-webrtc between local machines without hacking icem_conncheck_start in src/ice/connchk.c like this: This patch fixes the timing issue and I don't need the hack anymore. |
|
great, thanks for testing. @sreimers please let me know if you are planning to test this patch. |
|
Sure (maybe this weekend). |
|
First tests looks great! Many thanks, i will test a bit more. Only if I call a turn only peer:
So the recognition works, but no audio connection is established. But that could be a problem on my end. |
|
great, thanks for reporting and testing. The ICE saga is not over yet, the code still needs improvements. I will close this for now. If you find more ICE bugs, please create a new issue. |
|
@alfredh, how is librew compared to libre ? I mean what is wrong with librew ? |
= libre Changelog == [v1.1.0] - 2020-10-04 === Added - tls: functions to get the certificate issuer and subject [#18] - uri: Added path field to struct uri and its decode to uri_decode [#22] - tcp: add tcp_connect_bind [#24] - http: support bind to laddr in http_request [#24] - sipreg: support Cisco REGISTER keep-alives [#19] - sip: websocket support [#26] === Fixed - tls/openssl: fix X509_NAME win32/wincrypt.h conflict - dns: listen on IPv4 and IPv6 socket [#27] - main: fix/optimize windows file descriptors [#25] === Contributors (many thanks) - Alfred E. Heggestad - Christian Spielberger - Christoph Huber - Franz Auernigg - Juha Heinanen - Sebastian Reimers == [v1.0.0] - 2020-09-08 === Added - sip: add trace - sdp: sdp_media_disabled API function [#2] - tls: add tls_set_selfsigned_rsa [#6] - tls: add functions to verify server cert, purpose and hostname [#10] - http: client should set SNI [#10] - http: client should use tls functions to verify server certs, purpose and hostname [#10] - sipreg: add proxy expires field and get function [#13] - sipreg: make re-register interval configurable [#13] === Changed - debian: Automatic cleanup after building debian package === Fixed - Set SDK path (SYSROOT) using xcrun (fix building on macOS 10.14) - tcp: close socket on windows if connection is aborted or reset [#1] - rtmp: Fix URL path parsing (creytiv#245) - ice: various fixes [baresip/baresip#925] - openssl/tls: replace deprecated openssl 1.1.0 functions [#5] === Contributors (many thanks) - Alfred E. Heggestad - Christian Spielberger - Christoph Huber - Franz Auernigg - juha-h - Juha Heinanen - Richard Aas - Sebastian Reimers [#25]: baresip/re#25 [#27]: baresip/re#27 [#26]: baresip/re#26 [#19]: baresip/re#19 [#24]: baresip/re#24 [#22]: baresip/re#22 [#18]: baresip/re#18 [#13]: baresip/re#13 [#10]: baresip/re#10 [#6]: baresip/re#6 [#5]: baresip/re#5 [#2]: baresip/re#2 [#1]: baresip/re#1 [v1.0.0]: baresip/re@v0.6.1...v1.0.0 [v1.1.0]: baresip/re@v1.0.0...v1.1.0 [Unreleased]: baresip/re@v1.1.0...HEAD
Hi Alfred,
i just tested ICE with the current master (9180775) branch.
Do you have any idea why connection fails with:
Full logs (both sides): https://gist.github.com/sreimers/10c367e146b6f026359713cc9d0db501
Account config looks like this:
The text was updated successfully, but these errors were encountered: