feat(sid): pick tunes the C64's own SID chips can play - #272
Merged
Conversation
A tune the machine doesn't suit stays that way on a link that can't re-place chips — the chip is what it is, and the resolved-audio verdict can only say so. The free variable is which tune: a waveform scene whose `file` spec resolves to a directory or glob already picks one at random per setup(), and it can pick with the declared chips in mind. [hardware].host_sid_tune_match adds "prefer" (fitting tunes first, the rest as a fallback tail) and "require" (misfits dropped). A tune fits when the declarations would produce a clean verdict for it, and host_chip_fit routes through the same renderers as that verdict rather than re-deriving the match — a picker that disagreed with the line logged moments later would be worse than no picker. Candidates are judged from the PSID header alone, so an HVSC-sized directory costs short reads rather than a load per tune. Three cases return no opinion and leave the pool untouched: a link that reads the real SID state (it re-places chips per tune), an assumed NTSC/PAL model (weak enough to warn on, not strong enough to drop files out of someone's directory), and nothing declared at all. Default is "off" for the same reason — a directory the user pointed at is a statement of what they want played. "require" falls back to the full pool with a warning when nothing fits, so a mis-declared machine surfaces as a log line, not a scene that can never start. Only host_sid_chips can skip a 2SID tune: host_sid_model names one chip without claiming it is the only one, and inferring a chip count from it would reject working tunes on a machine whose second chip we were never told about.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #272 +/- ##
==========================================
+ Coverage 82.00% 82.02% +0.02%
==========================================
Files 142 142
Lines 24655 24705 +50
Branches 3612 3622 +10
==========================================
+ Hits 20219 20265 +46
- Misses 3645 3647 +2
- Partials 791 793 +2 ☔ 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.
What
Follow-up to #271. That PR made c64cast describe a machine's internal SID chips accurately. This one acts on the description in the only place it can: tune selection.
On a link that can't re-place chips, a tune the machine doesn't suit stays that way — the chip is what it is, and the resolved-audio verdict can only say so. But a
waveformscene whosefilespec resolves to a directory or glob already picks a random candidate persetup(), and it can pick with the declared chips in mind.off(default)preferrequireA tune fits when the declarations would produce a clean verdict for it — right model on every chip, and a chip declared at every address the tune drives. So a single-SID machine stops landing on 2SID tunes whose second voice-set goes nowhere, and a 6581 machine stops landing on tunes composed on an 8580.
Design notes
The predicate shares the verdict's renderers.
host_chip_fitcallsdescribe_declared_chips/describe_declared_audiorather than re-deriving the match. A picker that disagreed with the line logged moments later would be worse than no picker at all.Only the PSID header is read (
_HOST_FIT_HEADER_BYTES = 0x80— the header ends at$7Cand the extra-SID address bytes are its last relevant fields), so ordering an HVSC-sized directory costs short reads, not a load per tune.Three no-opinion cases leave the pool exactly as it was:
Default is
offfor the same reason: a directory the user pointed at is a statement of what they want played, and quietly narrowing it is their call.requirestill falls back to the unfiltered pool, with aWARNING, when nothing fits — a mis-declared machine surfaces as a log line rather than a scene that can never start.The two declarations buy different things.
host_sid_chipsdescribes the whole machine, so an undeclared tune address fails — that is what skips 2SID tunes.host_sid_modeljudges the primary chip's model alone: it names a chip without claiming it is the only one, and inferring a chip count from it would reject working tunes on a machine whose second chip we were never told about.The shuffle happens first, so the grouping is a bias on a random order rather than a reordering that would make one tune the deterministic pick for a whole class of machines.
Verification
make check(ruff,mypy --strict, pyright, 3729 tests) andmake site-checkgreen.29 new/changed tests: the predicate across all declaration shapes and no-opinion cases (
test_sid_resolved.py), profile resolution (test_backend.py), field validation (test_config.py), and pool ordering end-to-end through scene construction (test_waveform.py) — prefer/require/off, 2SID skipping, the assumed-model no-op, the require fallback warning, and an unreadable candidate sorting to the back.Also exercised offline against real HVSC files, which the synthetic test headers don't cover:
requirereduces a pool of four real 2SID tunes plus one 1SID tune to the 1SID tune;$D400/$D420correctly rejects both the 6581+6581 and 8580+8580 2SID tunes;No hardware run: this change adds no device I/O. It selects a different file before the existing player path runs, and everything downstream of the pick is untouched.