fix: U cell / udp:// slots — lossy pushed stream decoded instead of staying black (#133)

[compot-dev]

Closes #133.

Cause: the UDP stall check (#125) only ran for RTSP over UDP (playsOverUdp()); a pushed stream (udp:// slot, the U cell) is listening, so it never ran. And with every key frame losing a piece, NalAssembler dropped every key frame: nothing to decode, ever.

Fix

• The stall check covers pushed streams (RtspSession.listens()).
• After 8 s of packets without a decoded frame, the cell's NalAssembler switches to salvage: a NAL unit with a lost piece is passed on cut short at the gap instead of being dropped (at least 16 bytes collected). The decoder conceals the missing part. Over RTSP, Auto/Multicast still go to TCP as before (the new connection turns salvage off again in onSdp).
• Corner text · damaged while salvaging; /stats.json: nal_damaged (count, only while on).
• Unit tests: salvagePassesTheNalOnCutShortAtTheGap, salvageOffDropsAsBefore.

Device test (AG-216, CI build 36755bd), U cell receiving H.265 1080p30 pushed by a lab camera with steady losses:

• before: in_fps ≈ 30, out_fps 0, black cell;
• after: salvage on after ~10 s, then 1920×1088 H.265 30/30 fps · damaged, nal_damaged 19 in a minute; the picture looks clean — the concealed tail of a key frame is not visible at grid size.
• 0 FATAL EXCEPTION.

[compot-dev]

/review

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

Code Review by Qodo

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

1. Lost configuration can block playback ✓ Resolved

Description

dropOrSalvage() passes any fragmented NAL with at least 16 collected bytes, including truncated
H.264/H.265 parameter sets. When a parameter-set fragment is lost, MediaCodecManager.decodeFrame()
can mark that incomplete set as received and finish its configuration gate once the other sets
arrive, leaving subsequent slices dependent on invalid decoder configuration.

Code

android/app/src/main/java/com/openipc/decoder/NalAssembler.java[R219-223]

+        if (!salvage || nalSize < 16) {
+            drop(reason);
+            return;
+        }
+        Frame out = framePool.obtain(nalSize);

Evidence

The salvage path forwards the collected NAL without a type check. The decoder identifies parameter
sets by that NAL type, flags them as codec configuration, and records them in configSeen before
queueing.

android/app/src/main/java/com/openipc/decoder/NalAssembler.java[217-231]
android/app/src/main/java/com/openipc/decoder/MediaCodecManager.java[424-449]

Agent prompt

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

## Issue description
Salvage forwards truncated codec parameter sets, which the decoder treats as valid configuration.
## Fix Focus Areas
- android/app/src/main/java/com/openipc/decoder/NalAssembler.java[217-231]
- android/app/src/main/java/com/openipc/decoder/MediaCodecManager.java[424-449]
## Recommended Fix
Check the reconstructed NAL type before salvage. Continue dropping incomplete H.264 SPS/PPS and H.265 VPS/SPS/PPS; salvage eligible video slices only, and add tests for fragmented parameter-set loss.

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

---

2. Recovered slices still count as dropped 🐞 Bug ◔ Observability

Description

NalAssembler.assemble() increments lostDropped before dropOrSalvage() can pass the incomplete
slice to the sink. Once salvage is active, each qualifying gap therefore increases both the reported
nal_dropped count and nal_damaged, even though the slice was passed on rather than dropped.

Code

android/app/src/main/java/com/openipc/decoder/NalAssembler.java[104]

+            dropOrSalvage("packet loss inside a fragmented NAL unit", sink);

Evidence

The gap path increments lostDropped and then calls the new salvage path, which can invoke
sink.onNal(out) instead of drop(). QuadCell.nalDropped() exposes lossCount() as
nal_dropped, while the added nalSalvaged() exposes emitted cut-short slices as nal_damaged.

android/app/src/main/java/com/openipc/decoder/NalAssembler.java[101-105]
android/app/src/main/java/com/openipc/decoder/NalAssembler.java[216-233]
android/app/src/main/java/com/openipc/decoder/QuadCell.java[458-462]
android/app/src/main/java/com/openipc/decoder/Decoder.java[1402-1408]

Agent prompt

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

## Issue description
Salvaged slices are included in `nal_dropped` because the packet-loss counter increments before the assembler decides whether to emit or drop the slice.
## Fix Focus Areas
- android/app/src/main/java/com/openipc/decoder/NalAssembler.java[101-105]
- android/app/src/main/java/com/openipc/decoder/NalAssembler.java[216-233]
- android/app/src/main/java/com/openipc/decoder/QuadCell.java[423-445]
## Recommended Fix
Count a NAL as dropped only when it is actually discarded. Preserve a separate packet-loss signal for the quad stall detector so changing the diagnostic count does not prevent stall detection.

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

---

3. Recovered streams keep the damaged mode ✓ Resolved

Description

checkUdpStall() enables salvage, but the added reset runs only in onSdp(). A pushed sender can
go quiet and resume with the same payload type without another onSdp() callback, so recovered
video remains in salvage mode and continues to show the damaged marker.

Code

android/app/src/main/java/com/openipc/decoder/QuadCell.java[R398-400]

+            if (!nalAssembler.salvaging()) {
+                nalAssembler.setSalvage(true);
+                Log.w(TAG, tag + ": no complete key frame over UDP, decoding damaged ones");

Evidence

The listener keeps running through sender silence and calls onSdp only for an initial or changed
payload type. Same-type packets resume the connected state without reaching the sole added salvage
reset.

android/app/src/main/java/com/openipc/decoder/RtspSession.java[524-560]
android/app/src/main/java/com/openipc/decoder/QuadCell.java[393-404]
android/app/src/main/java/com/openipc/decoder/QuadCell.java[458-462]

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 pushed stream can disconnect and resume without a new SDP, leaving salvage enabled after recovery.
## Fix Focus Areas
- android/app/src/main/java/com/openipc/decoder/QuadCell.java[393-404]
- android/app/src/main/java/com/openipc/decoder/QuadCell.java[458-462]
- android/app/src/main/java/com/openipc/decoder/RtspSession.java[524-560]
## Recommended Fix
Clear salvage at an appropriate pushed-stream recovery transition, or after a defined period of healthy decoding without loss. Preserve the existing reset on a new SDP.

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

---

4. Damage count includes discarded units ✓ Resolved

Description

nalSalvaged() exports an assembler counter incremented before sink.onNal() as the count of
damaged NALs decoded. When the NAL queue is full or the decoder has no input buffer, the unit can be
discarded after that increment, so nal_damaged rises without a corresponding decode.

Code

android/app/src/main/java/com/openipc/decoder/QuadCell.java[R410-411]

+    /** NAL units decoded cut short at a lost piece, or -1 while that is off (#133). */
+    int nalSalvaged() { return nalAssembler.salvaging() ? nalAssembler.salvagedCount() : -1; }

Evidence

The assembler increments before handing off the frame, while both the queue sink and decoder have
paths that discard it. The new accessor describes the resulting value as decoded.

android/app/src/main/java/com/openipc/decoder/NalAssembler.java[223-231]
android/app/src/main/java/com/openipc/decoder/QuadCell.java[124-132]
android/app/src/main/java/com/openipc/decoder/MediaCodecManager.java[464-477]
android/app/src/main/java/com/openipc/decoder/QuadCell.java[410-411]

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 exported decoded-damage count includes partial NALs discarded downstream.
## Fix Focus Areas
- android/app/src/main/java/com/openipc/decoder/NalAssembler.java[223-231]
- android/app/src/main/java/com/openipc/decoder/QuadCell.java[124-132]
- android/app/src/main/java/com/openipc/decoder/QuadCell.java[410-411]
- android/app/src/main/java/com/openipc/decoder/Decoder.java[1410-1412]
## Recommended Fix
Choose and document the metric's actual boundary. If it means decoded units, count them after successful decoder submission; otherwise label and document the value as partial NALs emitted by the assembler rather than decoded.

ⓘ 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]

/review

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

Code review by qodo was updated up to the latest commit 0c64fcc

[compot-dev]

Merged v2 (#139) into the branch: two conflicts in QuadCell (corner text, onSdp) resolved keeping both sides; plus c9d3685 — an MJPEG cell never turns NAL salvage on and never shows · damaged. /review

[compot-dev]

/review

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

2. Recovered slices still count as dropped 🐞 Bug ◔ Observability

NalAssembler.assemble() increments lostDropped before dropOrSalvage() can pass the incomplete
slice to the sink. Once salvage is active, each qualifying gap therefore increases both the reported
nal_dropped count and nal_damaged, even though the slice was passed on rather than dropped.

Agent Prompt

## Issue description
Salvaged slices are included in `nal_dropped` because the packet-loss counter increments before the assembler decides whether to emit or drop the slice.

## Fix Focus Areas
- android/app/src/main/java/com/openipc/decoder/NalAssembler.java[101-105]
- android/app/src/main/java/com/openipc/decoder/NalAssembler.java[216-233]
- android/app/src/main/java/com/openipc/decoder/QuadCell.java[423-445]

## Recommended Fix
Count a NAL as dropped only when it is actually discarded. Preserve a separate packet-loss signal for the quad stall detector so changing the diagnostic count does not prevent stall detection.

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

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

Code review by qodo was updated up to the latest commit c9d3685

[compot-dev]

Qodo on c9d3685 — Recovered slices still count as dropped: intended. nal_dropped (lossCount()) counts NAL units hit by a lost packet, and it is also the loss signal of the stall detector, which must keep seeing the loss while salvage is on. With salvage, nal_damaged is the subset of those that were passed on cut short instead of being discarded (so discarded = nal_dropped − nal_damaged). No change.

Device re-test of the merged branch (c9d3685, AG-216): U cell 1920×1088 H.265 key frames · 1.0 fps · damaged, nal_damaged 39 after a minute, 0 FATAL. A Paused: decoder overloaded on slot 8 at startup turned out to be older (reproduced on v2 without this PR) → #148.

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

Qodo on c9d3685 — Recovered slices still count as dropped: intended. nal_dropped (lossCount()) counts NAL units hit by a lost packet, and it is also the loss signal of the stall detector, which must keep seeing the loss while salvage is on. With salvage, nal_damaged is the subset of those that were passed on cut short instead of being discarded (so discarded = nal_dropped − nal_damaged). No change.

Device re-test of the merged branch (c9d3685, AG-216): U cell 1920×1088 H.265 key frames · 1.0 fps · damaged, nal_damaged 39 after a minute, 0 FATAL. A Paused: decoder overloaded on slot 8 at startup turned out to be older (reproduced on v2 without this PR) → #148.

Understood. For finding 2, this metric behavior is intentional: nal_dropped counts NALs affected by packet loss and remains the stall detector’s loss signal, while nal_damaged counts the subset salvaged and emitted. Thus nal_dropped − nal_damaged represents affected NALs actually discarded. The device re-test supports the behavior, so no code change is needed.