Implement missing XM effects and improve mixing/spec compliance - #33
Implement missing XM effects and improve mixing/spec compliance#33mrdoob wants to merge 93 commits into
Conversation
Smaller buffer gives ~21 callbacks/sec at 44100Hz instead of ~2.7, making oscilloscope and pattern view updates much smoother. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds tremolo effect which oscillates volume using the same waveform types as vibrato. Adds voloffset support to the mixer so tremolo (and later tremor) can modify volume without changing ch.vol. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Allows setting the tremolo oscillator waveform type, same as E4x does for vibrato (0=sine, 1=ramp-down, 2=square, 4-7=continuous variants). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When enabled (E31), portamento slides are rounded to the nearest semitone. Set E30 to disable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Retriggers the current note every x ticks within the row. Unlike Rxy, this is a simple retrigger without volume change. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Slides the channel panning left (y) or right (x), with parameter memory like volume slide. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
X1x slides pitch up by x units (1/4 the resolution of fine porta E1x). X2x slides pitch down similarly. Both are tick-0 only effects. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Triggers a key-off (note release) at the specified tick within the row, allowing the volume envelope's release phase to begin. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sets the volume and panning envelope tick position to the specified value, allowing jumps within the envelope. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Rapidly switches channel volume on and off. x+1 ticks on, y+1 ticks off. Uses voloffset to mute during off phase without modifying ch.vol. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Delays the note trigger by x ticks. On tick 0, the note data is stored but not triggered. When the specified tick is reached, triggerNote() fires and the note plays. This enables echo/groove effects. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Repeats the current row for x additional tempo cycles before advancing to the next row. Only the first EEx command per row takes effect. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds support for panning slide left (D0-DF) and panning slide right (E0-EF) in the volume column. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
A few demodulate tracks still do not play correctly. Investigating... |
|
Okay, tracks sound much better now! |
|
LMAO! thanks mrdoob and Claude! will take a look... |
|
can we separate the worklet stuff from the effects work? I realize it's way past time to move on from ScriptProcessorNode... |
|
curious: which reference are you using for the proper effect definitions? did you track down the ft2 source? |
I'm using this as reference: |
Okay, once I'm done with a few more improvements I'll make a fork before the worklet commit and ask it to reimplement the improvements from the other branch. I'll leave the worklet change out of the PR. |
- Effects 9, E1x, E2x, X1x, X2x now remember parameters per spec - K00 triggers key-off on tick 0, EC0 cuts note on tick 0 - Panning formula matches spec: (EnvPan-32)*(128-|Pan-128|)/32 - Proper per-tick FadeOut tracking instead of baked-in envelope - Envelope loop continues after release (only sustain stops) - Sustain check properly gated on sustain flag (type bit 1) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Read vibrato type/sweep/depth/rate from instrument header. Apply pitch modulation every tick with gradual sweep-in. Supports all 4 waveform types: sine, square, ramp up, ramp down. Sweep freezes on key-off per FT2 behavior. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The portamento special handler reset ch.release when it detected a released note, which meant key-off (^^^) + portamento (3xx) would never actually stop the sound. Now only reset release when a real note (0-95) retriggers with portamento, not for key-off (96). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The accumulator was clamped with Math.max/Math.min (float range) instead of wrapping like a real int16. This caused 16-bit samples (e.g. hihats) to decode incorrectly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Pad each sample with one extra value so linear interpolation can safely read samp[k+1] at loop boundaries without out-of-bounds access. Looping samples wrap to loop start; non-looping pad with 0. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove the 2-pole Butterworth filter from the mixing pipeline and use linear interpolation between adjacent samples instead. This matches ft2-clone behavior and produces a cleaner, less muffled sound. Uses sample padding (previous commit) to avoid out-of-bounds reads at loop boundaries. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
FT2 deactivates the voice when sample position >= sampleEnd (including loop end for looping samples). Previously looping samples would wrap the position back into the loop producing unintended sound. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Found a song that does't play correctly. |
Note value 0 (C-0) was treated as falsy by `if (ch.note)` checks, skipping period calculation entirely. Notes played at the default channel period instead of the correct one. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Pheww, fixed. |
|
Hmm, one note is wrong in Edit: Never mind, all good! |
|
first impressions: the anti-pop volume ramp is way too slow, it completely eats the transients on snare drums. I actually find that unlistenable -- Something is odd about it, though; setting quickRampSamples to a tiny value fixes the first drum hit, but all subsequent ones are still muddy. It turns out that it's using a full tick length to ramp later note triggers rather than the quickRamp value. codex suggests this fix, and it sounds much better: diff --git a/xm.js b/xm.js
index 690b2ca..d041166 100644
--- a/xm.js
+++ b/xm.js
@@ -38,7 +38,8 @@ var _note_names = [
var f_smp = 44100; // updated by play callback, default value here
-var quickRampSamples = Math.round(f_smp / 200); // ~5ms crossfade
+var quickRampSamples = Math.max(1, Math.round(f_smp / 200)); // ~5ms crossfade
+player.quickRampSamples = quickRampSamples;
// Pre-allocated VU buffer for audio callback (sized on first use)
var vuBuffer = null;
@@ -83,6 +84,21 @@ function getstring(dv, offset, len) {
return str.join('');
}
+function getQuickRampSamples() {
+ var n = player.quickRampSamples;
+ if (typeof n !== "number" || !isFinite(n)) n = quickRampSamples;
+ n = n | 0;
+ if (n < 1) n = 1;
+ return n;
+}
+
+function startVoiceQuickRamp(ch) {
+ ch.vL = 0;
+ ch.vR = 0;
+ ch.rampSamplesLeft = getQuickRampSamples();
+}
+player.startVoiceQuickRamp = startVoiceQuickRamp;
+
// Amiga period LUT (1936 entries, stored in 1/4-scale to match JS period convention)
// FT2 formula: round(109568 / 2^((368+i)/192))
var amigaPeriodLUT = new Float64Array(1936);
@@ -199,13 +215,14 @@ function keyOff(ch) {
function snapshotFadeVoice(ch) {
if (ch.inst && ch.samp && ch.vL + ch.vR > 0) {
+ var ramp = getQuickRampSamples();
var fv = ch.fadeVoice || (ch.fadeVoice = {});
fv.inst = ch.inst; fv.samp = ch.samp;
fv.off = ch.off; fv.doff = ch.doff;
fv.vL = ch.vL; fv.vR = ch.vR;
- fv.volDeltaL = -ch.vL / quickRampSamples;
- fv.volDeltaR = -ch.vR / quickRampSamples;
- fv.rampSamplesLeft = quickRampSamples;
+ fv.volDeltaL = -ch.vL / ramp;
+ fv.volDeltaR = -ch.vR / ramp;
+ fv.rampSamplesLeft = ramp;
}
}
player.snapshotFadeVoice = snapshotFadeVoice;
@@ -371,8 +388,7 @@ function nextRow() {
triggerInstrument(ch, inst);
}
// new voice ramps up from zero
- ch.vL = 0; ch.vR = 0;
- ch.rampSamplesLeft = 0;
+ startVoiceQuickRamp(ch);
}
// handleEffects_TickZero: volume column effects (named functions defined above nextRow)
@@ -476,8 +492,7 @@ function triggerNote(ch) {
ch.pan = (d.volColumn & 0x0f) << 4;
}
// new voice ramps up from zero
- ch.vL = 0; ch.vR = 0;
- ch.rampSamplesLeft = 0;
+ startVoiceQuickRamp(ch);
}
player.triggerNote = triggerNote;
player.triggerInstrument = triggerInstrument;
@@ -661,23 +676,30 @@ function MixChannelIntoBuf(ch, start, end, dataL, dataR) {
// non-looping: position past end, voice naturally done
}
ch.off = newOff;
+ ch.rampSamplesLeft = 0;
return 0;
}
// FT2: if position already past sample end (e.g. 9xx offset), voice is inactive
if (ch.off >= sample_end) {
ch.vL = volL; ch.vR = volR;
+ ch.rampSamplesLeft = 0;
return 0;
}
var k = ch.off;
var dk = ch.doff;
var Vrms = 0;
- // linear per-sample volume ramp (tick-length)
+ // linear per-sample volume ramp (tick-length, or quick note-on ramp)
var ticklen = end - start;
var vL = ch.vL;
var vR = ch.vR;
- var volDeltaL = (volL - vL) / ticklen;
- var volDeltaR = (volR - vR) / ticklen;
+ var rampSamples = ticklen;
+ if (ch.rampSamplesLeft > 0) {
+ rampSamples = Math.min(rampSamples, ch.rampSamplesLeft);
+ }
+ var rampLeft = rampSamples;
+ var volDeltaL = (volL - vL) / rampSamples;
+ var volDeltaR = (volR - vR) / rampSamples;
var i = start;
var failsafe = 100;
@@ -693,14 +715,20 @@ function MixChannelIntoBuf(ch, start, end, dataL, dataR) {
ch.lastSample = samp[Math.min(k | 0, samp.length - 1)] || 0;
// snap to target volume (not mid-ramp) so silence path works on next tick
ch.vL = volL; ch.vR = volR;
+ ch.rampSamplesLeft = 0;
return Vrms;
}
}
var next_event = Math.max(1, Math.min(end, i + (sample_end - k) / dk));
+ var segEnd = next_event;
+ if (rampLeft > 0) {
+ segEnd = Math.min(segEnd, i + rampLeft);
+ }
+ var segStart = i;
// unrolled 8x with linear interpolation
var ki, kf, s;
- for (; i + 7 < next_event; i+=8) {
+ for (; i + 7 < segEnd; i+=8) {
ki=k|0; kf=k-ki; s=samp[ki]+(samp[ki+1]-samp[ki])*kf; k+=dk;
dataL[i]+=vL*s; dataR[i]+=vR*s; Vrms+=(vL+vR)*s*s; vL+=volDeltaL; vR+=volDeltaR;
ki=k|0; kf=k-ki; s=samp[ki]+(samp[ki+1]-samp[ki])*kf; k+=dk;
@@ -719,19 +747,31 @@ function MixChannelIntoBuf(ch, start, end, dataL, dataR) {
dataL[i+7]+=vL*s; dataR[i+7]+=vR*s; Vrms+=(vL+vR)*s*s; vL+=volDeltaL; vR+=volDeltaR;
}
- for (; i < next_event; i++) {
+ for (; i < segEnd; i++) {
ki=k|0; kf=k-ki; s=samp[ki]+(samp[ki+1]-samp[ki])*kf;
dataL[i]+=vL*s; dataR[i]+=vR*s;
Vrms+=(vL+vR)*s*s;
vL+=volDeltaL; vR+=volDeltaR;
k+=dk;
}
+
+ if (rampLeft > 0) {
+ rampLeft -= segEnd - segStart;
+ if (rampLeft <= 0) {
+ rampLeft = 0;
+ vL = volL;
+ vR = volR;
+ volDeltaL = 0;
+ volDeltaR = 0;
+ }
+ }
}
ch.off = k;
ch.lastSample = s;
// snap to target to avoid float drift
ch.vL = volL;
ch.vR = volR;
+ ch.rampSamplesLeft = rampLeft;
return Vrms * 0.5;
}
@@ -1173,7 +1213,7 @@ function init() {
}
// compute quickRampSamples once from actual sample rate
f_smp = player.audioctx.sampleRate;
- quickRampSamples = Math.round(f_smp / 200);
+ quickRampSamples = Math.max(1, Math.round(f_smp / 200));
player.quickRampSamples = quickRampSamples;
if (player.audioctx.createScriptProcessor === undefined) {
jsNode = player.audioctx.createJavaScriptNode(16384, 0, 2);
diff --git a/xmeffects.js b/xmeffects.js
index 75cb822..357ef05 100644
--- a/xmeffects.js
+++ b/xmeffects.js
@@ -257,8 +257,7 @@ function eff_t1_e(ch) { // extended effects tick 1+
ch.period = player.periodForNote(ch, ch.note);
}
ch.off = 0;
- ch.vL = 0; ch.vR = 0;
- ch.rampSamplesLeft = 0;
+ player.startVoiceQuickRamp(ch);
// FT2: triggerInstrument(ch) — reset envelopes, fadeout, vibrato, etc.
player.triggerInstrument(ch, inst);
}
@@ -356,8 +355,7 @@ function doMultiNoteRetrig(ch) {
// FT2: triggerNote restarts voice with quick volume ramp (crossfade)
player.snapshotFadeVoice(ch);
ch.off = 0;
- ch.vL = 0; ch.vR = 0;
- ch.rampSamplesLeft = 0;
+ player.startVoiceQuickRamp(ch);
}
}
|
|
I'll take a look later today 👌 |
|
Sounds good to me! Patch applied. |
The volume ramp on note triggers was using the full tick length instead of the short ~5ms quick ramp, eating snare drum transients and making subsequent hits sound muddy. Separate the two ramp modes so new voices ramp up over quickRampSamples (~5ms) while normal volume changes still ramp over the full tick. Co-Authored-By: a1k0n <a1k0n@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Some sounds in Edit: My memory was not correct apparently. |
What songs are you testing it with? |
Replace Math.sin() generated LUT with FT2's bit-exact vibratoTab[32] half-wave table. Fix ramp waveform to use FT2's index<<3 / bitwise NOT formula instead of modulo arithmetic. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
FT2 only sets volume envelope position if volume envelope is enabled, and has a bug where panning envelope position is conditional on the volume envelope's sustain flag instead of panning envelope's enabled flag. Store original file flags and replicate both behaviors. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Arpeggio (0xy): fix note order to match FT2's countdown tick indexing - E9x retrig: fix reversed modulo timing formula - Volume column portamento (Fxy): fix speed 4x too fast (<<4 → *4) - keyOff: reproduce FT2's inverted panning envelope tick backup bug - Tremolo: reproduce FT2's vibratoPos sign bug in ramp/square/sine Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Revert vol column porta speed to (nibble<<4) — our previous fix to (nibble*4) was 4x too slow vs FT2's (nibble<<4)*4 full-scale. Fix tremolo overall sign to use tremoloPos, not vibratoPos — the FT2 vibratoPos bug only affects the ramp waveform shape. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
well primariy kamel.xm, the one on my blog. literally the first thing jsxm was made for :) https://www.a1k0n.net/code/jsxm/ - all the snare hits except the first one were basically missing. |
|
Alright, test again whenever you can. |
…ions Reuse a fixed scopeBuffers array in audio_cb instead of creating new Float32Array objects every tick. Reduces GC pressure in the ScriptProcessor callback which runs on the main thread. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove the GainNode and apply post-mix amplification directly in the audio callback, matching FT2's fAudioNormalizeMul approach. Expose setAmplification() for UI volume control. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Hmm, still trying to get rid of some clicks in some songs... |
When rampLeft was decremented by a non-integer segment length, floating-point arithmetic could leave a tiny epsilon (~9e-13) instead of reaching zero. This made rampLeft > 0 true but too small to advance the sample index, deadlocking the mixer loop until the failsafe killed the voice mid-tick with no ramp. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Clicks are gone. |
Remove unused prettify functions and _note_names array, replace eff_unimplemented with null to skip no-op calls, store sample name in parsed object, drop createJavaScriptNode/noteOn/noteOff legacy fallbacks, fix var scope redeclaration and comment typo. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Route audio through a GainNode (jsNode → gainNode → destination) so the UI can control volume via player.gainNode.gain.value. Remove the internal amplification loop and setAmplification method. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Okay, ready to test again. |
- Fix Amiga period constant: 3583964 → 3579364 (8363*1712/4), was ~2 cents sharp - Fix global volume slide (Hxx) to use per-channel memory like FT2 - Add TODOs for missing period2NotePeriod() needed for Amiga mode arpeggio and glissando Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
This PR addresses two long-standing issues:
Implement missing XM effects (fixes Adding the missing effects. #5) — adds 15 previously unimplemented effects and features:
Mixing and spec compliance fixes — numerous improvements to match FT2 behavior
Spec compliance fixes
(EnvPan-32)*(128-|Pan-128|)/32FT2 accuracy fixes
Mixing improvements
Performance & cleanup
snapshotFadeVoice()helper to deduplicate 4 crossfade sites; reuses fadeVoice objectEnvelopeFollowerinstances viareset()method instead of allocating new onesquickRampSamplescomputation toinit()(sample rate doesn't change)MixSilenceIntoBuf(inline return 0),isNaNguards,typeofchecktriggerInstrument()helper (envelope/vibrato/retrig reset, 4 call sites)envtick,time_sound_started), fix implicit global, merge noop functionsOther fixes
setAmplification()for UI volume controlXM spec coverage
Compared against the XM format specification, here is where things stand after this PR:
All standard effects — implemented
All extended effects — implemented
Extra fine effects — implemented
All volume column commands — implemented
Other features
🤖 Generated with Claude Code