retina
retina copied to clipboard
Add Onvif backchannel support
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.
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:
- 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_typescrate 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 #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 haveretinatake care of the protocol details.
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.
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
SendingPacketsounds good. (And to be able to send whole frames, a matchingSendingCodecItemor something at theDemuxedlayer, but you might want to start with the packet-level stuff and add the higher-level thing later.) I don't think leveragingrtsp_types::Messagewould 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_typescrate 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 haveretinatake 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 (forTransport::Tcp) or directly into a UDP packet (forTransport::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.
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.