Vibrato optimizations (#530)

Optimize vibrato to even exceed vanilla's implementation, all without restricting usability from the current implementation. This should save up to about 1ms of audio runtime.

sound/sequences/00_sound_player.s

@@ -226,7 +226,7 @@ chan_end
 .delay_interrupt:
 chan_setpanmix 127
 chan_setvolscale 127
-chan_setvibratoextent 0
+chan_setvibratoextent 0xff // 0xff represents disabled value
 chan_ioreadval 1 // IO slots 0-3 are reset to -1 when read; restore the value
 chan_iowriteval 0
 chan_break // break out of the loop
@@ -1147,6 +1147,9 @@ chan_setbank 4
 chan_setinstr 14
 chan_setdecayrelease 12
 chan_setvibratoextent 10
+chan_setval 0x7f
+chan_call .delay
+chan_setvibratoextent 0xff
 chan_end
 
 .layer_68F:
@@ -1317,7 +1320,7 @@ chan_setlayer 0, .layer_79D
 chan_setlayer 1, .layer_79B
 chan_setval 36
 chan_call .delay
-chan_setvibratoextent 0
+chan_setvibratoextent 0xff
 chan_end
 
 .layer_79B:
@@ -3025,6 +3028,9 @@ chan_setvibratorate 60
 chan_setval 25
 chan_call .set_reverb
 chan_setlayer 0, .layer_11E4
+chan_setval 0x45
+chan_call .delay
+chan_setvibratoextent 0xff
 chan_end
 
 .layer_11E4:
@@ -3070,7 +3076,7 @@ chan_setbank 4
 chan_setinstr 6
 chan_setval 49
 chan_call .delay
-chan_setvibratoextent 0
+chan_setvibratoextent 0xff
 chan_end
 
 .layer_1242:
@@ -4389,7 +4395,7 @@ chan_setlayer 0, .layer_1AEB
 chan_setlayer 1, .layer_1AE9
 chan_setval 35
 chan_call .delay
-chan_setvibratoextent 0
+chan_setvibratoextent 0xff
 chan_end
 
 .layer_1AE9:
@@ -6055,7 +6061,7 @@ chan_setvibratorate 60
 chan_setlayer 0, .layer_259B
 chan_setval 30
 chan_call .delay
-chan_setvibratoextent 0
+chan_setvibratoextent 0xff
 chan_end
 
 .layer_259B:
@@ -6189,7 +6195,7 @@ chan_setvibratorate 5
 chan_setlayer 0, .layer_2684
 chan_setval 88
 chan_call .delay
-chan_setvibratoextent 0
+chan_setvibratoextent 0xff
 chan_end
 
 .layer_2684:
@@ -6360,7 +6366,7 @@ chan_setlayer 0, .layer_27B7
 chan_setlayer 1, .layer_27B5
 chan_setval 56
 chan_call .delay
-chan_setvibratoextent 0
+chan_setvibratoextent 0xff
 chan_end
 
 .layer_27B5:
@@ -7067,6 +7073,9 @@ chan_setenvelope .envelope_3444
 chan_setvibratorate 1
 chan_setvibratoextent 100
 chan_setlayer 0, .layer_2CA0
+chan_setval 0x12
+chan_call .delay
+chan_setvibratoextent 0xff
 chan_end
 
 .layer_2CA0:
@@ -7094,7 +7103,7 @@ chan_call .delay
 chan_setvibratoextent 80
 chan_setval 67
 chan_call .delay
-chan_setvibratoextent 0
+chan_setvibratoextent 0xff
 chan_end
 
 .layer_2CD6:
@@ -7292,6 +7301,13 @@ chan_setval 70
 chan_call .delay
 chan_setbank 10
 chan_setinstr 8
+chan_setval 0x7f
+chan_call .delay
+chan_setval 0x7f
+chan_call .delay
+chan_setval 0x4
+chan_call .delay
+chan_setvibratoextent 0xff
 chan_end
 
 .layer_2E28:

src/audio/effects.c

@@ -189,23 +189,33 @@ s32 get_vibrato_pitch_change(struct VibratoState *vib) {
 #endif
 
 f32 get_vibrato_freq_scale(struct VibratoState *vib) {
+    s32 vibratoExtentTarget;
+
     if (vib->delay != 0) {
         vib->delay--;
         return 1;
     }
 
+    // This needs to be set locally because changing the original value to 0 overrides the whole channel,
+    // effectively negating the ability to disable vibrato and bypass this function in the first place.
+    // This function isn't huge but it otherwise would get called many thousands of times per second.
+    vibratoExtentTarget = vib->seqChannel->vibratoExtentTarget;
+    if (vibratoExtentTarget >= VIBRATO_DISABLED_VALUE) {
+        vibratoExtentTarget = 0;
+    }
+
     if (vib->extentChangeTimer) {
         if (vib->extentChangeTimer == 1) {
-            vib->extent = (s32) vib->seqChannel->vibratoExtentTarget;
+            vib->extent = vibratoExtentTarget;
         } else {
             vib->extent +=
-                ((s32) vib->seqChannel->vibratoExtentTarget - vib->extent) / (s32) vib->extentChangeTimer;
+                (vibratoExtentTarget - vib->extent) / (s32) vib->extentChangeTimer;
         }
 
         vib->extentChangeTimer--;
-    } else if (vib->seqChannel->vibratoExtentTarget != (s32) vib->extent) {
+    } else if (vibratoExtentTarget != (s32) vib->extent) {
         if ((vib->extentChangeTimer = vib->seqChannel->vibratoExtentChangeDelay) == 0) {
-            vib->extent = (s32) vib->seqChannel->vibratoExtentTarget;
+            vib->extent = vibratoExtentTarget;
         }
     }
 
@@ -246,11 +256,11 @@ void note_vibrato_update(struct Note *note) {
         note->vibratoFreqScale = get_vibrato_freq_scale(&note->vibratoState);
     }
 #else
-    if (note->vibratoState.active) {
+    if (note->vibratoState.activeFlags & VIBMODE_PORTAMENTO) {
         note->portamentoFreqScale = get_portamento_freq_scale(&note->portamento);
-        if (note->parentLayer != NO_LAYER) {
-            note->vibratoFreqScale = get_vibrato_freq_scale(&note->vibratoState);
-        }
+    }
+    if ((note->vibratoState.activeFlags & VIBMODE_VIBRATO) && note->parentLayer != NO_LAYER) {
+        note->vibratoFreqScale = get_vibrato_freq_scale(&note->vibratoState);
     }
 #endif
 }
@@ -265,18 +275,23 @@ void note_vibrato_init(struct Note *note) {
 
     struct VibratoState *vib = &note->vibratoState;
 
-/* This code was probably removed from EU and SH for a reason; probably because it's dumb and makes vibrato harder to use well.
 #if defined(VERSION_JP) || defined(VERSION_US)
-    if (note->parentLayer->seqChannel->vibratoExtentStart == 0
-        && note->parentLayer->seqChannel->vibratoExtentTarget == 0
-        && note->parentLayer->portamento.mode == 0) {
-        vib->active = FALSE;
+    vib->activeFlags = VIBMODE_NONE;
+    if (note->parentLayer->portamento.mode != 0) {
+        vib->activeFlags |= VIBMODE_PORTAMENTO;
+        note->portamento = note->parentLayer->portamento;
+    }
+
+    if (!(note->parentLayer->seqChannel->vibratoExtentStart == 0
+        && note->parentLayer->seqChannel->vibratoExtentTarget >= VIBRATO_DISABLED_VALUE)) {
+        vib->activeFlags |= VIBMODE_VIBRATO;
+    } else {
         return;
     }
-#endif
-*/
-
+#else
     vib->active = TRUE;
+#endif
+
     vib->time = 0;
 
 #if defined(VERSION_EU) || defined(VERSION_SH)
@@ -313,8 +328,6 @@ void note_vibrato_init(struct Note *note) {
         vib->rate = seqChannel->vibratoRateStart;
     }
     vib->delay = seqChannel->vibratoDelay;
-
-    note->portamento = note->parentLayer->portamento;
 #endif
 }
 

src/audio/effects.h

@@ -31,6 +31,12 @@ enum ADSRDelays {
     ADSR_RESTART = -3,
 };
 
+enum VibratoModes {
+    VIBMODE_NONE       = 0,
+    VIBMODE_VIBRATO    = (1 << 0),
+    VIBMODE_PORTAMENTO = (1 << 1),
+};
+
 // Envelopes are always stored as big endian, to match sequence files which are
 // byte blobs and can embed envelopes. Hence this byteswapping macro.
 #if IS_BIG_ENDIAN

src/audio/internal.h

@@ -27,6 +27,8 @@
 #endif
 #endif // EXPAND_AUDIO_HEAP
 
+#define VIBRATO_DISABLED_VALUE (0xFF * 8)
+
 #define NO_LAYER ((struct SequenceChannelLayer *)(-1))
 
 enum MuteBehaviors {
@@ -139,7 +141,7 @@ struct VibratoState {
     /*    , 0x14*/ u8 active;
 #else
     /*0x08,     */ s8 *curve;
-    /*0x0C,     */ u8 active;
+    /*0x0C,     */ u8 activeFlags;
     /*0x0E,     */ u16 rate;
     /*0x10,     */ u16 extent;
 #endif

src/audio/playback.c

@@ -1423,6 +1423,7 @@ void note_init_all(void) {
         note->prevParentLayer = NO_LAYER;
 #if defined(VERSION_EU) || defined(VERSION_SH)
         note->waveId = 0;
+        note->vibratoState.active = FALSE;
 #else
         note->reverbVol = 0;
         note->usesHeadsetPanEffects = FALSE;
@@ -1432,12 +1433,12 @@ void note_init_all(void) {
         note->targetVolRight = 0;
         note->frequency = 0.0f;
         note->unused1 = 0x3f;
+        note->vibratoState.activeFlags = VIBMODE_NONE;
 #endif
         note->attributes.velocity = 0.0f;
         note->adsrVolScale = 0;
         note->adsr.state = ADSR_STATE_DISABLED;
         note->adsr.action = 0;
-        note->vibratoState.active = FALSE;
         note->portamento.cur = 0.0f;
         note->portamento.speed = 0.0f;
 #if defined(VERSION_SH)

src/audio/seqplayer.c

@@ -64,7 +64,7 @@ void sequence_channel_init(struct SequenceChannel *seqChannel) {
 #endif
     seqChannel->vibratoRateTarget = 0x800;
     seqChannel->vibratoRateStart = 0x800;
-    seqChannel->vibratoExtentTarget = 0;
+    seqChannel->vibratoExtentTarget = VIBRATO_DISABLED_VALUE;
     seqChannel->vibratoExtentStart = 0;
     seqChannel->vibratoRateChangeDelay = 0;
     seqChannel->vibratoExtentChangeDelay = 0;
@@ -1942,7 +1942,7 @@ void sequence_channel_process_script(struct SequenceChannel *seqChannel) {
                         break;
 
                     case 0xec:
-                        seqChannel->vibratoExtentTarget = 0;
+                        seqChannel->vibratoExtentTarget = VIBRATO_DISABLED_VALUE;
                         seqChannel->vibratoExtentStart = 0;
                         seqChannel->vibratoExtentChangeDelay = 0;
                         seqChannel->vibratoRateTarget = 0;