Repository navigation
feat(core,frontend): v1.1.0 "Latchkey" -- open-bus fix + emu-thread audio/lifecycle + accuracy research - #61
Conversation
…udio/lifecycle + accuracy research Fixes SuperFxBoard::map's Game-Pak-RAM-ownership open-bus gap (a CPU/DMA read while the GSU owned RAM always returned a hardcoded 0 instead of the real bus-latched byte), verified with zero regressions across the full --features test-roms battery. Closes emu-thread's two biggest documented gaps: real audio output via a thread-owned AudioProducer, and a proper pause/ROM-loaded/speed lifecycle via EmuControl driving a thread-owned Pacer, plus a PresentBuffer lock-free framebuffer handoff. Full parity (cheats/watchpoints/breakpoints/run-ahead/ rewind/movies/scripting/netplay-pause/RetroAchievements) remains a documented follow-up -- each needs a new shared-mutable-state design, not a mechanical port. Investigates without landing code: the harder open-bus-via-DMA-latch bug (substantially narrowed, still open), DRAM refresh timing (empirically measured to already be correct -- the originally-planned additive stall would have been a ~10,480-clock/frame regression), and a fractional-timebase refactor go/no-go assessment (conclusion: not warranted by any current residual). Also fixes two pre-existing broken intra-doc links from v1.0.1's per-voice- mute work, surfaced by running the RUSTDOCFLAGS="-D warnings" cargo doc gate as part of this release's regression pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements the v1.1.0 release milestones, focusing on completing the emu-thread feature with real audio output, a proper lifecycle control, and a lock-free triple-buffered framebuffer handoff. It also fixes a Super FX open-bus mapping bug and documents research on DRAM refresh and DMA open-bus behavior. The review feedback highlights critical improvements for the new code: correcting setpriority to target only the emulation thread rather than the entire process on Linux, casting a literal in prctl to prevent compilation failures on 32-bit targets, avoiding a costly per-frame framebuffer clone to reduce allocator pressure, and adding a defensive guard against a potential division-by-zero panic in the audio resampler when the ring capacity is zero.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| eprintln!("rustysnes: emu thread elevated to SCHED_RR priority 5."); | ||
| } else { | ||
| // SAFETY: see above — `setpriority` on the calling thread. | ||
| let niced = unsafe { libc::setpriority(libc::PRIO_PROCESS, 0, -10) == 0 }; |
There was a problem hiding this comment.
setpriority(libc::PRIO_PROCESS, 0, -10) sets the nice value of the entire process (all threads in the thread group) on Linux, rather than just the calling thread. This contradicts the comment on line 320 and the module's goal of only elevating the emulation thread's priority. To target only the calling thread, use the thread's TID (via libc::syscall(libc::SYS_gettid)) instead of 0.
let niced = unsafe {
let tid = libc::syscall(libc::SYS_gettid) as libc::id_t;
libc::setpriority(libc::PRIO_PROCESS, tid, -10) == 0
};| // SAFETY: `prctl(PR_SET_TIMERSLACK, ...)` sets this thread's timer slack (always permitted | ||
| // for one's own thread); extra args are ignored. | ||
| unsafe { | ||
| libc::prctl(libc::PR_SET_TIMERSLACK, 1_000_u64, 0, 0, 0); |
There was a problem hiding this comment.
Passing 1_000_u64 directly to libc::prctl will cause a compilation error on 32-bit Linux targets (such as armv7 or i686) because prctl expects arguments of type c_ulong (which is u32 on 32-bit platforms and u64 on 64-bit platforms). To ensure portability across both 32-bit and 64-bit architectures, cast the literal to libc::c_ulong.
| libc::prctl(libc::PR_SET_TIMERSLACK, 1_000_u64, 0, 0, 0); | |
| libc::prctl(libc::PR_SET_TIMERSLACK, 1_000 as libc::c_ulong, 0, 0, 0); |
| active.present.take_into(&mut active.present_staging); | ||
| #[cfg(feature = "emu-thread")] | ||
| let (fb, dims) = (emu.framebuffer().to_vec(), emu.fb_dims()); | ||
| let fb = active.present_staging.clone(); |
There was a problem hiding this comment.
Cloning active.present_staging every frame causes a heap allocation of up to ~900 KiB 60 times per second, which introduces significant allocator pressure and potential frame stuttering. Since the downstream rendering and texture upload APIs typically only require a read-only borrow of the framebuffer, you can avoid this clone entirely by taking a reference to active.present_staging.
| let fb = active.present_staging.clone(); | |
| let fb = &active.present_staging; |
| let cap = self.ring.capacity(); | ||
| let ratio = drc_ratio(self.ring.occupancy(), cap / 2, cap) * f64::from(speed); | ||
| self.resampler.process(samples, ratio, &self.ring); |
There was a problem hiding this comment.
If the audio device fails to initialize or is not yet ready, self.ring.capacity() can return 0. In this case, calling drc_ratio with cap = 0 can lead to a division by zero or panic. Add a defensive guard to return early if cap == 0, matching the check already present in health_pct.
let cap = self.ring.capacity();
if cap == 0 {
return;
}
let ratio = drc_ratio(self.ring.occupancy(), cap / 2, cap) * f64::from(speed);
self.resampler.process(samples, ratio, &self.ring);There was a problem hiding this comment.
Pull request overview
This PR advances RustySNES to v1.1.0 “Latchkey” with a core accuracy fix (Super FX open-bus behavior) and substantial native emu-thread parity work (audio + lifecycle + framebuffer handoff), plus corresponding documentation/research write-ups and workspace version bumps.
Changes:
- Fix
SuperFxBoard::mapso CPU/DMA reads of Game Pak RAM while the GSU owns RAM correctly resolve to open bus (threading the bus-latched byte viaCart::read24). - Extend native
emu-threadwith thread-owned audio production (AudioProducer), a pause/ROM-loaded/speed lifecycle (EmuControl+ thread-ownedPacer), and a triple-buffer framebuffer handoff (PresentBuffer) plus a nativeAppEvent::EmuFrameping. - Update roadmap/status/audit docs (DMA open-bus investigation status, DRAM-refresh measurement conclusion, and a fractional-timebase go/no-go assessment), and bump all crate/workspace versions to
1.1.0.
Reviewed changes
Copilot reviewed 32 out of 33 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| to-dos/VERSION-PLAN.md | Adds detailed v1.1.0 plan/summary entry and regression gate notes. |
| to-dos/ROADMAP.md | Updates phase spine narrative to include v1.1.0 scope/results. |
| README.md | Updates roadmap section to mention v1.1.0 in-progress scope. |
| docs/STATUS.md | Adds v1.1.0 in-progress status summary and updates frontend row to include threaded parity progress. |
| docs/scheduler.md | Expands DMA open-bus research notes; documents Super FX open-bus fix, watchpoint instrumentation, and DRAM refresh measurement conclusion. |
| docs/rom-test-corpus.md | Adds legitimate ROM sourcing leads section for known gaps (research-only). |
| docs/frontend.md | Updates emu-thread documentation to reflect audio/lifecycle/present-buffer changes and remaining gaps. |
| docs/audit/README.md | Updates audit index (SPC7110 conclusion clarification + adds fractional-timebase assessment entry). |
| docs/audit/fractional-timebase-go-no-go-2026-07-11.md | New audit document: evidence-based assessment that ADR-0002 gate is not met. |
| docs/adr/0002-fractional-timebase-refactor.md | Adds dated status update pointing to the go/no-go audit. |
| crates/rustysnes-test-harness/Cargo.toml | Bumps crate + internal dependency versions to 1.1.0. |
| crates/rustysnes-script/Cargo.toml | Bumps crate + internal dependency versions to 1.1.0. |
| crates/rustysnes-savestate/Cargo.toml | Bumps crate version to 1.1.0. |
| crates/rustysnes-ppu/Cargo.toml | Bumps crate + internal dependency versions to 1.1.0. |
| crates/rustysnes-netplay/Cargo.toml | Bumps crate + internal dependency versions to 1.1.0. |
| crates/rustysnes-frontend/src/present_buffer.rs | New triple-buffer framebuffer handoff implementation + tests. |
| crates/rustysnes-frontend/src/lib.rs | Exposes present_buffer module behind native emu-thread cfg. |
| crates/rustysnes-frontend/src/emu_thread.rs | Adds EmuControl, audio output, pacing/lifecycle, present publishing, and Linux priority elevation. |
| crates/rustysnes-frontend/src/config.rs | Improves cross-references in audio config docs. |
| crates/rustysnes-frontend/src/audio.rs | Adds AudioOutput::make_producer and AudioProducer for thread-owned audio production. |
| crates/rustysnes-frontend/src/app.rs | Wires emu-thread lifecycle sync, PresentBuffer consumption, native EventLoopProxy + AppEvent::EmuFrame. |
| crates/rustysnes-frontend/Cargo.toml | Bumps version/deps; updates emu-thread feature description; adds optional Linux libc dep for thread priority. |
| crates/rustysnes-cpu/Cargo.toml | Bumps crate + savestate dependency to 1.1.0. |
| crates/rustysnes-core/src/bus.rs | Refactors watchpoint checking into helper and adds DMA/HDMA visibility into watchpoints (debug-hooks). |
| crates/rustysnes-core/Cargo.toml | Bumps crate + internal dependency versions to 1.1.0. |
| crates/rustysnes-cheevos/Cargo.toml | Bumps crate version to 1.1.0. |
| crates/rustysnes-cart/src/coproc/superfx.rs | Fixes Super FX RAM-ownership open-bus mapping logic; clarifies unreachable hardcoded-0 read arm under normal cart path. |
| crates/rustysnes-cart/Cargo.toml | Bumps crate + savestate dependency to 1.1.0. |
| crates/rustysnes-apu/src/dsp.rs | Adjusts docs for per-voice mute setter to avoid broken private-item links. |
| crates/rustysnes-apu/Cargo.toml | Bumps crate + savestate dependency to 1.1.0. |
| CHANGELOG.md | Adds 1.1.0 release notes (fixes, added features, and research-only investigations). |
| Cargo.toml | Bumps workspace package version to 1.1.0. |
| Cargo.lock | Updates lockfile to reflect 1.1.0 crate versions and new dependency edges. |
| let idle = | ||
| !control.has_rom.load(Ordering::Acquire) || control.user_paused.load(Ordering::Acquire); | ||
| if idle { | ||
| std::thread::park_timeout(IDLE_PARK); | ||
| continue; | ||
| } |
| let dims = emu.fb_dims(); | ||
| #[cfg(feature = "emu-thread")] | ||
| active.present.take_into(&mut active.present_staging); | ||
| #[cfg(feature = "emu-thread")] | ||
| let (fb, dims) = (emu.framebuffer().to_vec(), emu.fb_dims()); | ||
| let fb = active.present_staging.clone(); |
Six findings from gemini-code-assist and copilot-pull-request-reviewer on the emu-thread parity work: - emu_thread.rs: setpriority(PRIO_PROCESS, 0, -10) niced the whole process instead of just the emu thread; address by TID via SYS_gettid. - emu_thread.rs: cast the prctl(PR_SET_TIMERSLACK, ...) literal to libc::c_ulong so it compiles on 32-bit targets. - emu_thread.rs: reset the wallclock pacer on idle/pause entry so resuming doesn't burst up to MAX_CATCHUP_FRAMES. - audio.rs: guard AudioProducer::push against a zero-capacity ring (device not yet ready), matching health_pct's existing guard. - app.rs: drop the emu mutex before copying the PresentBuffer's published frame, instead of holding it across take_into + clone — the copy no longer serializes the present path against the emu thread's next run_frame(). - app.rs: reuse a persistent fb_scratch buffer instead of Vec::clone-ing present_staging every frame, so steady-state presents make zero fresh allocations. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Summary
Opens the Reach-phase backlog arc (v1.1.0 -> v1.2.0 -> v1.3.0). A research + accuracy cut:
SuperFxBoard::map's Game-Pak-RAM-ownership open-bus gap -- a CPU/DMA read of GamePak RAM while the GSU owned it always returned a hardcoded
0instead of the real bus-latchedbyte, bypassing
Cart::read24's generic open-bus fallback entirely. Verified independently:zero regressions across the full
--features test-romsbattery with this fix alone.emu-thread(opt-in feature) gained real audio output (a thread-ownedAudioProducer) and a proper pause/ROM-loaded/speed lifecycle (EmuControldriving athread-owned
Pacer), plus aPresentBufferlock-free framebuffer handoff. Native builds nowcarry an
EventLoopProxy<AppEvent>(previously wasm32-only) for the thread'sAppEvent::EmuFrameping. Full parity (cheats/watchpoints/breakpoints/run-ahead/rewind/movies/scripting/netplay-pause/
RetroAchievements) is a documented follow-up, not silently claimed.
DRAM refresh timing (empirically proven the current model is already correct -- implementing the
originally-planned stall would have been a regression), and a fractional-timebase-refactor
go/no-go assessment (conclusion: not warranted by any current residual).
v1.0.1's per-voice-mute work, surfaced byrunning the
RUSTDOCFLAGS="-D warnings" cargo docgate as part of this release's regression pass.Test plan
cargo fmt --all --checkcargo clippy --workspace --all-targets -- -D warnings(default, flags-off,full,emu-thread,debug-hooks,emu-thread,debug-hooks)cargo test --workspace(all suites green)cargo test -p rustysnes-frontend --features emu-thread(72 tests)cargo test -p rustysnes-test-harness --features test-roms --release(27 accuracy/oraclesuites, zero regressions, no golden hash changed)
cargo build -p rustysnes-core --target thumbv7em-none-eabihf --no-default-features(no_std)RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-depswasm-winit,wasm-canvas) viatrunk build --releasexvfb-runlaunch of theemu-threadbinary against a staged commercial ROM(no panics over several seconds of runtime)
🤖 Generated with Claude Code