fix: Auto falls back to TCP when UDP delivers packets but no frame decodes (#113)

[compot-dev]

Closes #113.

Cause (details in #113): the lab cameras send RTSP-over-UDP heavily reordered (up to ~42 packets, which RtpReorder handles) and .126 also loses ≈ 1.5 packets/s on the way. Nearly every H.265 key frame then has a hole and NalAssembler drops it whole; on key frames only (#85) the cell shows nothing although 30 packets/s arrive. TCP has no loss.

Fix

• QuadCell: once a second, if RTSP plays over UDP/multicast, packets arrive (≥ 5 fps) and no frame was decoded for 8 s (longer than any lab camera's key frame interval), it tells the session (reportUdpBroken).
• RtspSession: with Auto or Multicast, a broken UDP stream ends the receive loop and the URL is marked UDP-failed, so the reconnect uses TCP — the same path as no video over UDP, using TCP today; log …: packets but no decoded frame for 8 s (N NAL units dropped) over UDP, using TCP. An explicit UDP setting stays on UDP.
• /stats.json: per cell nal_dropped (NAL units dropped because one of their packets was lost).

Unblocks Auto as the default (#112).

Device check: Aceline-1, slot 3 (.126, H.265) on Auto in the 3×3 grid → falls back to TCP within ~10 s and shows key frames; slot 2 (.133, H.264) on Auto stays on UDP.

[compot-dev]

/review

[qodo-free-for-open-source-projects]

Code Review by Qodo

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

1. Black cells can evade UDP recovery ✓ Resolved

Description

checkUdpStall() gates recovery on inFps, which is derived from marker-ended access units rather
than received RTP packets. When packet loss or fragmentation leaves fewer than five marker packets
per second while UDP traffic continues and no frame decodes, the gate never reports udpBroken, so
Auto or multicast remains on the black UDP stream.

Code

android/app/src/main/java/com/openipc/decoder/QuadCell.java[R351-352]

+        boolean stalled = session.playsOverUdp() && "connected".equals(state)
+                && pausedReason == null && inFps >= 5 && outFps == 0;

Evidence

The changed stall condition requires inFps >= 5, but inFps is computed from framesIn, and
framesIn increments only when an RTP packet has the marker bit. The session delivers packets
individually, so fragmented or incomplete access units can produce ongoing packet traffic without
satisfying the frame-rate gate; this prevents the fallback required for packet-arriving, no-frame
streams.

Restore video for multi-view key-frame cells over UDP Auto
android/app/src/main/java/com/openipc/decoder/QuadCell.java[351-352]
android/app/src/main/java/com/openipc/decoder/QuadCell.java[266-281]
android/app/src/main/java/com/openipc/decoder/QuadCell.java[431-437]
android/app/src/main/java/com/openipc/decoder/RtspSession.java[676-681]

Agent prompt

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

## Issue description
The UDP-stall detector uses `inFps`, which counts marker-ended video access units rather than incoming RTP packets. A stream can continue receiving packets without decoding frames while staying below the `inFps >= 5` threshold, preventing Auto or multicast from switching to TCP.
## Fix Focus Areas
- android/app/src/main/java/com/openipc/decoder/QuadCell.java[351-352]
- android/app/src/main/java/com/openipc/decoder/QuadCell.java[431-437]
## Recommended Fix
Track received video RTP packets independently from marker-ended frame counts, compute a per-second packet rate for the stall detector, and use that packet rate when deciding whether to call `reportUdpBroken`. Preserve the existing decoded-frame check and reset the packet-stall counter when traffic resumes or a frame is decoded.

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

---

2. Decoder failures trigger a needless switch ✓ Resolved

Description

checkUdpStall() treats incoming frames with no rendered output as broken UDP without checking
whether NALs were lost or whether the decoder can render. A decoder-limit error or invalid surface
satisfies the predicate, so Auto and Multicast switch to TCP even though changing transport cannot
resolve the rendering failure.

Code

android/app/src/main/java/com/openipc/decoder/QuadCell.java[R351-353]

+        boolean stalled = session.playsOverUdp() && "connected".equals(state)
+                && pausedReason == null && inFps >= 5 && outFps == 0;
+        udpStallSamples = stalled ? udpStallSamples + 1 : 0;

Evidence

The predicate uses only connection state and sampled input/output rates. Decoder-limit errors
populate problem, and decode() returns without rendering for an invalid surface; neither
condition prevents the new predicate from reporting UDP broken.

android/app/src/main/java/com/openipc/decoder/QuadCell.java[101-112]
android/app/src/main/java/com/openipc/decoder/QuadCell.java[350-358]
android/app/src/main/java/com/openipc/decoder/QuadCell.java[441-454]
android/app/src/main/java/com/openipc/decoder/RtspSession.java[657-664]

Agent prompt

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

## Issue description
The UDP stall detector also fires when decoding or rendering fails for reasons unrelated to packet loss.
## Fix Focus Areas
- android/app/src/main/java/com/openipc/decoder/QuadCell.java[350-358]
## Recommended Fix
Exclude decoder and surface failures from the transport diagnosis, and require evidence of UDP assembly loss before reporting the stream broken.

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

---

3. A new stream inherits an old stall ✓ Resolved

Description

udpStallSamples resets only when checkUdpStall() observes a non-stalled sample, not when the
session reconnects or receives a new SDP. If a disconnect and new UDP PLAY occur between one-second
UI samples, the new connection inherits the previous count and can be sent to TCP before it has had
eight seconds to decode a frame.

Code

android/app/src/main/java/com/openipc/decoder/QuadCell.java[R351-355]

+        boolean stalled = session.playsOverUdp() && "connected".equals(state)
+                && pausedReason == null && inFps >= 5 && outFps == 0;
+        udpStallSamples = stalled ? udpStallSamples + 1 : 0;
+        if (udpStallSamples >= UDP_STALL_SAMPLES) {
+            udpStallSamples = 0;

Evidence

The new counter has no connection identity and is not cleared by the SDP callback. The UI samples
approximately once per second while the RTSP session reconnects independently, allowing an entire
disconnect and reconnect between samples.

android/app/src/main/java/com/openipc/decoder/QuadCell.java[340-358]
android/app/src/main/java/com/openipc/decoder/QuadCell.java[401-409]
android/app/src/main/java/com/openipc/decoder/Decoder.java[926-951]
android/app/src/main/java/com/openipc/decoder/RtspSession.java[205-241]

Agent prompt

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

## Issue description
The stall count can span separate UDP connections when reconnection happens between samples.
## Fix Focus Areas
- android/app/src/main/java/com/openipc/decoder/QuadCell.java[340-358]
- android/app/src/main/java/com/openipc/decoder/QuadCell.java[399-410]
## Recommended Fix
Reset or key the stall count to the current connection when its status or SDP changes, so eight samples must come from the same UDP playback.

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

---

4. Loss statistics include oversized frames ✓ Resolved

Description

nalDropped() exposes NalAssembler.droppedCount() as a packet-loss count, but that counter also
increments when a NAL exceeds the assembly buffer. An oversized NAL therefore increases
nal_dropped and appears in the UDP fallback log even when none of its packets was lost.

Code

android/app/src/main/java/com/openipc/decoder/QuadCell.java[R361-362]

+    /** NAL units dropped so far because a packet of them was lost (#113). */
+    int nalDropped() { return nalAssembler.droppedCount(); }

Evidence

The assembler explicitly documents both packet loss and size as causes counted by droppedCount().
Its oversize branch calls drop(), which increments the same counter now exposed in the statistic
and fallback message.

android/app/src/main/java/com/openipc/decoder/NalAssembler.java[57-60]
android/app/src/main/java/com/openipc/decoder/NalAssembler.java[151-155]
android/app/src/main/java/com/openipc/decoder/NalAssembler.java[194-199]
android/app/src/main/java/com/openipc/decoder/Decoder.java[1294-1295]
android/app/src/main/java/com/openipc/decoder/QuadCell.java[356-362]

Agent prompt

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

## Issue description
The new packet-loss diagnostic includes NALs rejected for exceeding the assembly buffer.
## Fix Focus Areas
- android/app/src/main/java/com/openipc/decoder/QuadCell.java[356-362]
- android/app/src/main/java/com/openipc/decoder/NalAssembler.java[145-153]
- android/app/src/main/java/com/openipc/decoder/NalAssembler.java[190-200]
## Recommended Fix
Track packet-loss and size-limit drops separately, then use the packet-loss count for the new statistic and fallback message.

ⓘ 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 turn these tips off under Display > Comment parts

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[compot-dev]

/agentic_review

[qodo-free-for-open-source-projects]

Code Review by Qodo

Sorry, something went wrong

We weren't able to complete the code review on our side. Please try again manually by commenting /agentic_review on this PR.

[compot-dev]

/agentic_review

[qodo-free-for-open-source-projects]

Code review by qodo was updated up to the latest commit 1a44445

[compot-dev]

Qodo review — all four accepted and fixed:

1. Black cells can evade UDP recovery — the check now uses received video RTP packets (≥ 10 per second), not marker-ended frames.
2. Decoder failures trigger a needless switch — a second counts only with a decoder that is fine (no problem, a valid surface) and once NAL units were lost to packet loss within the window.
3. A new stream inherits an old stall — the count restarts on every status change and on a new SDP.
4. Loss statistics include oversized frames — NalAssembler.lossCount() counts only units dropped for a lost packet (missing start, missing end, a gap inside); nal_dropped and the log use it; oversized units stay in droppedCount() only.

[qodo-free-for-open-source-projects]

Qodo review — all four accepted and fixed:

1. Black cells can evade UDP recovery — the check now uses received video RTP packets (≥ 10 per second), not marker-ended frames.
2. Decoder failures trigger a needless switch — a second counts only with a decoder that is fine (no problem, a valid surface) and once NAL units were lost to packet loss within the window.
3. A new stream inherits an old stall — the count restarts on every status change and on a new SDP.
4. Loss statistics include oversized frames — NalAssembler.lossCount() counts only units dropped for a lost packet (missing start, missing end, a gap inside); nal_dropped and the log use it; oversized units stay in droppedCount() only.

The diff reflects all four reported fixes: packet-based UDP stall detection, decoder-health and packet-loss gating, stall resets on status/SDP changes, and separate packet-loss NAL accounting. The published Qodo review itself failed to complete, so these changes still need a fresh review to verify them end to end.

[compot-dev]

Device check (Aceline-1, b3923f8): slot 3 (.126, H.265) on Auto, a healthy UDP stream at the moment (nal_dropped 1) — stays on UDP, key frames decoded, no false fallback over 30 s; /stats.json shows nal_dropped. The fallback itself was shown with 1a44445 on a lossy stream: Quad-2: packets but no decoded frame for 8 s (9 NAL units dropped) over UDP, using TCP → playing over TCP, picture back.