Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UDP #30

Closed
scottlamb opened this issue Aug 26, 2021 · 10 comments
Closed

UDP #30

scottlamb opened this issue Aug 26, 2021 · 10 comments
Labels
enhancement New feature or request interop interoperability problem with another (often broken) RTSP implementation

Comments

@scottlamb
Copy link
Owner

scottlamb commented Aug 26, 2021

It'd be nice to support RTP/UDP in addition to RTP/TCP (interleaved channels) for at least a couple reasons:

  • counterintuitively, I think this would be more reliable with some cameras:
  • possibly lower latency, depending on network conditions and implementation. At least in theory this comes at the expense of more dropped packets.

Work for receiving data over UDP:

  • separate "sessions" from "connections". They're separate types already, but they're always created and destroyed together, and I think some fields might be in the wrong place.
  • manage the UDP socket(s). I think the simplest model is for each UDP session to own its UDP socket(s) (maybe just one, or maybe up to two per stream: one for RTP, one for RTCP). If a session is dropped without TEARDOWN, it gets handed off to some background task that retries the teardown periodically until success or timeout, then closes the socket(s). Note this is more IO library-specific logic. (It's possible to share UDP sockets between sessions, but I'm not seeing any real advantage right now, and it adds complexity.)
  • in combination with custom TCP/UDP transporter support #7: we'd need to define a trait for UDP, and we might want to take an "opener" rather than an existing TCP connection, so that we can retry opening the connection if it closes when we still want to send a TEARDOWN or other request.
  • consider how to handle out-of-order packets. On a LAN, I think we might get away with just dropping them, and that might be the only situation I care about for now. But I think other software like ffmpeg implements reorder buffers: if we're waiting for packet n and n+1 shows up, queue n+1 until at least x ms pass or the queue size exceeds y bytes.
  • send RTCP RR packets (reception reports) to help the server pace packets appropriately.

When sending data (a client using the ONVIF audio back channel, or a server, neither of which is implemented yet anyway), we'd also need to implement pacing.

@scottlamb scottlamb added enhancement New feature or request interop interoperability problem with another (often broken) RTSP implementation labels Aug 26, 2021
scottlamb added a commit that referenced this issue Aug 27, 2021
* make handle_keepalive_timer, handle_response, and handle_data proper
  methods. (pin-project is still confusing for me, but I think I'm
  starting to get the hang of it.)
* swap a couple fields between Session and RtspConnection to improve
  clarity for #30
scottlamb added a commit that referenced this issue Aug 27, 2021
This is a work in progress. I'll likely force-push over it as I continue
to rework it, it's gotten very little testing, and it has several
obvious problems:

* several unwrap()s and unimplemented!()s in error paths
* other TODOs indicating things to improve
* doesn't do TEARDOWN yet
* doesn't send RTCP RRs yet (so even well-implemented servers won't
  be able to properly pace packets to a slower connection)
@scottlamb
Copy link
Owner Author

@jlpoolen if you're very brave, you can try out the wip-udp branch with the new --transport=udp option. It's only received about one minute of testing, and it has several known problems mentioned in the commit message, but in the one minute it did appear to work well with Reolink cameras.

I won't prepare a Moonfire NVR version with this until it's more solid.

scottlamb added a commit that referenced this issue Aug 27, 2021
This is a work in progress. I'll likely force-push over it as I continue
to rework it, it's gotten very little testing, and it has several
obvious problems:

* several unwrap()s and unimplemented!()s in error paths
* other TODOs indicating things to improve
* doesn't do TEARDOWN yet
* doesn't send RTCP RRs yet (so even well-implemented servers won't
  be able to properly pace packets to a slower connection)
@jlpoolen
Copy link

