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

Switch from webrtc-rs to str0m #3659

Open
Tracked by #3515
thomaseizinger opened this issue Mar 22, 2023 · 15 comments
Open
Tracked by #3515

Switch from webrtc-rs to str0m #3659

thomaseizinger opened this issue Mar 22, 2023 · 15 comments

Comments

@thomaseizinger
Copy link
Contributor

thomaseizinger commented Mar 22, 2023

Description

The webrtc-rs implementation has a few fundamental issues due to its design. async callbacks require locks and prevent the idiomatic use of &mut. This results in problems such as webrtc-rs/webrtc#413 which currently prevent us from upgrading to the latest version: #3552

The new kid on the block is str0m, a sans IO implementation of the webrtc stack: https://github.com/algesten/str0m

In the tests, it looks like they already support data channels, meaning we might be able to replace webrtc with it.

Motivation

  • webrtc is an extremely heavy dependency and it bloats our dependency tree
  • It doesn't follow idiomatic Rust and was hard to integrate into our codebase
  • From the above follows: Easier maintenance
  • A production-grade webrtc implementation

Related issues:

Are you planning to do it yourself in a pull request?

No.

@thomaseizinger
Copy link
Contributor Author

cc @melekes

Depending on how many problems you run into once you deploy libp2p-webrtc, we might want to consider devoting some time to try this out instead of patching webrtc-rs.

@thomaseizinger thomaseizinger changed the title webrtc: consider switching from webrtc-rs to str0m webrtc: attempt to switch from webrtc-rs to str0m Mar 22, 2023
@thomaseizinger
Copy link
Contributor Author

@mxinden Any chance we can get this on the roadmap?

@thomaseizinger
Copy link
Contributor Author

@mxinden Any chance we can get this on the roadmap?

If we want to do libp2p/specs#544 (which I think is a really good idea), I believe this issue is a blocker.

@drHuangMHT
Copy link
Contributor

Related: #3138 with upstream issue on webrtc #104.

@mxinden
Copy link
Member

mxinden commented Apr 27, 2023

@mxinden Any chance we can get this on the roadmap?

Can you create a pull request @thomaseizinger? Allows us to discuss priorities with the other items.

@thomaseizinger
Copy link
Contributor Author

I've opened an issue for the failed SDP parsing which is where we hit a roadblock when @mxinden and I last explored what it would take to switch: algesten/str0m#164

@thomaseizinger
Copy link
Contributor Author

The playground @mxinden and I have been using is here: https://github.com/thomaseizinger/str0m/tree/libp2p-webrtc-playground

Will keep exploring on the side.

@thomaseizinger
Copy link
Contributor Author

thomaseizinger commented May 12, 2023

The playground @mxinden and I have been using is here: thomaseizinger/str0m@libp2p-webrtc-playground

Will keep exploring on the side.

I've tried #164 out and with an additional small change to the SDP, this now parses correctly. I've pushed it to the branch above. The new output is now:

2023-05-12T07:47:36.455616Z  INFO str0m::ice::agent: Set remote credentials: IceCreds { ufrag: "libp2p+webrtc+v1/b01d95e45dee6a3f4e77bb87081eb97b:libp2p+webrtc+v1/b01d95e45dee6a3f4e77bb87081eb97b", pass: "libp2p+webrtc+v1/b01d95e45dee6a3f4e77bb87081eb97b:libp2p+webrtc+v1/b01d95e45dee6a3f4e77bb87081eb97b" }
2023-05-12T07:47:36.455660Z  INFO str0m: DTLS setup is: false
2023-05-12T07:47:36.455720Z  INFO str0m::sctp: Uninited => AwaitRemoteAssociation
2023-05-12T07:47:36.455760Z DEBUG str0m::channel: Allocate channel id: ChannelId(0)
2023-05-12T07:47:36.455801Z TRACE str0m::ice::agent: Enqueueing event: IceRestart(IceCreds { ufrag: "77hg", pass: "ADebVBR6nBhkiq5R8a1iSa" })
2023-05-12T07:47:36.455829Z  INFO str0m::ice::agent: State change (new connection): New -> Checking
2023-05-12T07:47:36.455845Z TRACE str0m::ice::agent: Enqueueing event: IceConnectionStateChange(Checking)
2023-05-12T07:47:36.455864Z TRACE str0m::ice::agent: Stop timeout since ice-lite do no checks
2023-05-12T07:47:36.455871Z TRACE str0m::sctp: Handle timeout: Instant { tv_sec: 4949, tv_nsec: 489601509 }
2023-05-12T07:47:36.455882Z DEBUG str0m::channel: Associate stream id ChannelId(0) => 1
2023-05-12T07:47:36.455902Z TRACE str0m::ice::agent: Poll event: Some(IceRestart(IceCreds { ufrag: "77hg", pass: "ADebVBR6nBhkiq5R8a1iSa" }))
2023-05-12T07:47:36.455911Z TRACE str0m::ice::agent: Poll event: Some(IceConnectionStateChange(Checking))
2023-05-12T07:47:36.455918Z DEBUG str0m: IceConnectionStateChange(Checking)

This hang there which might be due to what is mentioned in algesten/str0m#164 (comment).

@thomaseizinger
Copy link
Contributor Author

New tracking issue on the str0m side: algesten/str0m#166

@thomaseizinger
Copy link
Contributor Author

@altonen Did I understand you in algesten/str0m#166 correctly that you are working on this? Mind sharing a draft-PR? I am very excited what this looks like :)

@altonen
Copy link

altonen commented Sep 4, 2023

I have a small experiment going on. It's not production-ready and still very rough around the edges (including WebRTC). If this turns out to be just a waste of time, I'll upstream the str0m-based WebRTC implementation to libp2p.

@thomaseizinger
Copy link
Contributor Author

I have a small experiment going on. It's not production-ready and still very rough around the edges (including WebRTC). If this turns out to be just a waste of time, I'll upstream the str0m-based WebRTC implementation to libp2p.

Looks great! Thanks for sharing :)

In case somebody picks this up, I am sure your WebRTC implementation is a good reference point.

Btw: we are working on a similar builder API: #4120

@altonen
Copy link

altonen commented Sep 4, 2023

Cheers, I hope the implementation is of small help to you if I don't make the PR myself at some point.

@thomaseizinger
Copy link
Contributor Author

FTR, the main implementation is here: https://github.com/altonen/litep2p/blob/master/src/transport/webrtc

@thomaseizinger thomaseizinger changed the title webrtc: attempt to switch from webrtc-rs to str0m Switch from webrtc-rs to str0m Sep 20, 2023
@DougAnderson444
Copy link
Contributor

I'm going to see if I can get this integrated into rust-libp2p

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants