Skip to content

fix(videoout): wait for flips with the high-resolution condition on Windows - #1655

Merged
adriadam10 merged 2 commits into
boykopovar:mainfrom
mohdfaour03:fix/videoout-present-precise-wait
Oct 11, 2026
Merged

adriadam10 merged 2 commits into
boykopovar:mainfrom
mohdfaour03:fix/videoout-present-precise-wait

Conversation

@mohdfaour03

Copy link
Copy Markdown
Contributor

What

One of the host waits #1471 lists as still affected: VideoOutDriver::presentLoop waits on flipQueue->changed (a std::condition_variable) with a 10 ms timeout while no flip is queued, and pumps SDL events and the pad after each wake. On Windows that wait goes through winpthreads, which ends timed waits on GetTickCount64 steps, so the poll ended 11 to 24 ms later instead of 10 ms. WaitForFlipRoom waits on the same condition with a 60 s bound and has the same granularity.

A plain high-resolution sleep is not a replacement here: a flip submitted while the present thread waits must wake it at once. libkernel already has the notify-capable precise wait the guest side uses, TimedWait::Condition (event plus high-resolution waitable timer per waiting thread, std::condition_variable_any under APS5_COARSE_TIMED_WAITS), and libSceVideoOut already links libkernel. The change makes FlipQueue::changed a TimedWait::Condition, turns the two timed waits into WaitUntil with the same bounds and predicates, and the eight notify_all into NotifyAll. On Linux TimedWait::Condition is the condition_variable_any path, so nothing changes there. This is the first use of TimedWait::Condition outside libkernel.

Tested

Windows 11 (10.0.26200), WinLibs GCC 15.2.0 (15.2.0posix-14.0.0-ucrt-r7), Release, BUILD_TESTING=ON.

  • Standalone measurement against the built libkernel.prx, tick raised to 0.5 ms as Time.cpp does, 60 iterations each:

    wait p50 min max
    std::condition_variable::wait_for(10 ms), no notify 11.65 ms 10.86 24.43
    TimedWait::Condition::WaitUntil(10 ms), no notify 10.01 ms 10.00 11.25
    TimedWait::Condition::WaitUntil(10 ms), notified from another thread 0.13 ms 0.06 1.54
    std::condition_variable::wait_for(10 ms), notified from another thread 0.14 ms 0.06 2.36

    The notified rows show the wake-on-flip path is unchanged.

  • cmake --build build --target libSceVideoOut video_out_flip_tests; video_out_flip_tests decode and video_out_flip_tests controls with SDL_VIDEODRIVER=dummy exit 0 (the modes video_out_flip_tests is not in ctest and its lifetime and reopen modes terminate on main #1040 reports as passing on main; the others need a param.json).

  • python tools/check_conventions.py --base upstream/main and git diff --check pass.

  • No title run; no Linux host here (the Linux path is the unchanged condition_variable_any fallback inside TimedWait::Condition).

Checklist

🤖 Generated with Claude Code

@oneandonlydean

Copy link
Copy Markdown
Collaborator

This breaks loading on Linux. With this PR on main (25cc6fa), libSceVideoOut.prx imports TimedWait::DeadlineNanos (_ZN9TimedWait13DeadlineNanosEm), which libkernel.prx contains but doesn't export, so a title stops at load with symbol lookup error ... undefined symbol and patched_libraries fails. nm -D on the PR's own build shows the same, so it isn't from main. Could you either export it from libkernel or keep the helper inside libSceVideoOut?

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

@adriadam10 adriadam10 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @oneandonlydean for catching this. Confirmed from the diff: VideoOutDriver.cpp now calls TimedWait::DeadlineNanos (declared in libkernel/Time/include/TimedWait.hpp:15), a plain function that libkernel.prx contains but doesn't export. libSceVideoOut.prx therefore imports an unresolved symbol and the title stops at load.

To fix, either:

  • export the TimedWait API that libSceVideoOut needs from libkernel (and check patched_libraries passes), or
  • keep the deadline computation inside libSceVideoOut (e.g. a local steady_clock-based deadline) and only use what libkernel already exports.

Please also run patched_libraries (and a load of a module importing it) before pushing; the PR CI no longer builds/runs ctest, so it won't show there.

mohdfaour03 and others added 2 commits October 11, 2026 11:19
…indows

The flip queue's condition variable went through winpthreads, whose
timed waits end on GetTickCount64 steps: the present thread's 10 ms poll
for SDL events and the pad, taken while no flip is queued, ended 11 to
24 ms later on Windows, and WaitForFlipRoom's 60 s bound had the same
granularity. Use TimedWait::Condition, which waits on an event and a
high-resolution timer, so the poll ends at 10 ms and a submitted flip
still wakes the present thread at once.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ng it from libkernel

libkernel.prx does not export TimedWait::*, so libSceVideoOut.prx had an
unresolved import and titles stopped at load. Compile TimedWait.cpp into
libSceVideoOut, and use NotifyAll for the notify added on main.
@adriadam10
adriadam10 force-pushed the fix/videoout-present-precise-wait branch from 25cc6fa to b41db48 Compare October 11, 2026 09:22

@adriadam10 adriadam10 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @oneandonlydean for catching this. Confirmed: libkernel.prx doesn't export TimedWait::*, so libSceVideoOut.prx had an unresolved import.

Fixed on the branch:

  • rebased onto current main (conflict in FlipRequest::Fail after ReleaseLocked; the new notify_all is now NotifyAll)
  • TimedWait.cpp (self-contained) is compiled into libSceVideoOut (CMakeLists.txt) instead of being imported from libkernel

Checked: nm -D libSceVideoOut.prx has no TimedWait undefined symbol; patched_libraries and the VideoOut ctest group pass (flip modes needing a Vulkan window skip). The Windows timing numbers are the author's and weren't reproduced here. Merging on green CI.

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 11, 2026
@adriadam10
adriadam10 merged commit c6ca798 into boykopovar:main Oct 11, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants