sdroege / rtsp-types

RTSP (RFC 7826) types and parsers/serializers
MIT License
26 stars 13 forks source link

interleaved binary data support #5

Closed lattice0 closed 3 years ago

lattice0 commented 3 years ago

Hello, I'm willing to contribute to this library. I'm writing software that receives video from cameras. Currently I call a C++ RTSP library from Rust.

I'd like to know if there's support for binary interleaved RTP messages in RTSP. If not, I can add support for it.

Do you have a client example that I can begin with? I understand that the library is just a parser but if there's a client example for me to test it'd be much faster for my development

sdroege commented 3 years ago

Interleaved binary messages are supported via Message::Data.

I'm writing an RTSP server around this. You can find an initial version here but there's still a lot more work to do. It's mostly under construction at this point :)

lattice0 commented 3 years ago

@sdroege I took a look at this but I didn't read much and I thoguht you were using gstreamer to stream RTSP as it's in the dependencies.

I'm looking to create a client. Guess I'll have to write one myself then, but very nice that it already has support for interleaved binary data :)

sdroege commented 3 years ago

Writing a client on top of this should be relatively simple too. If you're doing it async, feel free to take this.

Also let me know if you run into any problems or find bugs

lattice0 commented 3 years ago

@sdroege thanks. I evaluated your rtsp-types and https://github.com/sgodwincs/rtsp-rs, and both are in the same stage: they parse and serialize very well but the state machines for client and server need to be implemented.

I'll make a prototype of a client. I noticed that you are using rust's uri crate. Isn't that a bit of an overkill? It brings too many dependencies to the project and I don't see any way to prevent the credentials to be hidden in a url like rtsp://admin:12345@example.com. It needs to be hidden because, correct if I'm wrong, we receive the nonce for hashing just after sending a DESCRIBE. However, to send a DESCRIBE, currently it serializes the url with credentials.

Wouldn't it be better to copy https://github.com/sgodwincs/rtsp-rs/blob/19205ae9af24d8a837b2bf112db90320165e6f5d/rtsp-2/src/uri/request.rs? It implements a simple URL parser with support for credentials, and we can make it serialize into strings both with and without credentials. sgodwincs/rtsp-rs also sent my credentials right away in the DESCRIBE, making my camera simply close the connection without sending the nonce.

I'd remove lazy_static from his implementation too, I like very very minimal dependencies for security. What you think of this?

sdroege commented 3 years ago

but the state machines for client and server need to be implemented.

See the link I sent above for the server state machine, or big parts of it at least.

It needs to be hidden because, correct if I'm wrong, we receive the nonce for hashing just after sending a DESCRIBE. However, to send a DESCRIBE, currently it serializes the url with credentials.

That's something for the client to implement. You would first send an URI without credentials, and if you get "not authorized" back you would take care of sending the credentials. That has nothing to do with using the uri crate or not :)

Note that some servers also require credentials only starting at SETUP or already for an initial OPTIONS.

Wouldn't it be better to copy https://github.com/sgodwincs/rtsp-rs/blob/19205ae9af24d8a837b2bf112db90320165e6f5d/rtsp-2/src/uri/request.rs?

I'd prefer to not implement my own URI parser as that's not exactly trivial to get right. Copying some other implementation in is even worse than just depending on a widely used one.

I'd remove lazy_static from his implementation too, I like very very minimal dependencies for security. What you think of this?

That makes no sense to me. lazy_static is very widely used and where it's used is completely irrelevant for security aspects too. It would however make sense to instead use once_cell because that API has moved into std already and will be stabilized at some point.