Skip to content

Audio: revert clamps superseded by INT32 ramp overflow root-cause fix#153

Merged
JRickey merged 3 commits intomainfrom
agent/audio-revisit
May 8, 2026
Merged

Audio: revert clamps superseded by INT32 ramp overflow root-cause fix#153
JRickey merged 3 commits intomainfrom
agent/audio-revisit

Conversation

@JRickey
Copy link
Copy Markdown
Owner

@JRickey JRickey commented May 8, 2026

Summary

Two audio "fix" commits turned out to be band-aids over the int32 envelope-mixer ramp wrap that #108 / e0160c0 (audio_envmix_ramp_overflow_2026-05-08) addressed at the root. With the int32→int64 ramp clamp in place, voice samples no longer peak ±32767, so the downstream symptoms those band-aids were chasing don't materialize. Both reverts have been verified manually against the original fail conditions.

  • Revert d9e6058 ("pre-attenuate output low-pass biquad to absorb overshoot"). The biquad's 14.6% bilinear-warp overshoot only clipped because input was rail-pinned by the ramp wrap. Recovers ~1.4 dB on the output-filtered path. Verified: DK win-pose growl plays cleanly without kHeadroom = 0.85.
  • Revert d30a967 ("Fix N_AUDIO bus clipping for loud SFX") plus the dependent code added by 0297c35 (FX delay-line save/load mirror) and 5432b06's scoped fixed-bus clear. d30a967 had reintroduced int32 wet/dry bus accumulators that f0e2402 had previously removed for being non-canonical (OOT and SM64 use a saturating int16 DMEM bus with per-voice clamp16). With the ramp clamp in place, per-voice contributions stay in range, and the cumulative-clipping symptom doesn't surface. Verified: Sector Z multi-fighter brawls play cleanly against the canonical s16 saturating bus. −151 +20 net in port/audio/mixer.c.

The two superseded bug docs are kept and marked SUPERSEDED 2026-05-08 so the discovery history (and the discrete-IIR step-response analysis) remains traceable. The 14.5 kHz Butterworth output LPF that 5432b06 also introduced is unrelated (anti-imaging filter for content near Nyquist) and is kept.

Bonus: the same branch fixes scripts/new-worktree.sh to clone the Battle-ShipYard submodule alongside the others, which the original loop missed.

Test plan

  • DK win-pose growl plays cleanly (revert 1 fail-condition test).
  • Sector Z layered loud SFX play cleanly (revert 2 fail-condition test).
  • Smoke: BTT / 1P stages / sound test menu all play without harsh saturation regressions.
  • Smoke: reverb still audible on Hyrule Castle (sanity check that 0297c35's removed mirror isn't load-bearing in some path I didn't think through).
  • Verify the int32 ramp clamp continues to handle hard-pan edge SFX on stages other than Sector Z.

🤖 Generated with Claude Code

JRickey and others added 3 commits May 8, 2026 16:58
This reverts d9e6058. The 14.6% bilinear-warp overshoot it documented is
real, but the only reason it produced audible square-wave clipping was
that voice samples were being driven to int16 max — and that was the
INT32 envelope-mixer ramp wrap, fixed at the root in e0160c0
(audio_envmix_ramp_overflow_2026-05-08).

With the ramp clamp in place, voices no longer peak ±32767 (the e0160c0
verification shows L peak dropping 32767→27720 on the same scenario).
The biquad's worst-case input is now well under the rails, the discrete
step-response overshoot stays inside int16, and clampS16FromDouble
never flat-tops. DK win-pose growl plays cleanly without the kHeadroom
factor — verified manually in the worktree.

Recovers ~1.4 dB of headroom on the output-filtered path that was
previously thrown away to mask a different bug.

The bug doc is kept and marked SUPERSEDED; the discrete-IIR step-
response analysis remains a useful reference for future near-Nyquist
filter work.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This reverts d30a967, plus the dependent code added by 0297c35
("Fix reverb wet bus invisible to FX delay-line save/load") and
5432b06's scoped fixed-bus clear in aClearBufferImpl.

d30a967 reintroduced int32 wet/dry bus accumulators (dry_left_acc[],
wet_left_acc[], etc.) plus the fixed_bus_acc / sample_read /
sample_write / sample_add infrastructure to dodge "loud SFX bus
clipping." f0e2402 had previously removed the same int32-accumulator
approach (originally introduced in 69fe2a7) on the grounds that it
diverges from canonical N64 RSP behavior — both OOT (Ship of Harkinian)
and SM64 reference ports use a saturating int16 DMEM bus with per-voice
clamp16, not a wide accumulator clamped at the final stage.

The "loud SFX bus clipping" symptom that motivated d30a967 was actually
the same INT32 envelope-mixer ramp wrap fixed in e0160c0 manifesting
through a different path: the wrapped ramp pushed individual voice
contributions to ±32767, and stacking several of those on the
saturating bus produced the cumulative-clipping artifact. With the
ramp clamp in place, per-voice contributions stay in their intended
range, and Sector Z multi-fighter brawls play cleanly against the
canonical s16 saturating bus — verified manually in the worktree.

0297c35's aLoadBufferImpl/aSaveBufferImpl mirror logic existed only to
keep the int32 bus state coherent with the s16 DMEM region across the
FX delay-line DRAM round-trip. With the int32 accumulators gone, the
s16 region is the only state and memcpy preserves it correctly; the
mirror code is removed. Reverb is still audible because the bus path
now matches what it was before the d30a967/0297c35 layering.

5432b06's scoped fixed-bus clear in aClearBufferImpl is removed for
the same reason — there are no per-bus accumulators left to clear.
The 14.5 kHz Butterworth output low-pass filter that 5432b06 also
introduced is unrelated (anti-imaging concern, separate code path)
and is kept.

Net: 171 lines removed, audio path restored to the simpler RSP-
faithful clamp16 saturating bus. The two superseded bug docs
(audio_output_filter_overshoot_clip_2026-04-29 and
audio_int32_acc_save_load_bridge_2026-04-29) are kept and marked
SUPERSEDED so the discovery history remains traceable.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The submodule loop only iterated libultraship / torch / decomp; Battle-
ShipYard (the modkit that builds BattleShip.fromsource.o2r) was
registered in .gitmodules but never cloned into the worktree, which
broke cmake configure on any branch where CMakeLists.txt's
Battle-ShipYard/cmake/RelocData.cmake include is reachable.

The existing skip-if-not-configured guard makes this safe on older
base branches that don't yet track the submodule.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@JRickey JRickey merged commit be01c97 into main May 8, 2026
@JRickey JRickey deleted the agent/audio-revisit branch May 8, 2026 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant