Repository navigation
refactor: shared RtspSession for single view and Quad (#33, #9, #18, #19, #27) - #45
Merged
Merged
Conversation
RtspSession is one RTSP-over-TCP session: DESCRIBE/SETUP/PLAY, the interleaved receive loop, an 8 s watchdog and reconnection with capped back-off that never gives up. Stopping is non-blocking, and threads of an older generation deliver no callbacks. - Sdp: track URLs resolved per RFC 2326 (session-level "*", Content-Base, absolute and root-relative controls), the video codec from a=rtpmap with the legacy 96/97 fallback, the first supported payload of the m=video line, and audioUrl only when there is an audio track. - The audio SETUP is skipped without an audio track or for unsupported codecs, and its failure is not fatal. - Responses are parsed into status + headers + Content-Length body; interleaved frames before a reply are skipped. - Lenient URL parsing: the last '@' separates credentials, percent decoding, IPv6; credentials are kept out of the request line. Tests: Sdp, URL parsing, responses, and RtspSession against a scripted server on localhost (handshake, no audio track, failed audio SETUP, video-only mode, more than 10 failures, reconnect, non-blocking stop).
RtspClient keeps only what is specific to the single view (NAL assembly, audio routing, jitter) on top of RtspSession; QuadCell drops its copy of the RTSP client and keeps only decoding. Packets are matched by channel and by the video payload type from the SDP. Fixes #33, fixes #9, fixes #18, fixes #19, fixes #27.
PR Summary by QodoShare RTSP sessions across single view and Quad
AI Description
Diagram
High-Level Assessment
Files changed (12)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
- Sdp: each m=video section keeps its own payloads, rtpmap and control; the payload type and SETUP URL come from the same section. - No H.264/H.265 video track in the SDP fails the session, so it is retried with back-off instead of showing "connected" with no picture. - DESCRIBE replies without Content-Length: the body is read until 300 ms of silence (at most 64 KiB), as the old client did with buffered bytes. - onPacket() reports whether a video packet was usable; only usable packets feed the watchdog and the "connected" status. - The reconnect delay including jitter is capped at maxRetryMs. - QuadCell: a decoder created on the network thread after stop() is released instead of kept.
This was referenced Oct 12, 2026
compot-dev
added a commit
that referenced
this pull request
Oct 12, 2026
- Screenshots on Android 6-9 request WRITE_EXTERNAL_STORAGE first and continue once it is granted; the PNG is encoded and saved on a background thread instead of the UI thread (fixes #28). - Back is no longer intercepted in onKeyDown(): the menu PopupWindow and the dialogs handle it themselves, including predictive back gestures; enableOnBackInvokedCallback opts Android 13-15 into the same behaviour (fixes #30). - Cleanup (fixes #35): - removed the ProGuard rule for the no longer existing Decoder$Frame; - FramePool keeps small and large frames in separate, bounded classes with rounded capacities, so small requests no longer take key-frame buffers and key frames reuse them; - the single-view NAL buffer and KEY_MAX_INPUT_SIZE are both 2 MiB, so 4K key frames fit; - (the 5-thread pool and the suspicious indentation went away with RtspSession, #45.) - Lint baseline refreshed: no errors left in it.
compot-dev
added a commit
that referenced
this pull request
Oct 15, 2026
- Screenshots on Android 6-9 request WRITE_EXTERNAL_STORAGE first and continue once it is granted; the PNG is encoded and saved on a background thread instead of the UI thread (fixes #28). - Back is no longer intercepted in onKeyDown(): the menu PopupWindow and the dialogs handle it themselves, including predictive back gestures; enableOnBackInvokedCallback opts Android 13-15 into the same behaviour (fixes #30). - Cleanup (fixes #35): - removed the ProGuard rule for the no longer existing Decoder$Frame; - FramePool keeps small and large frames in separate, bounded classes with rounded capacities, so small requests no longer take key-frame buffers and key frames reuse them; - the single-view NAL buffer and KEY_MAX_INPUT_SIZE are both 2 MiB, so 4K key frames fit; - (the 5-thread pool and the suspicious indentation went away with RtspSession, #45.) - Lint baseline refreshed: no errors left in it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #33, fixes #9, fixes #18, fixes #19, fixes #27.
What changes
RtspClientandQuadCelleach had their own copy of the RTSP client (handshake, SDP, interleaved read, watchdog, reconnect), and the copies had already drifted apart. Both now use one class.RtspSession(new)stop()does not block; threads of an older generation deliver no callbacks.Sdp(new)AudioConfigRtspClientRtspSession(595 → 165 lines)QuadCellIssues fixed
a=rtpmap(H.265 on PT 96, H.264 on PT 97 and so on). Withoutrtpmapthe old 96/97 convention applies. Ifm=videolists several PTs, the first H.264/H.265 one is used.SETUPis not fatal: video plays without sound.*,Content-Base, absolute and root-relative (/trackID=0) controls.Other changes
@separates the credentials, so a password may contain@. Percent-escapes are decoded, and IPv6[::1]hosts work. Credentials do not appear in the request line.Content-Lengthbody. Interleaved RTP frames that arrive before a reply are skipped.Thread.sleep(interruptible) instead ofSystemClock.sleep. As a result, stopping the RTSP part no longer blocks the UI. onPause can block the UI thread for seconds (ANR risk), worst in Quad mode #32 is not closed by this:onPausestill waits for the decode executors.Tests
51 JVM tests pass in total; 19 of them are new.
RtspSessionTest: runs against a scripted RTSP server on localhost. Covers:Authorization,User-Agent,Session, statuses);SETUP(Single-camera mode never connects when the camera has no audio track #18);SETUP404: still plays (Single-camera mode never connects when the camera has no audio track #18);stop()returns in under 200 ms and no callbacks arrive afterwards.SdpParserTest: adds*,Content-Base, root-relative controls, no audio, codec fromrtpmap, several PTs.RtspUrlTest: credentials,@in the password, percent-encoding, IPv6, invalid URLs.lintDebug: no new issues. The baseline is updated because theSuspiciousIndentationwarning inRtspClientdisappeared with the rewrite.Known limitation
stop()no longer waits for the network thread. One packet from the old session may still land in the NAL queue afternalQueue.clear(). Every callback checks the session generation, so the window is a few instructions; at worst one frame of the previous camera reaches the new decoder.Not tested on a device. Things to check: