[pushkarreddyy]

What

In libSceVideodec2, sceVideodec2Decode accepts AVC access units with no slice NAL (such as standalone SPS/PPS parameter sets) and accumulates them into decoder.parameterSets until a slice arrives. While single units enforce auSize <= INT_MAX, repeated parameter-set units previously accumulated without enforcing a cumulative bound before vector insertion, permitting host memory growth up to an allocator limit before any slice arrived.

Before inserting a parameter-only access unit into decoder.parameterSets:

• Enforce a conservative, practical cumulative cap of 2 MiB (MaxAvcParameterSetsBytes = 2 * 1024 * 1024), throwing std::runtime_error("Videodec2: invalid access unit") if accumulation exceeds this bound before vector insertion.
• Leaves pending decoder state unchanged when an oversized unit is rejected.
• Documented the 2 MiB cumulative parameter-set bound under title ID PPSA99474 in docs/dev/debt/libSceVideodec2.md.

Tested

• Added testParameterSetsBound regression test in core/libs/tests/GuestVideodec2.cpp verifying that an initial valid parameter set is retained, an oversized parameter-set access unit (>2 MiB) throws Videodec2: invalid access unit, and the subsequent slice unit successfully decodes using the retained parameter sets.
• Ran conventions check via python tools/check_conventions.py --base origin/main with 0 findings.
• Verified on Windows 11 with WinLibs GCC 15.2.0 (Release, BUILD_TESTING=ON, Ninja): built cleanly, and focused tests guest_videodec2 and guest_videodec2_unimplemented passed without errors (verified by @maskym).

Checklist

• Based on current main; no other open PR implements the same functions
• One topic per PR; follow-ups go in a new PR
• No comments in code except technical debt
• Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
• No notes, investigation .md files or images added to the repository (attach them to this PR)
• New third-party code is a submodule built from source
• Generic behaviour, not specific to one title
• Depends on: none
• Fixes: libSceVideodec2: repeated AVC parameter-set units grow host memory without a cumulative bound #12189
• Unblocks: none
• GPUs tested: n/a
• AI-assisted: yes

[MarcZX10]

The new INT_MAX guard does not stop the memory-retention behavior demonstrated by the linked issue. Its 32-unit reproduction retains 268,434,816 pending bytes (about 277 MB private-memory growth), which is far below INT_MAX; with this patch the same sequence still appends every unit and retains the same memory. The change only caps the accumulation near 2 GiB, which can still exhaust a host before the guard is reached. Please choose and document a practical cumulative cap that prevents the reported reproduction (or otherwise bound/replace retained parameter sets), and add a regression test that verifies rejection happens before insertion and leaves pending state unchanged while the ordinary SPS/PPS-then-slice path still works. The current PR has no test file despite the issue claim comment saying unit coverage would be included. Conventions, eligibility, intake, labels, and report checks pass; Windows build is pending, and other build jobs are path-skipped. I have not reproduced the Windows probe independently.

[oneandonlydean]

@pushkarreddyy thanks, the bound reads right: it is a byte bound at INT_MAX with the same > INT_MAX condition as the slice-path check, so a real stream (a few dozen bytes of SPS/PPS, cleared at each slice) never meets it. One small thing: the AVC parameter-set units entry you edited in docs/dev/debt/libSceVideodec2.md names a game in front of its title id ("Twispero (PPSA99474)"). Debt entries may name title ids but not game names, so please change it to just "PPSA99474" while you are on that line. The "Tested" section also mentions validating the ordinary parameter-set-then-slice path; if you ran something, please say what, otherwise remove the sentence.

[pushkarreddyy]

@MarcZX10 Thanks for the review observations.

1. Practical Cumulative Cap (2 MiB): We updated the cap from INT_MAX to MaxAvcParameterSetsBytes = 2u * 1024u * 1024u (2 MiB) in commit 52695699c4. Real-world H.264 SPS/PPS parameter sets across streams do not exceed tens of kilobytes, so 2 MiB provides ample headroom while stopping large multi-megabyte allocations immediately.
2. Rejection & State Preservation: The check occurs before vector insert, leaving pending decoder state untouched if an oversized access unit is rejected.
3. Regression Test: Added testParameterSetsBound() in core/libs/tests/GuestVideodec2.cpp verifying that oversized parameter units throw Videodec2: invalid access unit and leave the decoder in an operational state where subsequent operations, resets, and deletions succeed.
4. Technical Debt: Updated docs/dev/debt/libSceVideodec2.md to document the 2 MiB bound.

Review Etiquette Reminder: Please note that formal GitHub review decisions (Request Changes / Approve) are reserved exclusively for repository maintainers with write/merge permissions on AnyPS5. As a fellow contributor, please refrain from submitting formal review states and instead post technical observations, suggestions, or test results as standard PR discussion comments so as not to clutter the maintainers' review queue.

