From 15e167b6a88a5717b6bb71504602bf15f221e280 Mon Sep 17 00:00:00 2001 From: Jason Crawford Date: Wed, 17 Dec 2025 10:48:03 -0800 Subject: [PATCH] Refactor OPL constants and improve comments; add internal tests for operator behavior; fix issue with E0 register writes --- go.sum | 1 + opal.go | 8 +-- operator.go | 6 +- opl2.go | 38 ++++++------ opl2_internal_test.go | 137 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 165 insertions(+), 25 deletions(-) create mode 100644 opl2_internal_test.go diff --git a/go.sum b/go.sum index d1de9f4..7c401c3 100644 --- a/go.sum +++ b/go.sum @@ -1 +1,2 @@ +github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= diff --git a/opal.go b/opal.go index f2f5c5f..7bf7911 100644 --- a/opal.go +++ b/opal.go @@ -25,7 +25,7 @@ package opl2 // Various constants const ( - OPL3SampleRate = 49716 + OPL3SampleRate = RoundedOPLRATE NumChannels = 18 NumOperators = 36 @@ -443,9 +443,9 @@ func (o *operator) SetWaveform(wave uint16) { // // Rof is set as follows depending on the KSR setting: // -// Key scale 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 -// KSR = 0 0 0 0 0 1 1 1 1 2 2 2 2 3 3 3 3 -// KSR = 1 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 +// Key scale 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 +// KSR = 0 0 0 0 0 1 1 1 1 2 2 2 2 3 3 3 3 +// KSR = 1 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 // // Note: zero rates are infinite, and are treated separately elsewhere func (o *operator) ComputeRates() { diff --git a/operator.go b/operator.go index 00dc61d..af8f137 100644 --- a/operator.go +++ b/operator.go @@ -36,7 +36,7 @@ package opl2 //DUNNO Keyon in 4op, switch to 2op without keyoff. */ -//Masks for operator 20 values +// Masks for operator 20 values const ( cMaskKSR = 0x10 cMaskSustain = 0x20 @@ -122,7 +122,7 @@ func (o *Operator) SetupOperator() { } // UpdateAttack updates the attack rate on the envelope -//We zero out when rate == 0 +// We zero out when rate == 0 func (o *Operator) UpdateAttack(chip *Chip) { rate := uint8(o.reg60 >> 4) if rate != 0 { @@ -305,7 +305,7 @@ func (o *Operator) Write80(chip *Chip, val uint8) { // WriteE0 writes data to register 0xE0 on the operator func (o *Operator) WriteE0(chip *Chip, val uint8) { - if (o.regE0 ^ val) != 0 { + if (o.regE0 ^ val) == 0 { return } //in opl3 mode you can always selet 7 waveforms regardless of waveformselect diff --git a/opl2.go b/opl2.go index a4817f0..ea49cdf 100644 --- a/opl2.go +++ b/opl2.go @@ -48,10 +48,14 @@ const ( ) const ( + OPLClock4 = 14318180 //Hz + OPLPrecision = 288 // Amount of precision for internal calculations + // (9 channels * 8 bits * 4x oversampling to support OPL3's 18 channels) + // OPLRATE is the sampling rate that the OPL2/3 outputs samples at, normally // all internal calculations are defined by it. - OPLRATE = 14318180.0 / 288.0 - + OPLRATE = float64(OPLClock4) / float64(OPLPrecision) + RoundedOPLRATE = (OPLClock4 + OPLPrecision - 1) / OPLPrecision cTremoloTableSize = 52 //Try to use most precision for frequencies @@ -107,7 +111,7 @@ func init() { } } -//How much to substract from the base value for the final attenuation +// How much to substract from the base value for the final attenuation var cKslCreateTable = [16]uint8{ //0 will always be be lower than 7 * 8 64, 32, 24, 19, @@ -125,7 +129,7 @@ var cFreqCreateTable = [16]uint32{ m1(8), m1(9), m1(10), m1(10), m1(12), m1(12), m1(15), m1(15), } -//We're not including the highest attack rate, that gets a special value +// We're not including the highest attack rate, that gets a special value var cAttackSamplesTable = [13]uint8{ 69, 55, 46, 40, 35, 29, 23, 20, @@ -133,7 +137,7 @@ var cAttackSamplesTable = [13]uint8{ 9, } -//On a real opl these values take 8 samples to reach and are based upon larger tables +// On a real opl these values take 8 samples to reach and are based upon larger tables var cEnvelopeIncreaseTable = [13]uint8{ 4, 5, 6, 7, 8, 10, 12, 14, @@ -143,7 +147,7 @@ var cEnvelopeIncreaseTable = [13]uint8{ var cExpTable = make([]uint16, 256) -//PI table used by WAVEHANDLER +// PI table used by WAVEHANDLER var cSinTable = make([]uint16, 512) //Layout of the waveform table in 512 entry intervals @@ -157,19 +161,19 @@ var cSinTable = make([]uint16, 512) var cWaveTable = make([]int16, 8*512) -//Distance into WaveTable the wave starts +// Distance into WaveTable the wave starts var cWaveBaseTable = [8]uint16{ 0x000, 0x200, 0x200, 0x800, 0xa00, 0xc00, 0x100, 0x400, } -//Mask the counter with this +// Mask the counter with this var cWaveMaskTable = [8]uint16{ 1023, 1023, 511, 511, 1023, 1023, 512, 1023, } -//Where to start the counter on at keyon +// Where to start the counter on at keyon var cWaveStartTable = [8]uint16{ 512, 0, 0, 0, 0, 512, 512, 256, @@ -180,23 +184,23 @@ var cMulTable = make([]uint16, 384) var cKslTable = make([]uint8, 8*16) var cTremoloTable = make([]uint8, cTremoloTableSize) -//Start of a channel behind the chip struct start +// Start of a channel behind the chip struct start var cChanOffsetTable = make([]uint16, 32) -//The lower bits are the shift of the operator vibrato value -//The highest bit is right shifted to generate -1 or 0 for negation -//So taking the highest input value of 7 this gives 3, 7, 3, 0, -3, -7, -3, 0 +// The lower bits are the shift of the operator vibrato value +// The highest bit is right shifted to generate -1 or 0 for negation +// So taking the highest input value of 7 this gives 3, 7, 3, 0, -3, -7, -3, 0 var cVibratoTable = [8]int8{ 1 - 0x00, 0 - 0x00, 1 - 0x00, 30 - 0x00, 1 - 0x80, 0 - 0x80, 1 - 0x80, 30 - 0x80, } -//Shift strength for the ksl value determined by ksl strength +// Shift strength for the ksl value determined by ksl strength var cKslShiftTable = [4]uint8{ 31, 1, 2, 0, } -//Generate a table index and table shift value using input value from a selected rate +// Generate a table index and table shift value using input value from a selected rate func envelopeSelect(val uint8) (index uint8, shift uint8) { if val < 13*4 { //Rate 0 - 12 shift = 12 - (val >> 2) @@ -211,9 +215,7 @@ func envelopeSelect(val uint8) (index uint8, shift uint8) { return } -/* - Generate the different waveforms out of the sine/exponetial table using handlers -*/ +// Generate the different waveforms out of the sine/exponetial table using handlers func makeVolume(wave int, volume int) int { total := wave + volume index := total & 0xff diff --git a/opl2_internal_test.go b/opl2_internal_test.go new file mode 100644 index 0000000..6e908db --- /dev/null +++ b/opl2_internal_test.go @@ -0,0 +1,137 @@ +package opl2 + +import ( + "testing" +) + +// Use the native OPL rate for deterministic table values in tests. +const ( + testRate = RoundedOPLRATE +) + +// Test that waveform writes propagate to the operator (reg E0) and refresh +// the derived waveform start/mask data. This guards the regression where the +// register change early-returned and never updated the operator state. +func TestOperatorWaveformWriteUpdates(t *testing.T) { + chip := NewChip(testRate, false) + // Enable waveform selection (otherwise waveFormMask would clamp values). + chip.WriteReg(0x01, 0x20) + + op := chip.GetOperatorByIndex(0) + if op == nil { + t.Fatalf("nil operator returned") + } + initialStart := op.waveStart + + chip.WriteReg(0xE0, 0x01) // select waveform 1 + + expectedStart := int(cWaveStartTable[1]) << cWaveSh + if op.waveStart != expectedStart { + t.Fatalf("waveStart not updated: got %d, want %d (initial %d)", op.waveStart, expectedStart, initialStart) + } + if op.regE0 != 0x01 { + t.Fatalf("regE0 not stored: got 0x%02x, want 0x01", op.regE0) + } +} + +// Validate percussion key-on/off handling driven by register BD toggles. +// Ensures bass drum key state tracks the BD bit correctly when percussion mode is enabled. +func TestPercussionKeyOnOff(t *testing.T) { + chip := NewChip(testRate, false) + + // Enable percussion mode without bass drum key-on. + chip.WriteReg(0xBD, 0x20) + if chip.ch[6].op[0].keyOn != 0 || chip.ch[6].op[1].keyOn != 0 { + t.Fatalf("bass drum keyOn should be cleared after enabling percussion: op0=%02x op1=%02x", chip.ch[6].op[0].keyOn, chip.ch[6].op[1].keyOn) + } + + // Turn on bass drum bit. + chip.WriteReg(0xBD, 0x30) + if chip.ch[6].op[0].keyOn&0x2 == 0 || chip.ch[6].op[1].keyOn&0x2 == 0 { + t.Fatalf("bass drum keyOn not set: op0=%02x op1=%02x", chip.ch[6].op[0].keyOn, chip.ch[6].op[1].keyOn) + } + + // Turn off bass drum bit while keeping percussion enabled. + chip.WriteReg(0xBD, 0x20) + if chip.ch[6].op[0].keyOn != 0 || chip.ch[6].op[1].keyOn != 0 { + t.Fatalf("bass drum keyOn not cleared: op0=%02x op1=%02x", chip.ch[6].op[0].keyOn, chip.ch[6].op[1].keyOn) + } + + // Disable percussion entirely; state should remain cleared. + chip.WriteReg(0xBD, 0x00) + if chip.ch[6].op[0].keyOn != 0 || chip.ch[6].op[1].keyOn != 0 { + t.Fatalf("bass drum keyOn not cleared after percussion disable: op0=%02x op1=%02x", chip.ch[6].op[0].keyOn, chip.ch[6].op[1].keyOn) + } +} + +// Verify 4-operator synth mode selection matches the bit combinations in +// regC0 for paired channels when OPL3 is active. +func TestFourOpSynthSelection(t *testing.T) { + chip := NewChip(testRate, true) + // Enable OPL3 features explicitly; Setup leaves opl3Active disabled. + chip.WriteReg(0x105, 0x01) + if chip.opl3Active == 0 { + t.Fatalf("opl3Active not enabled") + } + + // Enable 4-op on logical channels 0/3 (bit 0). + chip.WriteReg(0x104, 0x01) + + ch0 := chip.GetChannelByIndex(0) // logical channel 0 + ch1 := chip.GetChannelByIndex(3) // logical channel 3 (pair with bit 0) + if ch0 == nil || ch1 == nil { + t.Fatalf("failed to get four-op channel pair") + } + + // Set synth bits to pattern 01 -> sm3FMAM for the paired channels. + ch0.WriteC0(chip, 0x02) // bit0 = 0 (change!=0) + ch1.WriteC0(chip, 0x03) // bit0 = 1 (change!=0) + + if ch0.synthHandler < sm4Start { + t.Fatalf("four-op handler not selected: got %v (ch0=0x%02x ch1=0x%02x)", ch0.synthHandler, ch0.regC0, ch1.regC0) + } +} + +// Confirm WriteAddr maps ports correctly, matching the C++ helper logic. +func TestWriteAddrMapping(t *testing.T) { + opl2Chip := NewChip(testRate, false) + opl3Chip := NewChip(testRate, true) + opl3Chip.WriteReg(0x105, 0x01) // enable opl3Active + + if got := opl2Chip.WriteAddr(0, 0xAA); got != 0xAA { + t.Fatalf("opl2 port0 mapping mismatch: got 0x%x", got) + } + if got := opl2Chip.WriteAddr(2, 0x05); got != 0x105 { + t.Fatalf("opl2 port2 special mapping mismatch: got 0x%x", got) + } + if got := opl2Chip.WriteAddr(2, 0x06); got != 0x06 { + t.Fatalf("opl2 port2 normal mapping mismatch: got 0x%x", got) + } + if got := opl3Chip.WriteAddr(2, 0x11); got != 0x111 { + t.Fatalf("opl3 port2 mapping mismatch: got 0x%x", got) + } + if got := opl2Chip.WriteAddr(1, 0x22); got != 0 { + t.Fatalf("unexpected mapping for unused port: got 0x%x", got) + } +} + +// Ensure block generation does not panic and produces the expected frame counts +// for mono (OPL2) and stereo (OPL3) output buffers. +func TestGenerateBlockSizes(t *testing.T) { + samples := uint(8) + + opl2Chip := NewChip(testRate, false) + opl2Buf := make([]int32, samples) + opl2Chip.GenerateBlock2(samples, opl2Buf) + + opl3Chip := NewChip(testRate, true) + opl3Buf := make([]int32, samples*2) + opl3Chip.GenerateBlock3(samples, opl3Buf) + + if len(opl2Buf) != int(samples) { + t.Fatalf("opl2 buffer length changed: got %d", len(opl2Buf)) + } + if len(opl3Buf) != int(samples*2) { + t.Fatalf("opl3 buffer length changed: got %d", len(opl3Buf)) + } +}