Skip to content

feat(libSceVideoOut): implement sceVideoOutSubmitChangeBufferAttribute - #15247

Open
AlexWhitehouse wants to merge 3 commits into
boykopovar:mainfrom
AlexWhitehouse:feat/libscevideoout-change-buffer-attribute
Open

AlexWhitehouse wants to merge 3 commits into
boykopovar:mainfrom
AlexWhitehouse:feat/libscevideoout-change-buffer-attribute

Conversation

@AlexWhitehouse

@AlexWhitehouse AlexWhitehouse commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

What

Implements sceVideoOutSubmitChangeBufferAttribute (NID IOdgHlCGU-k), the PS4-era call that replaces the attribute of a registered legacy buffer group. It was a throwing inventory stub. The PS5 form, sceVideoOutSubmitChangeBufferAttribute2, and legacy registration through sceVideoOutRegisterBuffers were already implemented.

The behaviour follows shadPS4 commit 2183807e592b36d4440384878165b43d930c1dee:

In core/libs/prx/libSceVideoOut/src/Buffer.cpp:

  • Returned errors:

    • a null attribute is VIDEO_OUT_ERROR_INVALID_ADDRESS (0x80290002) for any handle and set index. It is checked first, as in the 13.42 module's disassembly quoted in review;
    • then an unknown or closing handle is VIDEO_OUT_ERROR_INVALID_HANDLE;
    • an aspect ratio other than 0 is INVALID_ASPECT_RATIO (0x80290008);
    • a width above the pitch is INVALID_PITCH (0x80290004);
    • a tiling mode above linear is INVALID_TILING_MODE (0x80290007).

    The three new codes are added to VideoOutDriver.hpp.

  • Thrown errors: these throw, as legacy registration and ...Attribute2 already do for the same inputs, rather than returning shadPS4's INVALID_VALUE:

    • an out-of-range or negative set index;
    • an unregistered group;
    • non-zero reserved fields;
    • a non-zero option.

    A negative index is out of bounds in shadPS4. Changing a PS5 group through the PS4 call isn't settled by any source, so it throws too. Unsupported PS4 pixel formats keep their existing throw.

  • Update: the legacy attribute is converted the same way sceVideoOutRegisterBuffers converts it. Every buffer in the group is checked against guest memory with the new attribute before the group is replaced, as ...Attribute2 does. Buffer addresses and pending flips are kept.

  • Tiled: a tiled attribute (tiling mode 0) is stored without a pitch and the group's PS4 pitch is recomputed, as registration and ...Attribute2 do. Its pitch must still be at least its width, as in shadPS4, so a tiled change with pitch 0 returns INVALID_PITCH; registration here instead pads a zero tiled pitch to 128.

  • No changes to existing functions: the new export is self-contained, so sceVideoOutRegisterBuffers and ...Attribute2 are untouched.

Debt (docs/dev/debt/libSceVideoOut.md):

  • A new entry records the 13.42 order of the null-attribute and handle checks, where the remaining checks differ from shadPS4, the tiled pitch rule, and that their PS5 order and timing are unverified.
  • The export's inventory row is removed and the entry count goes from 170 to 169, keeping main's "of the original 182" wording.

Tested

  • video_out_flip_* (core/libs/prx/libSceVideoOut/tests/Flip.cpp): the PS4 buffer test now registers a second legacy group with different dimensions and pitch, then changes the first one. Every pre-existing assertion is kept. It checks:
    • the converted fields;
    • unchanged buffer addresses and the untouched neighbouring group;
    • each returned and thrown validation failure, with state unchanged after each rejection;
    • a null attribute returning INVALID_ADDRESS ahead of an invalid handle or set index;
    • a change to tiled at pitches 128 and 256, presented as a GCN display buffer with that pitch, a tiled change without a pitch returning INVALID_PITCH, and the change back to linear;
    • a change while a flip is pending, followed by that flip completing;
    • the throw for a PS5 group.
  • Linux, lavapipe (Mesa 26.0.8, Ubuntu 26.04, Xvfb), with ANYPS5_REQUIRE_VULKAN=1, ANYPS5_NO_SHADER_CACHE=1 and VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation:
    • on main (d60ebec) with only the test added, 27/28 pass; the failure is sceVideoOutSubmitChangeBufferAttribute not implemented;
    • with this PR (8ea336b), 28/28 pass, and the whole VideoOut group passes 53 of 54 with 1 skipped (the display lacks 120 Hz);
    • no VUID- lines and no missing-layer messages in either log.
  • Windows 11, NVIDIA RTX 3060 (driver 610.88, Vulkan 1.4.341, MinGW GCC 15.2.0, ANYPS5_VIDEO_OUT_TEST_DRIVER=windows), with the same three variables (the validation layer is confirmed loaded in every run):
    • On main (d60ebec): 27/28 pass. video_out_flip_ps4 fails with sceVideoOutSubmitChangeBufferAttribute not implemented. To build the new test against main's throwing stub, only the typed declaration and the three error constants were added.
    • With this PR (8ea336b): 28/28 pass.
    • The VideoOut group: 51 of 52 pass, with 1 skipped: guest_video_out_wqhd_detection_no_vulkan reports no VK_KHR_surface.
    • Validation: no VUID- lines and no missing-layer messages in any log.
  • Mutation check: swapping width and pitch in the conversion fails the flip test; so does updating the neighbouring group. Checking the handle or the set index before the null attribute, or throwing for a null attribute, fails video_out_flip_ps4 too. So does each of these on the tiled path: dropping the ps4Pitch recompute, keeping the pitch in a tiled attribute, refusing tiled changes, or skipping the pitch check for a tiled attribute.
  • python3 tools/check_conventions.py --base origin/main: all conventions checks passed. The public-header audit passes.
  • python3 tools/pr_overlap.py: no other PR implements this export.

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: none
  • Unblocks: none
  • GPUs tested: NVIDIA GeForce RTX 3060, driver 610.88, Windows 11 (build 26300); Mesa lavapipe 26.0.8 (LLVM 21.1.8), Ubuntu 26.04
  • AI-assisted: yes

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Progress report (6ed10cc → 8ea336b)

📈 System libraries: 29.24% (+0.01%, +1 implemented functions)

✅ 1 implemented
Library Name
libSceVideoOut sceVideoOutSubmitChangeBufferAttribute

Changed libraries (implemented exports)

Library Before After Change
libSceVideoOut 69/269 (25.65%) 70/269 (26.02%) +0.37%

@TechLevelZero

Copy link
Copy Markdown
Contributor

✅ NVIDIA GPU test: passed

PR head 2f2b317 merged onto main 9090972 · tested 2026-11-02 22:43 UTC · 15 min

GPU NVIDIA RTX 2000 Ada Generation, driver 595.99.02, Vulkan 1.4.329
Host Ubuntu 26.04.1 LTS, kernel 7.0.0-38-generic, container ubuntu:24.04
Logs build and test logs for this run
Build ✅ passed (Release, -DANYPS5_ENABLE_SPIRV_TOOLS=ON)
Tests ✅ 967 passed · 2 failed (0 new, 1 also fail on main, 1 unstable on main) · 0 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 969 of 3,607 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 (456 tests run on the GPU)
Slow tests (>30 s, GPU tests timed with validation layers) ⚠️ agc_driver_pipeline_journal 120.1 s
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)
agc_driver_pipeline_journal: failed in 3 of the last 10 main runs
Failing on main too (1, not counted against this PR)
agc_driver_image_table
Validation IDs already present on main (4 IDs in 384 tests)
VUID-VkDeviceCreateInfo-pNext-pNext (384 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 376 more
VUID-vkCmdCopyImage-srcImage-01548 (7 tests): agc_driver_depth_clear_hle, agc_driver_depth_plane_buffer_memory, agc_driver_depth_plane_partial_store, agc_driver_depth_plane_sampling, agc_driver_depth_storage_dispatch, agc_driver_depth_surface_from_storage, agc_driver_dispatch_move_depth
VUID-vkCmdCopyImage-srcImage-01551 (7 tests): agc_driver_depth_clear_hle, agc_driver_depth_plane_buffer_memory, agc_driver_depth_plane_partial_store, agc_driver_depth_plane_sampling, agc_driver_depth_storage_dispatch, agc_driver_depth_surface_from_storage, agc_driver_dispatch_move_depth
VUID-VkPipelineColorBlendStateCreateInfo-pAttachments-00605 (1 test): agc_driver_prepared_exports
Reproduce
git fetch origin 2f2b31783da2eba0fbd5f567395843a1c72eef69 9090972b7e91fd266819cbd77a8b058e177b5682
git checkout 9090972b7e91fd266819cbd77a8b058e177b5682 && git merge --no-ff 2f2b31783da2eba0fbd5f567395843a1c72eef69
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 32 --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 4 --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 NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Nov 3, 2026
@AlexWhitehouse
AlexWhitehouse force-pushed the feat/libscevideoout-change-buffer-attribute branch from 2f2b317 to 963fc0e Compare November 3, 2026 19:34
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Nov 3, 2026

@BunnyFl BunnyFl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexWhitehouse thanks for this. The firmware has a different check order at the start, and one of the new throws should be a returned code.

The null attribute is checked first, and it returns a code. In firmware 13.42 system/common/lib/libSceVideoOut.sprx (sha256 975b229324655a2d2a945c755fc0339e7feecb62863efc28ad3b2d91a95ad215), sceVideoOutSubmitChangeBufferAttribute (IOdgHlCGU-k, 0x112e0) begins, read from disassembly and not run:

112e0: test   %rdx,%rdx
112e3: je     0x1133c                ; attribute == nullptr
...
112f8: call   0x13f00                ; port lookup by handle
112fd: test   %rax,%rax
11300: je     0x11342
...
1133c: mov    $0x80290002,%eax       ; returned before any handle lookup
11341: ret
11342: mov    $0x8029000b,%eax

So, on the console:

  • (any handle, i, nullptr) returns 0x80290002, which is VIDEO_OUT_ERROR_INVALID_ADDRESS (-2144796670 in VideoOutDriver.hpp). That includes an unknown handle.
  • A non-null attribute with an unknown handle returns 0x8029000b (VIDEO_OUT_ERROR_INVALID_HANDLE), as you have it.

On this head (core/libs/prx/libSceVideoOut/src/Buffer.cpp):

  • (badHandle, 0, nullptr) returns INVALID_HANDLE: the handle check is at Buffer.cpp:221, before the null check at :226.
  • (validHandle, 0, nullptr) throws VIDEO_OUT_ERROR_INVALID_ADDRESS (:227) instead of returning it. The setIndex throw (:223) also comes before the null check here, while the console reaches the handler with any index once the pointer and handle pass.

Suggested fix: make if (attribute == nullptr) return VIDEO_OUT_ERROR_INVALID_ADDRESS; the first statement, then check the handle, and add a test row for each.

After those two checks the module doesn't validate anything itself. It reaches the port's handler through 0x8(%rax) → 0xa8, which is call *%rax at 1131e with (*port, setIndex, attribute) (11315: mov (%rbx),%rdi). If that handler is null it returns 0x8029001b. On a non-negative result it calls 0x1a160(port, setIndex, attribute, -1). So the index, reserved-field, aspect-ratio, pitch and tiling checks and their order sit in the handler, which this export doesn't show. That fits your debt entry, which leaves them as shadPS4's and unverified on the PS5. I didn't follow the handler pointer, so I can't say where it lands.

What I ran: this head merged onto main (merge 14f2bb9558), Windows 11, WinLibs GCC, Release, AMD Radeon RX 9070 XT. With ANYPS5_REQUIRE_VULKAN=1, ANYPS5_NO_SHADER_CACHE=1 and VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation, all 28 video_out_flip_* tests pass (28 of 28), video_out_flip_ps4 included. I haven't compared VUID- output: these runs were without -V, and I didn't run main alongside. The finding above is about the CPU-side order and doesn't depend on it.

@AlexWhitehouse

Copy link
Copy Markdown
Contributor Author

Thanks, done in a760a3e: a null attribute is now checked first and returns VIDEO_OUT_ERROR_INVALID_ADDRESS for any handle and set index, then the handle check returns INVALID_HANDLE. Flip.cpp checks both orders with state unchanged afterwards, and the debt entry records the 13.42 order and leaves the handler's checks unverified. The VideoOut group passes on lavapipe and an RTX 3060 with validation and no VUID- lines, and video_out_flip_ps4 fails if the handle or index is checked before the null attribute or if a null attribute throws.

@TechLevelZero

Copy link
Copy Markdown
Contributor

✅ AMD GPU test: passed

PR head a760a3e merged onto main 1c8c0ef · tested 2026-11-04 00:46 UTC · 13 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,121 passed · 4 failed (0 new, 3 also fail on main, 1 unstable on main) · 8 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,133 of 3,842 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 (546 tests run on the GPU)
Slow tests (>30 s, GPU tests timed with validation layers) ⚠️ agc_driver_pipeline_journal 120.1 s, ⚠️ public_headers 58.1 s, ⚠️ agc_driver_image_sample_unnormalized 47.0 s, ⚠️ agc_driver_lazy_resolved_abi 39.6 s, ⚠️ agc_driver_sample_multisample_target 32.0 s
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)
agc_driver_pipeline_journal: failed in 8 of the last 10 main runs
Failing on main too (3, not counted against this PR)
agc_driver_image_table
agc_driver_native_depth_bda_clear
agc_driver_native_multisample_depth
Validation IDs already present on main (8 IDs in 467 tests)
VUID-VkDeviceCreateInfo-pNext-pNext (467 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 459 more
VUID-vkCmdCopyImage-srcImage-01548 (4 tests): agc_driver_depth_clear_hle, agc_driver_depth_storage_dispatch, agc_driver_depth_surface_from_storage, agc_driver_dispatch_move_depth
VUID-vkCmdCopyImage-srcImage-01551 (4 tests): agc_driver_depth_clear_hle, agc_driver_depth_storage_dispatch, agc_driver_depth_surface_from_storage, agc_driver_dispatch_move_depth
VUID-VkPipelineColorBlendStateCreateInfo-pAttachments-00605 (1 test): agc_driver_srgb8_color_target
VUID-VkSamplerCreateInfo-addressModeU-01079 (1 test): agc_driver_image_gather_volume
VUID-VkSamplerCreateInfo-addressModeU-parameter (1 test): agc_driver_image_gather_volume
VUID-VkSamplerCreateInfo-addressModeV-parameter (1 test): agc_driver_image_gather_volume
VUID-VkSamplerCreateInfo-addressModeW-parameter (1 test): agc_driver_image_gather_volume
Skipped (8)
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_gcn_display_present: skipped, no display: Vulkan support is either not configured in SDL or not available in current SDL video driver (offscreen) or platform
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 a760a3ea0cd5ac6001128096c604a5ffd8c08050 1c8c0efc8c41314402e6966da5bef6ea3d639d10
git checkout 1c8c0efc8c41314402e6966da5bef6ea3d639d10 && git merge --no-ff a760a3ea0cd5ac6001128096c604a5ffd8c08050
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.

@BunnyFl BunnyFl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexWhitehouse thanks, this matches the 13.42 order I described: the null attribute now returns VIDEO_OUT_ERROR_INVALID_ADDRESS as the first statement (Buffer.cpp:220), before the handle check, and no longer throws. The new rows in Flip.cpp cover a null attribute with indices 0, 1, 3, -1 and VIDEO_OUT_BUFFER_ATTRIBUTE_NUM_MAX, and with handles -1, 0 and VIDEO_OUT_NUM_MAX, and the debt entry now separates the two checks read from the module from the handler's checks, which stay unverified.

What I ran: head a760a3ea0c merged onto main 1c8c0efc8c, Windows 11, WinLibs GCC, Release, AMD Radeon RX 9070 XT, with ANYPS5_REQUIRE_VULKAN=1, ANYPS5_NO_SHADER_CACHE=1 and VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation:

  • all 28 video_out_flip_* tests pass (28 of 28);
  • mutation A (handle checked before the null attribute): video_out_flip_ps4 fails with "the legacy change checked its handle before a null attribute";
  • mutation B (a null attribute throws VIDEO_OUT_ERROR_INVALID_ADDRESS instead of returning it): video_out_flip_ps4 fails on the uncaught throw;
  • tools/check_conventions.py --base upstream/main on the head: all checks passed.

I didn't compare VUID- output myself (no -V run on main alongside); your run and the AMD CI comment above report none new. Approving.

@TechLevelZero

Copy link
Copy Markdown
Contributor

✅ NVIDIA GPU test: passed

PR head a760a3e merged onto main 1c8c0ef · tested 2026-11-04 02:01 UTC · 19 min

GPU NVIDIA RTX 2000 Ada Generation, driver 595.99.02, Vulkan 1.4.329
Host Ubuntu 26.04.1 LTS, kernel 7.0.0-38-generic, container ubuntu:24.04
Logs build and test logs for this run
Build ✅ passed (Release, -DANYPS5_ENABLE_SPIRV_TOOLS=ON)
Tests ✅ 1,126 passed · 2 failed (0 new, 1 also fail on main, 1 unstable on main) · 5 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,133 of 3,842 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 (546 tests run on the GPU)
Slow tests (>30 s, GPU tests timed with validation layers) ⚠️ agc_driver_pipeline_journal 120.1 s, ⚠️ agc_driver_lookup_log_fits_build 47.2 s
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)
agc_driver_pipeline_journal: failed in 4 of the last 10 main runs
Failing on main too (1, not counted against this PR)
agc_driver_image_table
Validation IDs already present on main (8 IDs in 467 tests)
VUID-VkDeviceCreateInfo-pNext-pNext (467 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 459 more
VUID-vkCmdCopyImage-srcImage-01548 (7 tests): agc_driver_depth_clear_hle, agc_driver_depth_plane_buffer_memory, agc_driver_depth_plane_partial_store, agc_driver_depth_plane_sampling, agc_driver_depth_storage_dispatch, agc_driver_depth_surface_from_storage, agc_driver_dispatch_move_depth
VUID-vkCmdCopyImage-srcImage-01551 (7 tests): agc_driver_depth_clear_hle, agc_driver_depth_plane_buffer_memory, agc_driver_depth_plane_partial_store, agc_driver_depth_plane_sampling, agc_driver_depth_storage_dispatch, agc_driver_depth_surface_from_storage, agc_driver_dispatch_move_depth
VUID-VkPipelineColorBlendStateCreateInfo-pAttachments-00605 (1 test): agc_driver_srgb8_color_target
VUID-VkSamplerCreateInfo-addressModeU-01079 (1 test): agc_driver_image_gather_volume
VUID-VkSamplerCreateInfo-addressModeU-parameter (1 test): agc_driver_image_gather_volume
VUID-VkSamplerCreateInfo-addressModeV-parameter (1 test): agc_driver_image_gather_volume
VUID-VkSamplerCreateInfo-addressModeW-parameter (1 test): agc_driver_image_gather_volume
Skipped (5)
agc_driver_gcn_display_present: skipped, no display: Vulkan support is either not configured in SDL or not available in current SDL video driver (offscreen) or platform
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
agc_driver_stencil_ref_export: skipped, the device lacks VK_EXT_shader_stencil_export
Reproduce
git fetch origin a760a3ea0cd5ac6001128096c604a5ffd8c08050 1c8c0efc8c41314402e6966da5bef6ea3d639d10
git checkout 1c8c0efc8c41314402e6966da5bef6ea3d639d10 && git merge --no-ff a760a3ea0cd5ac6001128096c604a5ffd8c08050
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 32 --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 4 --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 NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs.

@AlexWhitehouse
AlexWhitehouse force-pushed the feat/libscevideoout-change-buffer-attribute branch from a760a3e to 723e10f Compare November 5, 2026 21:39
@TechLevelZero

Copy link
Copy Markdown
Contributor

✅ NVIDIA GPU test: passed

PR head 723e10f merged onto main 55064c8 · tested 2026-11-05 22:36 UTC · 17 min

GPU NVIDIA RTX 2000 Ada Generation, driver 595.99.02, Vulkan 1.4.329
Host Ubuntu 26.04.1 LTS, kernel 7.0.0-38-generic, container ubuntu:24.04
Logs build and test logs for this run
Build ✅ passed (Release, -DANYPS5_ENABLE_SPIRV_TOOLS=ON)
Tests ✅ 1,333 passed · 2 failed (0 new, 1 also fail on main, 1 unstable on main) · 2 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,337 of 4,105 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 (637 tests run on the GPU)
Slow tests (>30 s, GPU tests timed with validation layers) ⚠️ agc_driver_pipeline_journal 120.1 s
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)
agc_driver_pipeline_journal: failed in 1 of the last 10 main runs
Failing on main too (1, not counted against this PR)
agc_driver_image_table
Validation IDs already present on main (9 IDs in 549 tests)
VUID-VkDeviceCreateInfo-pNext-pNext (549 tests): agc_driver_address_draw_space_reuse, 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, and 541 more
VUID-VkShaderModuleCreateInfo-pCode-08737 (14 tests): agc_driver_dpp16_operands, agc_driver_dpp8_pk_fmac, agc_driver_dx10_clamp, agc_driver_f16_fma_denormal_modes, agc_driver_f16_fma_invalid_nan, agc_driver_float16_fma_rounding, agc_driver_float16_modifiers, agc_driver_float16_native, and 6 more
VUID-VkShaderModuleCreateInfo-pCode-08739 (14 tests): agc_driver_dpp16_operands, agc_driver_dpp8_pk_fmac, agc_driver_dx10_clamp, agc_driver_f16_fma_denormal_modes, agc_driver_f16_fma_invalid_nan, agc_driver_float16_fma_rounding, agc_driver_float16_modifiers, agc_driver_float16_native, and 6 more
VUID-VkShaderModuleCreateInfo-pCode-08741 (14 tests): agc_driver_dpp16_operands, agc_driver_dpp8_pk_fmac, agc_driver_dx10_clamp, agc_driver_f16_fma_denormal_modes, agc_driver_f16_fma_invalid_nan, agc_driver_float16_fma_rounding, agc_driver_float16_modifiers, agc_driver_float16_native, and 6 more
VUID-vkCmdCopyImage-srcImage-01548 (7 tests): agc_driver_depth_clear_hle, agc_driver_depth_plane_buffer_memory, agc_driver_depth_plane_partial_store, agc_driver_depth_plane_sampling, agc_driver_depth_storage_dispatch, agc_driver_depth_surface_from_storage, agc_driver_dispatch_move_depth
VUID-vkCmdCopyImage-srcImage-01551 (7 tests): agc_driver_depth_clear_hle, agc_driver_depth_plane_buffer_memory, agc_driver_depth_plane_partial_store, agc_driver_depth_plane_sampling, agc_driver_depth_storage_dispatch, agc_driver_depth_surface_from_storage, agc_driver_dispatch_move_depth
VUID-VkPipelineColorBlendStateCreateInfo-pAttachments-00605 (1 test): agc_driver_prepared_exports
VUID-VkPipelineShaderStageCreateInfo-module-parameter (1 test): agc_driver_float16_unused_nan_payloads
VUID-VkPipelineShaderStageCreateInfo-pSpecializationInfo-06849 (1 test): agc_driver_float16_unused_nan_payloads
Skipped (2)
agc_driver_gcn_display_present: skipped, no display: Vulkan support is either not configured in SDL or not available in current SDL video driver (offscreen) or platform
agc_driver_vertex_elected_atomic_max: skipped, the device runs vertex programs in subgroups, so they keep their cross-lane reductions
Reproduce
git fetch origin 723e10f82ba7940b78609e864d1d924b1b0cf1e6 55064c81fb67060082e3e9f6f4262ba348d95a0a
git checkout 55064c81fb67060082e3e9f6f4262ba348d95a0a && git merge --no-ff 723e10f82ba7940b78609e864d1d924b1b0cf1e6
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 32 --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 4 --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 NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs.

