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 Onvif backchannel support #91

Open cody-the-casual-dev opened 6 months ago

cody-the-casual-dev commented 6 months ago

This PR is an attempt to start addressing #35. I'm learning Rust and ONVIF/RTSP, so this is a very rough starting draft. However, I was hoping to get some early feedback and direction.

Is this close to the approach you had in mind? Instead of creating a SendingPacket type, should I just be leveraging rtsp-types::Message?

I'm also having trouble figuring out the best way to actually get payload into a message to send back.

I would greatly appreciate any and all critical feedback.

scottlamb commented 6 months ago

Awesome!

Is this close to the approach you had in mind? Instead of creating a SendingPacket type, should I just be leveraging rtsp-types::Message?

Yes, this looks on-track. A new type like SendingPacket sounds good. (And to be able to send whole frames, a matching SendingCodecItem or something at the Demuxed layer, but you might want to start with the packet-level stuff and add the higher-level thing later.) I don't think leveraging rtsp_types::Message would make sense for several reasons:

I'm also having trouble figuring out the best way to actually get payload into a message to send back.

Have you seen retina::rtp::RawPacketBuilder? You'll want to make a RTP packet with that, and then stuff that into a RTSP data message (for Transport::Tcp) or directly into a UDP packet (for Transport::Udp). Ideally we'd support both, but I think it'd be fine to start with just one, returning a not supported error if the caller tries to set things up for the other.

cody-the-casual-dev commented 6 months ago

Thanks a ton! I had started on this work a few months ago, but had somewhat stalled. I won't promise a quick turnaround, but your feedback is helpful and encouraging.

Yes, this looks on-track. A new type like SendingPacket sounds good. (And to be able to send whole frames, a matching SendingCodecItem or something at the Demuxed layer, but you might want to start with the packet-level stuff and add the higher-level thing later.) I don't think leveraging rtsp_types::Message would make sense for several reasons:

  • The caller doesn't know a lot of the details that get stuffed in there, such as the RTP payload type, RTSP channel number, and RTSP headers (cseq, user-agent, authorization). It'd be confusing for them to have to put in dummy values and then have retina clear them out and add in its own stuff.
  • If using Transport::Udp, these messages don't go over the RTSP stream at all, so even using an RTSP type at all would be confusing.
  • The rtsp_types crate isn't exposed in Retina's public API. I'd prefer to keep it that way. I wouldn't rule out switching to our own RTSP code some day for consider more efficient buffering model #6 or some other reason.
  • The caller shouldn't have to serialize a RTP packet header either, which would be required to make the body of a rtsp_message::Data. They should be able to just specify the payload and timestamp and have retina take care of the protocol details.

Those all make sense to me!

Have you seen retina::rtp::RawPacketBuilder? You'll want to make a RTP packet with that, and then stuff that into a RTSP data message (for Transport::Tcp) or directly into a UDP packet (for Transport::Udp). Ideally we'd support both, but I think it'd be fine to start with just one, returning a not supported error if the caller tries to set things up for the other.

I had not seen that, I'll take a look.

scottlamb commented 6 months ago

Thanks a ton! I had started on this work a few months ago, but had somewhat stalled.

I know that story all too well. No pressure from me to complete it quickly, and I'm happy to review and give guidance.