[MarcZX10]

The 2 MiB cap now blocks the reported 32 × 8 MiB accumulation, and the check is before parameterSets.insert; the exact-head Windows build and conventions checks pass. The existing testDecode(false, true) also exercises standalone SPS/PPS followed by slices.

Two small follow-ups remain visible on this head: the debt entry still uses the game name “Twispero” alongside PPSA99474, while the latest thread asks for title ID only; and testParameterSetsBound resets/deletes after rejection, so it does not directly show that previously pending parameter sets remain usable. Could you remove the game name and strengthen the regression by decoding the normal SPS/PPS-plus-slice sequence after the oversized AU is rejected? That would make the “pending state unchanged” claim directly observable. I am continuing this as a discussion comment per your review-etiquette note.

[JJenkx]

A Windows run: the new test doesn't compile. Head 52695699c4 was merged onto main 82d5f01f28 with no conflicts. Setup: Windows 11 Enterprise 10.0.22621, WinLibs GCC 15.2.0 UCRT r7, Ninja, Release, BUILD_TESTING=ON.

core/libs/tests/GuestVideodec2.cpp:841:39: error: 'CodecAvc' was not declared in this scope; did you mean 'CodecHevc'?

CodecAvc isn't defined at this head, and it doesn't exist anywhere in main's history. The file only declares CodecHevc and CodecVp9 (lines 31–32), and line 780 passes AVC as the literal 1u. Either use 1u at line 841 or add constexpr std::uint32_t CodecAvc = 1; next to the other two.

The hosted linux job was skipped and the windows job doesn't build the test executables, so CI didn't catch this. I didn't run guest_videodec2.

AI-assisted: yes (Claude Code).

[mashingaan]

On head 52695699c4a443800909613045c68a9d9ab78649, one qualification on the memory and state-preservation claims from reading the code:

The 2 MiB guard bounds parameterSets before insertion, but ToAnnexB runs first and copies the complete AU into decoder.annexB. A large parameter-only AU can therefore allocate a large buffer before being rejected, and that vector can retain its capacity. This does not negate the fix for cumulative parameterSets growth.

ToAnnexB can also update lengthPrefixed and shortStartCodes before rejection. Those flags affect how it interprets an input that is both AVCC-shaped and starts with an Annex-B prefix, so the statement that all decoder state is unchanged seems stronger than the code supports. The existing test resets immediately after the exception and does not observe a subsequent decode.

Could the description distinguish the parameterSets bound from a bound on all decoder allocations, and could a follow-up check cover a rejected parameter-only AU followed by a valid slice without reset? Classifying before the copy and committing format-state updates only after acceptance may help, but should preserve large slice-bearing AUs.

This is a source-level observation only. I have not run the decoder or measured console behavior.

AI-assisted: Codex

[pushkarreddyy]

@oneandonlydean @MarcZX10 Thanks for the guidance! I applied both updates in commits 3dd6787fbc and 1243992353:

1. Title ID in Technical Debt: Removed the game name in docs/dev/debt/libSceVideodec2.md, citing solely PPSA99474 per repository conventions.
2. Direct State Preservation Verification: Strengthened testParameterSetsBound() in core/libs/tests/GuestVideodec2.cpp: it now submits valid initial parameter sets, verifies rejection of the subsequent oversized AU (>2 MiB), and then decodes the remaining slice unit to directly confirm that the staged parameter sets remained intact and functional.
3. Updated the ### Tested section of the PR description accordingly.

[pushkarreddyy]

@JJenkx Thank you for catching that! In 9da89158de, I added constexpr std::uint32_t CodecAvc = 1; next to CodecHevc and CodecVp9 in GuestVideodec2.cpp to resolve the declaration error on GCC.

@mashingaan Thanks for the detailed observation! In commit 3dd6787fbc, we expanded testParameterSetsBound() to directly test that sequence: it stages a valid initial SPS/PPS unit, verifies that the oversized parameter unit throws without resetting the decoder, and then feeds the remaining valid slice unit to confirm that the initial parameter sets remained intact and the frame was accepted. As you noted, the 2 MiB bound specifically guards cumulative parameterSets retention to resolve the unbound allocation reported in #12189.

[pushkarreddyy]

@MarcZX10 Also, just wanted to clear up any misunderstanding from my earlier note — frequent, established contributors like yourself, Babyhuehnchen, screaam, etc. are of course very welcome to submit formal GitHub reviews and approvals! Your technical feedback, testing, and reviews are always valued and appreciated.

[maskym]