@BunnyFl BunnyFl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-checked — approved

@AlexWhitehouse re-approved at 723e10f82b. My approval sat on a760a3ea, which you force-pushed over, so I re-ran everything rather than carrying it forward.

What changed since the head I approved. git range-diff from a760a3ea over the two merge bases shows the null-attribute commit identical (=) and the first commit differing only in the debt count line, which the rebase onto newer main re-resolved. No code changed.

The debt count. 174 is right, counted from the file rather than by subtracting: the bullet's number is the length of the table under it, and it reads 174 with 174 rows, against main's 175 with 175 rows. The removed row is sceVideoOutSubmitChangeBufferAttribute, which is also the one placeholder dropped from InventoryExports.cpp and InventoryExports.hpp. Keeping of the original 182, 107 match unchanged is correct now that main phrases that clause against the original set.

The firmware line, re-read. libSceVideoOut.sprx, firmware 13.42, sha256 975b229324655a2d2a945c755fc0339e7feecb62863efc28ad3b2d91a95ad215, from disassembly only, not measured on a console:

112e0: test %rdx,%rdx ; 112e3: je 0x1133c   ; attribute == NULL
1133c: mov $0x80290002,%eax ; 11341: ret    ; INVALID_ADDRESS, before anything else
112f8: call 0x13f00                         ; port lookup from the handle
112fd: test %rax,%rax ; 11300: je 0x11342
11342: mov $0x8029000b,%eax                 ; INVALID_HANDLE
11305: mov 0x8(%rax),%rax ; 11309: mov 0xa8(%rax),%rax
11310: test %rax,%rax ; 11313: je 0x11349   ; -> 0x8029001b when the handler is absent
1131e: call *%rax                           ; the port's handler, which I did not read

So the entry's "returns INVALID_ADDRESS for a null attribute, then INVALID_HANDLE; every other check is in the port's handler, not read" is accurate, and the order you now implement is the console's. I followed 0x112e0 to the ret at 0x11341 and to the three error arms above; the handler behind call *%rax is unread here too.

The three new constants also exist in that module as immediates: 0x80290004 three times, 0x80290007 three times, 0x80290008 once. That is corroboration that they are real libSceVideoOut codes, not evidence of which check uses which, since the handler behind 0x1131e is not something I read.

The shadPS4 attribution matches the code it names, read at commit 2183807e592b36d4440384878165b43d930c1dee: video_out.cpp lines 452-460 return INVALID_HANDLE for a missing or closed port and delegate to ChangeBufferAttribute, and driver.cpp lines 198-234 check the index and occupancy, the reserved fields, aspect_ratio, width > pitch_in_pixel and the tiling range, in that order, before copying the attribute. Your relative order is the same for the four checks you share with it, and the entry is right that AnyPS5 diverges by throwing INVALID_INDEX and INVALID_OPTION where shadPS4 returns INVALID_VALUE. Checking setIndex < 0 as well is a genuine improvement on that source, which only rejects the upper bound.

What I ran. Windows 11 Pro, WinLibs GCC, Release, BUILD_TESTING=ON, head merged into current main:

  • ^video_out_flip: 28 of 28, nothing unbuilt and nothing skipped. video_out_flip_ps4 is the one that reaches the new export, and it passed. These skip without a display; I checked the log for skips and there were none.
  • Validation layer on for that run (ANYPS5_REQUIRE_VULKAN=1, ANYPS5_NO_SHADER_CACHE=1, VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation, ctest -V): no missing-layer line, no VUID- id and no SYNC-HAZARD id.
  • GPUs tested: AMD Radeon RX 9070 XT, driver 32.0.32015.12012, Windows 11 Pro.
  • A plain negative control is not available here: restoring main's Buffer.cpp leaves Flip.cpp calling an export main does not declare, so the link fails rather than a test failing. So I used a mutation probe instead, on the merged tree: moving the null-attribute check below the HasConfig handle check, which is exactly what the firmware above rules out. The swap is one block and the diff confirmed it landed only inside the new function; after rebuilding libs and video_out_flip_tests, video_out_flip_ps4 failed on its own assertion, the legacy change checked its handle before a null attribute. I restored the file afterwards and the tree came back clean.

Overlap, checked myself at this head with tools/pr_overlap.py --branch. One comparison reported a shared line, and it is with my own open #14954, on the docs/dev/debt/libSceVideoOut.md entry at L68 rather than on code; whichever of the two lands second will re-resolve that line. A search of open pull requests for the export name returns only this one; nothing else implements it. Nine comparisons could not be completed because those pull requests conflict with main, so I looked at the three that share Buffer.cpp through GitHub's own file lists instead of a local diff.

One thing to coordinate, not a change request

#9676 (MERGEABLE) and #6977 both rework this same area, and the interaction is worth settling before either lands rather than after:

  • They add a Ps4TiledPitch helper and a group.ps4Pitch field, and make DescribeVideoOutBuffer use AgcDriver::DisplayTilingGcn with that pitch when a PS4 group has tiling_mode == 0.
  • They change the registration path in Buffer.cpp from if (attribute->tiling_mode != 1) throw ... "PS4 tiled display buffers are not implemented" to if (attribute->tiling_mode > 1) throw ... INVALID_TILING_MODE, which makes PS4 tiled buffers supported there.
  • They recompute group.ps4Pitch inside sceVideoOutSubmitChangeBufferAttribute2 for exactly that reason.

Your new sceVideoOutSubmitChangeBufferAttribute keeps main's refusal of tiling_mode != 1, which is correct against main as it stands today (the same refusal is at Buffer.cpp:200 there), and it copies the group before editing, so ps4Pitch survives from registration unrecomputed. If #9676 lands first, a title that registers a tiled PS4 group and then changes its attributes would be refused by the new call, and a group whose width or pitch changes would keep a ps4Pitch computed for the old values. If this one lands first, #9676 needs the same one-line recompute added to the new function. Either way it is one line plus a decision about the tiling refusal, and a Depends on: between you and @ntamas94 would pin the order. I haven't built the two merged together, so I am describing their diffs, not a run.

Optional, non-blocking

Ps5PixelFormat hardcodes its caller's name: Buffer.cpp:84 formats "sceVideoOutRegisterBuffers: unsupported PS4 pixel format 0x%08x". The new export calls it at Buffer.cpp:251, so an unsupported format rejected through that path names the wrong entry point to whoever is debugging. The assertion at Flip.cpp:274 matches only the unsupported PS4 pixel format substring, so it does not notice the prefix. CommitBufferGroup(__func__, ...) in the same file already shows the pattern for threading the name through, if you want it.

Nothing above blocks the merge.

AI-assisted: yes (Claude Code)

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Nov 6, 2026
@AlexWhitehouse
AlexWhitehouse force-pushed the feat/libscevideoout-change-buffer-attribute branch from 723e10f to 08d1a11 Compare November 6, 2026 09:41
@AlexWhitehouse

Copy link
Copy Markdown
Contributor Author

Thanks for re-checking. On #9676: this PR keeps main's behaviour and leaves the shared helpers alone, so whichever of the two lands second adds the ps4Pitch recompute to the new function and lifts its tiling refusal, with the matching test row and debt entry; if #9676 merges first I'll do that here when I rebase. The Ps5PixelFormat caller-name fix touches the lines #9676 edits, so I'll leave it until #9676 has landed. Since your approval the branch is rebased onto the merged #14954 (08d1a11, inventory count 169 recounted from the stubs, no code change), and the VideoOut group still passes on lavapipe with validation and no VUID- lines.

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Nov 6, 2026
@TechLevelZero

Copy link
Copy Markdown
Contributor

✅ NVIDIA GPU test: passed

PR head 08d1a11 merged onto main 5299939 · tested 2026-11-06 12:09 UTC · 17 min

