Repository navigation
feat(libSceNgs2): implement sceNgs2VoiceGetMatrixInfo and sceNgs2CustomRackGetModuleInfo - #10024
Conversation
…omRackGetModuleInfo
Progress report (9d6adeb → 1f1ddc9)📈 System libraries: 45.18% (+0.01%, +2 implemented, -2 removed functions) ✅ 2 implemented
🗑️ 2 removed
|
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed head 1f1ddc9. The two exported getters match the new ABI layouts (260-byte matrix info, 32-byte custom module info), use the existing lock/handle lookup patterns, validate output sizes and bounds, and only write output on success. The added tests cover unset and populated matrices (including the 64-level cap/clamping), invalid matrix/voice/rack/module indices, destroyed handles, and metadata for UserFx2 and filter modules. I also ran and All conventions checks passed.; both passed. I attempted CMake configuration with BUILD_TESTING=ON, but generation stopped because this checkout is missing ; therefore I could not execute the new native tests locally. Hosted product jobs are skipped, so runtime behavior remains supported by the author's reported 13/13 QEMU tests, not by a test run in this review environment. The ABI/layout claims are based on the cited compatibility references and are documented as unverified against PS5 hardware.
MarcZX10
left a comment
There was a problem hiding this comment.
Follow-up to clarify the review evidence for head 1f1ddc9: the exported getters match the new ABI layouts (260-byte matrix info, 32-byte custom module info), use existing lock and handle lookup patterns, validate output sizes and bounds, and only write output on success. The added tests cover unset and populated matrices (including the 64-level cap and clamping), invalid matrix/voice/rack/module indices, destroyed handles, and metadata for UserFx2 and filter modules.
I ran git diff --check upstream/main...upstream/pr-10024 and python3 tools/check_conventions.py --base upstream/main --head upstream/pr-10024; both passed. I attempted CMake configuration with BUILD_TESTING=ON, but generation stopped because this checkout is missing 3rdparty/vgmstream/src/coding/libs/atrac9_dec.c. I could not execute the new native tests locally. Hosted product jobs are skipped, so runtime behavior remains supported by the author's reported 13/13 QEMU tests, not by a test run in this review environment. The ABI/layout claims are based on the cited compatibility references and are documented as unverified against PS5 hardware.
|
@MarcZX10 thanks for the review. The missing file is from the vgmstream submodule, it was added to main last week for the AJM ATRAC9 decoder, so running git submodule update --init --recursive like in BUILD.md should get the CMake step through. |
|
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.
|
What
sceNgs2VoiceGetMatrixInfoandsceNgs2CustomRackGetModuleInfowere throwing stubs inlibSceNgs2/PendingExports.cpp. They now read back state the library already keeps, and live in the sharedlibSceNgs2.nativesources, assceNgs2VoiceGetOwnerdoes; the PS5 3.20 export inventory lists both inlibSceNgs2andlibSceNgs2.native.sceNgs2VoiceGetMatrixInfo(voice, matrixId, info, infoSize)reports the levels stored for one of the voice's matrices: a count and up to 64 levels, the 260-byte layout of shadPS4'sOrbisNgs2VoiceMatrixInfo. A matrix whose levels were never set reports 0 levels.sceNgs2CustomRackGetModuleInfo(rack, moduleIndex, info, infoSize)reports one module of a custom rack as it was given at creation: module id, the three buffer ids, state offset and state size, with both reserved words 0. The 32-byte layout is shadPS4'sOrbisNgs2CustomModuleInfoand SharpProspero'sSceNgs2CustomModuleInfo. The rack now keeps that info next to the module ids it already kept.Errors follow the neighbouring getters: a null info gives
INVALID_OUT_ADDRESS, an unknown handleINVALID_VOICE_HANDLEorINVALID_RACK_HANDLE, and a matrix info size other than 260 bytesINVALID_OUT_SIZE, as insceNgs2VoiceGetPortInfo. Cases whose PS5 error code is unknown throw: a matrix id past the rack's matrices, a module index past the rack's modules (so any index on a rack that is not custom), and a module info size under 32 bytes. shadPS4 gives these separate PS4 codes (INVALID_MATRIX_INDEX,INVALID_MODULE_INDEX,INVALID_MODULE_INFO_SIZE), and AnyPS5's PS5 codes do not map onto its PS4 ones. A module info size over 32 bytes throws too, because shadPS4 has longer per-module info (reverb, envelope) whose extra fields are not kept. Both are recorded indocs/dev/debt/libSceNgs2.native.md, and the two names leave the pending list indocs/dev/debt/libSceNgs2.md.shadPS4 and KytyPlus only log these calls, so no reference gives their behaviour beyond the layouts; none of this was measured on a console.
tools/progress.py --compare: +2 implemented inlibSceNgs2.native, the same 2 removed from thelibSceNgs2pending list.tools/pr_overlap.py --branchreports no overlap. #1384, #5548 and #5797 could not be compared because they conflict withmain; they share files but none touches these functions,customModulesor the new structures.Tested
main9d6adeb: all 13 NGS2 tests pass (ngs2_core_renderand the 12ngs2_*tests). The host is Apple silicon and Rosetta has no AVX2, so they ran underqemu-x86_64 -cpu max(QEMU 10.2.1).ngs2_voicehandlescovers unset, partial, clamped and full 64-level matrices, every error above, a rack without matrices and a destroyed rack;ngs2_customcovers two modules of different kinds, reserved words, every error above, a non-custom rack and a destroyed rack. Five mutations (no level copy, reserved words copied, always module 0, larger module info accepted, matrix info size ignored) each make a test fail.-fsyntax-only -Wall -Wextrawith no new warnings againstmain.python3 tools/check_conventions.py --base origin/mainpasses.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)