fix(sid): play vsync tunes at the tune's own frame rate - #277
Merged
Conversation
The C64-side SID player chains PLAY onto the kernal's CIA #1 Timer A interrupt, which the KERNAL runs at ~60 Hz on *both* standards -- it is a wall-clock service (TI$, SCNKEY, cursor blink), not a frame interrupt. Nothing in the .sid path ever reprogrammed it, so a tune composed for PAL's 50.12 Hz ran at 60.0 on a PAL machine as much as an NTSC one: +19.7% tempo, across roughly 80% of a full HVSC. [ultimate64].sid_play_rate (default "auto") sets the latch to the tune's own frame rate. "off" keeps the previous behaviour for anyone who knows these tunes at NTSC speed; a number pins every vsync tune to one rate. The latch is written after INIT, not before: the Ultimate kicks the player via run_prg, which soft-resets the C64, and the KERNAL's reset path reloads Timer A. Two gates keep it off self-timed tunes -- the header's per-subtune speed flag must say vsync, and the latch actually in place after INIT must not already look like a multispeed one. The sample burst behind that second gate went 8 -> 16 reads: the max of a free-running down-counter is biased low with a fat tail, and an 8-sample burst against a kernal 60 Hz jiffy measured 75.6 Hz on hardware. Also fixed: both the ASID ring player and the REU audio pump wrote $4025 back at teardown while documenting it as the NTSC default. $4025 is PAL's; NTSC is $4295. Both now go through c64.kernal_cia1_latch(system). Pitch is a separate error with a separate lever, so it gets separate settings. sid_video_mode (default "off") retimes the machine itself via the Ultimate 64's System Mode, correcting the 3.8% clock difference; hdmi_scan_resolution (default "auto") raises SD to HD when it does, since PAL timing at SD puts 576p50 on the wire and not every capture device locks to that. Both live, volatile, restored at teardown. [ultimate64].system now defaults to "auto" and is read from the machine's live System Mode. One field feeds the CPU clock, frame rate, DAC NMI latches and PLAY rate, and a hand-set value that disagreed moved all of them at once, silently. An explicit value still wins, warns on disagreement, and is an error-level --doctor finding. Verified on hardware: PAL vsync tune retuned to 50.12 Hz (latch $4CC7), NTSC tune to 59.83, a multispeed tune left self-timed at ~204 Hz, the System Mode switch and both restores round-tripping, and the SD -> HD raise and its restore.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #277 +/- ##
==========================================
- Coverage 82.08% 82.08% -0.01%
==========================================
Files 142 142
Lines 24788 25013 +225
Branches 3639 3675 +36
==========================================
+ Hits 20347 20531 +184
- Misses 3647 3677 +30
- Partials 794 805 +11 ☔ View full report in Codecov by Harness. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The defect
The C64-side SID player chains PLAY onto the kernal's CIA #1 Timer A interrupt.
That interrupt is a wall-clock service — TI$, SCNKEY, cursor blink — not a
frame interrupt, so the KERNAL programs it to ≈60 Hz on both standards
(
$4025PAL,$4295NTSC). Nothing in the.sidpath ever reprogrammed it.A tune composed for PAL's 50.12 Hz therefore played at 60.0 — +19.7% tempo —
on a PAL machine as much as on an NTSC one. That's roughly 80% of a full HVSC
(PAL + vsync); PAL CIA-timed tunes (~10%) self-time from their own INIT and drift
only by the clock ratio.
Tempo and pitch are two separate errors with two separate levers, and keeping
them apart is what shapes this change:
sid_play_rate)sid_video_mode)Hence the defaults: the tempo fix is on, the pitch fix is opt-in.
What's here
[ultimate64].sid_play_rate(default"auto") sets the latch to the tune'sown frame rate.
"off"is exactly the previous behaviour — a real preference,since plenty of people know these tunes at the speed an NTSC machine played
them — and an explicit number in Hz pins every vsync tune to one rate.
The latch is written after INIT, not before: the Ultimate kicks the player
via
run_prg, which soft-resets the C64, and the KERNAL's reset path reloadsTimer A, so a pre-kick write is gone before the first PLAY. Two gates keep it off
self-timed tunes — the header's per-subtune speed flag must say vsync, and the
latch actually in place after INIT must not already look like a multispeed one.
That second gate is what protects tunes whose header lies.
[ultimate64].systemdefaults to"auto", read from the machine's liveSystem Mode. This one field feeds the CPU clock, the frame rate, the DAC NMI
latches and the PLAY rate, so a hand-set value that disagreed moved all of them
at once, silently. An explicit value still wins (it's how you describe a
TeensyROM-driven C64), warns on disagreement, and is an error-level
--doctorfinding.
[ultimate64].sid_video_mode(default"off") retimes the machine via theUltimate 64's System Mode, correcting pitch.
[ultimate64].hdmi_scan_resolution(default
"auto") raises SD to HD when it does — PAL timing at SD puts 576p50 onthe wire and not every capture device locks to that, while the same machine at
720p50 captures cleanly.
"auto"only acts when c64cast itself retimed themachine: clean up after our own change, leave a machine we didn't touch alone.
Two bugs found on the way
the REU audio pump both wrote
$4025back at teardown while documenting it asthe NTSC default.
$4025is PAL's; NTSC is$4295. The jiffy clock ran ~3.8%fast after either teardown until the next reset.
$DC04/$DC05are write-only, so thelatch is estimated as the max over a burst of reads of a free-running
down-counter — which averages n/(n+1) of the true value with a fat tail. At
n=8, a burst against a kernal 60 Hz jiffy measured 75.6 Hz on hardware.
Raised to 16, which keeps that tail off the self-timed gate.
Note on the System Mode enum
The
-50/-60suffix selects the machine timing; thePAL/NTSCprefix selectsonly the analog chroma encoding. So
NTSC-50is a PAL-timed machine andPAL-60is an NTSC-timed one, and over HDMI the pairs are identical.
SYSTEM_MODE_TIMINGtherefore looks backwards on purpose, and says so.
On composite the prefix is the whole colour signal, so retiming costs more there
than over HDMI: the chroma encoding is preserved, but the field rate changes
underneath it and a single-standard set may not lock. Documented in caveats and
both books.
Verification
Full gate green:
make check,make lint,make typecheck(mypy --strict +pyright),
make doctor,make site-check,make books. Schema and generatedappendices regenerated.
On hardware:
$4CC7), confirmed by thepost-write sample reading ~50 Hz.
sid_play_rate = "off"→ no retune, previous behaviour intact.PAL → PAL-60, C64 reset, restore toPAL; the retune thenused the NTSC-clock latch (
$42C6), confirming the profile re-folded after theswitch.
--doctorreports both the agreeing and the disagreeingsystemcases.