GPU NVIDIA RTX 2000 Ada Generation, driver 595.99.02, Vulkan 1.4.329
Host Ubuntu 26.04.1 LTS, kernel 7.0.0-38-generic, container ubuntu:24.04
Logs build and test logs for this run
Build ✅ passed (Release, -DANYPS5_ENABLE_SPIRV_TOOLS=ON)
Tests ✅ 1,346 passed · 4 failed (0 new, 3 also fail on main, 1 unstable on main) · 3 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,353 of 4,210 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 (643 tests run on the GPU)
Slow tests (>30 s, GPU tests timed with validation layers) ⚠️ agc_driver_pipeline_journal 120.1 s
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)
agc_driver_pipeline_journal: failed in 1 of the last 10 main runs
Failing on main too (3, not counted against this PR)
agc_driver_image_table
guest_socket_options_multicast
web_library_assets
Validation IDs already present on main (9 IDs in 549 tests)
VUID-VkDeviceCreateInfo-pNext-pNext (549 tests): agc_driver_address_draw_space_reuse, 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, and 541 more
VUID-VkShaderModuleCreateInfo-pCode-08737 (14 tests): agc_driver_dpp16_operands, agc_driver_dpp8_pk_fmac, agc_driver_dx10_clamp, agc_driver_f16_fma_denormal_modes, agc_driver_f16_fma_invalid_nan, agc_driver_float16_fma_rounding, agc_driver_float16_modifiers, agc_driver_float16_native, and 6 more
VUID-VkShaderModuleCreateInfo-pCode-08739 (14 tests): agc_driver_dpp16_operands, agc_driver_dpp8_pk_fmac, agc_driver_dx10_clamp, agc_driver_f16_fma_denormal_modes, agc_driver_f16_fma_invalid_nan, agc_driver_float16_fma_rounding, agc_driver_float16_modifiers, agc_driver_float16_native, and 6 more
VUID-VkShaderModuleCreateInfo-pCode-08741 (14 tests): agc_driver_dpp16_operands, agc_driver_dpp8_pk_fmac, agc_driver_dx10_clamp, agc_driver_f16_fma_denormal_modes, agc_driver_f16_fma_invalid_nan, agc_driver_float16_fma_rounding, agc_driver_float16_modifiers, agc_driver_float16_native, and 6 more
VUID-vkCmdCopyImage-srcImage-01548 (7 tests): agc_driver_depth_clear_hle, agc_driver_depth_plane_buffer_memory, agc_driver_depth_plane_partial_store, agc_driver_depth_plane_sampling, agc_driver_depth_storage_dispatch, agc_driver_depth_surface_from_storage, agc_driver_dispatch_move_depth
VUID-vkCmdCopyImage-srcImage-01551 (7 tests): agc_driver_depth_clear_hle, agc_driver_depth_plane_buffer_memory, agc_driver_depth_plane_partial_store, agc_driver_depth_plane_sampling, agc_driver_depth_storage_dispatch, agc_driver_depth_surface_from_storage, agc_driver_dispatch_move_depth
VUID-VkPipelineColorBlendStateCreateInfo-pAttachments-00605 (1 test): agc_driver_prepared_exports
VUID-VkPipelineShaderStageCreateInfo-module-parameter (1 test): agc_driver_float16_unused_nan_payloads
VUID-VkPipelineShaderStageCreateInfo-pSpecializationInfo-06849 (1 test): agc_driver_float16_unused_nan_payloads
Skipped (3)
agc_driver_gcn_display_present: skipped, no display: Vulkan support is either not configured in SDL or not available in current SDL video driver (offscreen) or platform
agc_driver_scattered_runs: skipped, host imports are unavailable: /dev/udmabuf is not accessible
agc_driver_vertex_elected_atomic_max: skipped, the device runs vertex programs in subgroups, so they keep their cross-lane reductions
Reproduce
git fetch origin 08d1a1157921dd6aec8b4a67e42337eacf7d34a4 5299939ecfa9e186e1eb457c6f4bd133bdb087c7
git checkout 5299939ecfa9e186e1eb457c6f4bd133bdb087c7 && git merge --no-ff 08d1a1157921dd6aec8b4a67e42337eacf7d34a4
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 32 --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 4 --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 NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs.

@BunnyFl BunnyFl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexWhitehouse re-checked at 08d1a11579. My approval was on 723e10f82b. PS4 tiled display buffer support is now on main (Buffer.cpp:225 on main), merged with #9432 as 85e291c12b; we had both expected it from #9676. That makes the follow-up you described for "if #9676 merges first" due here, and one test row has to go.

What main does now (at 1ff0fe98aa):

  • On main, PS4 registration refuses only tiling_mode > 1 (Buffer.cpp:215 on main).
  • On main, registration stores group.ps4Pitch from Ps4TiledPitch (Buffer.cpp:225 on main).
  • On main, the Attribute2 change recomputes ps4Pitch for a PS4 group (Buffer.cpp:258 on main).
  • On main, Flip.cpp:204-213 registers a tiled PS4 group and checks it is presented as DisplayTilingGcn padded to 128 pixels, and Flip.cpp:177-187 on main changes a PS4 group to tiled through ...Attribute2.
  • On main, the debt entry "PS4 tiled display buffers" (docs/dev/debt/libSceVideoOut.md:38 on main) records that support.

What this head adds against that. The new export still refuses tiling_mode != 1 with "PS4 tiled display buffers are not implemented" (Buffer.cpp:246-247 at this head), and the test pins it:

