Skip to content

video: serve RTSP viewers over interleaved TCP - #47

Merged
tridge merged 2 commits into
ArduPilot:mainfrom
tridge:pr-rtsp-mp
Oct 21, 2026
Merged

tridge merged 2 commits into
ArduPilot:mainfrom
tridge:pr-rtsp-mp

Conversation

@tridge

@tridge tridge commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

RTSP connections previously entered publisher admission, preventing clients such as Mission Planner from viewing proxied streams by RTSP URL. Reserve /view/vN for authenticated playback of the existing MPEG-TS ring as RTP, preserving publisher splices and disconnecting viewers that fall behind.

Bound unauthenticated admission, support RTCP keepalives and TCP transport negotiation, and provide a copyable Mission Planner pipeline in the video UI.

RTSP connections previously entered publisher admission, preventing clients
such as Mission Planner from viewing proxied streams by RTSP URL. Reserve
/view/vN for authenticated playback of the existing MPEG-TS ring as RTP,
preserving publisher splices and disconnecting viewers that fall behind.

Bound unauthenticated admission, support RTCP keepalives and TCP transport
negotiation, and provide a copyable Mission Planner pipeline in the video UI.
@tridge tridge added the AIReview Request an automated AI review; picked up by the reviewprs sweep label Oct 20, 2026
@AP-Review

AP-Review commented Oct 20, 2026 •

Copy link
Copy Markdown

Deprecated — see below for the updated review.

Previous review (2026-10-20)

Automated review note — AI-generated (Claude+Codex), validated against the live diff. Please sanity-check before acting.
Verdict: COMMENT

Reviewed at head 4d3b5c8606.
Full report: https://firmware.ardupilot.org/Tools/APReview/DevCallReviews/PRReviews/ardupilot/supportproxy/47/1.html#prSupportProxy-47

This adds RTSP-over-TCP playback for viewers. It builds cleanly and its new tests pass, but two viewer-side issues should be fixed.

Should fix

  • videortspview.cpp:277 Closing viewer busy-spins the video child: when closing_ it returns without recv, but EPOLLIN stays level-triggered, so leftover input or a half-close after TEARDOWN spins the shared child at full CPU until the stall timeout (~10s). Drain and discard input (return false on EOF) while closing, or drop EPOLLIN.

Low

  • videortspview.cpp:255 PAUSE then PLAY reloads read_pos_ from scanner.join_offset(), rewinding to the latest keyframe and resending already-delivered TS data (PTS goes backwards in the same RTP session). Keep read_pos_ on resume while it is still resident, and re-anchor only if it was lapped.

Drain closing connections so unread input and half-closes cannot keep the
shared video child busy while its final response is blocked. Preserve a
paused viewer's buffered cursor on resume, re-anchoring only for the first
PLAY or after that position has been overwritten to avoid replaying video
within the same RTP session.

Add live regressions for TCP backpressure, half-close handling, exact
resume continuity and keyframe rejoining after a paused viewer is lapped.
@AP-Review

Copy link
Copy Markdown

Automated review note — AI-generated (Claude+Codex), validated against the live diff. Please sanity-check before acting.
Verdict: ACCEPT

Reviewed at head 7e45231576.
Full report: https://firmware.ardupilot.org/Tools/APReview/DevCallReviews/PRReviews/ardupilot/supportproxy/47/2.html#prSupportProxy-47

This RTSP-over-TCP viewer change now looks good to merge: it builds cleanly at 7e45231 and the RTSP viewer tests pass under the review harness.

Since the last review (at 4d3b5c8), commit 7e45231 fixed both earlier issues. A closing viewer now drains its input instead of busy-spinning the video child, and PAUSE/PLAY resumes from the buffered position instead of replaying from the last keyframe. New regression tests fail on the old code and pass now.

@tridge
tridge merged commit 21ca252 into ArduPilot:main Oct 21, 2026
2 checks passed
@rmackay9

Copy link
Copy Markdown

This was tested successfully yesterday on my aerial photography copter, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AIReview Request an automated AI review; picked up by the reviewprs sweep

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants