Fix for issue #474, and a few other bugs in MODGenerator (#478)

Fixes #474

This bug was already present in stellaplayer. Obviously "effectNumber" must be checked, because "effectParameterX" just holds the "upper nibble" of effectParameter

- do channel mixing in 32bit, to avoid noise from over/underflow
- write audio output as "signed int" (not unsigned)
- add int16 saturation check ("clipping")
* avoid crash when MOD file has to many CHANNELS
- avoid guru meditation, by logging an error and refusing playback in case MOD.numberOfChannels > CHANNELS

Author: softhack007

src/AudioGeneratorMOD.cpp

@@ -232,6 +232,11 @@ bool AudioGeneratorMOD::LoadHeader()
     Mod.numberOfChannels = (temp[0] - '0') * 10 + temp[1] - '0';
   else
     Mod.numberOfChannels = 4;
+  
+  if (Mod.numberOfChannels > CHANNELS) {
+    audioLogger->printf("\nAudioGeneratorMOD::LoadHeader abort - too many channels (configured: %d, needed: %d)\n", CHANNELS, Mod.numberOfChannels);
+    return(false);
+  }
 
   return true;
 }
@@ -417,7 +422,7 @@ bool AudioGeneratorMOD::ProcessRow()
 
     if (sampleNumber) {
       Player.lastSampleNumber[channel] = sampleNumber - 1;
-      if (!(effectParameter == 0xE && effectParameterX == NOTEDELAY))
+      if (!(effectNumber == 0xE && effectParameterX == NOTEDELAY))
         Player.volume[channel] = Mod.samples[Player.lastSampleNumber[channel]].volume;
     }
 
@@ -735,13 +740,14 @@ bool AudioGeneratorMOD::RunPlayer()
 
 void AudioGeneratorMOD::GetSample(int16_t sample[2])
 {
-  int16_t sumL;
-  int16_t sumR;
+  int32_t sumL;
+  int32_t sumR;
   uint8_t channel;
   uint32_t samplePointer;
   int8_t current;
   int8_t next;
   int16_t out;
+  int32_t out32;
 
   if (!running) return;
 
@@ -779,7 +785,7 @@ void AudioGeneratorMOD::GetSample(int16_t sample[2])
         samplePointer >= FatBuffer.samplePointer[channel] + fatBufferSize - 1 ||
         Mixer.channelSampleNumber[channel] != FatBuffer.channelSampleNumber[channel]) {
 
-      uint16_t toRead = Mixer.sampleEnd[Mixer.channelSampleNumber[channel]] - samplePointer + 1;
+      uint32_t toRead = Mixer.sampleEnd[Mixer.channelSampleNumber[channel]] - samplePointer + 1;
       if (toRead > fatBufferSize) toRead  = fatBufferSize;
 
       if (!file->seek(samplePointer, SEEK_SET)) {
@@ -800,27 +806,46 @@ void AudioGeneratorMOD::GetSample(int16_t sample[2])
 
     out = current;
 
-    // Integer linear interpolation
+    // Integer linear interpolation - only works correctly in 16bit
     out += (next - current) * (Mixer.channelSampleOffset[channel] & ((1 << FIXED_DIVIDER) - 1)) >> FIXED_DIVIDER;
 
     // Upscale to BITDEPTH
-    out <<= BITDEPTH - 8;
+    out32 = (int32_t)out << (BITDEPTH - 8);
 
     // Channel volume
-    out = out * Mixer.channelVolume[channel] >> 6;
+    out32 = out32 * Mixer.channelVolume[channel] >> 6;
 
     // Channel panning
-    sumL += out * min(128 - Mixer.channelPanning[channel], 64) >> 6;
-    sumR += out * min(Mixer.channelPanning[channel], 64) >> 6;
+    sumL += out32 * min(128 - Mixer.channelPanning[channel], 64) >> 6;
+    sumR += out32 * min(Mixer.channelPanning[channel], 64) >> 6;
+  }
+  
+  // Downscale to BITDEPTH - a bit faster because the compiler can replaced division by constants with proper "right shift" + correct handling of sign bit
+  if (Mod.numberOfChannels <= 4) {
+      // up to 4 channels
+      sumL /= 4;
+      sumR /= 4;
+  } else {
+    if (Mod.numberOfChannels <= 6) {
+      // 5 or 6 channels - pre-multiply be 1.5, then divide by 8 -> same as division by 6
+      sumL = (sumL + (sumL/2)) / 8;
+      sumR = (sumR + (sumR/2)) / 8;      
+    } else {
+      // 7,8, or more channels
+      sumL /= 8;
+      sumR /= 8;
+    }
   }
 
-  // Downscale to BITDEPTH
-  sumL /= Mod.numberOfChannels;
-  sumR /= Mod.numberOfChannels;
-
-  // Fill the sound buffer with unsigned values
-  sample[AudioOutput::LEFTCHANNEL] = sumL + (1 << (BITDEPTH - 1));
-  sample[AudioOutput::RIGHTCHANNEL] = sumR + (1 << (BITDEPTH - 1));
+  // clip samples to 16bit (with saturation in case of overflow)
+  if(sumL <= INT16_MIN) sumL = INT16_MIN;
+    else if (sumL >= INT16_MAX) sumL = INT16_MAX;
+  if(sumR <= INT16_MIN) sumR = INT16_MIN;
+    else if (sumR >= INT16_MAX) sumR = INT16_MAX;
+  
+  // Fill the sound buffer with signed values
+  sample[AudioOutput::LEFTCHANNEL] = sumL;
+  sample[AudioOutput::RIGHTCHANNEL] = sumR;
 }
 
 bool AudioGeneratorMOD::LoadMOD()

src/AudioOutputI2S.cpp

@@ -300,7 +300,7 @@ bool AudioOutputI2S::ConsumeSample(int16_t sample[2])
     {
       int16_t l = Amplify(ms[LEFTCHANNEL]) + 0x8000;
       int16_t r = Amplify(ms[RIGHTCHANNEL]) + 0x8000;
-      s32 = (r << 16) | (l & 0xffff);
+      s32 = ((r & 0xffff) << 16) | (l & 0xffff);
     }
     else
     {