Repository navigation
fix(ngs2): correct UserFx ids and preserve Delay compatibility - #9337
Conversation
|
Thanks for picking this up so quickly, it's good to see Two things worth a look before it goes in:
For what it's worth, the merge of #8609 in your branch is unchanged, so there's no conflict with mine, and the conflicts with #9233 are only textual, as your merge-order note already says. |
|
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.
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.
|
|
Title data point: BLUE REFLECTION Quartet (PPSA33655). Its racks are listed in #8851:
The stop moves forward with each PR. Same scripted runs, natively on Linux 7.2.9, Radeon RX 6650 XT, RADV Mesa 26.2.4, GCC Release:
So:
No audio was produced or checked; this only shows rack creation getting further. Combining the two PRs conflicted in |
|
@Th3EvilGod thanks, the firmware error codes for the rack checks look good. One change needs more before we can take it: UserFx 0x1d → 0x19. Could you:
The branch also conflicts with |
|
@oneandonlydean thanks for the review. The source is fma-rc's reading of the PS5 12.70 module, not a disassembly or console measurement I performed. It identifies UserFx as Agreed that the current PR reintroduces the #5055 startup stop; documenting it does not preserve the working title. The remaining gap is Delay's default processing before any parameter is supplied. That is explicitly asked in the follow-up firmware questions, and I have not found an answer establishing that pass-through is the console behavior. Would you accept retaining main's existing If retaining that behavior conflicts with the project's requirement to throw on unknown semantics, I will hold the mapping change until Delay's defaults are sourced rather than inventing a successful implementation. I am also inspecting the current-main conflicts so they can be resolved without dropping upstream changes. |
|
@Th3EvilGod thanks, that answers where On
One more thing: #9233 carries the same rack-creation checks and is ahead of this in the merge queue. Please rebase onto #9233's branch (or onto |
7903c6d to
b553950
Compare
What
Separate UserFx (
0x19,0x40001900 | index) from Delay (0x1d,0x40001dxx). Reuse the existing 40-byte UserFx callback parameter under the corrected IDs, reject non-zero UserFx module state withINVALID_STATE_SIZE, and reject non-null UserFx options and custom-sampler UserFx until their unsupported contracts are established.Preserve the existing Delay audio pass-through as requested by the maintainer. A one-buffer custom submixer with eight ports, no module state and option
20, 0, 4, 1000.0f, 0now passes creation, preserving the rack shape reported for Void Terrarium (PPSA03061) in #5055. The path handles the general supported option family, without a title ID or those specific values in production code: type 0/1, 1..8 taps, finite length 1..20000 and reserved 0. Unknown options, module state and custom-sampler Delay throw. Every0x40001dxxparameter throws an explicit Delay-not-implemented error before any callback or parameter state is changed.Evidence: fma-rc's PS5 firmware 12.70 module reading, parameter dispatch
0x25ec4, rack creation0x13b68, identifies UserFx and Delay, the 40-byte UserFx parameter (8-byte header, handler, three user-data words), and the Delay option layout/ranges. The earlier reading identifies UserFx as having no option or module state. These are attributed findings, not disassembly or hardware measurements I performed.Delay processing/defaults remain unverified; this deliberately preserves existing host behavior and does not claim a working delay effect. The existing UserFx callback context, flags, null-handler behavior and callback-error model also remain unverified on console. Both limits are recorded in separate debt entries. Multi-buffer racks and other custom DSP modules remain outside this change; no title is claimed unblocked.
Rebased on #9233 at
3e00103497(including its #8609 prerequisite), with main1a745ae38emerged. This PR's own change is the single commitb553950ef, affecting four files relative to the combined prerequisite base154ca3754e; Gain code, its ramp tests and its debt entry are retained. The main-targeting diff includes the prerequisite until it lands.Tested
Windows 11 Pro, supported WinLibs MinGW-w64 GCC 15.2.0 r7, Release, full-runtime configuration with
BUILD_TESTING=ON.068856ef5),ngs2_customaborts at rack creation withcustom module id 0x1d is not implemented.libsbuild and both NID-patched NGS2 libraries: pass.ctest --test-dir build/windows-cnd -R '^ngs2_' -V: 13/13 pass, 3.63 seconds.0x1dand the 20-byte option. It compares every stereo PCM sample over consecutive grains, silence before input and after stop, and unchanged output after rejected parameters. It covers 1, 2 and 24 Delay modules, both option types and range endpoints, all 2560x40001dxxIDs, and confirms no legacy UserFx callback runs. Null/wrong options, invalid option fields, NaN/infinity, module state/offset and custom-sampler Delay are rejected.0x19/0x40001900 | index, verify PCM and callback state at positions 0, 1 and 23, and retain invalid-state, parameter-size/index and custom-sampler rejection checks. Gain regressions remain enabled.tools/check_conventions.py --base origin/main: passes with no findings; whitespace check passes.No actual title, console-hardware or Linux run, no full repository test-suite run, and no performance claim. The synthetic Delay regression establishes preservation of existing PCM behavior, not console equivalence or game compatibility.
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)