Skip to content

feat(ngs2): add experimental I3DL2 reverb - #1384

Draft
TibixDev wants to merge 1 commit into
boykopovar:mainfrom
TibixDev:pr/ngs2-reverb
Draft

TibixDev wants to merge 1 commit into
boykopovar:mainfrom
TibixDev:pr/ngs2-reverb

Conversation

@TibixDev

@TibixDev TibixDev commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

What

Draft with overlapping upstream DSP work. Current main (d146ec1b6) already implements this rack and its I3DL2 controls. In particular, e15f1644e384ab055f721347a4d31cdff02e9a47 replaced the earlier approximation with the implementation checked against PS4 firmware 11.00 output; 473ca04b2 rejects unsupported rack sizes and sample rates. Main retains the PS5-comparison limitation in TechnicalDebt.

This branch contains an unverified eight-line feedback delay network and channel conversion that current main still rejects. A fresh Skyrim boot on main d146ec1 reaches Vulkan setup and then throws NGS2: reverb channel conversion is not implemented. The draft therefore retains a distinct compatibility experiment, although its DSP should not replace the newer reference-based implementation. A mergeable version must preserve that implementation and establish the missing channel-conversion semantics. The experimental revision may be used as an explicitly identified, identical support dependency on both sides of title benchmarks; this does not establish console equivalence.

Tested

This follow-up is a source/history review, not a new console or game run. The earlier Fedora 44 software-model tests only established this branch's internal behavior, not console equivalence, and do not validate replacement of the current implementation.

Checklist

  • Current main without duplicate behavior: reconcile the DSP with main and validate the remaining channel-conversion path.
  • One topic.
  • No new code comments in this follow-up.
  • Console-equivalent behavior or a throw: this branch's DSP remains unverified.
  • No notes, logs, or images in the repository.
  • No new third-party code.
  • General behavior.
  • Depends on: none.
  • AI-assisted: yes.

Integration follow-up

A separate main-based test library combining current panning/ATRAC9 with this branch's experimental reverb now gets PPSA03747 into recurring draws and dispatches. This is an integration baseline, not vanilla-main or console-validation evidence. The draft still needs the reference-preserving DSP/channel-conversion rework described above.

Title artifact correction

A binary-hash audit found that the earlier title runtime copies all contained the same patched main AGC library. Those runs establish the main + experimental NGS baseline only. They do not validate #1312/#1313 in-game. The separately built CTests and CPU benchmarks used the new unpatched libraries/source and remain valid. The earlier PR-specific title claim is withdrawn; corrected results follow.

Verified runtime rerun

Corrected runtime result: the NID-patched #1312 library has SHA-256 a15e22c1b1305f79ab39089c7640ab9b6dd0fb9e6bffc0910d053cddfc7d51eb, differs from baseline, and contains the fingerprint verification code. /proc confirms the game loaded that file and the shared experimental NGS library. A fresh 120-second run with APS5_VERIFY_REGISTER_KEYS=1 reported at least 1,706 register-key lookups, no fingerprint mismatch, and presented frames. It stopped at the imposed timeout (124). No performance comparison is claimed with this verification enabled.

Extract the approximate reverb rack and DSP from the combined fork, including parameter diagnostics and rounded-boundary handling. Test the same model through core and native libraries. Console output equivalence remains unverified and is documented in TechnicalDebt. AI-assisted: yes.
@TibixDev

TibixDev commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

Rebased on bc1fbe81. Kept PCM streaming coverage. This remains a draft: the console-reference blocker is unchanged.

Fedora 44 Release: ngs2_core_render_tests, ngs2_core_reverb_tests, ngs2_render_tests, ngs2_atrac9_tests, ngs2_filter_tests, ngs2_custom_tests, ngs2_parse_tests, ngs2_stream_tests, ngs2_reverb_tests build; the targeted tests and contribution checks pass. Windows CI is pending.

@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Oct 8, 2026
@TibixDev

TibixDev commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor Author

Reviewed against main d146ec1b6. e15f1644e replaced the prior approximation with DSP checked against PS4 firmware 11.00 output; unsupported-size/rate rejection followed in 473ca04b2. The PS5 comparison limitation is already documented on main.

The overlap is substantial, but this draft also accepts channel conversion that main still rejects: a fresh Skyrim boot with rebuilt main libraries reaches Vulkan setup, then throws NGS2: reverb channel conversion is not implemented. That compatibility experiment remains distinct. A mergeable rework must preserve main's reference-based DSP and settle channel-conversion semantics, rather than substitute this approximate eight-line FDN.

The description retains the unchecked reference requirement. This draft may serve as an explicitly named, identical support dependency on both sides of local title benchmarks, which is separate from validating it for merging. No console equivalence is claimed. AI-assisted review.

@TibixDev

TibixDev commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor Author

Further integration check: the unmerged implementation does provide a distinct channel-conversion path. Loading its entire old NGS library first got past main's reverb conversion error but dropped main's newer panning implementation (sceNgs2PanInit). I then adapted just its experimental reverb state/processing and channel routing into a separate main-based test library, keeping current panning, ATRAC9 and other NGS code.

Skyrim PPSA03747 now reaches recurring draws/dispatches with that common test dependency; a 180-second run completed to the imposed time limit without the original audio exception. This is useful as a shared integration baseline for graphics experiments. It is not vanilla main, a console reference check, or a claim of correct/complete game rendering. The source diff and binary hashes are recorded locally.

This PR remains a draft. Main's reference-checked equal-channel DSP should be preserved in any mergeable rework, and missing channel conversion needs its own reference validation. The approximate old DSP should not replace the newer implementation just to obtain a boot. AI-assisted integration investigation.

Artifact correction: the title runtime copies were subsequently found to contain the same patched main AGC library. The saved PRESS ANY BUTTON frame therefore validates the main + experimental NGS integration, not #1312 or #1313. The CTests/CPU benchmarks used the new unpatched builds and remain valid. The earlier PR-specific title claim is withdrawn; corrected results follow.

Corrected runtime result: the NID-patched #1312 library has SHA-256 a15e22c1b1305f79ab39089c7640ab9b6dd0fb9e6bffc0910d053cddfc7d51eb, differs from baseline, and contains the fingerprint verification code. /proc confirms the game loaded that file and the shared experimental NGS library. A fresh 120-second run with APS5_VERIFY_REGISTER_KEYS=1 reported at least 1,706 register-key lookups, no fingerprint mismatch, and presented frames. It stopped at the imposed timeout (124). No performance comparison is claimed with this verification enabled.

@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

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 907 docs/dev/TechnicalDebt.md
PR 1601 docs/dev/TechnicalDebt.md
PR 1835 docs/dev/TechnicalDebt.md
PR 3986 docs/dev/TechnicalDebt.md
PR 5021 docs/dev/TechnicalDebt.md
PR 5548 core/libs/prx/libSceNgs2.native/CMakeLists.txt
core/libs/prx/libSceNgs2.native/src/Ngs2Internal.hpp
core/libs/prx/libSceNgs2.native/src/Render.cpp
core/libs/prx/libSceNgs2.native/src/Voice.cpp
core/libs/prx/libSceNgs2/CMakeLists.txt
PR 5797 core/libs/prx/libSceNgs2.native/CMakeLists.txt
core/libs/prx/libSceNgs2.native/Ngs2Library.cmake
core/libs/prx/libSceNgs2.native/include/Ngs2Types.hpp
core/libs/prx/libSceNgs2.native/src/Ngs2Internal.hpp
core/libs/prx/libSceNgs2.native/src/Render.cpp
core/libs/prx/libSceNgs2.native/src/Voice.cpp
PR 6039 docs/dev/TechnicalDebt.md
PR 6044 core/libs/prx/libSceNgs2.native/CMakeLists.txt
core/libs/prx/libSceNgs2/CMakeLists.txt
PR 8238 core/libs/prx/libSceNgs2.native/src/Ngs2Internal.hpp
core/libs/prx/libSceNgs2.native/src/Rack.cpp
core/libs/prx/libSceNgs2.native/src/Render.cpp
core/libs/prx/libSceNgs2.native/src/System.cpp
core/libs/prx/libSceNgs2.native/src/Voice.cpp
PR 8241 docs/dev/TechnicalDebt.md
PR 8322 core/libs/prx/libSceNgs2.native/src/Ngs2Internal.hpp
core/libs/prx/libSceNgs2.native/src/Rack.cpp
PR 8327 core/libs/prx/libSceNgs2.native/src/Voice.cpp
PR 8341 core/libs/prx/libSceNgs2.native/src/System.cpp
PR 8513 core/libs/prx/libSceNgs2.native/src/Ngs2Internal.hpp
core/libs/prx/libSceNgs2.native/src/Rack.cpp
core/libs/prx/libSceNgs2.native/src/Render.cpp
core/libs/prx/libSceNgs2.native/src/System.cpp
core/libs/prx/libSceNgs2.native/src/Voice.cpp
PR 8609 core/libs/prx/libSceNgs2.native/include/Ngs2Types.hpp
core/libs/prx/libSceNgs2.native/src/Ngs2Internal.hpp
core/libs/prx/libSceNgs2.native/src/Rack.cpp
PR 8692 core/libs/prx/libSceNgs2.native/include/Ngs2Types.hpp

This was referenced Oct 15, 2026

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

Labels

build docs needs rebase Conflicts with main, rebase needed prx

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant