From e8a9fe282a53817ca7fa2aa65be724045632a6c6 Mon Sep 17 00:00:00 2001 From: Stephen Illingworth <58750902+JetSetIlly@users.noreply.github.com> Date: Wed, 7 Aug 2024 09:18:42 +0100 Subject: [PATCH 1/2] Improvement to volume sampling in TIA audio (#1038) volume of audio channels sampled every on every tick. sum of samples is averaged and a new sample output twice per scanline this fixes issues with ROMs that change the volume of the audio multiple times per scanline. for example, the experimental ROM in the following thread now works correctly https://forums.atariage.com/topic/370460-8-bit-digital-audio-from-2600/ (note that the ROM does not initialise the machine cleanly and so running the emulator with developer options (random memory etc.) can cause incorrect audio) --- src/emucore/tia/Audio.cxx | 14 +++++++++++--- src/emucore/tia/Audio.hxx | 16 ++++++++++++++-- src/emucore/tia/AudioChannel.cxx | 7 ++++++- src/emucore/tia/AudioChannel.hxx | 4 +++- 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/src/emucore/tia/Audio.cxx b/src/emucore/tia/Audio.cxx index 7d35c9a89..49b3145f6 100644 --- a/src/emucore/tia/Audio.cxx +++ b/src/emucore/tia/Audio.cxx @@ -47,6 +47,9 @@ void Audio::reset() { myCounter = 0; mySampleIndex = 0; + sumChannel0 = 0; + sumChannel1 = 0; + sumCt = 0; myChannel0.reset(); myChannel1.reset(); @@ -62,10 +65,15 @@ void Audio::setAudioQueue(const shared_ptr& queue) } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -void Audio::phase1() +void Audio::createSample() { - const uInt8 sample0 = myChannel0.phase1(); - const uInt8 sample1 = myChannel1.phase1(); + // calculate average of all recent volume samples. the average for each + // channel is mixed to create a single audible value + const uInt8 sample0 = (uInt8)(sumChannel0/sumCt); + const uInt8 sample1 = (uInt8)(sumChannel1/sumCt); + sumChannel0 = 0; + sumChannel1 = 0; + sumCt = 0; addSample(sample0, sample1); #ifdef GUI_SUPPORT diff --git a/src/emucore/tia/Audio.hxx b/src/emucore/tia/Audio.hxx index 9a054c5b0..a18019b80 100644 --- a/src/emucore/tia/Audio.hxx +++ b/src/emucore/tia/Audio.hxx @@ -57,7 +57,7 @@ class Audio : public Serializable bool load(Serializer& in) override; private: - void phase1(); + void createSample(); void addSample(uInt8 sample0, uInt8 sample1); private: @@ -68,6 +68,10 @@ class Audio : public Serializable AudioChannel myChannel0; AudioChannel myChannel1; + uInt32 sumChannel0; + uInt32 sumChannel1; + uInt32 sumCt; + std::array myMixingTableSum; std::array myMixingTableIndividual; @@ -92,6 +96,12 @@ class Audio : public Serializable // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void Audio::tick() { + // volume for each channel is sampled every color clock. the average of + // these samples will be taken twice a scanline in the phase1() function + sumChannel0 += (uInt32)myChannel0.actualVolume(); + sumChannel1 += (uInt32)myChannel1.actualVolume(); + sumCt++; + switch (myCounter) { case 9: case 81: @@ -101,7 +111,9 @@ void Audio::tick() case 37: case 149: - phase1(); + myChannel0.phase1(); + myChannel1.phase1(); + createSample(); break; default: diff --git a/src/emucore/tia/AudioChannel.cxx b/src/emucore/tia/AudioChannel.cxx index 72db5a076..ad82fd801 100644 --- a/src/emucore/tia/AudioChannel.cxx +++ b/src/emucore/tia/AudioChannel.cxx @@ -77,7 +77,7 @@ void AudioChannel::phase0() } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -uInt8 AudioChannel::phase1() +void AudioChannel::phase1() { if (myClockEnable) { bool pulseFeedback = false; @@ -118,7 +118,12 @@ uInt8 AudioChannel::phase1() } } } +} +// the actual volume of a chaneel is the volume register multiplied by the +// lowest of the pulse counter +uInt8 AudioChannel::actualVolume() +{ return (myPulseCounter & 0x01) * myAudv; } diff --git a/src/emucore/tia/AudioChannel.hxx b/src/emucore/tia/AudioChannel.hxx index 6302dcba3..ed62afafb 100644 --- a/src/emucore/tia/AudioChannel.hxx +++ b/src/emucore/tia/AudioChannel.hxx @@ -30,7 +30,9 @@ class AudioChannel : public Serializable void phase0(); - uInt8 phase1(); + void phase1(); + + uInt8 actualVolume(); void audc(uInt8 value); From 637c194f80824fa75e3d34a329b11c97a369d006 Mon Sep 17 00:00:00 2001 From: Stephen Anthony Date: Wed, 7 Aug 2024 12:14:07 -0230 Subject: [PATCH 2/2] Various fixes to latest sound code (style, clang-tidy, etc.) --- src/common/StateManager.hxx | 2 +- src/emucore/tia/Audio.cxx | 30 +++++++++++++++++++----------- src/emucore/tia/Audio.hxx | 14 +++++++------- src/emucore/tia/AudioChannel.cxx | 5 +++-- src/emucore/tia/AudioChannel.hxx | 2 +- 5 files changed, 31 insertions(+), 22 deletions(-) diff --git a/src/common/StateManager.hxx b/src/common/StateManager.hxx index 1576a969d..018f07dad 100644 --- a/src/common/StateManager.hxx +++ b/src/common/StateManager.hxx @@ -18,7 +18,7 @@ #ifndef STATE_MANAGER_HXX #define STATE_MANAGER_HXX -#define STATE_HEADER "06070002state" +#define STATE_HEADER "06070010state" class OSystem; class RewindManager; diff --git a/src/emucore/tia/Audio.cxx b/src/emucore/tia/Audio.cxx index 49b3145f6..76506da43 100644 --- a/src/emucore/tia/Audio.cxx +++ b/src/emucore/tia/Audio.cxx @@ -36,8 +36,10 @@ namespace { // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - Audio::Audio() { - for (uInt8 i = 0; i <= 0x1e; ++i) myMixingTableSum[i] = mixingTableEntry(i, 0x1e); - for (uInt8 i = 0; i <= 0x0f; ++i) myMixingTableIndividual[i] = mixingTableEntry(i, 0x0f); + for (uInt8 i = 0; i <= 0x1e; ++i) + myMixingTableSum[i] = mixingTableEntry(i, 0x1e); + for (uInt8 i = 0; i <= 0x0f; ++i) + myMixingTableIndividual[i] = mixingTableEntry(i, 0x0f); reset(); } @@ -45,11 +47,9 @@ Audio::Audio() // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void Audio::reset() { + mySumChannel0 = mySumChannel1 = mySumCt = 0; myCounter = 0; mySampleIndex = 0; - sumChannel0 = 0; - sumChannel1 = 0; - sumCt = 0; myChannel0.reset(); myChannel1.reset(); @@ -67,13 +67,11 @@ void Audio::setAudioQueue(const shared_ptr& queue) // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void Audio::createSample() { - // calculate average of all recent volume samples. the average for each + // Calculate average of all recent volume samples. the average for each // channel is mixed to create a single audible value - const uInt8 sample0 = (uInt8)(sumChannel0/sumCt); - const uInt8 sample1 = (uInt8)(sumChannel1/sumCt); - sumChannel0 = 0; - sumChannel1 = 0; - sumCt = 0; + const auto sample0 = static_cast(mySumChannel0 / mySumCt); + const auto sample1 = static_cast(mySumChannel1 / mySumCt); + mySumChannel0 = mySumChannel1 = mySumCt = 0; addSample(sample0, sample1); #ifdef GUI_SUPPORT @@ -115,6 +113,11 @@ bool Audio::save(Serializer& out) const if (!myChannel0.save(out)) return false; if (!myChannel1.save(out)) return false; + + out.putInt(mySumChannel0); + out.putInt(mySumChannel1); + out.putInt(mySumCt); + #ifdef GUI_SUPPORT out.putLong(static_cast(mySamples.size())); out.putByteArray(mySamples.data(), mySamples.size()); @@ -144,6 +147,11 @@ bool Audio::load(Serializer& in) if (!myChannel0.load(in)) return false; if (!myChannel1.load(in)) return false; + + mySumChannel0 = in.getInt(); + mySumChannel1 = in.getInt(); + mySumCt = in.getInt(); + #ifdef GUI_SUPPORT const uInt64 sampleSize = in.getLong(); ByteArray samples(sampleSize); diff --git a/src/emucore/tia/Audio.hxx b/src/emucore/tia/Audio.hxx index a18019b80..161e0c91f 100644 --- a/src/emucore/tia/Audio.hxx +++ b/src/emucore/tia/Audio.hxx @@ -68,9 +68,9 @@ class Audio : public Serializable AudioChannel myChannel0; AudioChannel myChannel1; - uInt32 sumChannel0; - uInt32 sumChannel1; - uInt32 sumCt; + uInt32 mySumChannel0{0}; + uInt32 mySumChannel1{0}; + uInt32 mySumCt{0}; std::array myMixingTableSum; std::array myMixingTableIndividual; @@ -96,11 +96,11 @@ class Audio : public Serializable // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void Audio::tick() { - // volume for each channel is sampled every color clock. the average of + // Volume for each channel is sampled every color clock. the average of // these samples will be taken twice a scanline in the phase1() function - sumChannel0 += (uInt32)myChannel0.actualVolume(); - sumChannel1 += (uInt32)myChannel1.actualVolume(); - sumCt++; + mySumChannel0 += static_cast(myChannel0.actualVolume()); + mySumChannel1 += static_cast(myChannel1.actualVolume()); + mySumCt++; switch (myCounter) { case 9: diff --git a/src/emucore/tia/AudioChannel.cxx b/src/emucore/tia/AudioChannel.cxx index ad82fd801..679a92cd3 100644 --- a/src/emucore/tia/AudioChannel.cxx +++ b/src/emucore/tia/AudioChannel.cxx @@ -120,9 +120,10 @@ void AudioChannel::phase1() } } -// the actual volume of a chaneel is the volume register multiplied by the +// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +// The actual volume of a chaneel is the volume register multiplied by the // lowest of the pulse counter -uInt8 AudioChannel::actualVolume() +uInt8 AudioChannel::actualVolume() const { return (myPulseCounter & 0x01) * myAudv; } diff --git a/src/emucore/tia/AudioChannel.hxx b/src/emucore/tia/AudioChannel.hxx index ed62afafb..fc292ff6d 100644 --- a/src/emucore/tia/AudioChannel.hxx +++ b/src/emucore/tia/AudioChannel.hxx @@ -32,7 +32,7 @@ class AudioChannel : public Serializable void phase1(); - uInt8 actualVolume(); + uInt8 actualVolume() const; void audc(uInt8 value);