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

libpeer does not work in Firefox, and takes ~40s to load in Chrome #59

Open
darkuranium opened this issue Nov 28, 2023 · 2 comments
Open

Comments

@darkuranium
Copy link

I strongly suspect the two issues in the title are related, hence grouping them to the same ticket.

I've run into it during the course of implementing #53, and after some investigation, I believe I may have found a clue, which is related to another issue.


First, the other issue; (which lead to the clue; I'm putting it here because I believe it's part of the solution, even though it's not the entire one)
The following line in agent.c is a no-op:

  // remove last \n
  description[strlen(description)] = '\0';

Since strlen(description) is the length of the string, foo[strlen(foo)] will always be \0. I assume you meant strlen(description) - 1.

However, there is also a problem in ice_candidate_to_description(...), namely:

  snprintf(description, length, "a=candidate:%d %d %s %" PRIu32 " %d.%d.%d.%d %d typ %s\n",

... I'm pretty sure that should have been a \r\n, which would make the other line strlen(description) - 2.

In other words: (diff minimized for brevity)

--- a/src/ice.c
+++ b/src/ice.c
@@ ... @@ void ice_candidate_to_description(IceCandidate *candidate, char *description, in
-  snprintf(description, length, "a=candidate:%d %d %s %" PRIu32 " %d.%d.%d.%d %d typ %s\n",
+  snprintf(description, length, "a=candidate:%d %d %s %" PRIu32 " %d.%d.%d.%d %d typ %s\r\n",
--- a/src/agent.c
+++ b/src/agent.c
@@ ... @@ void agent_get_local_description(Agent *agent, char *description, int length) {
   // remove last \n
-  description[strlen(description)] = '\0';
+  description[strlen(description) - 2] = '\0';

Fixing the above issue reveals an error message in Firefox (as opposed to it simply dropping the connection after a few seconds):

Uncaught (in promise) DOMException: SIPCC Failed to parse SDP: SDP Parse Error on line 17: Warning: Invalid token ordering detected, token c= found after token a=
SDP Parse Error on line 29: Warning: Invalid token ordering detected, token c= found after token a=
SDP Parse Error on line 29: c= line specified twice at same level, parse failed.
SDP Parse Error on line 46: End of line beyond end of buffer.

... comparing with other (working) WebRTC implementations, it looks the c=... line must immediately follow the m=... line, whereas libpeer intermixes them:

libpeer (non-working)

...
m=application 50712 UDP/DTLS/SCTP webrtc-datachannel
a=mid:datachannel
a=sctp-port:5000
c=IN IP4 0.0.0.0
... (other a=... entries)

https://test.antmedia.io:5443/LiveApp/webrtc-test-tool.html (working)

...
m=application 9 UDP/DTLS/SCTP webrtc-datachannel
c=IN IP4 0.0.0.0
... (other a=... entries)

I'll continue the investigation, but perhaps this'll be useful to someone in the meantime.

@darkuranium
Copy link
Author

I got it working!

Above fixes lead me to the ultimate one, which is to add STUN_ATTR_TYPE_XOR_MAPPED_ADDRESS to STUN mappings. (I've implemented this in agent.c)

It's still not a full solution — it makes the prflx candidate work, but not host candidates. That might be a feature of Firefox though, as I have this vague memory that Firefox rejects all such candidates on LAN for privacy and/or security reasons. I'm not 100%, I'll have to investigate this further. But I do have a working solution now!

@darkuranium
Copy link
Author

See #60

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

1 participant