// Flip.cpp:270-271 at 08d1a11579
invalid.tiling_mode = 0;
rejected([&] { sceVideoOutSubmitChangeBufferAttribute(handle, 0, &invalid); }, "tiled display buffers are not implemented");
  • That row asserts that an unimplemented path throws, which pins the gap as if it were the contract (the point raised on #14950). It has to be removed either way.
  • The message is no longer true on main: tiled PS4 buffers are implemented there, just not through this call.

Requested change, one of:

  • implement the tiled case as you planned: accept tiling_mode == 0, store the pitch as registration does (Buffer.cpp:224 on main), recompute group.ps4Pitch, and replace the pinned row with one that changes a group to tiled and checks the presented tiling and padded pitch, as main's Flip.cpp:181-186 does for ...Attribute2;
  • or keep the refusal, drop the pinned row, and reword the throw and the debt entry so they say this call doesn't change a group to tiled yet.

One thing to settle if you implement it: main's own tiled registration passes pitch_in_pixel = 0 (Flip.cpp:206 on main). At this head the width > pitch_in_pixel check returns INVALID_PITCH (Buffer.cpp:241) before the tiling check is reached, so a tiled change written the same way would come back INVALID_PITCH. I haven't checked what the console expects in pitch_in_pixel for a tiled PS4 attribute.

A correction to my previous review: I said a group whose width or pitch changes through this call "would keep a ps4Pitch computed for the old values". With only linear changes accepted, that value is never read: DescribeVideoOutBuffer uses ps4Pitch only when tiling_mode == 0 (Buffer.cpp:30 on main). It matters only once the tiled case is accepted.

What I ran. This head merged onto main 1ff0fe98aa (merge 1fe7820f0b, clean), Windows 11 Pro, WinLibs GCC, Release, AMD Radeon RX 9070 XT, with ANYPS5_REQUIRE_VULKAN=1, ANYPS5_NO_SHADER_CACHE=1, VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation and ctest -V:

  • ^video_out_flip: 28 of 28 pass, none unbuilt; no missing-layer line, no VUID- and no SYNC-HAZARD ids. The pinned row is among the passing checks: it expects the throw (Flip.cpp:271).
  • tools/pr_overlap.py --branch at this head: no line conflicts. #13486 is listed for the same export; GitHub's file list for its head 0bde10d243 is three relinker files, none in libSceVideoOut. Six pull requests conflict with main and could not be compared (#4040, #5021, #12863, #13486, #13994, #14212).

The rest of what I approved at 723e10f82b still holds: the null-attribute and handle order from the 13.42 module, the returned codes and the converted fields.

AI-assisted: yes (Claude Code)

AlexWhitehouse and others added 3 commits November 6, 2026 13:56
Replace a legacy PS4 buffer group's attribute through the PS4 call,
following shadPS4 commit 2183807e592b36d4440384878165b43d930c1dee
(video_out.cpp:452-460, driver.cpp:198-240): an aspect ratio other than
0, a width above the pitch and a tiling mode above linear return their
VideoOut errors. The conversion matches legacy registration and the
update matches sceVideoOutSubmitChangeBufferAttribute2, keeping buffer
addresses and pending flips.

Missing groups, reserved fields, null attributes, out-of-range indices
and PS5 groups throw, as the registration and Attribute2 paths do for
the same inputs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ute first

The 13.42 module's sceVideoOutSubmitChangeBufferAttribute (0x112e0)
returns 0x80290002 for a null attribute before it looks up the handle,
then 0x8029000b for an unknown handle, and leaves every other check to
the port's handler. Check the attribute first and return the code
instead of throwing, then the handle.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
main now registers tiled PS4 display buffers, so the PS4 change call
accepts tiling mode 0 as well: it stores the attribute without a pitch
and recomputes the group's PS4 pitch as registration does. The width
against pitch check still applies to tiled attributes, as in shadPS4.
The test changes a group to tiled at two pitches and back to linear
instead of expecting the old refusal.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@AlexWhitehouse
AlexWhitehouse force-pushed the feat/libscevideoout-change-buffer-attribute branch from 08d1a11 to 8ea336b Compare November 6, 2026 14:09
@AlexWhitehouse

Copy link
Copy Markdown
Contributor Author

Done in 8ea336b, rebased onto current main: the call now accepts a tiled attribute, stores it without a pitch and recomputes ps4Pitch as registration does, and the pinned row is replaced by changes to tiled at pitches 128 and 256 that check the presented GCN tiling and pitch, then a change back to linear. On the pitch question I kept shadPS4's rule, which checks width > pitch_in_pixel in both its register and change paths whatever the tiling, so a tiled change with pitch 0 returns INVALID_PITCH; that is tested and recorded in the debt entry next to registration's padding to 128. The VideoOut group passes on lavapipe and an RTX 3060 with validation and no VUID- lines, and dropping the recompute, keeping the tiled pitch, refusing tiled changes or skipping the tiled pitch check each fails video_out_flip_ps4.

@BunnyFl BunnyFl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexWhitehouse re-checked at 8ea336be64, thanks for taking the tiled case. My change request was on 08d1a11579; both points are addressed.

  • Tiled changes: the new export stores a tiled attribute without a pitch and recomputes ps4Pitch (Buffer.cpp:270-271 at this head), the same two lines registration uses (Buffer.cpp:224-225). The pinned "not implemented" row is gone. Flip.cpp:293-303 now changes group 0 to tiled at pitches 128 and 256, checks that it is presented as DisplayTilingGcn with that pitch, and changes it back to linear.
  • Pitch 0: I read shadPS4 at 2183807e592b36d4440384878165b43d930c1dee. Its ChangeBufferAttribute returns INVALID_PITCH for width > pitch_in_pixel before the tiling check, whatever the tiling (driver.cpp#L213-L222), and its registration does the same (driver.cpp#L138-L147). So Buffer.cpp:258 and the test at Flip.cpp:291 match that source. The console's handler is still unread, and the debt entry (docs/dev/debt/libSceVideoOut.md:47 at this head) says so and records the difference from registration's padding.
  • Count: in the tree merged with main 7383be1a19, InventoryExports.cpp has 190 throwing stubs (191 on main) and the debt table has 169 rows, matching the entry's 169.

What I ran. This head merged onto main 7383be1a19 (merge 70c6063b4c, clean), Windows 11 Pro, WinLibs GCC, Release, AMD Radeon RX 9070 XT (driverInfo 26.10.2 (LLPC), driverVersion 2.0.416, Vulkan 1.4.359), with ANYPS5_REQUIRE_VULKAN=1, ANYPS5_NO_SHADER_CACHE=1, VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation and ctest -V:

  • ^video_out_flip: 28 of 28 pass, none unbuilt; no missing-layer line, no VUID- and no SYNC-HAZARD ids.
  • Restoring main's Buffer.cpp doesn't link (the test calls the new export), so the controls are four mutations of the new function, each rebuilt and run on the merge. Each one fails video_out_flip_ps4:
    • ps4Pitch set to 0 instead of recomputed;
    • the tiled pitch kept in the stored attribute;
    • tiling mode 0 refused (!= 1);
    • the pitch check skipped for a tiled attribute.
  • tools/pr_overlap.py --branch at this head: no line conflicts. #13486 is listed for the same export, but GitHub's file list for its head 0bde10d243 is three files and none of their patch lines names this export. #4040, #5021, #12863, #13486 and #14212 conflict with main and couldn't be compared; their GitHub patches have no added or removed line naming this export or its NID IOdgHlCGU-k.

AI-assisted: yes (Claude Code)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants