Skip to content

Conversation

@lichon
Copy link

@lichon lichon commented Nov 12, 2025

This pull request introduces WebRTC support to the project, allowing connections to be established using the WebRTC protocol in addition to existing TCP and WebSocket options. The main changes include adding new dependencies, implementing a WebRTC stream wrapper, updating the connection logic to detect and handle WebRTC endpoints

These changes collectively add WebRTC support as a first-class transport option, expanding the range of protocols supported by the project and providing a foundation for further development and testing of WebRTC-based features.

Any idea?

@lichon
Copy link
Author

lichon commented Nov 12, 2025

@fufesou any idea?

Repository owner locked and limited conversation to collaborators Nov 12, 2025
Repository owner unlocked this conversation Nov 12, 2025
@rustdesk rustdesk requested a review from Copilot November 12, 2025 12:32
@rustdesk
Copy link
Owner

rustdesk commented Nov 12, 2025

Please make this as an optional feature.

Copilot finished reviewing on behalf of rustdesk November 12, 2025 12:34
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request adds WebRTC transport support to the project, enabling peer-to-peer connections using the WebRTC protocol alongside existing TCP and WebSocket options. The implementation includes endpoint detection, connection establishment via SDP offer/answer exchange, and a stream wrapper that integrates with the existing Stream enum.

Key Changes:

  • Added webrtc crate dependency (v0.14.0) and implemented a WebRTCStream wrapper with send/receive methods
  • Extended the Stream enum and connection logic to detect and handle webrtc:// prefixed endpoints
  • Included an example demonstrating WebRTC data channel usage

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
src/webrtc.rs New WebRTC stream implementation with connection management, SDP handling, and data channel I/O operations
src/stream.rs Extended Stream enum to include WebRTC variant and added corresponding method dispatching
src/socket_client.rs Added WebRTC endpoint detection and connection initiation in the TCP connection flow
src/lib.rs Exposed the new webrtc module as a public API
examples/webrtc.rs Example code demonstrating WebRTC peer connection setup and data channel communication
Cargo.toml Added webrtc dependency and dev dependencies for the example

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@lichon
Copy link
Author

lichon commented Nov 12, 2025

@rustdesk

Please make this as an optional feature.

Like which kind of optional, cargo build features?

@rustdesk
Copy link
Owner

YES

@rustdesk
Copy link
Owner

Also, please write a working example for this webRTC stream.

@lichon
Copy link
Author

lichon commented Nov 12, 2025

I don't think it will affect other well-functioning code. It will only be activated when encountering specific webrtc:// connection endpoints.
Still need to be optional feature?
Or you just dont want to link too much unused libs?

@lichon
Copy link
Author

lichon commented Nov 12, 2025

Also, please write a working example for this webRTC stream.

Yea, I would try it, since we need a signal server here, it's not a simple example

@rustdesk
Copy link
Owner

rustdesk commented Nov 12, 2025

You can do a simple mock signal server.

@rustdesk
Copy link
Owner

rustdesk commented Nov 12, 2025

I don't think it will affect other well-functioning code. It will only be activated when encountering specific webrtc:// connection endpoints.

It will increase the binary size (disk space) and build time (cpu usage) if some one do not need this feature. webrtc is not a small crate.

you just dont want to link too much unused libs?

Yes, this is another reason.

@rustdesk
Copy link
Owner

rustdesk commented Nov 13, 2025

Remove all unwrap/expect. We do not allow those causes crash. All exception must be handled rather than crash directly.

Only mutex unwrap is allowed.

@lichon
Copy link
Author

lichon commented Nov 13, 2025

example updated

@rustdesk
Copy link
Owner

please put webrtc_dummy in examples/webrtc.rs

@lichon
Copy link
Author

lichon commented Nov 13, 2025

please put webrtc_dummy in examples/webrtc.rs

we need this in stream.rs when webrtc feature disabled

@lichon lichon requested a review from rustdesk November 13, 2025 15:09
@rustdesk
Copy link
Owner

rustdesk commented Nov 13, 2025

we need this in stream.rs when webrtc feature disabled

Why? Can not use #[cfg(feature = "webrtc")]? I suggest you ask chatgpt.

@lichon lichon requested a review from Copilot November 15, 2025 06:57
Copilot finished reviewing on behalf of lichon November 15, 2025 07:02
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 21 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Author

@lichon lichon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed all in next patch

@rustdesk rustdesk requested a review from Copilot November 17, 2025 05:32
Copilot finished reviewing on behalf of rustdesk November 17, 2025 05:35
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 21 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

lichon and others added 4 commits November 17, 2025 14:45
@lichon
Copy link
Author

lichon commented Nov 20, 2025

Update here, I build a rustdesk web terminal, both websocket and webrtc works fine with this patch

@rustdesk
Copy link
Owner

web terminal

Code?

add api to get webrtc stream
@lichon
Copy link
Author

lichon commented Nov 20, 2025

rustdesk-web-ts

@rustdesk
Copy link
Owner

What TURN server do you use?

@lichon
Copy link
Author

lichon commented Nov 20, 2025

It's webrtc P2P

@lichon
Copy link
Author

lichon commented Nov 20, 2025

If you need a relay, just disable webrtc, websocket relay still works

@rustdesk
Copy link
Owner

have you ever tested TURN?

@lichon
Copy link
Author

lichon commented Nov 20, 2025

500MB free plan here: metered.ca
force relay works, test from example

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

Successfully merging this pull request may close these issues.

2 participants