[widgetii]

Decodes pictures cut into several patches, the last single-patch restriction on what the Hi3516CV608 encoder emits. Its slice split cuts every N LCU rows.

Patch decoder: a picture is now decoded one patch at a time:

Call	Does
svac3_picture_begin()	picture-level state: coding-unit map, SAO parameters, frame, references
svac3_picture_patch()	one patch, in patch_index order; returns 1 at the last one
svac3_picture_end()	deblocking and SAO over the whole picture, then the temporal motion

What each patch does:

• Region: the patch covers its rectangle of LCUs. Uniform patches are laid out in raster order from patch_width_minus1 / patch_height_minus1.
• Fresh start: it restarts the entropy decoder and its contexts, and starts from its own patch_qp and fixed_patch_qp_flag.
• Exact end: it must end exactly on its last LCU.
• Availability: a neighbour in another patch is unavailable for intra reference samples, MPM, the median and skip/direct candidates, CABAC contexts, the QP predictor and SAO merge. A unit's patch is recorded in the coding-unit map.

svac3_patch_decode() / svac3_patch_parse() stay for single-patch pictures and single patches. Pictures whose loop filters stop at patch boundaries (cross_patch_loop_filter_enable_flag 0 with several patches) are refused: there is no stream to check them against.

Picture decoder: svac3_dec now keeps a picture open across its patch NAL units and outputs it after the last one. A picture that loses a patch is dropped, and the unit that reveals the loss returns SVAC3_ERR_CORRUPT. That unit is one of:

• a later patch of it, out of order;
• the next picture's first patch;
• an SPS, PPS or picture header arriving before the picture's last patch.

Corpus. Two new streams with the encoder's slice split on. Both are bit-exact against the reference decoder through svac3dec and through FFmpeg:

Stream	Patches per picture
cv608_patch4_1080p	9 (every 4 LCU rows)
cv608_patch2_720p	12 (every 2 LCU rows)

All earlier streams are unchanged, and the local 2560x1440 one is still 295/295.

Tests:

• test_dec -patches, on cv608_patch4_1080p:
  • the whole stream decodes;
  • dropping a picture's second patch makes each of its later patches CORRUPT and outputs nothing;
  • dropping its last patch makes the next picture header CORRUPT.
• test_patch covers:
  • the patch count, including huge patch sizes;
  • a wrong layout parsing out of sync;
  • patch_index past the picture;
  • the single-patch call refusing a multi-patch picture;
  • the cross-patch-filter refusal.
• Fixes the tests found: a huge patch_width_minus1 no longer divides by zero, and the reference count is not computed before it is validated (UBSan).
• Fuzzing: the CI fuzzer gains a multi-patch seed. Locally, 3 minutes on 6 workers found nothing.
• ctest: 23/23 in release and in ASan+UBSan.

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

PR Summary by Qodo

Decode SVAC3 pictures spanning multiple uniform patches

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Decode uniform multi-patch pictures with patch-local prediction and whole-picture filtering.
• Assemble patch NALs into complete pictures, rejecting missing or out-of-order patches.
• Validate bit-exact output with new encoder streams, golden hashes, and patch-loss tests.

Diagram

graph TD
  NAL["Patch NAL"] --> State["Picture state"] --> Decode["Patch decode"] --> Last{"Last patch?"}
  Last -->|yes| Filters["Loop filters"] --> DPB["DPB output"]
  Last -->|no| NAL
  State -->|patch lost| Drop["Drop picture"]
  DPB -->|references| State

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Buffer all patch NALs before decoding

• ➕ Keeps picture assembly separate from reconstruction.
• ➖ Adds compressed-data buffering and delays decoding.
• ➖ Still requires patch-aware prediction, entropy resets, and whole-picture filtering.

Recommendation: Keep the incremental picture lifecycle: patch boundaries affect decoding itself, so buffering NALs would not eliminate the core changes. Deferring filtering and DPB commits until completion also prevents incomplete pictures from becoming references.

Files changed (14) +473 / -126

Enhancement (3) +318 / -114

svac3_dec.cKeep pictures open across patch NAL units +109/-42

Keep pictures open across patch NAL units

• Splits picture decoding into start, patch, and commit stages, retaining references and output slots until the final patch. Drops incomplete pictures on patch-order errors or replacement headers and reports corruption.

source/svac3/svac3_dec.c

svac3_patch.cDecode patch regions within a shared picture +177/-70

Decode patch regions within a shared picture

• Adds picture-lifecycle operations, uniform patch geometry, per-patch entropy and QP resets, and patch-local neighbor availability. Defers loop filters and temporal-motion storage until completion, while retaining single-patch helpers and guarding oversized patch dimensions and invalid reference counts.

source/svac3/svac3_patch.c

svac3_patch.hExpose the multi-patch picture lifecycle +32/-2

Expose the multi-patch picture lifecycle

• Declares patch counting and picture begin, patch, end, and free operations. Clarifies which existing helpers accept individual patches or only single-patch pictures.

source/svac3/svac3_patch.h

Documentation (3) +10 / -6

README.mdDocument support for uniform multi-patch pictures +5/-3

Document support for uniform multi-patch pictures

• Updates the supported Main-profile features and clarifies that non-uniform patches and pictures whose filters stop at patch boundaries remain unsupported.

README.md

svac3d.hUpdate the public decoder capability comment +1/-1

Update the public decoder capability comment

• States that supported I and P pictures may contain several uniform patches.

source/svac3/svac3d.h

README.mdCatalog the new multi-patch encoder streams +4/-2

Catalog the new multi-patch encoder streams

• Documents the 1080p nine-patch and 720p twelve-patch streams and their reference-decoder golden hashes.

tests/README.md

Other (8) +145 / -6

ci.ymlSeed the CI patch fuzzer with a multi-patch stream +1/-0

Seed the CI patch fuzzer with a multi-patch stream

• Adds a sample from the new 1080p multi-patch stream to the fuzz corpus.

.github/workflows/ci.yml

CMakeLists.txtRegister the multi-patch decoder test +2/-0

Register the multi-patch decoder test

• Adds a CTest invocation of test_dec in its new -patches mode.

CMakeLists.txt

cv608_patch2_720p.framemd5Add golden hashes for the 720p patch stream +21/-0

Add golden hashes for the 720p patch stream

• Provides per-frame I420 MD5 values for all 20 pictures.

tests/golden/cv608_patch2_720p.framemd5

cv608_patch4_1080p.framemd5Add golden hashes for the 1080p patch stream +31/-0

Add golden hashes for the 1080p patch stream

• Provides per-frame I420 MD5 values for all 30 pictures.

tests/golden/cv608_patch4_1080p.framemd5

cv608_patch2_720p.svac3Add a twelve-patch-per-picture 720p stream +0/-0

Add a twelve-patch-per-picture 720p stream

• Adds a Hi3516CV608 stream split every two LCU rows for multi-patch decoding validation.

tests/streams/cv608_patch2_720p.svac3

cv608_patch4_1080p.svac3Add a nine-patch-per-picture 1080p stream +0/-0

Add a nine-patch-per-picture 1080p stream

• Adds a Hi3516CV608 stream split every four LCU rows for decoder tests and fuzzing.

tests/streams/cv608_patch4_1080p.svac3

test_dec.cTest picture assembly and missing-patch recovery +70/-4

Test picture assembly and missing-patch recovery

• Adds a -patches mode that decodes the complete stream, then verifies that losing an early or final patch suppresses output and reports corruption at the revealing unit.

tests/test_dec.c

test_patch.cTest patch layout and support boundaries +20/-2

Test patch layout and support boundaries

• Checks patch counts, oversized dimensions, incorrect layout and indices, single-patch API refusal of multi-patch pictures, and the cross-patch-filter restriction.

tests/test_patch.c

