MIDI with soundfont - interrupting a MIDI with BGM (OFF) and playing another MIDI will play a note of the previous MIDI before playing the new one

[Mimigris]

Player platform:

Windows, 64 bits continuous build of the Player.

Describe the issue in detail and how to reproduce it:

While using a custom soundfont (the default setting on Windows does not work for this issue), interrupt a MIDI track with the command Play BGM: (OFF), and then play another MIDI track (can be the same one). When the new MIDI track will play, some notes of the previous MIDI track will be heard while they should not.

You can test this with testgame: interact with the NPC on the left or right to play a MIDI, then the one at the center to stop it, and then the one on the left or right again to encounter the issue (make sure to have a custom soundfont to encounter the issue). MIDIissue.zip

Reported by ajie on X, with the following footage as an example: https://twitter.com/aj_108fn/status/1716463030903652845

[Ghabry]

I think the problem is that the fluidsynth synth is recycled for all MIDI files. When the BGM is stopped there are still notes marked as "released" so they slowly fade out.

When the next BGM plays these notes continue fading out.

This is a design limitation in our code: There is no way to keep the old decoder alive so this fade can finish while the next already plays.

Workaround: Force all sounds off in fluidsynth when the Midi track changes. This is slightly incorrect because notes will be turned hard off now instead of fading, so if there is some kind of "cross-fading" in a game used between midi tracks this is now lost (I don't know any game that does this).

(The Native Midi Out does not have this issue btw because this is managed by the operating system)

---

diff --git a/src/audio_decoder_midi.cpp b/src/audio_decoder_midi.cpp
index 6874c48e3..e6832225a 100755
--- a/src/audio_decoder_midi.cpp
+++ b/src/audio_decoder_midi.cpp
@@ -398,6 +398,8 @@ void AudioDecoderMidi::reset() {
        midi_msg = midimsg_make(midi_event_control_change, channel, midi_set_reg_param_lower, 0);
        mididec->SendMidiMessage(midi_msg);
    }
+
+   mididec->Reset();
 }

 void AudioDecoderMidi::reset_tempos_after_loop() {
diff --git a/src/audio_midi.cpp b/src/audio_midi.cpp
index 011fb480b..285625570 100644
--- a/src/audio_midi.cpp
+++ b/src/audio_midi.cpp
@@ -126,7 +126,7 @@ std::unique_ptr<AudioDecoderBase> MidiDecoder::CreateFmMidi(bool resample) {
    return mididec;
 }

-void MidiDecoder::Reset() {
+void MidiDecoder::ResetGlobalState() {
    works.fluidsynth = true;
    works.wildmidi = true;

diff --git a/src/audio_midi.h b/src/audio_midi.h
index 88c5fd73a..ca10bea15 100644
--- a/src/audio_midi.h
+++ b/src/audio_midi.h
@@ -60,7 +60,7 @@ public:
     *
     * The library must implement the following commands:
     * - SendMidiMessage
-    * - SendSysExMessage (nice to have)
+    * - SendSysExMessage
     *
     * When Midi messages are not supported (library uses own sequencer)
     * - Open
@@ -166,6 +166,12 @@ public:
        return -1;
    };

+   /**
+    * Called when the device is reset due to a Midi file change.
+    * Can be used when the normal reset sequence (GM reset etc.) is not sufficient.
+    */
+   virtual void Reset() {};
+
    /**
     * Returns the unique name of the Midi decoder.
     *
@@ -197,7 +203,7 @@ public:
    /**
     * Resets the global state of the midi libraries.
     */
-   static void Reset();
+   static void ResetGlobalState();

 protected:
    int frequency = EP_MIDI_FREQ;
diff --git a/src/decoder_fluidsynth.cpp b/src/decoder_fluidsynth.cpp
index a4d6bf3c9..a2168a99f 100644
--- a/src/decoder_fluidsynth.cpp
+++ b/src/decoder_fluidsynth.cpp
@@ -17,6 +17,7 @@

 #include "system.h"
 #include "decoder_fluidsynth.h"
+#include <fluidsynth/synth.h>

 #if defined(HAVE_FLUIDSYNTH) || defined(HAVE_FLUIDLITE)

@@ -339,6 +340,15 @@ void FluidSynthDecoder::SendSysExMessage(const uint8_t* data, std::size_t size)
        nullptr, nullptr, nullptr, 0);
 }

+void FluidSynthDecoder::Reset() {
+   // Prevent that old notes resume playing when BGM is stopped and resumed later
+   auto* instance_synth = GetSynthInstance();
+
+   for (int channel = 0; channel < 16; channel++) {
+       fluid_synth_all_sounds_off(GetSynthInstance(), channel);
+   }
+}
+
 fluid_synth_t *FluidSynthDecoder::GetSynthInstance() {
    if (use_global_synth) {
        return global_synth.get();
diff --git a/src/decoder_fluidsynth.h b/src/decoder_fluidsynth.h
index a5e2896d8..63c8e7953 100644
--- a/src/decoder_fluidsynth.h
+++ b/src/decoder_fluidsynth.h
@@ -61,6 +61,8 @@ public:

    void SendSysExMessage(const uint8_t* data, size_t size) override;

+   void Reset() override;
+
    std::string GetName() override {
 #if defined(HAVE_FLUIDSYNTH)
        return "FluidSynth";
diff --git a/src/scene_gamebrowser.cpp b/src/scene_gamebrowser.cpp
index 77be95d74..801af3fad 100644
--- a/src/scene_gamebrowser.cpp
+++ b/src/scene_gamebrowser.cpp
@@ -51,7 +51,7 @@ void Scene_GameBrowser::Continue(SceneType /* prev_scene */) {

    Cache::ClearAll();
    AudioSeCache::Clear();
-   MidiDecoder::Reset();
+   MidiDecoder::ResetGlobalState();
    lcf::Data::Clear();
    Main_Data::Cleanup();

[Mimigris]

Reopening it since I'm able to reproduce this issue on the continuous build on another game. In Wadanohara and the Great Blue Sea, this issue is still present if a soundfont is used, and on any kind of MIDI that I could find in the game (title screen, battle, maps outside of the main sea that uses a wav...). An example footage can be found here of the issue at 0:29 in the video: you can hear notes from the previous MIDI there.