fix(basic): RTSP handshake, EOF, thread lifecycle, SDP parsing, CI (#36–#40) by compot-dev · Pull Request #41 · OpenIPC/decoder

compot-dev · 2026-10-12T10:33:51Z

Fixes #36, fixes #37, fixes #38, fixes #39, fixes #40.

All changes stay inside the single-file design of basic. The debug APK is about 34 KB.

Changes

Issue	Problem	Fix
#36	`read += input.read()` hangs forever when the connection closes	Every read is checked for `-1` and throws `IOException`, so the outer loop reconnects
#37	`BufferedReader` swallowed the first RTP packets (SPS/PPS/IDR)	Responses are read byte by byte from the same `BufferedInputStream` that `tcp_stream()` uses. The SDP is read by `Content-Length`. Status codes are checked.
#38	Duplicate threads after a quick pause/resume; flags not `volatile`	Threads exit on a generation counter; flags are `volatile`; `onPause` closes the socket; a stale thread does not reset the state of a newer connection
#39	Telegram caption has empty fields	Variables are defined before use. Publishing is restricted to `basic`/`v2`, as on `v2`.
#40	trackID, PT and UDP ports are hard-coded	See below

#40 in detail

Track URLs come from a=control, with support for *, absolute URLs and the Content-Base header.
The video codec comes from a=rtpmap, so H.265 on PT 96 works. If there is no rtpmap, the old 96/97 convention is kept.
The audio SETUP is sent only when the SDP has an audio track, and its failure is not fatal.
Only L16 audio is played (as before), with the rate taken from the SDP. Without rtpmap, the old PT 100 and the old rate estimate are kept. PCMU/AAC are no longer played as noise.
The UDP code is removed: the mType flag was never set anywhere, so UDP was dead code (and only port 5000 was ever opened).
user:password is no longer included in the request line; it is sent only in Authorization: Basic.

Also: an odd-length L16 payload no longer throws. versionName is now 1.07.1; versionCode is unchanged (7), so the new versionCode cannot collide with builds already uploaded to Google Play.

Verification

assembleDebug and lintDebug locally: no new lint warnings.
JVM tests of the new logic, without a device (Decoder instance created bypassing the Activity constructor): 21/21 pass.
- response parsing: SDP by Content-Length, and the byte after the response stays in the stream;
- parseSdp: a=control:*, absolute control, no audio track, H.265 on PT 96, legacy 96/97/100, L16/16000, PCMU not played;
- tcp_stream: EOF in the body, in the header, and a clean EOF all end in IOException (no hang);
- a packet reaches the NAL queue; a stale generation exits the loop.
Not tested on a device or with a camera.

- Define GIT_HASH, GIT_BRANCH, TAG_NAME and TIME before they are used, so the Telegram caption is no longer empty (fixes #39). - Same branch policy as v2: only basic and v2 update the 'latest' release and post to Telegram; other branches upload a workflow artifact. A '/' in a branch name is replaced in the artifact name.

- tcp_stream(): every read is checked for end of stream and throws IOException, so a closed connection triggers a reconnect instead of an endless loop or a NegativeArraySizeException (fixes #36). - The handshake reads responses byte by byte from the same buffered stream that tcp_stream() uses, honours Content-Length for the SDP and checks status codes, so no RTP bytes are lost (fixes #37). - Worker threads exit on a generation counter instead of a shared boolean; flags are volatile and onPause closes the socket, so a quick pause/resume no longer leaves duplicate threads (fixes #38). - Track URLs come from a=control (including '*' and Content-Base), the video codec from a=rtpmap (legacy 96/97 without rtpmap), audio is only set up when present and only L16 is played, with the rate from the SDP. A failed audio SETUP is not fatal. The unused UDP code with fixed ports is removed, and credentials are no longer sent in the request line (fixes #40). - Odd-length L16 payloads no longer throw. Version 1.07.1.

qodo-free-for-open-source-projects · 2026-10-12T10:34:33Z

PR Summary by Qodo

Fix basic RTSP streaming, thread lifecycle, and release workflow

🐞 Bug fix ✨ Enhancement ⚙️ Configuration changes 🕐 40+ Minutes

AI Description

• Preserve RTP bytes during RTSP handshakes and reconnect cleanly when streams close.
• Use SDP track and codec metadata; stop stale workers after pause or resume.
• Restrict release publishing to basic and v2, repair Telegram captions, and bump versionName.

Diagram

graph TD
  A["Activity lifecycle"] --> B["Network worker"] --> C["RTSP handshake"] --> D["Response reader"] --> E["SDP parameters"] --> F["TCP RTP stream"] --> G["Media queues"]
  F -->|"EOF reconnect"| B
  A -->|"Stop generation"| B

High-Level Assessment

The following are alternative approaches to this PR:

1. Adopt an RTSP client library

➕ Could provide broader protocol interoperability and reduce custom parsing maintenance.
➖ Adds dependency and APK size to a deliberately small, single-file client.
➖ Requires a larger integration change than these targeted fixes.

Recommendation: Keep the targeted single-file implementation for this PR: it addresses the reported failures without changing the app's size and dependency strategy. Reconsider a library if broader RTSP authentication, transport, or SDP support becomes a requirement.

Files changed (3) +302 / -136

Bug fix (1) +283 / -134

Decoder.javaMake RTSP streaming SDP-aware and safe across disconnects +283/-134

Make RTSP streaming SDP-aware and safe across disconnects

• Reads RTSP responses and interleaved RTP through one buffered stream, checks status and EOF, and derives track URLs, payload types, codecs, and supported audio rate from SDP instead of fixed assumptions. Removes unused UDP handling, keeps credentials out of request URLs, and uses generation-scoped workers and socket closure for pause, resume, and reconnect. Also handles odd-length L16 payloads and avoids blocking when the video queue is full.

app/src/main/java/com/openipc/decoder/Decoder.java

Other (2) +19 / -2

build.ymlGate release publishing and populate Telegram captions +18/-1

Gate release publishing and populate Telegram captions

• Working branches now retain APK and AAB builds as workflow artifacts; only basic and v2 publish to the latest release and Telegram. Artifact names replace branch-name slashes, and the Telegram step defines its commit, branch, tag, and time fields.

.github/workflows/build.yml

build.gradleBump the displayed app version to 1.07.1 +1/-1

Bump the displayed app version to 1.07.1
• Changes versionName from 1.07 to 1.07.1 while leaving versionCode at 7.
app/build.gradle

qodo-free-for-open-source-projects · 2026-10-12T10:36:02Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~Early video packets break the handshake~~ ✓ Resolved

Description

readResponse() treats the next byte as the start of an RTSP status line and cannot skip an
interleaved RTP frame. If the camera sends video after video SETUP but before the audio SETUP or
PLAY response, the leading $ fails status parsing and the connection is retried.

Code

app/src/main/java/com/openipc/decoder/Decoder.java[R561-565]

+        String status = readLine(in);
+        Log.i(TAG, status);
+        String[] parts = status.split(" ", 3);
+        if (parts.length < 2 || !parts[0].startsWith("RTSP/")) {
+            throw new IOException("Bad RTSP response: " + status);

Evidence
request() calls readResponse() directly, which reads an unclassified status line. The only
$-frame reader runs after the PLAY response has succeeded, despite video being set up before the
later requests.
app/src/main/java/com/openipc/decoder/Decoder.java[559-565]
app/src/main/java/com/openipc/decoder/Decoder.java[594-600]
app/src/main/java/com/openipc/decoder/Decoder.java[721-743]
app/src/main/java/com/openipc/decoder/Decoder.java[753-770]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
RTP arriving between RTSP requests and responses is parsed as a status line, aborting the handshake.
## Fix Focus Areas
- app/src/main/java/com/openipc/decoder/Decoder.java[559-600]
- app/src/main/java/com/openipc/decoder/Decoder.java[721-743]
## Recommended Fix
Read and consume complete `$`-framed interleaved packets while awaiting a response, then continue looking for the RTSP status line without losing stream alignment.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~An old connection hides the live socket~~ ✓ Resolved

Description

rtsp_connect() assigns mSocket before checking whether its generation is current. After a quick
pause and resume, an old thread can replace the new thread's socket reference with its own
soon-closed socket, so closeSocket() no longer closes the live connection.

Code

app/src/main/java/com/openipc/decoder/Decoder.java[R698-699]
+            mSocket = s;
+            if (gen != listenerGen) return;

Evidence
The stale-generation check follows the assignment; stale cleanup is gated out, while pause and
watchdog close whichever socket the shared field holds.
app/src/main/java/com/openipc/decoder/Decoder.java[520-528]
app/src/main/java/com/openipc/decoder/Decoder.java[697-700]
app/src/main/java/com/openipc/decoder/Decoder.java[744-749]
app/src/main/java/com/openipc/decoder/Decoder.java[860-876]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A stale network thread can overwrite the socket reference used to stop the current connection.
## Fix Focus Areas
- app/src/main/java/com/openipc/decoder/Decoder.java[520-528]
- app/src/main/java/com/openipc/decoder/Decoder.java[697-700]
- app/src/main/java/com/openipc/decoder/Decoder.java[744-749]
## Recommended Fix
Synchronize socket publication and generation validation with pause/resume, and clear the reference only when it still points to that connection's socket.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. ~~A paused connection blocks reconnection~~ ✓ Resolved

Description

rtsp_connect() sets activeStream after PLAY without rechecking listenerGen. If pause happens
just before that assignment, the stale thread restores the flag and immediately leaves
tcp_stream(), while the resumed network thread skips connecting because the flag remains true.

Code

app/src/main/java/com/openipc/decoder/Decoder.java[R741-743]

+            lastFrame = SystemClock.elapsedRealtime();
          activeStream = true;
-            if (mType) {
-                try (DatagramSocket d = new DatagramSocket(5000)) {
-                    udp_stream(d);
-                }
-            } else {
-                tcp_stream(s.getInputStream());
+            tcp_stream(in, gen);

Evidence
Pause clears the flag and increments the generation, but the late assignment can reverse the flag.
The stream loop then rejects the old generation, its cleanup does not clear shared state, and the
new listener connects only when the flag is false.
app/src/main/java/com/openipc/decoder/Decoder.java[741-755]
app/src/main/java/com/openipc/decoder/Decoder.java[799-811]
app/src/main/java/com/openipc/decoder/Decoder.java[860-876]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A connection finishing after pause can mark an obsolete stream active and delay reconnection.
## Fix Focus Areas
- app/src/main/java/com/openipc/decoder/Decoder.java[738-749]
- app/src/main/java/com/openipc/decoder/Decoder.java[799-811]
- app/src/main/java/com/openipc/decoder/Decoder.java[860-876]
## Recommended Fix
Coordinate the final generation check and active-state update with pause/resume; do not mark an obsolete connection active, and keep cleanup scoped to its own connection.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

4. ~~Later codec choices lose their packets~~ ✓ Resolved

Description

parseSdp() reads only p[3] from each media line and considers a=rtpmap only for that first
payload type. When a media line advertises an unsupported choice before H.264, H.265, or L16, the
supported choice cannot be selected and processPacket() ignores its packets.

Code

app/src/main/java/com/openipc/decoder/Decoder.java[R625-628]

+                int pt = -1;
+                try {
+                    if (p.length >= 4) pt = Integer.parseInt(p[3]);
+                } catch (NumberFormatException ignored) {

Evidence
The parser stores only the fourth token of each m= line; the matching checks exclude mappings for
later tokens, and dispatch accepts only the stored payload values.
app/src/main/java/com/openipc/decoder/Decoder.java[622-656]
app/src/main/java/com/openipc/decoder/Decoder.java[665-677]
app/src/main/java/com/openipc/decoder/Decoder.java[777-795]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Only the first SDP payload is evaluated, so supported codecs listed later are ignored.
## Fix Focus Areas
- app/src/main/java/com/openipc/decoder/Decoder.java[622-656]
- app/src/main/java/com/openipc/decoder/Decoder.java[665-677]
## Recommended Fix
Parse every payload listed in each media line, associate each with its matching rtpmap entry, and select a supported payload and codec before publishing stream parameters.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

5. ~~Audio sharing a video payload goes silent~~ ✓ Resolved

Description

tcp_stream() drops the interleaved channel before processPacket() chooses a handler solely by
payload number, checking video first. If separate video and L16 audio tracks both advertise payload
96, packets from audio channel 2 are processed as video and never reach processAudio().

Code

app/src/main/java/com/openipc/decoder/Decoder.java[R769-770]

+            if ((channel == 0 || channel == 2) && len > 13) {
              processPacket(new Frame(data, len));

Evidence
SETUP assigns distinct interleaved channels, and the parser independently assigns each track's
payload. The TCP reader admits either channel but discards its identity, making the video-first
comparison win when payloads coincide.
app/src/main/java/com/openipc/decoder/Decoder.java[630-636]
app/src/main/java/com/openipc/decoder/Decoder.java[665-676]
app/src/main/java/com/openipc/decoder/Decoder.java[720-731]
app/src/main/java/com/openipc/decoder/Decoder.java[769-789]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Payload-only dispatch confuses separate tracks when they reuse a payload number.
## Fix Focus Areas
- app/src/main/java/com/openipc/decoder/Decoder.java[720-731]
- app/src/main/java/com/openipc/decoder/Decoder.java[753-791]
## Recommended Fix
Pass the interleaved channel to packet dispatch and match payload numbers within the corresponding video or audio track.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

View review recommended (2)

6. ~~Root-relative track URLs target the wrong path~~ ✓ Resolved

Description

resolveControl() appends every non-absolute control value to the full base URL. For
a=control:/trackID=0 and a base with a path, it constructs a URL containing that base path
followed by //trackID=0 rather than a track URL at the host root, so video SETUP targets the wrong
resource.

Code

app/src/main/java/com/openipc/decoder/Decoder.java[R603-606]

+    private static String resolveControl(String base, String control) {
+        if (control == null || control.equals("*")) return base;
+        if (control.startsWith("rtsp://") || control.startsWith("rtsps://")) return control;
+        return base.endsWith("/") ? base + control : base + "/" + control;

Evidence
The resolver checks only complete RTSP URLs before appending the control to the base, and its result
is used verbatim as the SETUP request URL.
app/src/main/java/com/openipc/decoder/Decoder.java[603-606]
app/src/main/java/com/openipc/decoder/Decoder.java[660-664]
app/src/main/java/com/openipc/decoder/Decoder.java[718-723]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
String concatenation produces an incorrect track URL for controls beginning with `/`.
## Fix Focus Areas
- app/src/main/java/com/openipc/decoder/Decoder.java[603-606]
- app/src/main/java/com/openipc/decoder/Decoder.java[660-664]
## Recommended Fix
Resolve control values by URL-reference semantics, handling absolute URLs, root-relative paths, and relative track names distinctly.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

7. ~~Audio keeps the previous stream's rate~~ ✓ Resolved

Description

parseSdp() updates audioRate on each connection, but playAudio() creates an AudioTrack only
when none exists. If the socket reconnects to audio with a different announced rate without an
activity pause, the existing track is reused and plays the new samples at the old rate.

Code

app/src/main/java/com/openipc/decoder/Decoder.java[R414-415]

+        // Rate from the SDP; legacy guess for servers that do not announce it
+        int sample = audioRate > 0 ? audioRate : length * 25;

Evidence
The new rate is stored for every parsed SDP but consulted only during track creation. Connection
cleanup does not close audio; only activity pause does.
app/src/main/java/com/openipc/decoder/Decoder.java[380-385]
app/src/main/java/com/openipc/decoder/Decoder.java[413-430]
app/src/main/java/com/openipc/decoder/Decoder.java[674-677]
app/src/main/java/com/openipc/decoder/Decoder.java[741-749]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new SDP rate does not update an AudioTrack retained across RTSP reconnections.
## Fix Focus Areas
- app/src/main/java/com/openipc/decoder/Decoder.java[380-385]
- app/src/main/java/com/openipc/decoder/Decoder.java[413-430]
- app/src/main/java/com/openipc/decoder/Decoder.java[674-677]
- app/src/main/java/com/openipc/decoder/Decoder.java[744-749]
## Recommended Fix
Release or reconfigure the audio track when a new session's audio format differs, coordinating that change with the audio worker and queued samples.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Tip of the day

💡 Did you know, you can commit Qodo's fix in one click with committable suggestions (GitHub & GitLab)

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

- Skip interleaved RTP frames that arrive before an RTSP response. - Publish the socket and set activeStream only for the current generation, under one lock shared with onPause/startListener; a stale thread only clears its own socket. - Pick the first supported payload type from each m= line instead of only the first one listed. - Dispatch packets by interleaved channel: channel 0 is video and channel 2 is audio, even when both use the same payload type. - Resolve root-relative a=control values against scheme://host:port. - Recreate the AudioTrack when a reconnect announces another rate.

compot-dev added 2 commits October 12, 2026 10:33

qodo-free-for-open-source-projects Bot reviewed Oct 12, 2026

View reviewed changes

compot-dev merged commit 6cf68b5 into basic Oct 12, 2026
1 check passed

compot-dev deleted the fix/basic branch October 13, 2026 17:18

compot-dev mentioned this pull request Oct 17, 2026

docs: CHANGELOG.md with the version history (#228) #231

Merged

Navigation Menu

Repository navigation

fix(basic): RTSP handshake, EOF, thread lifecycle, SDP parsing, CI (#36–#40) - #41

fix(basic): RTSP handshake, EOF, thread lifecycle, SDP parsing, CI (#36–#40)#41
compot-dev merged 3 commits into
basicfrom
fix/basic

compot-dev commented Oct 12, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 12, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 12, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

compot-dev commented Oct 12, 2026

Changes

#40 in detail

Verification

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 12, 2026

PR Summary by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

qodo-free-for-open-source-projects Bot commented Oct 12, 2026 •

edited

Loading