Brave as in a dog to a steak? Can you provide me an example of a URL for the UDP connection? I've not ventured in Reolink's UDP capability and would need an example to construct ones for my cameras.
(Thinking a scripted population of the configuration stuff (I'd write a Perl script) may be in order -- that is a time-consuming process.)

@scottlamb
Copy link
Owner Author

The URL doesn't change. With the additional --transport=udp parameter, Retina will (after setting up the sockets and such) tell the Reolink server to use UDP via the Transport: header of the SETUP method.

@jlpoolen
Copy link

Do you want me to leave tails of log files? I've tried three times, 10-20 seconds, then 2 almost 1 minute until a failure. Here my most recent:

jlpoole@taurus /usr/local/src/retina_branch/retina $ cargo run --example client mp4 --transport udp --url rtsp://192.168.1.54:554/h264Preview_01_main --username moon --password fire123  --duration 600 /tmp/retina_wip.mp4
    ...

2487874216 (mod-2^32: 2487874216), npt 70.144: 512-byte audio frame
2863836048 (mod-2^32: 2863836048), npt 71.176: 17430-byte video frame
2863841988 (mod-2^32: 2863841988), npt 71.242: 17374-byte video frame
E20210827 19:44:40.099 main client] Fatal: Lost 1 RTP packets mid-stream
jlpoole@taurus /usr/local/src/retina_branch/retina $ 

For posterity, here's what I did to switch to this branch after I clone into a new area, e.g. /usr/local/src/retina_branch,

jlpoole@taurus /usr/local/src/retina_branch $ git clone https://github.com/scottlamb/retina.git
jlpoole@taurus /usr/local/src/retina_branch $ cd retina/
	jlpoole@taurus /usr/local/src/retina_branch/retina $ git checkout 1f3abb7d0c018087d2140b7eb3905c98799a1782
Note: switching to '1f3abb7d0c018087d2140b7eb3905c98799a1782'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at 1f3abb7 WIP: UDP support (#30)
jlpoole@taurus /usr/local/src/retina_branch/retina $ cargo run --example client mp4 --transport udp --url rtsp://192.168.1.54:554/h264Preview_01_main --username moon --password fire123  --duration 10 /tmp/retina_wip.mp4
   Compiling retina v0.2.0 (/usr/local/src/retina_branch/retina)
    Finished dev [unoptimized + debuginfo] target(s) in 6.56s
     Running `target/debug/examples/client mp4 --transport udp --url 'rtsp://192.168.1.54:554/h264Preview_01_main' --username moon --password fire123 --duration 10 /tmp/retina_wip.mp4`
3150027245 (mod-2^32: 3150027245), npt 0.000: 512-byte audio frame
3150028269 (mod-2^32: 3150028269), npt 0.064: 512-byte audio frame
3150029293 (mod-2^32: 3150029293), npt 0.128: 512-byte audio frame
3150030317 (mod-2^32: 3150030317), npt 0.192: 512-byte audio frame
3150031341 (mod-2^32: 3150031341), npt 0.256: 512-byte audio frame
3150032365 (mod-2^32: 3150032365), npt 0.320: 512-byte audio frame
3150033389 (mod-2^32: 3150033389), npt 0.384: 512-byte audio frame
3150034413 (mod-2^32: 3150034413), npt 0.448: 512-byte audio frame
3064035027 (mod-2^32: 3064035027), npt 0.000: 702428-byte video frame

I'm a Subversion fan, so git is a foreign language for me. I had to fumble around until I came upon the "git checkout ..." command.

@jlpoolen
Copy link

Here's my first try (precedes my posting above):

3150036461 (mod-2^32: 3150036461), npt 0.576: 512-byte audio frame
3150037485 (mod-2^32: 3150037485), npt 0.640: 512-byte audio frame
3064062027 (mod-2^32: 3064062027), npt 0.300: 15766-byte video frame
E20210827 19:39:47.923 main client] Fatal: Lost 17 RTP packets mid-stream
jlpoole@taurus /usr/local/src/retina_branch/retina $ 

@jlpoolen
Copy link

jlpoolen commented Aug 28, 2021

Was trying for a 10 minute run (--duration 600), just after a minute same kind of error. Note: when the Fatal error occurs, the output file is not usable by vlc.

2041404768 (mod-2^32: 2041404768), npt 64.704: 512-byte audio frame
2041405792 (mod-2^32: 2041405792), npt 64.768: 512-byte audio frame
1914628791 (mod-2^32: 1914628791), npt 67.599: 11122-byte video frame
E20210827 20:01:10.129 main client] Fatal: Lost 1 RTP packets mid-stream
jlpoole@taurus /usr/local/src/retina_branch/retina $ ls -laht /tmp |head
total 160M
drwxrwxrwt 12 root    root     12K Aug 27 20:01 .
-rw-r--r--  1 jlpoole jlpoole  49M Aug 27 20:01 retina_wip.mp4
-rw-r--r--  1 jlpoole jlpoole 309K Aug 27 19:45 vlc_log.html
-rw-r--r--  1 jlpoole jlpoole 9.7K Aug 27 19:22 Fri_Aug_27_19:21:59_2021_ffmpeg_STDERR_54.log
-rw-r--r--  1 jlpoole jlpoole 5.1M Aug 27 19:22 Fri_Aug_27_19:21:59_2021_ffmpeg_54.mp4
-rw-r--r--  1 jlpoole jlpoole 6.1K Aug 27 19:22 Fri_Aug_27_19:21:59_2021_live_54.log
-rw-r--r--  1 jlpoole jlpoole 5.4M Aug 27 19:22 Fri_Aug_27_19:21:59_2021_live_54.mp4
-rw-r--r--  1 jlpoole jlpoole 3.1M Aug 27 19:22 Fri_Aug_27_19:21:59_2021_retina_54.mp4
-rw-r--r--  1 jlpoole jlpoole  365 Aug 27 19:22 Fri_Aug_27_19:21:59_2021_retina_STDERR_54.log
jlpoole@taurus /usr/local/src/retina_branch/retina $ vlc /tmp/retina_wip.mp4 
VLC media player 3.0.14 Vetinari (revision )
[mov,mp4,m4a,3gp,3g2,mj2 @ 0x7eff50c59780] moov atom not found
[mov,mp4,m4a,3gp,3g2,mj2 @ 0x7eff04c03200] moov atom not found
QObject::~QObject: Timers cannot be stopped from another thread
jlpoole@taurus /usr/local/src/retina_branch/retina $ 

