From 43ff4d39442e57aeca98f539ae560a7c6b87c79c Mon Sep 17 00:00:00 2001 From: Ghabry Date: Fri, 22 Sep 2023 20:45:19 +0200 Subject: [PATCH 1/2] GenericAudio: Make static variables instance variables This way they are destroyed when the DisplayUi is destroyed instead of when the process terminates. Fixes a use-after-free in the Fluidsynth Decoder. Fix #3015 Fix #3035 Fix #3079 --- src/audio_generic.cpp | 31 ++++++++++--------------------- src/audio_generic.h | 23 ++++++++++++----------- 2 files changed, 22 insertions(+), 32 deletions(-) mode change 100644 => 100755 src/audio_generic.cpp mode change 100644 => 100755 src/audio_generic.h diff --git a/src/audio_generic.cpp b/src/audio_generic.cpp old mode 100644 new mode 100755 index 10d1df065b..f210b330ba --- a/src/audio_generic.cpp +++ b/src/audio_generic.cpp @@ -22,31 +22,20 @@ #include #include "audio_decoder_midi.h" #include "audio_generic.h" -#include "audio_generic_midiout.h" -#include "filefinder.h" #include "output.h" -GenericAudio::BgmChannel GenericAudio::BGM_Channels[nr_of_bgm_channels]; -GenericAudio::SeChannel GenericAudio::SE_Channels[nr_of_se_channels]; -bool GenericAudio::BGM_PlayedOnceIndicator; - -std::vector GenericAudio::sample_buffer = {}; -std::vector GenericAudio::scrap_buffer = {}; -unsigned GenericAudio::scrap_buffer_size = 0; -std::vector GenericAudio::mixer_buffer = {}; - -std::unique_ptr GenericAudio::midi_thread; - GenericAudio::GenericAudio(const Game_ConfigAudio& cfg) : AudioInterface(cfg) { int i = 0; for (auto& BGM_Channel : BGM_Channels) { BGM_Channel.id = i++; BGM_Channel.decoder.reset(); + BGM_Channel.instance = this; } i = 0; for (auto& SE_Channel : SE_Channels) { SE_Channel.id = i++; SE_Channel.decoder.reset(); + SE_Channel.instance = this; } BGM_PlayedOnceIndicator = false; midi_thread.reset(); @@ -492,8 +481,8 @@ void GenericAudio::BgmChannel::Stop() { stopped = true; if (midi_out_used) { midi_out_used = false; - midi_thread->GetMidiOut().Reset(); - midi_thread->GetMidiOut().Pause(); + instance->midi_thread->GetMidiOut().Reset(); + instance->midi_thread->GetMidiOut().Pause(); } else if (decoder) { decoder.reset(); } @@ -503,16 +492,16 @@ void GenericAudio::BgmChannel::SetPaused(bool newPaused) { paused = newPaused; if (midi_out_used) { if (newPaused) { - midi_thread->GetMidiOut().Pause(); + instance->midi_thread->GetMidiOut().Pause(); } else { - midi_thread->GetMidiOut().Resume(); + instance->midi_thread->GetMidiOut().Resume(); } } } int GenericAudio::BgmChannel::GetTicks() const { if (midi_out_used) { - return midi_thread->GetMidiOut().GetTicks(); + return instance->midi_thread->GetMidiOut().GetTicks(); } else if (decoder) { return decoder->GetTicks(); } @@ -521,7 +510,7 @@ int GenericAudio::BgmChannel::GetTicks() const { void GenericAudio::BgmChannel::SetFade(int fade) { if (midi_out_used) { - midi_thread->GetMidiOut().SetFade(0, std::chrono::milliseconds(fade)); + instance->midi_thread->GetMidiOut().SetFade(0, std::chrono::milliseconds(fade)); } else if (decoder) { decoder->SetFade(0, std::chrono::milliseconds(fade)); } @@ -529,7 +518,7 @@ void GenericAudio::BgmChannel::SetFade(int fade) { void GenericAudio::BgmChannel::SetVolume(int volume) { if (midi_out_used) { - midi_thread->GetMidiOut().SetVolume(volume); + instance->midi_thread->GetMidiOut().SetVolume(volume); } else if (decoder) { decoder->SetVolume(volume); } @@ -537,7 +526,7 @@ void GenericAudio::BgmChannel::SetVolume(int volume) { void GenericAudio::BgmChannel::SetPitch(int pitch) { if (midi_out_used) { - midi_thread->GetMidiOut().SetPitch(pitch); + instance->midi_thread->GetMidiOut().SetPitch(pitch); } else if (decoder) { decoder->SetPitch(pitch); } diff --git a/src/audio_generic.h b/src/audio_generic.h old mode 100644 new mode 100755 index b80493e608..04755a2c7d --- a/src/audio_generic.h +++ b/src/audio_generic.h @@ -21,10 +21,9 @@ #include "audio.h" #include "audio_secache.h" #include "audio_decoder_base.h" +#include "audio_generic_midiout.h" #include -class GenericAudioMidiOut; - /** * A software implementation for handling EasyRPG Audio utilizing the * AudioDecoder for BGM and AudioSeCache for fast SE playback. @@ -73,6 +72,7 @@ class GenericAudio : public AudioInterface { struct BgmChannel { int id; std::unique_ptr decoder; + GenericAudio* instance = nullptr; bool paused; bool stopped; bool midi_out_used = false; @@ -87,6 +87,7 @@ class GenericAudio : public AudioInterface { struct SeChannel { int id; std::unique_ptr decoder; + GenericAudio* instance = nullptr; bool paused; bool stopped; }; @@ -103,17 +104,17 @@ class GenericAudio : public AudioInterface { static constexpr unsigned nr_of_se_channels = 31; static constexpr unsigned nr_of_bgm_channels = 2; - static BgmChannel BGM_Channels[nr_of_bgm_channels]; - static SeChannel SE_Channels[nr_of_se_channels]; - static bool BGM_PlayedOnceIndicator; - static bool Muted; + BgmChannel BGM_Channels[nr_of_bgm_channels]; + SeChannel SE_Channels[nr_of_se_channels]; + mutable bool BGM_PlayedOnceIndicator; + bool Muted; - static std::vector sample_buffer; - static std::vector scrap_buffer; - static unsigned scrap_buffer_size; - static std::vector mixer_buffer; + std::vector sample_buffer = {}; + std::vector scrap_buffer = {}; + unsigned scrap_buffer_size = 0; + std::vector mixer_buffer = {}; - static std::unique_ptr midi_thread; + std::unique_ptr midi_thread; }; #endif From 2edb2174be1bb133e48c2662fe41c6e1206cacef Mon Sep 17 00:00:00 2001 From: Ghabry Date: Fri, 22 Sep 2023 21:17:03 +0200 Subject: [PATCH 2/2] Reinitialize the Midi libraries when returning to the game browser. Now custom soundfonts are always loaded when switching the game instead of only the first time. Fix #2916 --- src/audio_generic.cpp | 1 - src/audio_generic_midiout.h | 2 +- src/audio_midi.cpp | 13 +++++++++ src/audio_midi.h | 5 ++++ src/decoder_fluidsynth.cpp | 58 ++++++++++++++++++++----------------- src/decoder_fluidsynth.h | 1 + src/decoder_wildmidi.cpp | 26 +++++++++++++---- src/decoder_wildmidi.h | 1 + src/scene_gamebrowser.cpp | 2 ++ 9 files changed, 74 insertions(+), 35 deletions(-) diff --git a/src/audio_generic.cpp b/src/audio_generic.cpp index f210b330ba..1135064b9e 100755 --- a/src/audio_generic.cpp +++ b/src/audio_generic.cpp @@ -20,7 +20,6 @@ #include #include #include -#include "audio_decoder_midi.h" #include "audio_generic.h" #include "output.h" diff --git a/src/audio_generic_midiout.h b/src/audio_generic_midiout.h index 9858a0df80..f9538a3bf1 100644 --- a/src/audio_generic_midiout.h +++ b/src/audio_generic_midiout.h @@ -20,8 +20,8 @@ #include #include "system.h" +#include "audio_decoder_midi.h" -class AudioDecoderMidi; namespace Filesystem_Stream { class InputStream; } diff --git a/src/audio_midi.cpp b/src/audio_midi.cpp index 0468a8e7fd..011fb480b4 100644 --- a/src/audio_midi.cpp +++ b/src/audio_midi.cpp @@ -125,3 +125,16 @@ std::unique_ptr MidiDecoder::CreateFmMidi(bool resample) { return mididec; } + +void MidiDecoder::Reset() { + works.fluidsynth = true; + works.wildmidi = true; + +#ifdef HAVE_LIBWILDMIDI + WildMidiDecoder::ResetState(); +#endif + +#if defined(HAVE_FLUIDSYNTH) || defined(HAVE_FLUIDLITE) + FluidSynthDecoder::ResetState(); +#endif +} diff --git a/src/audio_midi.h b/src/audio_midi.h index 25fdd1ab7d..88c5fd73ae 100644 --- a/src/audio_midi.h +++ b/src/audio_midi.h @@ -194,6 +194,11 @@ class MidiDecoder { static std::unique_ptr CreateFmMidi(bool resample); + /** + * Resets the global state of the midi libraries. + */ + static void Reset(); + protected: int frequency = EP_MIDI_FREQ; }; diff --git a/src/decoder_fluidsynth.cpp b/src/decoder_fluidsynth.cpp index d7830517e7..a4d6bf3c92 100644 --- a/src/decoder_fluidsynth.cpp +++ b/src/decoder_fluidsynth.cpp @@ -95,6 +95,9 @@ static fluid_fileapi_t fluidlite_vio = { namespace { std::string preferred_soundfont; + + bool once = false; + bool init = false; } struct FluidSynthDeleter { @@ -208,10 +211,7 @@ FluidSynthDecoder::~FluidSynthDecoder() { } bool FluidSynthDecoder::Initialize(std::string& error_message) { - static bool init = false; - static bool once = false; - - // only initialize once + // only initialize once until a new game starts if (once) return init; once = true; @@ -220,40 +220,36 @@ bool FluidSynthDecoder::Initialize(std::string& error_message) { fluid_set_default_fileapi(&fluidlite_vio); #endif + global_settings.reset(new_fluid_settings()); if (!global_settings) { - global_settings.reset(new_fluid_settings()); - if (!global_settings) { - return false; - } - fluid_settings_setstr(global_settings.get(), "player.timing-source", "sample"); - fluid_settings_setint(global_settings.get(), "synth.lock-memory", 0); + return false; + } + fluid_settings_setstr(global_settings.get(), "player.timing-source", "sample"); + fluid_settings_setint(global_settings.get(), "synth.lock-memory", 0); - fluid_settings_setnum(global_settings.get(), "synth.gain", 0.6); - fluid_settings_setnum(global_settings.get(), "synth.sample-rate", EP_MIDI_FREQ); - fluid_settings_setint(global_settings.get(), "synth.polyphony", 256); + fluid_settings_setnum(global_settings.get(), "synth.gain", 0.6); + fluid_settings_setnum(global_settings.get(), "synth.sample-rate", EP_MIDI_FREQ); + fluid_settings_setint(global_settings.get(), "synth.polyphony", 256); #if defined(HAVE_FLUIDSYNTH) && FLUIDSYNTH_VERSION_MAJOR > 1 - fluid_settings_setint(global_settings.get(), "synth.reverb.active", 0); - fluid_settings_setint(global_settings.get(), "synth.chorus.active", 0); + fluid_settings_setint(global_settings.get(), "synth.reverb.active", 0); + fluid_settings_setint(global_settings.get(), "synth.chorus.active", 0); #else - fluid_settings_setstr(global_settings.get(), "synth.reverb.active", "no"); - fluid_settings_setstr(global_settings.get(), "synth.chorus.active", "no"); + fluid_settings_setstr(global_settings.get(), "synth.reverb.active", "no"); + fluid_settings_setstr(global_settings.get(), "synth.chorus.active", "no"); #endif - // Fluidsynth 1.x does not support VIO API for soundfonts + // Fluidsynth 1.x does not support VIO API for soundfonts #if defined(HAVE_FLUIDSYNTH) && FLUIDSYNTH_VERSION_MAJOR > 1 - // owned by fluid_settings - global_loader = new_fluid_defsfloader(global_settings.get()); - fluid_sfloader_set_callbacks(global_loader, - vio_open, vio_read, vio_seek, vio_tell, vio_close); + // owned by fluid_settings + global_loader = new_fluid_defsfloader(global_settings.get()); + fluid_sfloader_set_callbacks(global_loader, + vio_open, vio_read, vio_seek, vio_tell, vio_close); #endif - } + global_synth.reset(create_synth(error_message)); if (!global_synth) { - global_synth.reset(create_synth(error_message)); - if (!global_synth) { - return false; - } + return false; } init = true; @@ -261,6 +257,14 @@ bool FluidSynthDecoder::Initialize(std::string& error_message) { return init; } +void FluidSynthDecoder::ResetState() { + once = false; + init = false; + + global_synth.reset(); + global_settings.reset(); +} + void FluidSynthDecoder::SetSoundfont(StringView sf) { preferred_soundfont = ToString(sf); } diff --git a/src/decoder_fluidsynth.h b/src/decoder_fluidsynth.h index 740291b3a9..a5e2896d8f 100644 --- a/src/decoder_fluidsynth.h +++ b/src/decoder_fluidsynth.h @@ -45,6 +45,7 @@ class FluidSynthDecoder : public MidiDecoder { ~FluidSynthDecoder() override; static bool Initialize(std::string& error_message); + static void ResetState(); /** * Sets the name of the preferred soundfont. diff --git a/src/decoder_wildmidi.cpp b/src/decoder_wildmidi.cpp index c17c9d6617..1278fdfd0f 100644 --- a/src/decoder_wildmidi.cpp +++ b/src/decoder_wildmidi.cpp @@ -38,8 +38,16 @@ */ #define WILDMIDI_OPTS 0 +namespace { + bool once = false; + bool init = false; +} + static void WildMidiDecoder_deinit() { - WildMidi_Shutdown(); + if (init) { + WildMidi_Shutdown(); + init = false; + } } #if LIBWILDMIDI_VERSION >= 1027 // at least 0.4.3 @@ -80,10 +88,7 @@ bool WildMidiDecoder::Initialize(std::string& error_message) { std::string config_file; bool found = false; - static bool init = false; - static bool once = false; - - // only initialize once + // only initialize once until a new game starts if (once) return init; once = true; @@ -289,11 +294,20 @@ bool WildMidiDecoder::Initialize(std::string& error_message) { #endif // setup deinitialization - atexit(WildMidiDecoder_deinit); + static bool atexit_once = false; + if (!atexit_once) { + atexit_once = true; + atexit(WildMidiDecoder_deinit); + } return true; } +void WildMidiDecoder::ResetState() { + once = false; + WildMidiDecoder_deinit(); +} + bool WildMidiDecoder::Open(std::vector& data) { // this should not happen if (handle) { diff --git a/src/decoder_wildmidi.h b/src/decoder_wildmidi.h index 4ef83ce607..4142b3e09e 100644 --- a/src/decoder_wildmidi.h +++ b/src/decoder_wildmidi.h @@ -34,6 +34,7 @@ class WildMidiDecoder : public MidiDecoder { ~WildMidiDecoder(); static bool Initialize(std::string& error_message); + static void ResetState(); // Audio Decoder interface bool Open(std::vector& data) override; diff --git a/src/scene_gamebrowser.cpp b/src/scene_gamebrowser.cpp index c8bb105c8f..e48fcb1971 100644 --- a/src/scene_gamebrowser.cpp +++ b/src/scene_gamebrowser.cpp @@ -21,6 +21,7 @@ #include #include "options.h" #include "scene_settings.h" +#include "audio_midi.h" #include "audio_secache.h" #include "cache.h" #include "game_system.h" @@ -49,6 +50,7 @@ void Scene_GameBrowser::Continue(SceneType /* prev_scene */) { Cache::ClearAll(); AudioSeCache::Clear(); + MidiDecoder::Reset(); lcf::Data::Clear(); Main_Data::Cleanup();