fix(core): the APU clock is region-independent; it ran 0.92% slow on PAL - #316
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (2)
🧰 Additional context used📓 Path-based instructions (9)crates/**/*.rs📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
crates/rustysnes-{cpu,ppu,apu,cart,core}/**/*.rs📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.rs📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
crates/rustysnes-core/**/*.rs📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{rs,md}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
crates/rustysnes-*/**/*📄 CodeRabbit inference engine (Custom checks)
Files:
**/*.{rs,toml}📄 CodeRabbit inference engine (AGENTS.md)
Files:
crates/**⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (2)
WalkthroughThe SPC timing denominator now uses the active PPU region. NTSC keeps its existing divisor, while PAL uses a PAL-specific divisor. A regression test verifies PAL sample-rate scaling. APU and scheduler documentation now describe both denominators. ChangesAPU timing correction
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant advance_master
participant PPU
participant SPC accumulator
participant SMP
advance_master->>PPU: Read active region
advance_master->>SPC accumulator: Select NTSC or PAL denominator
SPC accumulator->>SMP: Release APU cycles
Possibly related PRs
🚥 Pre-merge checks | ✅ 8 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (8 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/rustysnes-core/src/bus.rs`:
- Around line 1737-1764: Update the smp_samples helper to record
bus.clock.master before advancing and return the elapsed master ticks alongside
the drained sample count, using bus.clock.master minus the starting value so
recursive DRAM_REFRESH_CLOCKS are included. Normalize the NTSC and PAL sample
counts by their measured elapsed ticks before comparing the rates and ratio,
rather than comparing raw sample totals.
- Around line 132-142: Unify the APU timing model across the documented sites:
in crates/rustysnes-core/src/bus.rs, docs/scheduler.md, docs/apu.md, and
docs/adr/0001-master-clock-lockstep-scheduler.md, distinguish the nominal 24.576
MHz resonator from the ares model’s 24,606,720 Hz SMP clock and align
accumulator ratios with 68,352/(715,909|709,379) and the region-specific master
clocks. In CHANGELOG.md, correct the Unreleased PAL calculation to approximately
2,031,856 rather than 2,031,850; update every listed site as required, with no
site treated as evidence-only.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 75b13ba2-4ace-4bd4-aa1c-19510e2adb49
📒 Files selected for processing (4)
CHANGELOG.mdcrates/rustysnes-core/src/bus.rsdocs/apu.mddocs/scheduler.md
📜 Review details
⏰ Context from checks skipped due to timeout. (6)
- GitHub Check: lint
- GitHub Check: accuracysnes
- GitHub Check: test-light
- GitHub Check: copilot-pull-request-reviewer
- GitHub Check: build demo + docs
- GitHub Check: review
🧰 Additional context used
📓 Path-based instructions (14)
docs/**/*.md
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Before changing a subsystem, consult
docs/architecture.md,docs/STATUS.md,CONTRIBUTING.md, the relevant subsystem documentation, and applicable ADRs.New subsystems must add documentation under
docs/.
Files:
docs/apu.mddocs/scheduler.md
**/*.{rs,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Chip-behavior changes must update both the chip implementation and the corresponding
docs/<subsystem>.mddocumentation.A chip change must update both the chip implementation and its corresponding
docs/<chip>.mddocumentation in the same change.
Files:
docs/apu.mddocs/scheduler.mdCHANGELOG.mdcrates/rustysnes-core/src/bus.rs
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Do not commit or vendor the generatedsnesdev_wiki/mirror; it is gitignored and intended only as a local reference.
Keep commits focused and use Conventional Commits:<type>(<scope>): <subject>, with an imperative subject of at most 72 characters.
Do not use emojis in code, comments, or commit messages.
Before opening a PR, ensure formatting, Clippy, workspace tests, the core embedded build, rustdoc with warnings denied, documentation coverage, and changelog requirements pass.
Ticket completion must be reflected in the relevantto-dos/sprint file.
**/*: Preserve the one-directional crate graph: chip crates must not depend on one another;rustysnes-coreties them together.
Never commit commercial ROMs; only commit derived screenshots and hashes.
Keepdocs/STATUS.mdas the authoritative per-subsystem status and update project documentation in the same PR as code changes.
Do not treat RustyNESv2.0orengine-lineageanchors as project releases.
Files:
docs/apu.mddocs/scheduler.mdCHANGELOG.mdcrates/rustysnes-core/src/bus.rs
docs/**/*
📄 CodeRabbit inference engine (docs/testing-strategy.md)
Chip crates should exceed 90% unit-test coverage, and each chip should be fuzzable in isolation.
Files:
docs/apu.mddocs/scheduler.md
docs/**
⚙️ CodeRabbit configuration file
docs/**: Docs are the spec, not a history log. Flag claims that contradict the code, counts that
contradict the generateddocs/accuracysnes-coverage.md, and any statement of coverage that
is broader than what the corresponding test actually asserts.
Files:
docs/apu.mddocs/scheduler.md
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: Docs are the spec, not a changelog. Flag prose that has drifted from the code it describes
rather than style nits. The markdownlint gate is pinned to v0.39.0 via pre-commit —
do not report rules that version does not have (MD060 in particular).
Files:
docs/apu.mddocs/scheduler.mdCHANGELOG.md
CHANGELOG.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
User-visible changes must be recorded under the
[Unreleased]section.For the full pull request diff against its base branch, modify
CHANGELOG.mdwhen user-visible behavior changes, including emulator output, frontend features, CLI flags, public APIs, or AccuracySNES cartridge contents. Do not require it for purely internal changes, tests, comments, or CI configuration.
Files:
CHANGELOG.md
crates/**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
crates/**/*.rs: Preserve the master-clock lockstep timing model.
rustysnes-core::Busowns mutable machine state, and the CPU borrows&mut Bus.
Preserve determinism: seed, ROM, and input must produce bit-identical output.
Treat test ROMs as the behavioral specification; when documentation disagrees with passing ROM behavior, update the documentation.
Keepunsafeconfined to existing allowed areas, namely frontend and FFI code, and document everyunsafeblock with a// SAFETY:comment.
Files:
crates/rustysnes-core/src/bus.rs
crates/rustysnes-{cpu,ppu,apu,cart,core}/**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Keep core chip implementation changes localized to the owning chip crate and preserve the workspace crate boundaries.
Files:
crates/rustysnes-core/src/bus.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use Rust edition 2024 and the toolchain pinned inrust-toolchain.toml(Rust 1.96).
Runcargo fmt --all --check; Rust code must remain rustfmt-compliant.
Run Clippy withcargo clippy --workspace --all-targets -- -D warnings; warnings must not remain.
New public Rust items must have rustdoc becausemissing_docsis a workspace lint.
Do not runcargo clippy --all-features;scriptingandscript-wasmare mutually exclusive. Use explicit per-feature jobs instead.
**/*.rs: Do not introduce.unwrap(),.expect(), orpanic!()on untrusted external input—such as ROM/save-state bytes, netplay messages, Lua or scripting input, or user-supplied paths—outside#[cfg(test)]code. Use typed errors at those boundaries; locally constructed values or values immediately protected by a checked invariant are allowed.
Every newunsafe { ... }block orunsafe fnmust have an adjacent// SAFETY:comment naming the relied-on invariant and its guarantor. Unsafe code outside the frontend and FFI shims should additionally be questioned becauseunsafe_codeis a workspace lint.
**/*.rs: Use Rust edition 2024 with the pinned 1.96 toolchain; satisfy workspacepedantic,nursery,missing_docs, andunsafe_codewarnings because CI runs with-D warnings. Document every public item.
Keepunsafecode restricted to the frontend and FFI, and include a// SAFETY:justification for each use.
Keep hot paths allocation-free.
Treatrustysnes_core::Busas the owner of mutable emulator state; the CPU borrows&mut Bus.
Use the master clock at 21477270 Hz as the timing master; advance the scheduler in lockstep and run other chips on their divisors.
Maintain determinism: seed, ROM, and input must produce bit-identical audio/video; frontend rate control must not alter emulation results.
When implementing hardware behavior, pin and run the failing test ROM first; treat test ROMs as the specification.
Files:
crates/rustysnes-core/src/bus.rs
crates/rustysnes-core/**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
The core crate must compile for
thumbv7em-none-eabihfwith--no-default-features; the chip stack must remainno_std.
Files:
crates/rustysnes-core/src/bus.rs
crates/rustysnes-*/**/*
📄 CodeRabbit inference engine (Custom checks)
For the full pull request diff against its base branch, any observable behavior change under
crates/rustysnes-<chip>/must be accompanied by an edit to the matchingdocs/<chip>.md; a crate change passes without documentation only when it does not alter observable behavior, with the non-behavioral change stated explicitly.
Files:
crates/rustysnes-core/src/bus.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,toml}: Additive features must be default-off so shipped/native,no_std, and wasm builds remain byte-identical.
Never use or configure--all-features; validate opt-in feature combinations individually as required by the project recipe.
Files:
crates/rustysnes-core/src/bus.rs
crates/**
⚙️ CodeRabbit configuration file
crates/**: Emulator core. Hot paths are allocation-free;unsaferequires a// SAFETY:comment
naming the invariant. Any change to save-stated fields needs aFORMAT_VERSIONbump and a
docs/adr/0006bump-log entry. Behavior changes must update the matchingdocs/<chip>.md
in the same change.
Files:
crates/rustysnes-core/src/bus.rs
🔇 Additional comments (2)
crates/rustysnes-core/src/bus.rs (1)
465-473: LGTM!Also applies to: 888-900
docs/apu.md (1)
273-276: LGTM!
| fn smp_samples(region: Region, master_ticks: u64) -> usize { | ||
| let mut bus = Bus::new(region); | ||
| let mut sink = std::vec::Vec::new(); | ||
| let mut n = 0; | ||
| while n < master_ticks { | ||
| bus.advance_master(MASTER_PER_DOT); | ||
| n += u64::from(MASTER_PER_DOT); | ||
| bus.apu.drain_audio(&mut sink); | ||
| } | ||
| sink.len() | ||
| } | ||
|
|
||
| // Many frames, so the ~0.92% difference is far larger than the one-sample quantisation. | ||
| let ntsc = smp_samples(Region::Ntsc, 425_568 * 40); | ||
| let pal = smp_samples(Region::Pal, 425_568 * 40); | ||
| assert!( | ||
| pal > ntsc, | ||
| "PAL must produce MORE audio per master tick than NTSC — its master clock is slower \ | ||
| (21.281370 vs 21.477270 MHz) while the APU crystal is the same, so one master tick is \ | ||
| worth more APU. Got NTSC {ntsc} samples, PAL {pal}" | ||
| ); | ||
| #[allow(clippy::cast_precision_loss)] | ||
| let ratio = (pal as f64) / (ntsc as f64); | ||
| // 21_477_270 / 21_281_370 = 1.0092053...; the tolerance is one accumulator tick either way. | ||
| assert!( | ||
| (ratio - 1.009_205).abs() < 0.000_05, | ||
| "the two divisors must differ by exactly the ratio of the two master clocks; got \ | ||
| {ratio} from NTSC {ntsc} samples, PAL {pal}" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Measure elapsed master ticks before comparing sample counts.
Bus::advance_master recursively advances DRAM_REFRESH_CLOCKS at each scanline. smp_samples increments n only by MASTER_PER_DOT. The test interval therefore includes master ticks that n does not count.
This input runs long enough to trigger DRAM refresh repeatedly. The raw pal / ntsc ratio combines the APU rate with unequal elapsed master-tick counts. Return bus.clock.master - start and normalize each sample count by its measured elapsed ticks.
Proposed fix
- fn smp_samples(region: Region, master_ticks: u64) -> usize {
+ fn smp_samples(region: Region, master_ticks: u64) -> (usize, u64) {
let mut bus = Bus::new(region);
let mut sink = std::vec::Vec::new();
- let mut n = 0;
- while n < master_ticks {
+ let start = bus.clock.master;
+ while bus.clock.master - start < master_ticks {
bus.advance_master(MASTER_PER_DOT);
- n += u64::from(MASTER_PER_DOT);
bus.apu.drain_audio(&mut sink);
}
- sink.len()
+ (sink.len(), bus.clock.master - start)
}
- let ntsc = smp_samples(Region::Ntsc, 425_568 * 40);
- let pal = smp_samples(Region::Pal, 425_568 * 40);
+ let (ntsc, ntsc_ticks) = smp_samples(Region::Ntsc, 425_568 * 40);
+ let (pal, pal_ticks) = smp_samples(Region::Pal, 425_568 * 40);
assert!(
pal > ntsc,
"PAL must produce MORE audio per master tick than NTSC ..."
);
#[allow(clippy::cast_precision_loss)]
- let ratio = (pal as f64) / (ntsc as f64);
+ let ratio = (pal as f64 / pal_ticks as f64) / (ntsc as f64 / ntsc_ticks as f64);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn smp_samples(region: Region, master_ticks: u64) -> usize { | |
| let mut bus = Bus::new(region); | |
| let mut sink = std::vec::Vec::new(); | |
| let mut n = 0; | |
| while n < master_ticks { | |
| bus.advance_master(MASTER_PER_DOT); | |
| n += u64::from(MASTER_PER_DOT); | |
| bus.apu.drain_audio(&mut sink); | |
| } | |
| sink.len() | |
| } | |
| // Many frames, so the ~0.92% difference is far larger than the one-sample quantisation. | |
| let ntsc = smp_samples(Region::Ntsc, 425_568 * 40); | |
| let pal = smp_samples(Region::Pal, 425_568 * 40); | |
| assert!( | |
| pal > ntsc, | |
| "PAL must produce MORE audio per master tick than NTSC — its master clock is slower \ | |
| (21.281370 vs 21.477270 MHz) while the APU crystal is the same, so one master tick is \ | |
| worth more APU. Got NTSC {ntsc} samples, PAL {pal}" | |
| ); | |
| #[allow(clippy::cast_precision_loss)] | |
| let ratio = (pal as f64) / (ntsc as f64); | |
| // 21_477_270 / 21_281_370 = 1.0092053...; the tolerance is one accumulator tick either way. | |
| assert!( | |
| (ratio - 1.009_205).abs() < 0.000_05, | |
| "the two divisors must differ by exactly the ratio of the two master clocks; got \ | |
| {ratio} from NTSC {ntsc} samples, PAL {pal}" | |
| fn smp_samples(region: Region, master_ticks: u64) -> (usize, u64) { | |
| let mut bus = Bus::new(region); | |
| let mut sink = std::vec::Vec::new(); | |
| let start = bus.clock.master; | |
| while bus.clock.master - start < master_ticks { | |
| bus.advance_master(MASTER_PER_DOT); | |
| bus.apu.drain_audio(&mut sink); | |
| } | |
| (sink.len(), bus.clock.master - start) | |
| } | |
| // Many frames, so the ~0.92% difference is far larger than the one-sample quantisation. | |
| let (ntsc, ntsc_ticks) = smp_samples(Region::Ntsc, 425_568 * 40); | |
| let (pal, pal_ticks) = smp_samples(Region::Pal, 425_568 * 40); | |
| assert!( | |
| pal > ntsc, | |
| "PAL must produce MORE audio per master tick than NTSC — its master clock is slower \ | |
| (21.281370 vs 21.477270 MHz) while the APU crystal is the same, so one master tick is \ | |
| worth more APU. Got NTSC {ntsc} samples, PAL {pal}" | |
| ); | |
| #[allow(clippy::cast_precision_loss)] | |
| let ratio = (pal as f64 / pal_ticks as f64) / (ntsc as f64 / ntsc_ticks as f64); | |
| // 21_477_270 / 21_281_370 = 1.0092053...; the tolerance is one accumulator tick either way. | |
| assert!( | |
| (ratio - 1.009_205).abs() < 0.000_05, | |
| "the two divisors must differ by exactly the ratio of the two master clocks; got \ | |
| {ratio} from NTSC {ntsc} samples, PAL {pal}" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/rustysnes-core/src/bus.rs` around lines 1737 - 1764, Update the
smp_samples helper to record bus.clock.master before advancing and return the
elapsed master ticks alongside the drained sample count, using bus.clock.master
minus the starting value so recursive DRAM_REFRESH_CLOCKS are included.
Normalize the NTSC and PAL sample counts by their measured elapsed ticks before
comparing the rates and ratio, rather than comparing raw sample totals.
There was a problem hiding this comment.
Pull request overview
This PR fixes RustySNES core scheduling so the APU (SPC700/S-DSP) runs at the same wall-clock rate on both NTSC and PAL by making the async-resync denominator region-dependent (master clock differs by region, APU crystal does not).
Changes:
- Make the SPC fractional-clock denominator depend on the PPU region (NTSC vs PAL) inside
Bus::advance_master. - Update scheduler/APU documentation to reflect the region-dependent denominator and the prior PAL slowdown.
- Add a core regression test asserting the NTSC/PAL divisor ratio matches the master-clock ratio, and document the fix in the changelog.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| docs/scheduler.md | Updates async-resync documentation to describe region-dependent SPC divisor and rationale. |
| docs/apu.md | Updates APU integration docs to describe the new region-dependent denominator behavior. |
| crates/rustysnes-core/src/bus.rs | Implements region-specific SPC divisor selection in the master-tick loop and adds a regression test. |
| CHANGELOG.md | Documents the PAL APU rate fix and verification strategy in Unreleased. |
| /// | ||
| /// **The APU's oscillator is region-independent and the master clock's is not**, so this ratio is | ||
| /// the one place in the core where which console a real machine is matters. The APU runs from its | ||
| /// own 24.576 MHz crystal at a fixed `2_050_560` Hz base rate in both regions; the master clock is |
| the SPC domain is bit-deterministic (`docs/adr/0004`). | ||
|
|
||
| **The denominator is the one place in the core where which console a real machine is matters, and | ||
| it was region-blind until `v1.29.0`.** The APU runs from its own 24.576 MHz crystal, so its rate is |
| - **The APU ran 0.92% slow on PAL: its clock divisor was pinned to the NTSC master rate.** | ||
| `Bus::advance_master` converts master ticks to SMP base clocks through a fixed rational, and the | ||
| denominator was `715_909` — the NTSC master clock — in **both** regions. The APU runs from its own | ||
| 24.576 MHz crystal, so its rate is the same on NTSC and PAL while the master clock's is not; |
Bus::advance_master converts master ticks to SMP base clocks through a fixed rational, and the denominator was 715_909 -- the NTSC master clock -- in BOTH regions. The APU runs from its own 24.576 MHz crystal, so its rate is the same on NTSC and PAL while the master clock's is not; holding the whole RATIO fixed therefore made the APU scale with the video clock. On PAL that is 21_281_370 x 68352/715909 = 2_031_850 base Hz against hardware's 2_050_560. Both references disagree with the old behaviour, from opposite directions. ares (sfc/system/system.cpp) region-sets cpuFrequency and never region-sets apuFrequency at all. snes9x (apu/apu.cpp) carries two explicit ratios, 15664/328125 and 34176/709379, which both work out to an APU rate of exactly 1,025,280 Hz and differ ONLY in the master-clock denominator -- 709_379 x 30 is 21_281_370, which is where the new constant comes from. The divisor is chosen from the PPU's region at the point of use rather than cached in Clock: the accumulator is already serialized, and a second field agreeing with it is one more thing that can disagree across a region change or a state restore. A stale doc comment had asserted the opposite -- sync_region_from_cart said "nothing else in the core depends on which oscillator frequency a real console would use". The SPC accumulator is exactly that one thing; the comment is corrected rather than left to mislead. docs/scheduler.md §async-resync and docs/apu.md updated in the same change. NTSC output is byte-identical. the_apu_rate_is_region_independent asserts the two divisors differ by exactly the ratio of the two master clocks, observed through emitted DSP samples so it needs no new counter; it fails when the region-blind behaviour is injected. The AccuracySNES battery (56 harness tests) and both images still cross-validate three ways. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
015e2c8 to
afc8137
Compare
…per tick Bot review, all three accepted. The region match sat inside the per-sub-tick loop -- the emulator's hottest path -- when nothing inside that loop can change the region. It is now resolved once per call. The test drained the audio FIFO once per DOT across ~4.2M iterations. It drains once per frame instead: ~640 samples a frame is far inside the 16,384-entry FIFO, so nothing is lost, and both regions still run over the same number of master ticks so the comparison is unchanged (APU-per-master-tick, not frame length). SPC_DEN_NTSC's summary now carries the unreduced 21_477_270 the way SPC_DEN_PAL's already carried 21_281_370. Re-verified: the test still fails when the region-blind behaviour is injected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
All three accepted.
Re-verified after the changes: the test still fails when the region-blind behaviour is injected, so the tightening did not cost the assertion. |
Antigravity review (Gemini via Ultra)This PR fixes PAL APU emulation timing by making the SPC700 master-clock denominator region-dependent (709,379 for PAL vs 715,909 for NTSC) so the APU runs at its native 24.576 MHz crystal frequency regardless of video region. Blocking issuesNone found. Suggestions
Nitpicks
Automated first-pass review by |
Bus::advance_masterconverts master ticks to SMP base clocks through a fixed rational, and the denominator was715_909— the NTSC master clock — in both regions.The APU runs from its own 24.576 MHz crystal, so its rate is the same on NTSC and PAL while the master clock's is not (21.477270 vs 21.281370 MHz). Holding the whole ratio fixed therefore made the APU scale with the video clock:
Both references disagree with the old behaviour, from opposite directions
ares (
sfc/system/system.cpp:70-81) region-setscpuFrequencyand leavesapuFrequency = 32040.0 * 768.0never region-set at all:snes9x (
apu/apu.cpp:20-23) carries two explicit ratios:Both work out to an APU rate of exactly 1,025,280 Hz, differing only in the master-clock denominator.
709_379 × 30 = 21_281_370— which is where the new constant comes from.The fix
SPC_DEN_NTSC/SPC_DEN_PAL, chosen from the PPU's region at the point of use rather than cached inClock. That is deliberate: the accumulator is already serialized, and a second field agreeing with it is one more thing that can disagree across a region change or a state restore — the trap a per-dot field hit previously.A stale doc comment had asserted the opposite.
sync_region_from_cartsaid "nothing else in the core depends on which oscillator frequency a real console would use." The SPC accumulator is exactly that one thing, because the APU's crystal is not the master clock's. Corrected rather than left to mislead the next reader;docs/scheduler.md§async-resync anddocs/apu.mdupdated in the same change.Verification
the_apu_rate_is_region_independentasserts the two divisors differ by exactly the ratio of the two master clocks, observed through emitted DSP samples so it needs no new counter. It passes with the fix and fails when the region-blind behaviour is injected.NTSC output is byte-identical — only the PAL path moves.
Cross-validation, both images:
snes9x: OK (15 known),Mesen2: OK (1 known),ares: OK (3 known),snes9x PAL: OK (15 known),Mesen2 PAL: OK (1 known), 54/54 scenes on both scene hosts,3 reference(s) agree with the cart.Found while designing
B2.07(NTSC 60.0988 Hz), which needs an APU-vs-video time ratio and so had to establish what the APU rate actually is.🤖 Generated with Claude Code
PAL APU timing was approximately 0.92% slow for PAL input because the SMP used the NTSC clock denominator. The change selects the denominator from the current PPU region, which corrects the PAL DSP sample rate. NTSC output remains byte-identical.
The claim is false if PAL still uses
SPC_DEN_NTSC, if a region switch or state restore selects the wrong denominator, or if NTSC output changes. Regression tests measure emitted DSP samples.No AccuracySNES dossier assertions were added or altered. AccuracySNES coverage remains 56 tests against 443 dossier assertions. The coverage denominator did not move.
Documentation records
715,909for NTSC and709,379for PAL.