Repository navigation
fix: streams above 1080p in the single view: decoder set up from the SPS (#166) - #220
Conversation
# Conflicts: # android/app/src/main/java/com/openipc/decoder/MediaCodecManager.java
…PS (#166) The test build of PR #220 did not help on the Rockchip HY300: its OMX decoders declare no size above 1080p, so KEY_MAX_WIDTH/HEIGHT was never set. Now the single view waits for the SPS, reads the picture size from it (SpsParser, H.264 and H.265, cropping included) and configures the decoder for that size, KEY_MAX_* covering it. A larger SPS mid-stream gets a new decoder. A hardware decoder at a size it does not declare that gives no frame within 5 s after its parameter sets hands the stream to the software decoder. /stats.json: size only while a picture is shown (it kept the size of an earlier stream), plus stream_size (from the SPS) and decoder.
compot-dev
changed the title
Mirror (#246) merged first: its GL path is released on the new failure and replace paths (failDecoder, dropDecoder); version 2.18.1, versionCode 44.
Version 2.18.2, versionCode 45 (2.18.1 went to the snapshot fix, #248).
PR Summary by QodoConfigure single-view decoders from SPS for streams above 1080p
AI Description
Diagram
High-Level Assessment
Files changed (7)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
…use, SPS bounds (#166) - A hardware decoder that fails setup or decoding at a size it does not declare marks that size for the software decoder, like a probation that ran out. - A size marked for software waits for a free software decoder instead of going back to the hardware one that showed nothing. - The probation remembers the size it started at; a later SPS does not change which size is marked. - A pause forgets the stream's size and parameter sets: the next stream sends its own SPS before a decoder is made. - SpsParser computes sizes and crops in long and rejects codes that would wrap round; tests for malformed SPS.
|
/review |
|
Code review by qodo was updated up to the latest commit 4046f6b |
…SPS (#166) softwareFor holds the last 8 sizes the hardware decoder gave no frame for, not only the latest. Marking a size makes the next decoder wait for a new SPS, so it gets the whole parameter set before the next key frame.
|
/review |
| if (isSps) { | ||
| spsSeen = true; | ||
| if (spsSize != null) { | ||
| streamWidth = spsSize[0]; | ||
| streamHeight = spsSize[1]; | ||
| } |
There was a problem hiding this comment.
3. A later valid sps cannot correct startup 🐞 Bug ≡ Correctness
decodeFrame() sets spsSeen even when SpsParser.size() returns null, allowing createDecoder() to configure the decoder at 1280×720. If the first SPS is unreadable and a later valid SPS fits within confWidth—or confWidth remains zero—the later size does not recreate the decoder, and probation is never armed for the incorrectly configured hardware decoder.
Agent Prompt
## Issue description
An unreadable first SPS opens the decoder-creation gate without providing its size, and a later readable SPS may not cause reconfiguration.
## Fix Focus Areas
- android/app/src/main/java/com/openipc/decoder/MediaCodecManager.java[690-706]
- android/app/src/main/java/com/openipc/decoder/MediaCodecManager.java[711-717]
## Recommended Fix
For large-frame playback, require a successfully parsed size before creating the decoder, or explicitly recreate a decoder initially configured with an unknown size when a valid SPS arrives.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| } else if (largeFrames && spsSize != null && confWidth > 0 | ||
| && (spsSize[0] > confWidth || spsSize[1] > confHeight)) { |
There was a problem hiding this comment.
4. A changed resolution can stall silently 🐞 Bug ☼ Reliability
decodeFrame() replaces the decoder only when a new SPS exceeds confWidth or confHeight, while probation is decided only when that decoder is created. If a stream changes to an undeclared resolution above 1080p that still fits within its configured maximum, the existing decoder receives that stream without a probation deadline or software fallback.
Agent Prompt
## Issue description
A new SPS within the configured maximum can describe a size the current hardware decoder does not declare, but the existing decoder has no fallback timer.
## Fix Focus Areas
- android/app/src/main/java/com/openipc/decoder/MediaCodecManager.java[711-725]
- android/app/src/main/java/com/openipc/decoder/MediaCodecManager.java[571-575]
## Recommended Fix
When a parsed SPS changes the stream size, check support for that size on the current codec and either start a fresh probation period or replace the decoder so fallback remains available.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if ((configSeen & needed) == needed && awaitingConfig) { | ||
| awaitingConfig = false; | ||
| // The probation runs from the first key frame on | ||
| if (probation) probationUntilMs = nowMs() + PROBATION_MS; |
There was a problem hiding this comment.
5. Delayed key frames trigger false fallback 🐞 Bug ≡ Correctness
decodeFrame() starts probationUntilMs when the final parameter set arrives, rather than when the first key frame is submitted. If that key frame arrives more than five seconds later, the expiry check runs before feeding it and marks the hardware decoder as failed despite it never having received a picture to decode.
Agent Prompt
## Issue description
Probation can expire before the hardware decoder receives its first key frame, incorrectly selecting software for that size.
## Fix Focus Areas
- android/app/src/main/java/com/openipc/decoder/MediaCodecManager.java[718-725]
- android/app/src/main/java/com/openipc/decoder/MediaCodecManager.java[742-750]
## Recommended Fix
Arm the five-second deadline after submitting the first key-frame NAL following the required parameter sets, rather than when parameter-set collection completes.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit 0fe8807 |
1 participant
Fixes #166 (to be confirmed on the reporter's HY300).
Problem. On a Rockchip HY300 (Android 11,
OMX.rk.video_decoder.avc/hevc) a 2304×1296 stream connects, the decoder starts and allocates buffers, but never outputs a frame. 1080p and 720p from the same camera play.What test 1 showed. The first version of this PR announced the largest size the decoder declares as
KEY_MAX_WIDTH/HEIGHT. On the HY300 the log of test build 1 has nopictures up toline: the Rockchip OMX decoders declare nothing above 1080p, so nothing was set and the decoder still started at 1280×720 (reports of 2026-10-19 in #166, for 2.14 and 2.17).Change.
SpsParserreads the picture size (with the cropping / conformance window) from the H.264 and H.265 SPS. Tested with SPS from libx264 / libx265 (High, Main, Baseline, High 4:4:4, interlaced; 2304×1296, 1080p, 720p).KEY_MAX_WIDTH/HEIGHTcovers the stream and the largest declared size. A larger SPS mid-stream gets a new decoder.c2.android.*), logged asNo frame from … : software decoder./stats.json(single):sizeonly while a picture is shown (it kept the size of an earlier stream, noted by the reporter), newstream_size(from the SPS) anddecoder.docs/debug.mdupdated.Version 2.18.2 (PATCH: a fix; it was 2.17.1 until Mirror, #246, merged first as 2.18.0, and the snapshot fix #248 took 2.18.1). Merged with
v2after #246: the mirror's GL path is also released on the paths this PR adds (decoder failure, replacing a decoder on probation).Local Docker build as CI:
testDebugUnitTest(360 tests),lintDebug(no new findings),assembleDebugAceline-1 (AG-216, Android 10), CI build 2.17.1
35f2bc8: single 2560×1920 H.265 (.184) at 20 fps, logstream 2560×1920, pictures up to 3840×2160,stats.jsonsize/stream_size2560x1920,decoderOMX.allwinner.video.decoder.hevc; 1080p H.264 (.248) and switching between them; grid unchanged (9 cells). The probation path cannot be reached here (the Allwinner decoders declare 4K).Reporter: 2304×1296 H.264 and H.265 on the HY300 with
hy300-166-test2HY300 (sansarus,
hy300-166-test2, 2026-10-20): 2560×1440 H.264 now shows a picture throughc2.android.avc.decoderafter the 5 s probation ofOMX.rk.video_decoder.avc, at ~18 fps; heavy for that device (the software decoder sometimes falls behind and restarts), a lower substream plays smoothly on the hardware decoder.Aceline-1, CI build
4046f6b(after the Qodo fixes): single H.265 2560×1920 and H.264 1080p, 3× Home → back (the size comes from the new SPS each time), grid; 0FATAL EXCEPTION/ANR in. The fallback path itself cannot be reached there (the Allwinner decoders declare 4K);0fe8807changes only that path.