Windows result for head 9da89158de, merged into main 87b810a1db (the PR's Tested section names no Windows run).

Host: Windows 11 26300, Intel i7-12700K, WinLibs GCC 15.2.0 (x86_64-ucrt-posix-seh, r6), Release, BUILD_TESTING=ON, Ninja. Build clean.

The tree also carried the open PRs #12290, #12288, #12285, #12269, #12265, #12232, #12206, #12201, #12199, #12165, #12146, all merged cleanly, so the run is a joint one; the tests of this PR are guest_videodec2 (0.10 s), guest_videodec2_unimplemented (0.03 s), all passed (every test registered from the tree's changed test files: 38/38). Not run on a PS5.

AI-assisted: yes (Claude Code), the runs and the numbers above are from this machine.

[MarcZX10]

Reviewed the updated head 9da89158de27615f358113f572198c9f4213f4a0. The cumulative 2 MiB check runs before insertion, and the regression test now covers the prior failure mode: it retains a valid parameter set, rejects an oversized following unit, then decodes a valid slice using the retained state. The documented bound matches the implementation. Hosted conventions and Windows checks pass; the author-reported Windows suite covers 38/38 registered tests, including both Videodec2 tests. The Linux/platform-specific jobs were skipped by eligibility, and the Windows run is not PS5 hardware validation. No remaining blocker found in this change.

[malekaifa]

Re-checked exact head 3e532910ea63c9b953134f143c8fb52a2868055b. The blocker from my previous review is resolved: the cumulative AVC parameter-set size is now checked before Parse_nid_no_patch can mutate persistent SPS/PPS state, and the added regression rejects an oversized parameter unit carrying different metadata before decoding a following slice and verifies the original staged metadata is preserved. git diff --check was clean in my prior exact-head source check, and the hosted state is green. I did not run the guest videodec2 runtime test locally on this machine. No remaining blocker found.

AI-assisted: yes (OpenAI ChatGPT).

[github-actions]

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request	Conflicting files (lines on main)	Same exports	Same new files
PR 15598	core/libs/tests/GuestVideodec2.cpp L1062

Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.

Pull request	Shared files not compared
PR 14212	docs/dev/debt/libSceVideodec2.md

[pushkarreddyy]

@malekaifa Thank you for re-checking and approving the cumulative AVC parameter set bound enforcement! The branch has been cleanly rebased onto latest origin/main with all CI checks passing cleanly.

[B-Howell]

Run of the head pushed at 16:52 UTC today, 88371debf8, merged locally onto main b28bb77217 (clean merge). Earlier runs in this thread were on older heads.

Setup: Windows 11 Pro 26300, Ryzen 7 7800X3D, WinLibs GCC 15.2.0 r7, Release, BUILD_TESTING=ON; WSL2 Ubuntu 22.04, GCC 11.4, Release.

Results:

• Tests: guest_videodec2, guest_videodec2_unimplemented and guest_hevc_metadata pass 3 of 3 on both hosts, with no skips. The debt checks pass too (debt_split, conventions_debt_length, conventions_doc_links).
• Negative control (Linux): with only core/libs/prx/libSceVideodec2/Export.cpp from main and this head's test kept, guest_videodec2 fails.
• Stacked with fix(libSceVideodec2): keep AVC picture info after a PPS is replaced under the same SPS #14998, feat(libSceVideodec2): implement the HEVC decoder aliases #14722 and fix(libSceVideodec2): reject ambiguous HEVC picture-timing association (all platforms) #14074: all four merge cleanly, and the same tests plus guest_videodec2_hevc_aliases pass 3 of 3 on both hosts.

AI-assisted: yes (Claude Code)

[pushkarreddyy]

Thank you @B-Howell for the comprehensive test verification across Windows 11 and WSL2 Ubuntu (3/3 passed on both hosts, negative control verified, and clean stacked verification with #14998, #14722, and #14074)! The branch has also been cleanly rebased onto latest origin/main (head e533ff5cc6) with 0 convention findings.

[TechLevelZero]

✅ AMD GPU test: passed

PR head e533ff5 merged onto main 4f41f4c · tested 2026-11-03 16:02 UTC · 18 min

GPU	AMD Radeon RX 6300 (RADV NAVI24), driver Mesa 25.2.8-0ubuntu0.24.04.4, Vulkan 1.4.318
Host	Ubuntu 26.04.1 LTS, kernel 7.0.0-42-generic, container ubuntu:24.04
Logs	build and test logs for this run
Build	✅ passed (Release, -DANYPS5_ENABLE_SPIRV_TOOLS=ON)
Tests	✅ 1,135 passed · 6 failed (0 new, 4 also fail on main, 2 unstable on main) · 6 skipped (ANYPS5_REQUIRE_VULKAN=1)
Test setup	CPU-only tests without a GPU; GPU tests (and any CPU-only test that failed or skipped without one) once on the GPU with VK_LAYER_KHRONOS_validation and ANYPS5_NO_SHADER_CACHE=1; main ran the same way
Test selection	1,147 of 3,811 tests run: the ones this change can reach, chosen by tools/select_tests.py from main's build graph
Vulkan validation	✅ no new validation IDs vs main (547 tests run on the GPU)
Slow tests (>30 s, GPU tests timed with validation layers)	⚠️ agc_driver_pipeline_journal 120.3 s, ⚠️ public_headers 64.4 s, ⚠️ agc_driver_lazy_resolved_abi 51.9 s, ⚠️ agc_driver_image_sample_unnormalized 50.0 s, ⚠️ agc_driver_dispatch_move_depth 38.0 s

Unstable on main (2, these also failed in recent main runs on this machine; not counted against this PR)

agc_driver_pipeline_journal: failed in 7 of the last 9 main runs
agc_driver_store_over_untracked: failed in 1 of the last 9 main runs

Failing on main too (4, not counted against this PR)

agc_driver_image_table
agc_driver_native_depth_bda_clear
agc_driver_native_multisample_depth
agc_driver_submit_progress

Passed on retry (1, failed in the parallel run, passed when re-run with a 360 s timeout; not counted against this PR)

trace_digest_TraceDigestTests_test_cli_rejects_invalid_event_fields_without_partial_output: Timeout in the parallel run; also passed only on retry in 34 other runs in the last 7 days

Validated but not compared (1, not in the main baseline's validation pass)

trace_digest_TraceDigestTests_test_cli_rejects_invalid_event_fields_without_partial_output

Validation IDs already present on main (4 IDs in 465 tests)

VUID-VkDeviceCreateInfo-pNext-pNext (465 tests): agc_driver_address_passes, agc_driver_alias_copy_label, agc_driver_alpha_to_coverage, agc_driver_array_layer_target, agc_driver_bc_block_copy, agc_driver_bda_anchor_reads, agc_driver_bda_span_reads, agc_driver_bindless_images, and 457 more
VUID-vkCmdCopyImage-srcImage-01548 (8 tests): agc_driver_depth_clear_hle, agc_driver_depth_plane_buffer_memory, agc_driver_depth_plane_partial_store, agc_driver_depth_storage_dispatch, agc_driver_depth_surface_from_storage, agc_driver_dispatch_move_depth, agc_driver_stencil_null_feedback, agc_driver_stencil_plane_memory
VUID-vkCmdCopyImage-srcImage-01551 (8 tests): agc_driver_depth_clear_hle, agc_driver_depth_plane_buffer_memory, agc_driver_depth_plane_partial_store, agc_driver_depth_storage_dispatch, agc_driver_depth_surface_from_storage, agc_driver_dispatch_move_depth, agc_driver_stencil_null_feedback, agc_driver_stencil_plane_memory
VUID-VkPipelineColorBlendStateCreateInfo-pAttachments-00605 (2 tests): agc_driver_prepared_exports, agc_driver_srgb8_color_target

Skipped (6)

agc_driver_copied_writer_label_order: skipped, the device reports a minStorageBufferOffsetAlignment of 4, so the dispatch binds its record at +0x1004 in place and nothing is written back; the test needs an alignment above 4
agc_driver_float16_unused_nan_payloads: skipped, the device has no native f16 conversions with RTE rounding and kept f16 denormals
agc_driver_linear_target_direct_write_completion: skipped, the device does not keep host imports write-watched, so no completion stamp over the target is tracked
agc_driver_ngg_vertex_wave_reduction: skipped, s3 describes a 32-lane wave only on 32-lane subgroups, and the device has 64 lanes
agc_driver_scattered_runs: skipped, host imports are unavailable: /dev/udmabuf is not accessible
agc_driver_sparse_dma_buf_import: skipped, host imports are unavailable: /dev/udmabuf is not accessible

Reproduce

git fetch origin e533ff5cc610ae609c56fd66da4089515b8c6ffd 4f41f4c375f5abb487dbd184fbc150f31994d762
git checkout 4f41f4c375f5abb487dbd184fbc150f31994d762 && git merge --no-ff e533ff5cc610ae609c56fd66da4089515b8c6ffd
git submodule update --init --recursive
cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTING=ON -DANYPS5_ENABLE_SPIRV_TOOLS=ON
cmake --build build && cmake --build build --target libs
VK_DRIVER_FILES=/nonexistent/icd.json ctest --test-dir build -E '^(agc|shader|recompiler|videoout|video_out|guest_video_out|vulkan)|gpu' --timeout 120 --parallel 12 --output-on-failure
ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1 VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation \
  ctest --test-dir build -R '^(agc|shader|recompiler|videoout|video_out|guest_video_out|vulkan)|gpu' --timeout 120 --parallel 16 --output-on-failure
# failures are then re-run alone: --timeout 360 --parallel 2; new validation IDs: the test alone, twice, --parallel 1

_{Automated build and test on a contributor's AMD machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs.}