You can let me know either through this thread, or by other means, if you would like further testing or a testing that might generate unique logs with different error messages What I could do is accumulate log files and then hash the errors to identify different ones.

@scottlamb
Copy link
Owner Author

Don't go to any trouble to gather logs and things; I haven't really done my due diligence on my own yet. But I should have mentioned: it probably should be used with --allow-loss as well. UDP is inherently somewhat lossy, although it's interesting that you're reliably seeing loss in the first minute.

@scottlamb
Copy link
Owner Author

I believe this is working well now.

@jlpoolen
Copy link

I have not updated since my initial pull. But I did do further testing and had several last until the specified duration of 10 minutes was reached. Do you want me to activate some tests? I've been working on a Perl script which automates testing and logging.

@scottlamb
Copy link
Owner Author

Sure; I am curious how it will do in your high-loss environment. It's merged to the main branch now, so you'll want to git checkout main back. It's also available in Moonfire NVR via --rtsp-transport=udp.

scottlamb added a commit that referenced this issue Aug 31, 2021
scottlamb added a commit that referenced this issue Sep 8, 2021
*   out-of-order packets should be ignored (except for logging).

*   after ignored RTP or RTCP packet, don't incorrectly return Pending
    without both underlying sockets actually returning Pending. This
    could lead to not rechecking the sockets until prompted for another
    reason, like an RTSP keepalive.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request interop interoperability problem with another (often broken) RTSP implementation
Projects
None yet
Development

No branches or pull requests

2 participants