Skip to content

fix(libSceVideoOut): propagate shutdown driver error and register flip tests - #1575

Closed
neuralsin wants to merge 1 commit into
boykopovar:mainfrom
neuralsin:fix/video-out-flip-tests
Closed

neuralsin wants to merge 1 commit into
boykopovar:mainfrom
neuralsin:fix/video-out-flip-tests

Conversation

@neuralsin

Copy link
Copy Markdown
Contributor

What

Resolves #1040:

  1. In VideoOutDriver::presentLoop, wraps AgcDriverShutdown_nid_postfix() in a try-catch block and records any shutdown exception into flipQueue->failure. This prevents Driver::Shutdown()'s stored worker failure from throwing unhandled on presentThread and invoking std::terminate(), allowing presentThread.join() to finish cleanly so VideoOutDriver::Shutdown() can propagate the error to the caller thread as expected by tests.
  2. In core/libs/prx/libSceVideoOut/CMakeLists.txt, builds video_out_flip_tests under if(BUILD_TESTING) and registers all three headless test cases (video_out_flip, video_out_flip_decode, video_out_flip_controls) in CTest with SDL_VIDEODRIVER=offscreen and Windows libc path prepend.
  3. In tools/check_test_registration.py, removes video_out_flip_tests from KNOWN_UNRUN.

Tested

  • Linux / WSL 2 Ubuntu 24.04 (x86_64), GCC 15.2.0, Ninja 1.13.2
  • ctest -R video_out_flip: 3/3 passed (100% tests passed: video_out_flip, video_out_flip_decode, video_out_flip_controls)
  • python tools/check_test_registration.py build_wsl: 100% passed (401 test executables, 3 built and not run as recorded, none new)
  • python tools/check_conventions.py --base origin/main: 100% passed

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 in TechnicalDebt
  • No notes, investigation .md files or images added to the repository
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • AI-assisted: yes

@github-actions

github-actions Bot commented Oct 8, 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
#1648 core/libs/prx/libSceVideoOut/CMakeLists.txt L29-33
core/libs/prx/libSceVideoOut/src/VideoOutDriver.cpp L555
#1801 core/libs/prx/libSceVideoOut/src/VideoOutDriver.cpp L554-555

@MarcZX10

MarcZX10 commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

One more overlap beyond the bot's #1648/#1801 list: open PR #1048 also catches AgcDriverShutdown_nid_postfix() in presentLoop() and registers the same three CTest cases; open PR #1225 does the same plus the Windows libc PATH setup. All three address the #1040 shutdown/test-registration path. #1225's latest Linux and Windows runs currently fail before compilation because it does not remove video_out_flip_tests from KNOWN_UNRUN; #1575 already has that removal.

Please coordinate with those authors/maintainers on the canonical change and list any actual dependency in the PR body before rebasing or resolving the shared VideoOutDriver.cpp / CMake hunks. That should prevent reviewers from merging conflicting versions of the same fix.

@MarcZX10

MarcZX10 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

I reviewed the full diff and compared it with the live PR list. #1225 is still open and already contains the same presentLoop shutdown-error propagation and registration of the same three headless modes, so this PR’s checklist claim that no other open PR implements the same functions is currently stale. Please coordinate with #1225 and keep one PR for this change. Also, these new CTest entries do not set a timeout; CONTRIBUTING requires tests to complete in under 30 seconds, so please set an explicit 30-second timeout. I’m holding review until those are resolved.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 9, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

Closing: #1225 makes the same presentLoop change and ctest registration for #1040 and was opened first. If something here is missing from #1225, a review comment there is the best place for it.

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

Labels

build needs rebase Conflicts with main, rebase needed prx

Projects

None yet

Development

Successfully merging this pull request may close these issues.

video_out_flip_tests is not in ctest and its lifetime and reopen modes terminate on main

3 participants