Skip to content

fix(libSceNgs2.native): return the module's buffer and allocator errors from sceNgs2SystemCreate and sceNgs2SystemCreateWithAllocator - #9316

Merged
oneandonlydean merged 7 commits into
boykopovar:mainfrom
MarrasFederico:feat/ngs2-system-create-arguments
Oct 23, 2026
Merged

oneandonlydean merged 7 commits into
boykopovar:mainfrom
MarrasFederico:feat/ngs2-system-create-arguments

Conversation

@MarrasFederico

Copy link
Copy Markdown
Contributor

What

sceNgs2SystemCreate and sceNgs2SystemCreateWithAllocator checked the output handle first and threw for a missing buffer or allocator. They now follow prosper's reading of the two exports in libSceNgs2.native.sprx (0x1d1e0 and 0x1d2f0):

  • sceNgs2SystemCreate:
    1. a NULL buffer info, or a NULL host_buffer in it, returns 0x804A8020;
    2. then a NULL handle returns 0x804A8010;
    3. then the option checks run.
  • sceNgs2SystemCreateWithAllocator:
    1. a NULL allocator, or a NULL alloc_handler, returns 0x804A8023;
    2. then the NULL handle;
    3. then the option checks.
  • No free_handler: the module checks only alloc_handler, so an allocator with no free_handler keeps throwing, after the option checks as before. The new debt entry records it.

Rack creation is left as it is. #8609's tests rely on its current order, and it will follow separately.

This builds on #9312, which makes the option checks return codes.

Tested

Ubuntu 26.04.1, kernel 7.0.0-38, GCC 15.2.0, Release, full build with BUILD_TESTING=ON. Windows was not compiled. No title was run.

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: fix(libSceNgs2.native): validate the system option with the module's error codes #9312
  • Fixes: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

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 9926 core/libs/prx/libSceNgs2.native/include/Ngs2Types.hpp L11
PR 10037 docs/dev/debt/libSceNgs2.native.md L33-34

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 1384 core/libs/prx/libSceNgs2.native/include/Ngs2Types.hpp
core/libs/prx/libSceNgs2.native/src/System.cpp
PR 5797 core/libs/prx/libSceNgs2.native/include/Ngs2Types.hpp
docs/dev/debt/libSceNgs2.native.md

@ycbuzi

ycbuzi commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

Windows run of 6279944 merged onto main 0c6a5bd (dcc541cae9, clean), the platform the description lists as not tested. Windows 11 Home 10.0.26200, MinGW-w64 GCC 15.2.0 (ucrt-posix-seh r7), Release, BUILD_TESTING=ON.

Built libSceNgs2.native and ngs2_handleerrors_tests with no errors or warnings. ctest --repeat until-fail:2 on ngs2_handleerrors: passes on both repeats. The head carries the PRs below it in its stack (#9312, #9299) too. Not run: a title.

@oneandonlydean oneandonlydean self-assigned this Oct 22, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

@MarrasFederico I read the stack: #9316 contains #9312, so I will review and land #9316 and treat #9312 as included. The module error codes and the new tests look right to me.

Before it can merge, Ngs2Types.hpp needs one fix. Since #9233 was merged, main already defines SCE_NGS2_ERROR_INVALID_OPTION_SIZE (0x804A8013) and SCE_NGS2_ERROR_INVALID_NUM_CHANNELS (0x804A8052), with the same values you use. This branch adds both again. Git merges that without a conflict, but the header then defines each name twice and the library does not compile. The CI runs on this branch started before #9233 landed, so they do not show it.

Could you rebase onto current main and:

  • delete those two lines from core/libs/prx/libSceNgs2.native/include/Ngs2Types.hpp, and keep the others you add (INVALID_SAMPLE_RATE, INVALID_NUM_GRAIN_SAMPLES, INVALID_MAX_GRAIN_SAMPLES, INVALID_BUFFER_INFO, INVALID_BUFFER_ALLOCATOR)?
  • fix(libSceNgs2): accept system option flag bit 1 #8341 is now in main (01f99305be), so please keep its rule for the system option flags inside your CheckSystemOption: it accepts bit 1 and throws for any other bit. In System.cpp replace the line if (option->flags != 0) throw std::runtime_error("NGS2: system option flags are not implemented"); with
    constexpr std::uint32_t knownFlags = 0x2;
    if ((option->flags & ~knownFlags) != 0) throw std::runtime_error("NGS2: system option flags " + Ngs2Hex(option->flags) + " are not implemented");

Ngs2Hex is already defined in that file. Without this, the new test that #8341 adds for the flags fails once both are in.

After the push CI runs again on the new head, and I will do a short delta review.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 23, 2026
…eate-arguments

# Conflicts:
#	core/libs/prx/libSceNgs2.native/include/Ngs2Types.hpp
@MarrasFederico

Copy link
Copy Markdown
Contributor Author

@oneandonlydean thanks, both are in the current head (ad5df50d6d). I merged rather than rebased so I wouldn't have to force-push:

On that tree the library builds and all 13 NGS2 test targets pass after a full rebuild, including #8341's flags test in Ngs2Render.cpp. tools/check_conventions.py --base upstream/main passes. I gave #9312 and #9315 the same merge, so all three are mergeable again.

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 23, 2026
@oneandonlydean
oneandonlydean merged commit 5255264 into boykopovar:main Oct 23, 2026
10 checks passed
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