From b212fa6486923da3928752b194f698bffca34a8b Mon Sep 17 00:00:00 2001 From: JetSetIlly Date: Sun, 4 Aug 2024 09:21:35 +0100 Subject: [PATCH] TIA audio sampled every colour clock sum of samples is averaged and output twice per scanline for an output sample rate of 31.4KHz this fixes issues with ROMs that change the volume of the audio multiple times per scanline added *.wav to .gitignore --- .gitignore | 1 + Makefile | 2 +- README.md | 10 +++-- gui/sdlaudio/audio.go | 2 +- gui/sdlimgui/win_dbgscr.go | 14 +++---- hardware/tia/audio/audio.go | 73 ++++++++++++++++++--------------- hardware/tia/audio/channels.go | 9 +++- hardware/tia/audio/doc.go | 8 ++++ hardware/tia/audio/registers.go | 19 ++++----- 9 files changed, 80 insertions(+), 58 deletions(-) diff --git a/.gitignore b/.gitignore index 0a5c1e2a4..dba2eec1c 100644 --- a/.gitignore +++ b/.gitignore @@ -36,3 +36,4 @@ armcode.map go.work go.work.sum REDUX_PLAYBACK_FILES +*.wav diff --git a/Makefile b/Makefile index f6565d339..e2740238f 100644 --- a/Makefile +++ b/Makefile @@ -5,7 +5,7 @@ goBinary = go gcflags = -c 3 -B -wb=false ldflags = -s -w ldflags_version = $(ldflags) -X 'github.com/jetsetilly/gopher2600/version.number=$(version)' -profilingRom = ./MattressMonkeys20240530rc4.bin +profilingRom = roms/Pitfall.bin # the renderer to use for the GUI # diff --git a/README.md b/README.md index 6d10936ff..edfddf86e 100644 --- a/README.md +++ b/README.md @@ -150,6 +150,10 @@ The TIA Audio implementation is based almost entirely on the work of Chris Brenn https://atariage.com/forums/topic/249865-tia-sounding-off-in-the-digital-domain/ +Additional work on volume sampling a result of this thread: + +https://forums.atariage.com/topic/370460-8-bit-digital-audio-from-2600/ + Musical information as seen in the tracker window taken from Random Terrain. https://www.randomterrain.com/atari-2600-memories-music-and-sound.html @@ -265,6 +269,6 @@ http://www.festvox.org/docs/manual-2.4.0/festival_toc.html At various times during the development of this project, the following people have provided advice and encouragement: Andrew Rice, David Kelly. And those from AtariAge who have provided testing, advice and most importantly, -encouragement (alphabetically): alex_79; Al Nafuur; Andrew Davie; DirtyHairy; -John Champeau; MarcoJ; MrSQL; Rob Bairos; Spiceware; Thomas Jenztsch; Zachary -Scolaro; ZeroPageHomebrew +encouragement (alphabetically): alex_79; Al Nafuur; Andrew Davie; Batari; +DirtyHairy; John Champeau; MarcoJ; MrSQL; Rob Bairos; Spiceware; Thomas +Jenztsch; Zachary Scolaro; ZeroPageHomebrew diff --git a/gui/sdlaudio/audio.go b/gui/sdlaudio/audio.go index 29069d198..c34dcd846 100644 --- a/gui/sdlaudio/audio.go +++ b/gui/sdlaudio/audio.go @@ -42,7 +42,7 @@ const bufferLength = 628 const realtimeDemand = bufferLength * 2 // if queued audio ever exceeds this value then push the audio into the SDL buffer -const maxQueueLength = 16384 +const maxQueueLength = audio.SampleFreq / 2 // Audio outputs sound using SDL. type Audio struct { diff --git a/gui/sdlimgui/win_dbgscr.go b/gui/sdlimgui/win_dbgscr.go index 5cb33e130..86680ba46 100644 --- a/gui/sdlimgui/win_dbgscr.go +++ b/gui/sdlimgui/win_dbgscr.go @@ -575,11 +575,11 @@ func (win *winDbgScr) drawOverlayComboTooltip() { }, true) case reflection.OverlayLabels[reflection.OverlayAudio]: win.img.imguiTooltip(func() { + imguiColorLabelSimple("Change", win.img.cols.reflectionColors[reflection.AudioChanged]) + imgui.Spacing() imguiColorLabelSimple("Phase 0", win.img.cols.reflectionColors[reflection.AudioPhase0]) imgui.Spacing() imguiColorLabelSimple("Phase 1", win.img.cols.reflectionColors[reflection.AudioPhase1]) - imgui.Spacing() - imguiColorLabelSimple("Change", win.img.cols.reflectionColors[reflection.AudioChanged]) }, true) case reflection.OverlayLabels[reflection.OverlayCoproc]: win.img.imguiTooltip(func() { @@ -759,16 +759,16 @@ func (win *winDbgScr) drawReflectionTooltip() { // no RSYNC specific hover information case reflection.OverlayLabels[reflection.OverlayAudio]: imguiSeparator() - if ref.AudioPhase0 || ref.AudioPhase1 || ref.AudioChanged { + if ref.AudioChanged || ref.AudioPhase0 || ref.AudioPhase1 { + if ref.AudioChanged { + reg := strings.Split(e.Operand.Resolve(), ",")[0] + imguiColorLabelSimple(fmt.Sprintf("%s updated", reg), win.img.cols.reflectionColors[reflection.AudioChanged]) + } if ref.AudioPhase0 { imguiColorLabelSimple("Audio phase 0", win.img.cols.reflectionColors[reflection.AudioPhase0]) } else if ref.AudioPhase1 { imguiColorLabelSimple("Audio phase 1", win.img.cols.reflectionColors[reflection.AudioPhase1]) } - if ref.AudioChanged { - reg := strings.Split(e.Operand.Resolve(), ",")[0] - imguiColorLabelSimple(fmt.Sprintf("%s updated", reg), win.img.cols.reflectionColors[reflection.AudioChanged]) - } } else { imgui.Text("Audio unchanged") } diff --git a/hardware/tia/audio/audio.go b/hardware/tia/audio/audio.go index df6ed1d4d..5447825cf 100644 --- a/hardware/tia/audio/audio.go +++ b/hardware/tia/audio/audio.go @@ -34,14 +34,10 @@ type Tracker interface { AudioTick(env TrackerEnvironment, channel int, reg Registers) } -// SampleFreq represents the number of samples generated per second. This is -// the 30Khz reference frequency desribed in the Stella Programmer's Guide. -const SampleFreq = 31400 +// SampleFreq represents the number of samples generated per second +const SampleFreq = 15700 * 2 -// Audio is the implementation of the TIA audio sub-system, using Ron Fries' -// method. Reference source code here: -// -// https://raw.githubusercontent.com/alekmaul/stella/master/emucore/TIASound.c +// Audio is the implementation of the TIA audio sub-system type Audio struct { env *environment.Environment @@ -51,6 +47,11 @@ type Audio struct { // twice in that time clock228 int + // the volume is sampled every colour clock and the volume at each clock is + // summed. at fixed points, the volume is averaged + sampleSum []int + sampleSumCt int + // From the "Stella Programmer's Guide": // // "There are two audio circuits for generating sound. They are identical but @@ -62,15 +63,20 @@ type Audio struct { Vol0 uint8 Vol1 uint8 + // the addition of a tracker is not required tracker Tracker registersChanged bool + samplePoint bool } // NewAudio is the preferred method of initialisation for the Audio sub-system. func NewAudio(env *environment.Environment) *Audio { - return &Audio{ - env: env, + au := &Audio{ + env: env, + sampleSum: make([]int, 2), } + + return au } // Plumb audio into emulation @@ -78,14 +84,6 @@ func (au *Audio) Plumb(env *environment.Environment) { au.env = env } -func (au *Audio) Reset() { - au.clock228 = 0 - au.channel0 = channel{} - au.channel1 = channel{} - au.Vol0 = 0 - au.Vol1 = 0 -} - // SetTracker adds a Tracker implementation to the Audio sub-system. func (au *Audio) SetTracker(tracker Tracker) { au.tracker = tracker @@ -115,6 +113,8 @@ func (au *Audio) UpdateTracker() { // 30Khz clock. func (au *Audio) Step() bool { au.registersChanged = false + au.samplePoint = false + if au.tracker != nil { // it's impossible for both channels to have changed in a single video cycle if au.channel0.registersChanged { @@ -128,35 +128,42 @@ func (au *Audio) Step() bool { } } - au.clock228++ - if au.clock228 >= 228 { - au.clock228 = 0 - return false - } + var changed bool + + // sum volume bits + au.sampleSum[0] += int(au.channel0.actualVolume()) + au.sampleSum[1] += int(au.channel1.actualVolume()) + au.sampleSumCt++ switch au.clock228 { case 10: - au.channel0.phase0() - au.channel1.phase0() - return false + fallthrough case 82: au.channel0.phase0() au.channel1.phase0() - return false case 38: - au.channel0.phase1() - au.channel1.phase1() + fallthrough case 150: au.channel0.phase1() au.channel1.phase1() - default: - return false + + // take average of sum of volume bits + au.Vol0 = uint8(au.sampleSum[0] / au.sampleSumCt) + au.Vol1 = uint8(au.sampleSum[1] / au.sampleSumCt) + au.sampleSum[0] = 0 + au.sampleSum[1] = 0 + au.sampleSumCt = 0 + + changed = true } - au.Vol0 = au.channel0.actualVol - au.Vol1 = au.channel1.actualVol + // advance 228 clock and reset sample counter + au.clock228++ + if au.clock228 >= 228 { + au.clock228 = 0 + } - return true + return changed } // HasTicked returns whether the audio channels were ticked on the previous diff --git a/hardware/tia/audio/channels.go b/hardware/tia/audio/channels.go index 102802be9..d37d6a08f 100644 --- a/hardware/tia/audio/channels.go +++ b/hardware/tia/audio/channels.go @@ -28,7 +28,7 @@ type channel struct { pulseCounter uint8 noiseCounter uint8 - actualVol uint8 + volumeChanged bool } func (ch *channel) String() string { @@ -119,6 +119,11 @@ func (ch *channel) phase1() { } } } +} - ch.actualVol = (ch.pulseCounter & 0x01) * ch.registers.Volume +// the actual volume of the channel is the volume in the register multiplied by +// the lower bit of the pulsecounter. this is then used in combination with the +// volume of the other channel to get the actual output volume +func (ch *channel) actualVolume() uint8 { + return (ch.pulseCounter & 0x01) * ch.registers.Volume } diff --git a/hardware/tia/audio/doc.go b/hardware/tia/audio/doc.go index 5dd3caef8..1c332a530 100644 --- a/hardware/tia/audio/doc.go +++ b/hardware/tia/audio/doc.go @@ -28,4 +28,12 @@ // Stella (published under the GNU GPL v2.0 licence) // https://github.com/stella-emu/stella/blob/e6af23d6c12893dd17711002971087f28f87c31f/src/emucore/tia/Audio.cxx // https://github.com/stella-emu/stella/blob/e6af23d6c12893dd17711002971087f28f87c31f/src/emucore/tia/AudioChannel.cxx +// +// Additional work on volume sampling a result of this thread: +// +// https://forums.atariage.com/topic/370460-8-bit-digital-audio-from-2600/ +// +// For reference, Ron Fries' audio method is represented here: +// +// https://raw.githubusercontent.com/alekmaul/stella/master/emucore/TIASound.c package audio diff --git a/hardware/tia/audio/registers.go b/hardware/tia/audio/registers.go index f8dc83286..64406ae9c 100644 --- a/hardware/tia/audio/registers.go +++ b/hardware/tia/audio/registers.go @@ -56,30 +56,27 @@ func (au *Audio) ReadMemRegisters(data chipbus.ChangedRegister) bool { switch data.Register { case cpubus.AUDC0: au.channel0.registers.Control = data.Value & 0x0f - au.channel0.reactAUDCx() + au.channel0.registersChanged = true case cpubus.AUDC1: au.channel1.registers.Control = data.Value & 0x0f - au.channel1.reactAUDCx() + au.channel1.registersChanged = true case cpubus.AUDF0: au.channel0.registers.Freq = data.Value & 0x1f - au.channel0.reactAUDCx() + au.channel0.registersChanged = true case cpubus.AUDF1: au.channel1.registers.Freq = data.Value & 0x1f - au.channel1.reactAUDCx() + au.channel1.registersChanged = true case cpubus.AUDV0: au.channel0.registers.Volume = data.Value & 0x0f - au.channel0.reactAUDCx() + au.channel0.volumeChanged = true + au.channel0.registersChanged = true case cpubus.AUDV1: au.channel1.registers.Volume = data.Value & 0x0f - au.channel1.reactAUDCx() + au.channel0.volumeChanged = true + au.channel1.registersChanged = true default: return true } return false } - -// changing the value of an AUDx registers causes some side effect. -func (ch *channel) reactAUDCx() { - ch.registersChanged = true -}