search

ayufan / camera-streamer

High-performance low-latency camera streamer for Raspberry PI's
293 stars 46 forks source link

Adding the ability to send a list of ICE servers to the SDP request so WebRTC can be used over WAN. #65

Closed QuinnDamerell closed 1 year ago

QuinnDamerell commented 1 year ago

This change allows clients to optionally send a list of ICE servers as part of the HTTP SDP request POST, which is used for the created peer connection. This will enable clients to use internet STUN and TURN servers and the local ICE candidates, thus allowing WAN-based WebRTC connections. If no ICE servers are sent, the existing logic remains the same, only offering a local ICE candidate.

Context: I'm the developer behind OctoEverywhere.com, a free remote access service used by the OctoPrint and Klipper communities. My project allows users full and secure access to their web portals, including webcam streaming. With the old mjpeg stream, I have my service set up to proxy the stream, but WebRTC has new challenges.

Both OctoPrint and Moonraker have recently adopted camera-streamer (which is fantastic 🤩), and I want to be able to support WebRTC over the WAN as well. I'm still exploring how to do it, but at a minimum, I need to be able to configure the WebRTC client to use STUN and possibly TURN servers for the connection so it attempts to connect over the WAN and the LAN.

I'm very open to feedback and excited to hear what you think!

ayufan commented 1 year ago

@QuinnDamerell Thank you. Will review this over weekend, but from quick glance I don't see problems. I need to consider whether to expose it on cmdline or both.

QuinnDamerell commented 1 year ago

@ayufan great! I appreciate it! The only thing I wanted to work out is that when setting ICE servers, the WebRTC lib seems to not auto-include the local ICE candidate. I will check the libdatachannel logic to see if I can keep that included; it's handy to have so users always connect locally if possible.

I'm also trying to get this change made ASAP, as OctoPrint and Moonraker are moving to camera-streamer right now, and as far as I know, once installed neither have a good update path for it.

ayufan commented 1 year ago

@QuinnDamerell The OctoPi is upgradeable. crowsnest I believe is not without user intervention.

QuinnDamerell commented 1 year ago

@ayufan I know that Octopi has that cool auto image creator, but I think on the image is made, there's no update path. So that means anyone who uses an old image is stuck on whatever version they got, as far as I know.

ayufan commented 1 year ago

@QuinnDamerell I made this change very similar to yours: https://github.com/ayufan/camera-streamer/commit/99c8415f02fe3a33a3d52a53dab6eeede4838e32. You can check this in develop branch.

QuinnDamerell commented 1 year ago

@ayufan thanks! Jw, is it disabled by default, or am I reading this wrong?

DEFINE_OPTION_DEFAULT(webrtc, disable_client_ice, bool, "1", "Ignore ICE servers provided in '/webrtc' request."),

Is there any reason to disable it? Since the /webrtc calls will exclude the ice servers, it's already an opt-in. It would be great the ice servers would be accepted by default if they are in the request, so I don't have to change any other systems that configure camera-streamer.

ayufan commented 1 year ago

@QuinnDamerell No, it is enabled by default. The 1 specifies what happens if you do --webrtc.disable_client_ice, it is resolved to --webrtc.disable_client_ice=1.

QuinnDamerell commented 1 year ago

Ah, that totally makes sense, sorry!

Btw, is there an easy way to dev camera-streamer locally on Linux? Is there some kind of dev flow you follow?

QuinnDamerell commented 1 year ago

Sorry for all of the questions, I only have one more. What's your cadence to go into master? I'm just trying to coordinate my work and was wondering if you had some timeline. :)

ayufan commented 1 year ago

I think those changes that are currently in develop have some urgency. I need to confirm that usage of develop will not break crowsnest before merge to master.

OctoPi uses precompiled packages (which is safer), crowsnest currently builds from branch which is less ideal.

On Fri, Jun 2, 2023 at 5:48 PM Quinn Damerell @.***> wrote:

Sorry for all of the questions, I only have one more. What's your cadence to go into master? I'm just trying to coordinate my work and was wondering if you had some timeline. :)

— Reply to this email directly, view it on GitHub https://github.com/ayufan/camera-streamer/pull/65#issuecomment-1573952817, or unsubscribe https://github.com/notifications/unsubscribe-auth/AASOSQNJLGFUZU7IALOWEOLXJIDOHANCNFSM6AAAAAAYUKEEA4 . You are receiving this because you were mentioned.Message ID: @.***>

QuinnDamerell commented 1 year ago

Perfect! Yeah, I was going to follow up with the OctoPrint peeps to get the package updated when it's all ready. I didn't know it was package based for OctoPrint; that's amazing!

Crowsnest / all of Klipper stuff usually targets git repos, which is less than ideal. One thought is that Crowsnest could at least target tagged releases or something, which might make it more stable. But also, as you said, Crowsnest will pull the repo when you run sudo make install, but after that, when you update Crowsnest from the Klipper frontends, it only updates the Crowsnest repo, not its compiled libs.

Regardless, thank you so much for your help and for being so responsive!

ayufan commented 1 year ago

Crowsnest / all of Klipper stuff usually targets git repos, which is less than ideal. One thought is that Crowsnest could at least target tagged releases or something, which might make it more stable. But also, as you said, Crowsnest will pull the repo when you run sudo make install, but after that, when you update Crowsnest from the Klipper frontends, it only updates the Crowsnest repo, not its compiled libs.

Yes, and this is a problem. In general I want people to use my distributed precompiled debian binaries, to which they bind via their tooling or they repo.

This allows to make master-focued workflow more robust, and those to be more stable by pinning version and ensuring compatibility with it.

Maybe crowsnest should have its own APT repo which it would use to distribute camera-streamer.

The Octo and crowsnest would pull precompiled deb and push to their repo for distribution.

QuinnDamerell commented 1 year ago

Yeah, that would be the best solution, mainly because Klipper frontends show system package updates directly in the update manager. Then most users would update the camera-streamer using the main update flow from the Klipper frontends..

Btw, I'm more than happy to help with any develop branch testing if it would help. Just let me know. :)

ayufan commented 1 year ago

I need to consider, maybe I will push debian repo for camera-streamer.