ym2612: fixing the bugs (#1775)

1. The cycle update order needed clean up, mostly to fix feedback.
2. Envelope update function was also reordered to avoid double update
issue.
3. LFO behavior was corrected (should apply proper attenuation when
disabled).
4. To avoid potential update issue, rate scaling was lightly refactored.
Avoids recalc of key scale factor and renames keyScale to rateScaling
(RS) for clarity.

Related issues: #1159, #1112

ares/component/audio/ym2612/channel.cpp

@@ -29,24 +29,25 @@ auto YM2612::Channel::Operator::updateKeyState() -> void {
 auto YM2612::Channel::Operator::runEnvelope() -> void {
   if(ym2612.envelope.clock & (1 << envelope.divider) - 1) return;
 
+  if(envelope.state == Attack && envelope.value == 0) {
+    envelope.state = Decay;
+    updateEnvelope();
+  }
+
+  u32 sustain = envelope.sustainLevel < 15 ? envelope.sustainLevel << 5 : 0x1f << 5;
+  if(envelope.state == Decay && envelope.value >= sustain) {
+    envelope.state = Sustain;
+    updateEnvelope();
+  }
+
   u32 value = ym2612.envelope.clock >> envelope.divider;
   u32 step = envelope.steps >> ((~value & 7) << 2) & 0xf;
-  u32 sustain = envelope.sustainLevel < 15 ? envelope.sustainLevel << 5 : 0x1f << 5;
 
   if(envelope.state == Attack) {
-    if(envelope.value == 0) {
-      envelope.state = Decay;
-      updateEnvelope();
-    } else if(envelope.rate < 62) {
-      // will stop updating if attack rate is increased to upper threshold during attack phase (confirmed behavior)
-      envelope.value += ~u16(envelope.value) * step >> 4;
-    }
+    // will stop updating if attack rate is increased to upper threshold during attack phase (confirmed behavior)
+    if(envelope.rate < 62) envelope.value += ~u16(envelope.value) * step >> 4;
   }
   if(envelope.state != Attack) {
-    if(envelope.state == Decay && envelope.value >= sustain) {
-      envelope.state = Sustain;
-      updateEnvelope();
-    }
     if(ssg.enable) step = envelope.value < 0x200 ? step << 2 : 0;  //SSG results in a 4x faster envelope
     envelope.value = min(envelope.value + step, 0x3ff);
   }
@@ -82,16 +83,14 @@ auto YM2612::Channel::Operator::runPhase() -> void {
 }
 
 auto YM2612::Channel::Operator::updateEnvelope() -> void {
-  u32 key = min(max((u32)pitch.value, 0x300), 0x4ff);
-  u32 ksr = (octave.value << 2) + ((key - 0x300) >> 7);
   u32 rate = 0;
 
   if(envelope.state == Attack)  rate += (envelope.attackRate  << 1);
   if(envelope.state == Decay)   rate += (envelope.decayRate   << 1);
   if(envelope.state == Sustain) rate += (envelope.sustainRate << 1);
   if(envelope.state == Release) rate += (envelope.releaseRate << 1);
 
-  rate += (ksr >> 3 - envelope.keyScale) * (rate > 0);
+  rate += (keyScale >> 3 - envelope.rateScaling) * (rate > 0);
   rate  = min(rate, 63);
 
   auto& entry = envelopeRates[rate >> 2];
@@ -107,15 +106,15 @@ auto YM2612::Channel::Operator::updatePitch() -> void {
   pitch.value = channel.mode ? pitch.reload : channel[3].pitch.reload;
   octave.value = channel.mode ? octave.reload : channel[3].octave.reload;
 
+  u32 key = min(max((u32)pitch.value, 0x300), 0x4ff);
+  keyScale = (octave.value << 2) + ((key - 0x300) >> 7);
+
   updatePhase();
   updateEnvelope();  //due to key scaling
 }
 
 auto YM2612::Channel::Operator::updatePhase() -> void {
-  u32 key = min(max((u32)pitch.value, 0x300), 0x4ff);
-  u32 ksr = (octave.value << 2) + ((key - 0x300) >> 7);
-  u32 tuning = detune & 3 ? detunes[(detune & 3) - 1][ksr & 7] >> (3 - (ksr >> 3)) : 0;
-
+  u32 tuning = detune & 3 ? detunes[(detune & 3) - 1][keyScale & 7] >> (3 - (keyScale >> 3)) : 0;
   u32 lfo = ym2612.lfo.clock >> 2 & 0x1f;
   s32 pm = (pitch.value * vibratos[channel.vibrato][lfo & 15] >> 9) * (lfo > 15 ? -1 : 1);
 
@@ -131,7 +130,7 @@ auto YM2612::Channel::Operator::updateLevel() -> void {
   bool invert = ssg.attack != ssg.invert && envelope.state != Release;
   n10 value = ssg.enable && invert ? 0x200 - envelope.value : 0 + envelope.value;
 
-  outputLevel = ((totalLevel << 3) + value + (lfoEnable ? lfo << 1 >> depth : 0)) << 3;
+  outputLevel = (totalLevel << 3) + value + (lfo << 1 >> depth) << 3;
 }
 
 auto YM2612::Channel::power() -> void {
@@ -149,6 +148,7 @@ auto YM2612::Channel::power() -> void {
     op.keyOn = 0;
     op.keyLine = 0;
     op.lfoEnable = 0;
+    op.keyScale = 0;
     op.detune = 0;
     op.multiple = 0;
     op.totalLevel = 0;
@@ -175,7 +175,7 @@ auto YM2612::Channel::power() -> void {
     op.envelope.steps = 0;
     op.envelope.value = 0x3ff;
 
-    op.envelope.keyScale = 0;
+    op.envelope.rateScaling = 0;
     op.envelope.attackRate = 0;
     op.envelope.decayRate = 0;
     op.envelope.sustainRate = 0;

ares/component/audio/ym2612/io.cpp

@@ -14,8 +14,10 @@ auto YM2612::writeData(n8 data) -> void {
   case 0x022: {
     lfo.rate = data.bit(0,2);
     lfo.enable = data.bit(3);
-    lfo.clock = 0;
-    lfo.divider = 0;
+    if(!lfo.enable) {
+      lfo.clock = 0;
+      lfo.divider = 0;
+    }
     break;
   }
 
@@ -114,10 +116,10 @@ auto YM2612::writeData(n8 data) -> void {
     break;
   }
 
-  //key scaling, attack rate
+  //rate scaling, attack rate
   case 0x050: {
     op.envelope.attackRate = data.bit(0,4);
-    op.envelope.keyScale = data.bit(6,7);
+    op.envelope.rateScaling = data.bit(6,7);
     channel[index].updateEnvelope();
     channel[index].updatePhase();
     break;

ares/component/audio/ym2612/serialization.cpp

@@ -44,6 +44,7 @@ auto YM2612::Channel::Operator::serialize(serializer& s) -> void {
   s(keyOn);
   s(keyLine);
   s(lfoEnable);
+  s(keyScale);
   s(detune);
   s(multiple);
   s(totalLevel);
@@ -68,7 +69,7 @@ auto YM2612::Channel::Operator::serialize(serializer& s) -> void {
   s(envelope.divider);
   s(envelope.steps);
   s(envelope.value);
-  s(envelope.keyScale);
+  s(envelope.rateScaling);
   s(envelope.attackRate);
   s(envelope.decayRate);
   s(envelope.sustainRate);

ares/component/audio/ym2612/ym2612.cpp

@@ -13,6 +13,25 @@ auto YM2612::clock() -> array<i16[2]> {
   s32 left  = 0;
   s32 right = 0;
 
+  timerA.run();
+  timerB.run();
+
+  if(++envelope.divider == 3) {
+    envelope.divider = 0;
+    if(!++envelope.clock) ++envelope.clock; // 12-bit counter: 1..4095 - zero-value is skipped (confirmed behavior)
+  }
+
+  if(lfo.enable && ++lfo.divider >= lfoDividers[lfo.rate]) {
+    lfo.divider = 0;
+    lfo.clock++;
+    for(auto& channel : channels) {
+      for(auto& op : channel.operators) {
+        op.updatePhase();  //due to vibrato
+        op.updateLevel();  //due to tremolo
+      }
+    }
+  }
+
   for(auto& channel : channels) {
     auto& op = channel.operators;
 
@@ -30,12 +49,18 @@ auto YM2612::clock() -> array<i16[2]> {
       return y < 0x1a00 ? pow2[y & 0x1ff] << 2 >> (y >> 9) : 0; // -78 dB floor
     };
 
-    s32 feedback = modMask & op[0].prior + op[0].priorBuffer >> 9 - channel.feedback;
-    s32 accumulator = 0;
-
     op[0].priorBuffer = op[0].prior; // only need to buffer the output for feedback
     for(auto n : range(4)) op[n].prior = op[n].output;
 
+    for(auto& op : channel.operators) {
+      op.runPhase();
+      if(envelope.divider) continue;
+      op.runEnvelope();
+    }
+
+    s32 feedback = modMask & op[0].prior + op[0].priorBuffer >> 9 - channel.feedback;
+    s32 accumulator = 0;
+
     op[0].output = wave(0, feedback * (channel.feedback > 0));
 
     // Note: Despite not emulating per cycle, operator pipelining has been accounted for.
@@ -122,33 +147,6 @@ auto YM2612::clock() -> array<i16[2]> {
     //if(channel.rightEnable) right += voiceData;
   }
 
-  timerA.run();
-  timerB.run();
-
-  if(lfo.enable && ++lfo.divider == lfoDividers[lfo.rate]) {
-    lfo.divider = 0;
-    lfo.clock++;
-    for(auto& channel : channels) {
-      for(auto& op : channel.operators) {
-        op.updatePhase();  //due to vibrato
-        op.updateLevel();  //due to tremolo
-      }
-    }
-  }
-
-  if(++envelope.divider == 3) {
-    envelope.divider = 0;
-    if(!++envelope.clock) ++envelope.clock; // 12-bit counter: 1..4095 - zero-value is skipped (confirmed behavior)
-  }
-
-  for(auto& channel : channels) {
-    for(auto& op : channel.operators) {
-      op.runPhase();
-      if(envelope.divider) continue;
-      op.runEnvelope();
-    }
-  }
-
   return {sclamp<16>(left), sclamp<16>(right)};
 }
 

ares/component/audio/ym2612/ym2612.hpp

@@ -107,6 +107,7 @@ struct YM2612 {
       n1 keyOn = 0;
       n1 keyLine = 0;
       n1 lfoEnable = 0;
+      n5 keyScale = 0;
       n3 detune = 0;
       n4 multiple = 0;
       n7 totalLevel = 0;
@@ -140,7 +141,7 @@ struct YM2612 {
         n32 steps = 0;
         n10 value = 0x3ff;
 
-        n2  keyScale = 0;
+        n2  rateScaling = 0;
         n5  attackRate = 0;
         n5  decayRate = 0;
         n5  sustainRate = 0;

ares/md/system/serialization.cpp

@@ -1,4 +1,4 @@
-static const string SerializerVersion = "v141";
+static const string SerializerVersion = "v141.1";
 
 auto System::serialize(bool synchronize) -> serializer {
   if(synchronize) scheduler.enter(Scheduler::Mode::Synchronize);