Fix issue with pitch bend up causing portomento to go up jagged.

The issue was with the midi_effectivePitchCents function.
rhargreaves · Jul 26, 2024 · bd5bfa4 · bd5bfa4
1 parent 35923b3
commit bd5bfa4
Show file tree

Hide file tree

Showing 6 changed files with 147 additions and 47 deletions.
diff --git a/src/midi.c b/src/midi.c
@@ -931,20 +931,25 @@ static void processChannelGlide(DeviceChannel* chan)
     }
 
     if (chan->glideTargetPitch > chan->pitch) {
-        chan->cents += increment;
-        if (chan->cents == 100) {
-            chan->pitch++;
-            chan->cents = 0;
-        }
-        chan->ops->pitch(chan->number, chan->pitch, chan->cents);
+
+        PitchCents pc = { .pitch = chan->pitch, .cents = chan->cents };
+        pc = midi_pitchShift(pc, increment);
+        chan->pitch = pc.pitch;
+        chan->cents = pc.cents;
+
+        pc = midi_effectivePitchCents(pc.pitch, pc.cents, chan->pitchBend);
+        chan->ops->pitch(chan->number, pc.pitch, pc.cents);
+
     } else if (chan->glideTargetPitch < chan->pitch
         || (chan->glideTargetPitch == chan->pitch && chan->cents > 0)) {
-        chan->cents -= increment;
-        if (chan->cents == 0 - increment) {
-            chan->pitch--;
-            chan->cents = 100 - increment;
-        }
-        chan->ops->pitch(chan->number, chan->pitch, chan->cents);
+
+        PitchCents pc = { .pitch = chan->pitch, .cents = chan->cents };
+        pc = midi_pitchShift(pc, 0 - increment);
+        chan->pitch = pc.pitch;
+        chan->cents = pc.cents;
+
+        pc = midi_effectivePitchCents(pc.pitch, pc.cents, chan->pitchBend);
+        chan->ops->pitch(chan->number, pc.pitch, pc.cents);
     }
 }
 
@@ -968,16 +973,29 @@ void midi_tick(void)
 
 PitchCents midi_effectivePitchCents(u8 pitch, s8 cents, u16 pitchBend)
 {
-    s16 newPitch = pitch + ((pitchBend - MIDI_PITCH_BEND_CENTRE) / 0x1000);
-    s16 newCents = cents + (((pitchBend - MIDI_PITCH_BEND_CENTRE) % 0x1000) / 41);
+    u16 totalCents = (pitch * 100) + cents;
+    s16 bendCents = ((pitchBend - MIDI_PITCH_BEND_CENTRE) * 25) / 1024; // 8192 pb = 200 cents
+    totalCents = totalCents + bendCents;
 
+    PitchCents pc = { .pitch = totalCents / 100, .cents = totalCents % 100 };
+    return pc;
+}
+
+PitchCents midi_pitchShift(PitchCents pc, s8 centsAdd)
+{
+    s16 newCents = pc.cents + centsAdd;
+    if (newCents < -100) {
+        newCents += 100;
+        pc.pitch--;
+    }
     if (newCents < 0) {
         newCents += 100;
-        newPitch--;
-    } else if (cents >= 100) {
+        pc.pitch--;
+    }
+    if (newCents >= 100) {
         newCents -= 100;
-        newPitch++;
+        pc.pitch++;
     }
-    PitchCents pc = { .cents = newCents, .pitch = newPitch };
+    pc.cents = newCents;
     return pc;
 }
diff --git a/src/midi.h b/src/midi.h
@@ -162,3 +162,4 @@ void midi_remap_channel(u8 midiChannel, u8 deviceChannel);
 void midi_reset(void);
 void midi_tick(void);
 PitchCents midi_effectivePitchCents(u8 pitch, s8 cents, u16 pitchBend);
+PitchCents midi_pitchShift(PitchCents pc, s8 centsAdd);
diff --git a/src/midi_psg.c b/src/midi_psg.c
@@ -120,25 +120,6 @@ static u16 toneFromPitchCents(PitchCents pc)
     return tone + (((toneForMidiKey(pc.pitch + 1) - tone) * pc.cents) / 100);
 }
 
-static PitchCents pitchShift(PitchCents pc, s8 centsAdd)
-{
-    s16 newCents = pc.cents + centsAdd;
-    if (newCents < -100) {
-        newCents += 100;
-        pc.pitch--;
-    }
-    if (newCents < 0) {
-        newCents += 100;
-        pc.pitch--;
-    }
-    if (newCents >= 100) {
-        newCents -= 100;
-        pc.pitch++;
-    }
-    pc.cents = newCents;
-    return pc;
-}
-
 static u16 envelopeTone(MidiPsgChannel* psgChan)
 {
     PitchCents pc = { .pitch = psgChan->pitch, .cents = psgChan->cents };
@@ -147,16 +128,16 @@ static u16 envelopeTone(MidiPsgChannel* psgChan)
     case 0:
         break;
     case 0x1:
-        pc = pitchShift(pc, 5);
+        pc = midi_pitchShift(pc, 5);
         break;
     case 0x2:
-        pc = pitchShift(pc, 10);
+        pc = midi_pitchShift(pc, 10);
         break;
     case 0x3:
-        pc = pitchShift(pc, 20);
+        pc = midi_pitchShift(pc, 20);
         break;
     case 0x4:
-        pc = pitchShift(pc, 50);
+        pc = midi_pitchShift(pc, 50);
         break;
     case 0x5:
         pc.pitch += 1;
@@ -168,16 +149,16 @@ static u16 envelopeTone(MidiPsgChannel* psgChan)
         pc.pitch += 5;
         break;
     case 0x8:
-        pc = pitchShift(pc, -5);
+        pc = midi_pitchShift(pc, -5);
         break;
     case 0x9:
-        pc = pitchShift(pc, -10);
+        pc = midi_pitchShift(pc, -10);
         break;
     case 0xA:
-        pc = pitchShift(pc, -20);
+        pc = midi_pitchShift(pc, -20);
         break;
     case 0xB:
-        pc = pitchShift(pc, -50);
+        pc = midi_pitchShift(pc, -50);
         break;
     case 0xC:
         pc.pitch -= 1;

diff --git a/tests/unit/main.c b/tests/unit/main.c
@@ -190,6 +190,10 @@ int main(void)
         midi_test(test_midi_effectivePitchCents_4),
         midi_test(test_midi_effectivePitchCents_5),
         midi_test(test_midi_effectivePitchCents_6),
+        midi_test(test_midi_effectivePitchCents_cents_with_partial_bend_down),
+        midi_test(test_midi_effectivePitchCents_high_cents_with_partial_bend_down),
+        midi_test(test_midi_effectivePitchCents_cents_with_full_bend_up),
+        midi_test(test_midi_effectivePitchCents_high_cents_with_full_bend_up),
 
         midi_pcm_test(test_midi_enables_dac),
         midi_pcm_test(test_midi_disables_dac),
@@ -387,6 +391,8 @@ int main(void)
         midi_portamento_test(test_midi_portamento_synth_note_off_triggered),
         midi_portamento_test(test_midi_portamento_zeros_any_residual_cents),
         midi_portamento_test(test_midi_portamento_glides_note_up_for_psg),
+        midi_portamento_test(test_midi_portamento_glides_note_up_with_pitch_bend),
+        midi_portamento_test(test_midi_portamento_glides_note_down_with_pitch_bend)
         // TODO: test for special mode glide
         // TODO: pitch bend conflicts
         // clang-format on

diff --git a/tests/unit/test_midi_fm.c b/tests/unit/test_midi_fm.c
@@ -773,13 +773,45 @@ static void test_midi_effectivePitchCents_5(UNUSED void** state)
     PitchCents pc = midi_effectivePitchCents(50, 0, 0x1800);
 
     assert_int_equal(pc.pitch, 49);
-    assert_int_equal(pc.cents, 51);
+    assert_int_equal(pc.cents, 50);
 }
 
 static void test_midi_effectivePitchCents_6(UNUSED void** state)
 {
     PitchCents pc = midi_effectivePitchCents(50, 0, 0x2800);
 
     assert_int_equal(pc.pitch, 50);
-    assert_int_equal(pc.cents, 49);
+    assert_int_equal(pc.cents, 50);
+}
+
+static void test_midi_effectivePitchCents_cents_with_partial_bend_down(UNUSED void** state)
+{
+    PitchCents pc = midi_effectivePitchCents(50, 25, 0x1800);
+
+    assert_int_equal(pc.pitch, 49);
+    assert_int_equal(pc.cents, 75);
+}
+
+static void test_midi_effectivePitchCents_high_cents_with_partial_bend_down(UNUSED void** state)
+{
+    PitchCents pc = midi_effectivePitchCents(50, 80, 0x1800);
+
+    assert_int_equal(pc.pitch, 50);
+    assert_int_equal(pc.cents, 30);
+}
+
+static void test_midi_effectivePitchCents_cents_with_full_bend_up(UNUSED void** state)
+{
+    PitchCents pc = midi_effectivePitchCents(50, 25, 0x4000);
+
+    assert_int_equal(pc.pitch, 52);
+    assert_int_equal(pc.cents, 25);
+}
+
+static void test_midi_effectivePitchCents_high_cents_with_full_bend_up(UNUSED void** state)
+{
+    PitchCents pc = midi_effectivePitchCents(50, 80, 0x4000);
+
+    assert_int_equal(pc.pitch, 52);
+    assert_int_equal(pc.cents, 80);
 }