[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. Later patches misdecode with small LCUs ✓ Resolved

Description

decode_patch() resets the QP values and entropy contexts for each patch but leaves num_delta_qp
from the preceding patch. When an LCU is smaller than the configured QP group and a coding unit
signals a QP delta, the next patch uses the previous patch’s counter to select its first delta
context, potentially desynchronizing decoding.

Code

source/svac3/svac3_patch.c[R1356-1359]

+    p->fixed_qp = hls->pps.fixed_picture_qp_flag ? 1 : hls->patch.fixed_patch_qp_flag;
+    p->patch_qp = hls->pps.fixed_picture_qp_flag ? hls->ph.picture_qp : hls->patch.patch_qp;
+    p->prev_cu_qp = p->lcu_qp = p->patch_qp;
  svac3_ctx_init(p->ctx, SVAC3_NUM_CTX);

Evidence

The counter selects and advances the CU-QP-delta context, but its only existing reset requires a
tree at least as large as the QP group. Successive patches reuse the same parser while restarting
other patch-local state.

source/svac3/svac3_patch.c[845-857]
source/svac3/svac3_patch.c[1019-1025]
source/svac3/svac3_patch.c[1356-1361]
source/svac3/svac3_patch.c[1411-1429]

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 parser persists `num_delta_qp` between patches even though each patch restarts entropy decoding. If the QP group is larger than an LCU, the normal group-root reset does not run, so a later patch selects the wrong CU-QP-delta context.
## Fix Focus Areas
- source/svac3/svac3_patch.c[1356-1359]
- source/svac3/svac3_patch.c[1019-1024]
## Recommended Fix
Reset `p->num_delta_qp` when initializing each patch, alongside its QP and entropy state. Add a multi-patch test with CU QP deltas and a QP group larger than the LCU.

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

---

2. One lost patch also drops the next picture ✓ Resolved

Description

decode_patch_nal and svac3_dec_nal return SVAC3_ERR_CORRUPT (via lost) for a unit they have
actually processed: a picture header that was parsed, or a next-picture first patch that was decoded
and may even have committed a frame into *out. svac3d_decode returns on any nonzero result. So
when a picture loses its last patch, the rest of the packet that reveals the loss is never fed: the
new picture's remaining patches, or the finished single-patch frame, are lost too, and the P
pictures that reference it then fail with 'reference not decoded' until the next I picture.

Code

source/svac3/svac3_dec.c[R378-379]

+    if (!svac3_nal_is_patch(t))
+        return lost ? SVAC3_ERR_CORRUPT : SVAC3_OK;

Evidence

svac3d_decode stops walking the buffer's NAL units on the first error (if (ret) return ret;),
before calling out() even when f is set. FFmpeg passes one access unit per packet, starting with
its picture header. The new lost ? SVAC3_ERR_CORRUPT : ... returns therefore cut off a picture
that otherwise decodes. When a single-patch picture completes, commit() has already set *out and
put the frame in the DPB, but it is never output. The svac3_dec.h contract says *out is set when the
unit completes a picture and gives no rule for doing that alongside an error.

source/svac3/svac3d.c[54-85]
source/svac3/svac3_dec.c[328-389]
source/svac3/svac3_dec.h[23-29]
ffmpeg/0001-avcodec-add-SVAC-3.0-decoding-via-libsvac3d.patch[305-315]

Agent prompt

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

## Issue description
When a picture loses patches, the NAL unit that reveals the loss (the next picture header, SPS/PPS, or the next picture's first patch) is fully processed but returns SVAC3_ERR_CORRUPT. svac3d_decode() stops on any error, so the rest of the access unit is dropped. A frame committed by that same unit (single-patch picture) is never handed to the output callback.
## Fix Focus Areas
- source/svac3/svac3_dec.c[328-389]
- source/svac3/svac3d.c[54-85]
- source/svac3/svac3_dec.h[23-29]
## Recommended Fix
Separate 'a previous picture was dropped' from the current unit's result. Either keep a sticky `lost` flag in svac3_dec_t that callers can query, or return a distinct non-fatal code. Then make svac3d_decode() keep going after it: deliver `f` when it is set and report the loss once at the end. Never set *out together with a fatal error. Document the behaviour in svac3_dec.h and add a test that feeds a whole access unit through svac3d_decode() after a lost last patch.

ⓘ 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 add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[widgetii]

Thanks, both valid; both fixed in the latest commit.

1. Later patches misdecode with small LCUs: fixed. decode_patch() now resets NumDeltaQp and the QP group origin (CuQpGroupX/Y = the patch's first LCU) along with the QPs and contexts it already reset, so a QP group never continues into the next patch. The corpus streams are unchanged: they have no QP group larger than an LCU.

2. One lost patch also drops the next picture: fixed at the API.

• svac3d_decode() no longer stops at the first NAL unit in error. The unit that reveals a loss is itself decoded and may complete a picture.
• Every completed picture now reaches the callback, including one completed alongside the error, and the first error is returned once the whole buffer is done.
• svac3_dec.h now says that *out can be set together with SVAC3_ERR_CORRUPT, and svac3d.h documents the new behaviour.
• The FFmpeg wrapper already returns the pictures first and then the error.

New test: test_api -lost takes cv608_patch4_1080p with picture 0's last patch removed and decodes it in one call. It expects SVAC3D_ERR_CORRUPT and the same pictures, by count and hash, as the internal decoder fed the same units one by one. Under the old behaviour it got zero pictures.

ctest: 24/24 in release, and the new and changed tests are clean under ASan+UBSan.