search

scottlamb / retina

High-level RTSP multimedia streaming library, in Rust
https://crates.io/crates/retina
Apache License 2.0
218 stars 46 forks source link

Add support for RTSP-over-SSL (RTSPS) #78

Open abigagli opened 1 year ago

abigagli commented 1 year ago

I'm playing with some cameras that only do Secure RTSP (RTSPS) so I tried to adapt retina to support it. I'm not sure you'll want to merge this in, because frankly:

  1. I'm not a Rust expert: there might be a better way than just hiding the difference between the TCP/TLS streams behind an enum, but I couldn't find an easy type-erasure mechanism and I didn't want to make this change bubble-up too much in the client code, so I've encapsulated it in the original tokio::Connection struct at the price of having to do quite a bit of pattern matching in the implementation of Stream and Sink
  2. I couldn't find the time to add proper testing, although the original tests are all running fine (i.e. I don't think I broke anything) and this now actually works with RSTPS streams...
scottlamb commented 1 year ago

Thank you for the contribution!

I'm not sure you'll want to merge this in, because...

I'm happy to give guidance on those things if you can take the time to work through it.

Before I get into the details, I'm assuming you've used this only with Transport::Tcp? I haven't played with rtsps before but my understanding is that it implies UDP communication is encrypted via SRTP, and I don't see any of that now. I think for now it'd be fine to return an unsupported error on the combination of rtsp and Transport::Udp. The current behavior (trying unencrypted UDP if the user requested encryption) seems sketchy/wrong.

abigagli commented 1 year ago

Yep, I'd like to get any guidance and improve, just keep in mind this is a side project for me and I'm not sure how much time I'll be able to allocate. That said, yes I completely disregarded UDP and I'm not even sure how is that supposed to work (my use case is only TCP at the moment, so that's what I focused on) but I agree that a better approach is to just refuse to work in case the user chooses rstps and udp. I'll update the PR with this approach as soon as I can

abigagli commented 1 year ago

Ok, I've gated the use of RTSPS so that any attempt to use it with UDP will generate an error. I've done it in the Session::setup method as it was the earliest point where both the url and the transport options were present together to avoid having to remember to check it in every client when processing command line arguments.