Repository navigation
feat: multi-view cell stats and status, decoder limits (#80, #82, #83) - #84
Conversation
…80, #82, #83) - Each cell shows small text in its bottom-left corner, updated every second: 'WxH codec decoded/received fps' while playing, otherwise Connecting…, Buffering…, Disconnected, a short connection error (Port in use, Connection refused, No answer from camera, Wrong login or password, …) or a decoder limit (#80, #82). The same line goes to logcat every 5 s. - Received frames are counted by the RTP marker bit, decoded frames by releaseOutputBuffer; MediaCodecManager exposes them with the output size. - Instance limit (#83): the first decoder of a type reports its declared limit (getMaxSupportedInstances, Android 6+) and throughput (logged). A cell that would exceed the limit shows 'Decoder limit: N × H.265' and retries with back-off until an instance is free. RtspSession reports connection failures through Callback.onFailure (default no-op). - Overload (#83): judged by measurement, not by the declared budget, which is far too low on some boxes. When a settled cell decodes under 85 % of what it receives for 5 s, the highest-numbered playing cell is paused ('Paused: decoder overloaded') until the grid is reopened. - Tests: CellStatsTest.
…own (#83) On the device the first version paused cells down to two: decoded/received also drops when frames are broken by packet loss on the network, which says nothing about the decoder. A cell now counts as behind only when the decoder makes it lose frames: its NAL queue overflows, or dequeueInputBuffer finds no free buffer (that NAL is dropped). After a pause the load gets 10 s to settle before the next decision. decoded/received stays in the corner text as information.
Device test on the AG-216 (Android 10, Allwinner decoder), 8 cameras in the 3×3 grid, all main streamsDeclared limits, logged by the app: Measured, corner text = decoded/received fps: with 8 main streams (1080p–5 MP, 7–30 fps) the cells showed e.g. First version of the relief ( With the fix it still settles at 2 cells, now for the right reason:
So on this box the declared budget (1080p60 in total) is accurate. It fits two 1080p30 cameras, about four 1080p15, or about ten D1 substreams. The 3×3 grid of main streams is beyond any box with this class of decoder, and the app now says so in each paused cell instead of showing broken or frozen video. |
|
/review |
|
Note: this PR was merged before Qodo's review arrived (a process slip on my side). Requesting the review now; findings will be fixed in a follow-up PR. |
Code Review by Qodo
1. Startup failures look like playback
|
| int h = decoder.outHeight(); | ||
| String size = w > 0 ? w + "×" + h + " " : ""; | ||
| return size + (codecH265 ? "H.265" : "H.264") + " " | ||
| + Math.round(outFps) + "/" + Math.round(inFps) + " fps"; |
There was a problem hiding this comment.
1. Startup failures look like playback 📎 Requirement gap ☼ Reliability
MediaCodecManager.createDecoder() catches codec configuration and startup failures without notifying the cell, while QuadCell leaves onDecoderFailed() empty. Once the RTSP session reports connected, sample() therefore falls through to an ordinary codec and zero-fps line even when no decoder exists.
Agent Prompt
## Issue description
Codec configuration or startup failures are logged and retried without setting a visible cell problem, so a connected cell can display normal zero-fps statistics while remaining black.
## Fix Focus Areas
- android/app/src/main/java/com/openipc/decoder/MediaCodecManager.java[232-253]
- android/app/src/main/java/com/openipc/decoder/QuadCell.java[106-114]
- android/app/src/main/java/com/openipc/decoder/QuadCell.java[270-282]
## Recommended Fix
Notify the listener when decoder creation, configuration, or startup fails, store a concise decoder failure message in the cell, and make the status sampler prefer that message until a subsequent successful decoder start clears it.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if (!InstanceBudget.tryAcquire(type)) { | ||
| int limit = InstanceBudget.limit(type); | ||
| synchronized (decoderLock) { | ||
| retry.onFailure(nowMs()); |
There was a problem hiding this comment.
2. Two cells can exceed decoder limit 📎 Requirement gap ☼ Reliability
createDecoder() reserves an instance slot before learnLimits() discovers the codec's declared maximum, so the initial reservations are allowed while the limit is unknown. If multiple cells start the same codec concurrently, they can all create decoders before the first limit is published and exceed that declared maximum.
Agent Prompt
## Issue description
The instance budget allows concurrent reservations while the codec limit is unknown, then learns the limit only after each decoder has already been created. Concurrent same-codec starts can therefore exceed the reported maximum.
## Fix Focus Areas
- android/app/src/main/java/com/openipc/decoder/MediaCodecManager.java[84-105]
- android/app/src/main/java/com/openipc/decoder/MediaCodecManager.java[121-136]
- android/app/src/main/java/com/openipc/decoder/MediaCodecManager.java[222-236]
## Recommended Fix
Coordinate first-codec creation and limit discovery with instance acquisition, or otherwise serialize unknown-limit startup so later same-codec requests wait for the learned limit before creating a decoder; release reservations on every failed or cancelled attempt.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if (c == null || c.isPaused()) continue; | ||
| active++; | ||
| last = i; | ||
| if (c.isSettled(now) && c.inFps() >= 5) { |
There was a problem hiding this comment.
3. Disconnected cells can be paused 📎 Requirement gap ☼ Reliability
relieveDecoder() increments active and sets last for every non-null, non-paused cell, even if it is connecting, disconnected, or has no decoder; only overload measurement checks whether a cell is settled. When a lower-numbered decoder is overloaded and a higher-numbered cell is not playing, the non-playing cell is paused instead, leaving the decoder load unchanged, potentially pausing a sole playing stream and delaying another relief attempt during the 10-second cooldown.
Agent Prompt
## Issue description
`relieveDecoder()` counts non-playing cells toward its active-cell threshold and can select them for overload relief. Pausing a connecting, disconnected, or decoder-less cell removes no decoder load and can delay a useful pause during the cooldown.
## Fix Focus Areas
- android/app/src/main/java/com/openipc/decoder/Decoder.java[892-919]
- android/app/src/main/java/com/openipc/decoder/QuadCell.java[290-295]
## Recommended Fix
Count toward `active` and update `last` only for cells that are connected and currently have a decoder, adding an accessor on `QuadCell` if needed. Keep `last` as the highest-numbered eligible cell, skip relief when fewer than two cells are decoding, and retain the settled-duration requirement for detecting frame loss rather than using connecting or disconnected cells as pause candidates.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if (c.isSettled(now) && c.inFps() >= 5) { | ||
| boolean behind = c.lostFps() >= Math.max(2f, 0.1f * c.inFps()); | ||
| c.behindSamples = behind ? c.behindSamples + 1 : 0; | ||
| if (c.behindSamples >= OVERLOAD_SAMPLES) overloaded = true; |
There was a problem hiding this comment.
4. Nal drops can trigger false overload 📎 Requirement gap ≡ Correctness
sample() sums queueFullDrops and inputDropped as lostFps, although both count dropped NAL units, while relieveDecoder() compares that rate with RTP-marker-counted inFps without using the decoded frame rate. Multi-slice frames, aggregated NAL packets, or parameter sets sent with keyframes can inflate the measured loss past the max(2, 10% of inFps) threshold and pause a cell even when decoded-to-received frame throughput remains healthy.
Agent Prompt
## Issue description
Overload detection compares dropped NAL units with received complete frames rather than measuring loss in a consistent unit. Multi-NAL access units can inflate the loss rate and cause a cell to be paused despite healthy decoded-to-received frame throughput.
## Fix Focus Areas
- android/app/src/main/java/com/openipc/decoder/QuadCell.java[51-71]
- android/app/src/main/java/com/openipc/decoder/QuadCell.java[116-125]
- android/app/src/main/java/com/openipc/decoder/QuadCell.java[255-287]
- android/app/src/main/java/com/openipc/decoder/MediaCodecManager.java[310-320]
- android/app/src/main/java/com/openipc/decoder/Decoder.java[901-904]
## Recommended Fix
Normalize both sides of the overload comparison to the same unit. Prefer tracking decoder-caused loss per access unit: mark the current frame when any of its NALs is dropped, increment a frame-loss counter only once at its marker bit or new access-unit boundary, and compare that rate with received frames. Alternatively, base relief on decoded versus marker-bit received frame rates over the configured sample window, or compare NAL drops with received NAL units. Add a pause-threshold test covering aggregated NAL packets.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| decoder = null; | ||
| releaseSlot(); |
There was a problem hiding this comment.
5. A freed slot can still hold a decoder 🐞 Bug ☼ Reliability
releaseSlot() decrements the shared instance count before the associated codec is stopped and released. When another cell starts during that interval, it can acquire the supposedly free slot and attempt to create a decoder while the device still has the maximum number alive.
Agent Prompt
## Issue description
Instance-budget slots become available before their native decoders are disposed, permitting concurrent creation above the declared limit.
## Fix Focus Areas
- android/app/src/main/java/com/openipc/decoder/MediaCodecManager.java[157-175]
- android/app/src/main/java/com/openipc/decoder/MediaCodecManager.java[182-195]
- android/app/src/main/java/com/openipc/decoder/MediaCodecManager.java[255-264]
## Recommended Fix
Keep each reservation until its codec has actually been released, including asynchronous and cancelled-creation paths. Ensure each path returns its reservation exactly once after disposal.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| public void onFailure(String message) { | ||
| problem = shortError(message); |
There was a problem hiding this comment.
6. A waiting cell can lose its limit warning 🐞 Bug ≡ Correctness
QuadCell.onFailure() unconditionally replaces a decoder-limit warning with a later RTSP connection
error, and onStatus("connected") clears that replacement. If the RTSP connection recovers while
decoder creation is still in back-off, the cell displays playback statistics instead of why it has
no decoder until another creation attempt runs.
Agent Prompt
## Issue description
A transient connection failure overwrites an outstanding decoder-limit warning, which disappears on reconnect before decoder retry completes.
## Fix Focus Areas
- android/app/src/main/java/com/openipc/decoder/QuadCell.java[105-114]
- android/app/src/main/java/com/openipc/decoder/QuadCell.java[220-231]
- android/app/src/main/java/com/openipc/decoder/QuadCell.java[270-282]
## Recommended Fix
Store connection failures and decoder-limit state separately. Show the connection reason while disconnected, then retain or restore the decoder-limit reason on reconnection until decoder creation succeeds.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| cells[last].pause(reason); | ||
| showQuadOverlay(last); | ||
| if (quadStats[last] != null) quadStats[last].setText(reason); |
There was a problem hiding this comment.
7. Freed decoder slot restores the overload 🐞 Bug ≡ Correctness
cells[last].pause() stops the cell, and setEnabled(false) calls releaseSlot(), which returns its InstanceBudget slot. A different cell that is waiting on Decoder limit keeps retrying tryAcquire (back-off of at most 10 s), takes the slot and starts decoding. The decoder load is back where it was, and after the cooldown another cell is paused, so cells drop one by one while the total load stays the same.
Agent Prompt
## Issue description
When a cell is paused for decoder overload, its instance slot is freed and a cell waiting on the instance limit immediately takes it, so the load is not reduced.
## Fix Focus Areas
- android/app/src/main/java/com/openipc/decoder/Decoder.java[892-919]
## Recommended Fix
When overload is detected, first pause (with the same reason) every cell currently blocked by the decoder limit, or mark them so they stop retrying. Only then pause the highest-numbered decoding cell. Another option: once an overload pause has happened, keep a grid-level cap on the number of decoders so waiting cells cannot start.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Qodo's post-merge findings are handled in the follow-up branch for #85 (
|
… slots freed late
- A decoder that fails to start is reported to the cell ('Decoder error') instead of
showing normal zero-fps stats
- While a codec's instance limit is unknown, its decoders start one at a time, so
concurrent cells cannot exceed the limit before it is learnt (Android 5 and errors
count as unlimited)
- The instance slot is given back only after the codec is released, so a waiting cell
cannot start while the device still holds the maximum
- A decoder limit message is not replaced by a connection error
- Only playing cells are demoted or paused (#85 planner)
… slots freed late
- A decoder that fails to start is reported to the cell ('Decoder error') instead of
showing normal zero-fps stats
- While a codec's instance limit is unknown, its decoders start one at a time, so
concurrent cells cannot exceed the limit before it is learnt (Android 5 and errors
count as unlimited)
- The instance slot is given back only after the codec is released, so a waiting cell
cannot start while the device still holds the maximum
- A decoder limit message is not replaced by a connection error
- Only playing cells are demoted or paused (#85 planner)
… slots freed late
- A decoder that fails to start is reported to the cell ('Decoder error') instead of
showing normal zero-fps stats
- While a codec's instance limit is unknown, its decoders start one at a time, so
concurrent cells cannot exceed the limit before it is learnt (Android 5 and errors
count as unlimited)
- The instance slot is given back only after the codec is released, so a waiting cell
cannot start while the device still holds the maximum
- A decoder limit message is not replaced by a connection error
- Only playing cells are demoted or paused (#85 planner)
… slots freed late
- A decoder that fails to start is reported to the cell ('Decoder error') instead of
showing normal zero-fps stats
- While a codec's instance limit is unknown, its decoders start one at a time, so
concurrent cells cannot exceed the limit before it is learnt (Android 5 and errors
count as unlimited)
- The instance slot is given back only after the codec is released, so a waiting cell
cannot start while the device still holds the maximum
- A decoder limit message is not replaced by a connection error
- Only playing cells are demoted or paused (#85 planner)
… slots freed late
- A decoder that fails to start is reported to the cell ('Decoder error') instead of
showing normal zero-fps stats
- While a codec's instance limit is unknown, its decoders start one at a time, so
concurrent cells cannot exceed the limit before it is learnt (Android 5 and errors
count as unlimited)
- The instance slot is given back only after the codec is released, so a waiting cell
cannot start while the device still holds the maximum
- A decoder limit message is not replaced by a connection error
- Only playing cells are demoted or paused (#85 planner)
Closes #80, #82, #83.
Corner text in every multi-view cell (bottom-left, small, updated each second):
1920×1080 H.265 25/25 fps, i.e. frames decoded / frames received;Connecting…,Buffering…,Disconnected, or a short reason:Port in use,Connection refused,No answer from camera,Wrong login or password,Stream not found(Multi-view: a cell that cannot start (e.g. udp:// port in use) is just black #80);Quad-N: …), for device reports (Multi-view: per-cell stats in the corner (resolution, codec, decoded/received fps) #82).Decoder limits (#83):
getMaxSupportedInstances, Android 6+) and its declared 1080p throughput (logged). A cell over the limit showsDecoder limit: N × H.265and starts on its own when an instance is free (retry with back-off).Paused: decoder overloaded) until the grid is reopened. After each pause the load gets 10 s to settle.Received frames are counted by the RTP marker bit, decoded frames by
releaseOutputBuffer.RtspSession.Callbackgets a defaultonFailure(message).Tests:
CellStatsTest(status texts, error shortening, instance budget). Device test with the CI build follows.