diff --git a/tests/unit/test_midi_portamento.c b/tests/unit/test_midi_portamento.c
@@ -310,3 +310,65 @@ static void test_midi_portamento_glides_note_up_for_psg(UNUSED void** state)
         midi_tick();
     }
 }
+
+static void test_midi_portamento_glides_note_up_with_pitch_bend(UNUSED void** state)
+{
+    for (u8 chan = 0; chan < MAX_FM_CHANS; chan++) {
+        debug_message("channel %d\n", chan);
+        __real_midi_cc(chan, CC_PORTAMENTO_ENABLE, 127);
+
+        expect_synth_pitch(chan, 2, 0x439);
+        expect_synth_volume_any();
+        expect_synth_noteOn(chan);
+        __real_midi_note_on(chan, MIDI_PITCH_A2, MAX_MIDI_VOLUME);
+
+        expect_synth_pitch(chan, 2, 0x47a);
+        __real_midi_pitch_bend(chan, 0x3000); // +1st
+
+        __real_midi_note_on(chan, MIDI_PITCH_C4, MAX_MIDI_VOLUME);
+        expect_synth_pitch(chan, 2, 0x480);
+        midi_tick();
+
+        for (u16 i = 0; i < 148; i++) {
+            expect_synth_pitch_any();
+            midi_tick();
+        }
+
+        expect_synth_pitch(chan, 4, 681);
+        midi_tick();
+
+        midi_tick();
+        midi_tick();
+    }
+}
+
+static void test_midi_portamento_glides_note_down_with_pitch_bend(UNUSED void** state)
+{
+    for (u8 chan = 0; chan < MAX_FM_CHANS; chan++) {
+        debug_message("channel %d\n", chan);
+        __real_midi_cc(chan, CC_PORTAMENTO_ENABLE, 127);
+
+        expect_synth_pitch(chan, 4, 0x284);
+        expect_synth_volume_any();
+        expect_synth_noteOn(chan);
+        __real_midi_note_on(chan, MIDI_PITCH_C4, MAX_MIDI_VOLUME);
+
+        expect_synth_pitch(chan, 4, 0x25f);
+        __real_midi_pitch_bend(chan, 0x1000); // -1st
+
+        __real_midi_note_on(chan, MIDI_PITCH_A2, MAX_MIDI_VOLUME);
+        expect_synth_pitch(chan, 3, 0x4b7);
+        midi_tick();
+
+        for (u16 i = 0; i < 148; i++) {
+            expect_synth_pitch_any();
+            midi_tick();
+        }
+
+        expect_synth_pitch(chan, 2, 0x3fd);
+        midi_tick();
+
+        midi_tick();
+        midi_tick();
+    }
+}