fix(host): a movie's end no longer waits ~250 ms for a picture that is not coming #2682

Closed
github-sync wants to merge 3 commits from github/pr/2682 into main
Owner

@SadTransGirl opened this pull request on GitHub · 2026-11-12 06:06 UTC · original
Merged on GitHub in cc04531b87

Every start has a ~250 ms freeze at the title-to-game load: the game's main thread joins the movie thread, which sits in the decoder's wait for one more picture. This makes that wait as short as it was meant to be, and none when nothing is coming.

Cause. dec_acquire (host/nn_video.c) waits for a picture another thread's submission may bring: 50 tries of a 2 ms sleep, meant as ~100 ms. A 2 ms sleep takes 4-5 ms here, so the wait was 245-257 ms. At a movie's end (Stop/Flush, then the thread joined) nothing more is submitted, so the wait always ran out in full. Found by profiling the main thread during the freeze: hos_thread_join on MovieOperateThread, which was in that loop.

Change.

  • The wait is ~100 ms by the clock instead of 50 tries.
  • No wait after a Stop or a Flush with nothing submitted since (Dec.stopped, set in dec_flush, cleared by the next submit).
  • A submission still pending is waited for as before.
  • HOS_VIDEO_ACQUIRE_TRIES=1 gives the 50 tries back.

Tests. tests/test_vcore_video_acquire.py, which compiles the real dec_acquire with fakes, gains four cases: 100 ms of 2 ms sleeps; sleeps that take 5 ms still stop at 100 ms (not 250); no wait after a stop; a pending submission still waited for. The existing cases are unchanged.

Measured. Town to Hyrule Field from the title, RTX 4070 SUPER, Windows 11, DLAA 2560x1440, unattended, two runs each on the same build with and without the change: the frame at the title-to-game load was 258 and 259 ms without it, and no frame in that stretch was over 60 ms with it.

Not tested. Linux, AMD, Intel; cutscene movies in play (the change only shortens a wait that ran out, but a movie that is stopped and started again is the case to watch: its first picture could show one frame later; likewise a Flush used to loop or seek, where an acquire between the flush and the next submission now returns the previous buffer at once instead of waiting).

Part of #995.

🤖 Generated with Claude Code

> [@SadTransGirl](https://github.com/SadTransGirl) opened this pull request on GitHub · 2026-11-12 06:06 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2682) > **Merged on GitHub** in https://git.magicbean.dev/MagicBeanDev/Hookshot/commit/cc04531b87c1dae3fc30d9eb69505d97bf52f676 Every start has a ~250 ms freeze at the title-to-game load: the game's main thread joins the movie thread, which sits in the decoder's wait for one more picture. This makes that wait as short as it was meant to be, and none when nothing is coming. **Cause.** `dec_acquire` (`host/nn_video.c`) waits for a picture another thread's submission may bring: 50 tries of a 2 ms sleep, meant as ~100 ms. A 2 ms sleep takes 4-5 ms here, so the wait was 245-257 ms. At a movie's end (`Stop`/`Flush`, then the thread joined) nothing more is submitted, so the wait always ran out in full. Found by profiling the main thread during the freeze: `hos_thread_join` on `MovieOperateThread`, which was in that loop. **Change.** - The wait is ~100 ms by the clock instead of 50 tries. - No wait after a `Stop` or a `Flush` with nothing submitted since (`Dec.stopped`, set in `dec_flush`, cleared by the next submit). - A submission still pending is waited for as before. - `HOS_VIDEO_ACQUIRE_TRIES=1` gives the 50 tries back. **Tests.** `tests/test_vcore_video_acquire.py`, which compiles the real `dec_acquire` with fakes, gains four cases: 100 ms of 2 ms sleeps; sleeps that take 5 ms still stop at 100 ms (not 250); no wait after a stop; a pending submission still waited for. The existing cases are unchanged. **Measured.** Town to Hyrule Field from the title, RTX 4070 SUPER, Windows 11, DLAA 2560x1440, unattended, two runs each on the same build with and without the change: the frame at the title-to-game load was 258 and 259 ms without it, and no frame in that stretch was over 60 ms with it. **Not tested.** Linux, AMD, Intel; cutscene movies in play (the change only shortens a wait that ran out, but a movie that is stopped and started again is the case to watch: its first picture could show one frame later; likewise a `Flush` used to loop or seek, where an acquire between the flush and the next submission now returns the previous buffer at once instead of waiting). Part of #995. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- github-sync:item:2682 -->
dec_acquire waits for a picture another thread's submission may bring: 50
tries of a 2 ms sleep, meant as ~100 ms. Such a sleep takes 4-5 ms, so the
wait was 245-257 ms, on MovieOperateThread, which the game's main thread
joins when a movie ends: a frame of ~250 ms at the title-to-game load,
every start.

The wait is now ~100 ms by the clock, and none after a Stop or a Flush with
nothing submitted since (no picture is coming then). A submission still
pending is waited for as before. HOS_VIDEO_ACQUIRE_TRIES=1: 50 tries, as
before.

tests/test_vcore_video_acquire.py, which compiles the real dec_acquire,
gains four cases: 100 ms of 2 ms sleeps; sleeps of 5 ms still 100 ms; no
wait after a stop; a pending submission still waited for.

Measured (town to field, RTX 4070 SUPER, two runs each): the frame at the
title-to-game load was 258 and 259 ms without the change, under 60 ms with
it.

Assisted-by: Claude Mythos 5.2, Claude Opus 5.5
Co-Authored-By: Claude Mythos 5.2 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Author
Owner

@Joey-Magnus approved on GitHub · 2026-11-12 14:34 UTC · original

I merged this into current main and ran tests/test_vcore_video_acquire.py with the other video/vcore/env checks: 9 passed. The four new cases cover the clock-based 100 ms limit, 5 ms sleeps, no wait after a stop, and a pending submission still being waited for. I read dec_acquire against dec_flush and dec_submit. stopped is set and cleared under d->lock, which dec_acquire holds when it reads it, so there's no race. After a Stop the idle path below the loop still returns the buffer with its sync not reached, the same as before. HOS_VIDEO_ACQUIRE_TRIES is read with env_on. The 258/259 ms before and under-60 ms after on the RTX 4070 SUPER is the measurement I'd want. The one case left to watch is the one you name: a Flush used to seek or loop a cutscene movie, where an acquire between the flush and the next submit now returns the previous buffer at once. If a Linux or AMD tester sees a cutscene movie start a frame late, HOS_VIDEO_ACQUIRE_TRIES=1 is the A/B.

> [@Joey-Magnus](https://github.com/Joey-Magnus) approved on GitHub · 2026-11-12 14:34 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2682#pullrequestreview-5876231696) I merged this into current main and ran `tests/test_vcore_video_acquire.py` with the other video/vcore/env checks: 9 passed. The four new cases cover the clock-based 100 ms limit, 5 ms sleeps, no wait after a stop, and a pending submission still being waited for. I read `dec_acquire` against `dec_flush` and `dec_submit`. `stopped` is set and cleared under `d->lock`, which `dec_acquire` holds when it reads it, so there's no race. After a Stop the `idle` path below the loop still returns the buffer with its sync not reached, the same as before. `HOS_VIDEO_ACQUIRE_TRIES` is read with `env_on`. The 258/259 ms before and under-60 ms after on the RTX 4070 SUPER is the measurement I'd want. The one case left to watch is the one you name: a Flush used to seek or loop a cutscene movie, where an acquire between the flush and the next submit now returns the previous buffer at once. If a Linux or AMD tester sees a cutscene movie start a frame late, `HOS_VIDEO_ACQUIRE_TRIES=1` is the A/B. <!-- github-sync:r:5876231696 -->
github-sync closed this pull request 2026-11-12 16:55:36 +00:00
Author
Owner

@hckrman101 approved on GitHub · 2026-11-12 16:31 UTC · original

Approving. I read dec_acquire, dec_flush and dec_submit at the head.

  • Early-out correctness: stopped is written in dec_flush and dec_submit and read in dec_acquire, all under d->lock, so there is no race. It is also re-checked every loop turn after the lock is re-taken, so a Stop that lands during a wait ends it. Skipping the wait cannot skip a real last frame: eos and the queue are tested first, and a Stop or Flush has already emptied the queue and reset eos. After a Stop, the no-picture path hands back the buffer as before, with the idle sync. Nothing about the decoder or the thread's teardown changes beyond a shorter wait. Dec is calloc'd, so stopped starts at 0 and the first start waits as it did.
  • The clock: one hos_now_ns() deadline replaces the try count, so a slow sleep can't stretch the wait, and HOS_VIDEO_ACQUIRE_TRIES=1 keeps the old behaviour for an A/B.
  • Evidence: the fix is shown gone in the PR body and in docs/tested-configurations.md: the title-to-game frame was 258 and 259 ms without the change and under 60 ms with it, two runs each, RTX 4070 SUPER, Windows 11, build ids named. That meets the rule for a fix. I agree with Joey-Magnus's review (the same reading of the locking), and he named the same remaining case.
  • Still untested, as the PR says: Linux, AMD and Intel, and a cutscene movie that is stopped and started again or a Flush used to loop or seek. There an acquire between the flush and the next submit now returns the previous buffer at once. That is not a blocker, since the env switch is the A/B if a tester sees a late first picture.

CI is green on the head and the branch merges cleanly into main. No other open PR touches host/nn_video.c or this test.

> [@hckrman101](https://github.com/hckrman101) approved on GitHub · 2026-11-12 16:31 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2682#pullrequestreview-5878183834) Approving. I read `dec_acquire`, `dec_flush` and `dec_submit` at the head. - **Early-out correctness:** `stopped` is written in `dec_flush` and `dec_submit` and read in `dec_acquire`, all under `d->lock`, so there is no race. It is also re-checked every loop turn after the lock is re-taken, so a Stop that lands during a wait ends it. Skipping the wait cannot skip a real last frame: `eos` and the queue are tested first, and a Stop or Flush has already emptied the queue and reset `eos`. After a Stop, the no-picture path hands back the buffer as before, with the idle sync. Nothing about the decoder or the thread's teardown changes beyond a shorter wait. `Dec` is calloc'd, so `stopped` starts at 0 and the first start waits as it did. - **The clock:** one `hos_now_ns()` deadline replaces the try count, so a slow sleep can't stretch the wait, and `HOS_VIDEO_ACQUIRE_TRIES=1` keeps the old behaviour for an A/B. - **Evidence:** the fix is shown gone in the PR body and in `docs/tested-configurations.md`: the title-to-game frame was 258 and 259 ms without the change and under 60 ms with it, two runs each, RTX 4070 SUPER, Windows 11, build ids named. That meets the rule for a fix. I agree with Joey-Magnus's review (the same reading of the locking), and he named the same remaining case. - **Still untested, as the PR says:** Linux, AMD and Intel, and a cutscene movie that is stopped and started again or a Flush used to loop or seek. There an acquire between the flush and the next submit now returns the previous buffer at once. That is not a blocker, since the env switch is the A/B if a tester sees a late first picture. CI is green on the head and the branch merges cleanly into main. No other open PR touches `host/nn_video.c` or this test. <!-- github-sync:r:5878183834 -->
Commenting is not possible because the repository is archived.
No description provided.