fix(apu): v2.2.7 "Timbre II" — recalibrate VRC6 to ~1.0x + exact 5B envelope DAC - #344
Open
doublegate wants to merge 3 commits into
Open
fix(apu): v2.2.7 "Timbre II" — recalibrate VRC6 to ~1.0x + exact 5B envelope DAC#344doublegate wants to merge 3 commits into
doublegate wants to merge 3 commits into
Conversation
|
Warning Review limit reached
Next review available in: 55 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughChangesExpansion audio fidelity
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 9✅ Passed checks (9 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
doublegate
force-pushed
the
feat/v2.2.7-timbre-ii-audio
branch
from
August 4, 2026 14:14
d1810d9 to
92428fc
Compare
There was a problem hiding this comment.
Pull request overview
Updates RustyNES to the v2.2.7 “Timbre II” release, focusing on expansion-audio fidelity by re-scaling VRC6 output to ~1.0× a 2A03 pulse and updating Sunsoft 5B envelope-mode volume to use the exact 5-bit 1.5 dB/step DAC curve. It also refreshes release/version documentation and the relevant audio oracle tests/snapshots.
Changes:
- Recalibrate VRC6 mixer scaling (
VRC6_MIX_SCALE979 → 650) and updatedb_vrc6a/boracle targets + snapshots. - Implement Sunsoft 5B envelope-mode DAC at full 5-bit resolution via a new 32-entry log table, with unit tests to lock invariants.
- Bump workspace/release version to 2.2.7 and update README/STATUS/docs/changelog accordingly.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Bumps displayed version and updates “Current Release” narrative + citation version. |
| docs/STATUS.md | Updates status header to v2.2.7 release summary. |
| docs/expansion-audio.md | Updates expansion-audio relative-level documentation (includes VRC6 + 5B envelope notes). |
| docs/apu-2a03.md | Updates the expansion-audio levels section to reflect VRC6 recalibration and 5B envelope DAC completion. |
| docs/accuracy-ledger.md | Updates ledger rows for VRC6 recalibration and 5B envelope DAC completion. |
| crates/rustynes-test-harness/tests/audio_expansion.rs | Updates VRC6 level oracle targets from ~1.506× to ~1.0×. |
| crates/rustynes-test-harness/tests/snapshots/audio_expansion__audio_expansion_db_vrc6a.snap | Re-blesses audio snapshot hash for VRC6a at new level. |
| crates/rustynes-test-harness/tests/snapshots/audio_expansion__audio_expansion_db_vrc6b.snap | Re-blesses audio snapshot hash for VRC6b at new level. |
| crates/rustynes-mappers/src/m069_sunsoft_fme7.rs | Adds 5B 32-step envelope DAC table and routes envelope mode through it (plus unit tests). |
| crates/rustynes-mappers/src/m024_vrc6.rs | Changes VRC6 scale constant and updates documentation/tests around expected mix output. |
| CHANGELOG.md | Adds v2.2.7 changelog entry describing the VRC6 + 5B envelope changes. |
| Cargo.toml | Bumps workspace.package version to 2.2.7. |
| Cargo.lock | Regenerates lockfile versions for workspace crates at 2.2.7. |
| AGENTS.md | Updates the “Current release” narrative to v2.2.7. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…Timbre II") v2.2.7 "Timbre II" is an expansion-audio accuracy release addressing NESdev-forum feedback (VRC6 "heavily imbalanced", Sunsoft 5B "off"). It is the product of a measure-first cross-reference of VRC6 and Sunsoft 5B against 11 reference emulators (Mesen2/MesenCE, ares, higan, nestopia, fceux, tetanes, rustico, GeraNES, puNES, BizHawk) plus the NESdev wiki -- prompted by the observation that a Mesen2-only comparison hides exactly the cases where Mesen2 is itself the outlier. These are expansion-only changes: `Mapper::mix_audio()` returns 0 for every non-expansion mapper, so the base 2A03 mix is an untouched additive term. The base 2A03 output is therefore byte-identical -- AccuracyCoin holds 141/141 (100.00%), nestest is 0-diff, and blargg/kevtris are unchanged. The base BLEP decimator was independently re-verified excellent (SFDR 81.6 dB via the rustynes-apu spectral test), confirming the reviewer's "harsh aliasing" is not in the base synthesis path. VRC6 level -- recalibrated to the field/hardware consensus - A full-volume VRC6 pulse is now ~1.0x a 2A03 pulse (was ~1.506x). `VRC6_MIX_SCALE` 979 -> 650 in `m024_vrc6.rs` (15*650/65536 = 0.1488 = the 2A03 `pulse_table[15]`, ratio 1.00). - Rationale: the prior 1.506x mirrored Mesen2's specifically LOUDER mixer convention (Mesen2 weights VRC6 `output*5` in `NesSoundMixer::GetOutputVolume`). The cross-reference shows Mesen2 is the loud outlier: the NESdev wiki says "at maximum volume, the pulse channels of the VRC6 are roughly equivalent to the pulse channels of the 2A03", and rustico / tetanes / BizHawk each encode a VRC6 pulse == a 2A03 pulse EXACTLY (ares/higan/nestopia reach the same via a `sum/61` normalization). bbbradsmith's `db_vrc6` matched-level intent agrees. - The `db_vrc6a/b` oracle target moved 1.506 -> 1.000 (`audio_expansion.rs`, verified passing). The two `db_vrc6a/b` snapshots were re-blessed: ONLY `audio_fnv1a64` changed -- the framebuffer FNV and cycle count are byte-identical, a pure audio-level change. - VRC6's per-channel balance (linear pulse+pulse+saw, saw 0-31 vs pulse 0-15) was already correct and matches the whole field; it is unchanged. Sunsoft 5B envelope -- exact 5-bit 1.5 dB/step DAC - Added `SUNSOFT5B_LOG_VOL32` (32 entries, x1.1885/step = +1.5 dB), and the envelope-mode amplitude path now indexes it with the FULL 5-bit envelope level instead of truncating to 4-bit via `>> 1`. The 4-bit truncation was the "3 dB per step" approximation the NESdev wiki explicitly names as inexact; the full 5-bit 1.5 dB/step DAC matches nestopia and rustico (the field's stricter references). - Fixed 4-bit volume tones (already the correct 3 dB/step, being the 4-bit projection of the same DAC) and the 5B absolute level (1.265x) are unchanged. The odd entries of the 32-level table equal the 16-entry 4-bit table exactly, guarded by the new `log_vol32_odd_entries_match_4bit` unit test. `volume()` -> `amplitude()` (returns the DAC value, not the 4-bit index). - All committed 5B snapshots stayed byte-identical (the extant 5B test ROMs do not exercise even envelope levels); envelope-modulated 5B music now gets the exact curve. Verification - `cargo test -p rustynes-test-harness --features test-roms --test audio_expansion`: 25/25 (VRC6 oracle now 1.0, all 5B green). `cargo test -p rustynes-mappers` VRC6 + 5B + parity unit tests green. `cargo fmt --all --check`, `cargo clippy --workspace --all-targets -D warnings`, `RUSTDOCFLAGS=-D warnings cargo doc`, markdownlint all clean. Docs (`apu-2a03.md`, `expansion-audio.md`, `accuracy-ledger.md`) updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
doublegate
force-pushed
the
feat/v2.2.7-timbre-ii-audio
branch
from
August 4, 2026 16:26
92428fc to
0fdf906
Compare
…B gap, README subject) - docs/expansion-audio.md + docs/apu-2a03.md: the 'levels match hardware and Mesen2' framing was self-contradictory now that VRC6 is recalibrated AWAY from Mesen2 (the loud outlier). Reworded to 'calibrated against the reference-emulator field; Mesen2 was the historical bar but is the VRC6 outlier'. - docs/expansion-audio.md: removed the stale claim that the Sunsoft 5B absolute level is a deferred i16-contract gap — that was remediated in v2.2.3 (i32 widening + level_db_5b). Only VRC7 FM remains an honest level gap. - README.md: disambiguated 'zero emulation-core behavior changes' to explicitly attribute it to v2.2.6 (not v2.2.7, which intentionally changes expansion audio). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@AGENTS.md`:
- Line 30: Update the stale current-release operating note in AGENTS.md from
v2.2.6 to v2.2.7, including its prohibition on later version claims, so it
matches the current release declaration already present in the document.
Preserve the surrounding repository-instruction wording and release guidance.
In `@CHANGELOG.md`:
- Around line 17-48: Add a concise `[Unreleased]` changelog entry summarizing
the VRC6 level recalibration and Sunsoft 5B envelope DAC changes described in
the 2.2.7 section. Place it under the existing `[Unreleased]` heading and keep
the detailed release entry unchanged.
In `@docs/apu-2a03.md`:
- Around line 340-353: Update stale documentation that no longer matches the
implementation. In docs/apu-2a03.md at line 325, change the Mapper::mix_audio
hook signature return type from i16 to i32. In docs/expansion-audio.md at line
8, update the same hook signature to return i32 instead of i16, and remove the
obsolete Sunsoft 5B absolute-level deferral text at lines 43-45 that claims the
level calibration is incomplete—the implementation now completes this
calibration with the trait returning i32 and using SUNSOFT5B_MIX_SCALE_NUM/DEN
for the level.
🪄 Autofix
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: e8015b82-0e24-4563-80c1-56fa32a4f01e
⛔ Files ignored due to path filters (3)
Cargo.lockis excluded by!**/*.lock,!Cargo.lockcrates/rustynes-test-harness/tests/snapshots/audio_expansion__audio_expansion_db_vrc6a.snapis excluded by!**/*.snapcrates/rustynes-test-harness/tests/snapshots/audio_expansion__audio_expansion_db_vrc6b.snapis excluded by!**/*.snap
📒 Files selected for processing (12)
AGENTS.mdCHANGELOG.mdCargo.tomlREADME.mdcrates/rustynes-mappers/src/lib.rscrates/rustynes-mappers/src/m024_vrc6.rscrates/rustynes-mappers/src/m069_sunsoft_fme7.rscrates/rustynes-test-harness/tests/audio_expansion.rsdocs/STATUS.mddocs/accuracy-ledger.mddocs/apu-2a03.mddocs/expansion-audio.md
…bbit #344) Line 188's current-release label + the 'never claim later than' guard still read v2.2.6; line 30 was already v2.2.7. Bring both to v2.2.7 'Timbre II'. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Antigravity review (Gemini via Ultra)This PR updates workspace crate versions to v2.2.7, recalibrates the VRC6 expansion audio mix scale from 979 to 650 to target ~1.0x 2A03 pulse output, and upgrades Sunsoft 5B envelope modulation to an exact 32-level 1.5 dB/step logarithmic DAC table. Blocking issuesNone found. Suggestions
Nitpicks
Automated first-pass review by |
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.
v2.2.7 "Timbre II" — expansion-audio fidelity (VRC6 + Sunsoft 5B)
Second of the v2.2.6 → v2.3.0 NESdev-remediation line. Addresses the forum feedback
that VRC6 is "heavily imbalanced" and Sunsoft 5B is "off", via a measure-first
cross-reference against 11 reference emulators (Mesen2/MesenCE, ares, higan, nestopia,
fceux, tetanes, rustico, GeraNES, puNES, BizHawk) + the NESdev wiki — because a
Mesen2-only comparison hides where Mesen2 is itself the outlier.
Base 2A03 is byte-identical
Expansion-only changes (
mix_audio()==0for non-expansion mappers), so AccuracyCoinholds 141/141 (100.00%), nestest is 0-diff, blargg/kevtris unchanged. The base BLEP was
independently verified excellent — SFDR 81.6 dB (
rustynes-apuspectral test) — so thereviewer's "harsh aliasing" is not in the base synthesis path.
VRC6 → ~1.0× a 2A03 pulse (was ~1.506×)
VRC6_MIX_SCALE979 → 650. The 1.506× mirrored Mesen2's louder mixer convention(weights VRC6
×5); the field says otherwise — the NESdev wiki ("roughly equivalent tothe pulse channels of the 2A03") and rustico / tetanes / BizHawk encode a VRC6 pulse ==
a 2A03 pulse exactly (ares/higan/nestopia via
sum/61).db_vrc6a/boracle target1.506 → 1.000; the two snapshots re-blessed (only
audio_fnv1a64moved — framebuffer +cycle count byte-identical). VRC6 per-channel balance was already correct, unchanged.
Sunsoft 5B envelope → exact 5-bit 1.5 dB/step DAC
New
SUNSOFT5B_LOG_VOL32(32-level, ×1.1885/step). The envelope path now indexes it atfull 5-bit resolution instead of truncating to 4-bit via
>> 1(the "3 dB/step"approximation the NESdev wiki explicitly names inexact) — matching nestopia/rustico.
Fixed 4-bit volume (already correct 3 dB/step) and the 5B level (1.265×) unchanged; the
odd entries of the 32-level table equal the 4-bit table exactly (new unit test). All 5B
snapshots stayed byte-identical; envelope-modulated music now gets the exact curve.
Checks
audio_expansion25/25 · mappers VRC6 + 5B + parity unit tests green ·cargo fmt --check·cargo clippy --workspace --all-targets -D warnings·RUSTDOCFLAGS=-D warnings cargo doc· markdownlint — all clean. Docs updated (apu-2a03.md,expansion-audio.md,accuracy-ledger.md).🤖 Generated with Claude Code
Summary by CodeRabbit