Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MacOS Fixes #3148

Merged
merged 3 commits into from
Nov 20, 2023
Merged

MacOS Fixes #3148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f7f9c78
Midi: basic validation to prevent synth crashes
tyrone-sudeium Nov 16, 2023
46e20b1
MacOS: Fix link issue with cmake xcode generator
tyrone-sudeium Nov 16, 2023
77b52c8
Midi: opt FluidSynth into NeedsSoftReset (Fixes #3135)
tyrone-sudeium Nov 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -551,8 +551,8 @@ endif()

if(${PLAYER_TARGET_PLATFORM} STREQUAL "SDL2")
target_sources(${PROJECT_NAME} PRIVATE
src/platform/sdl/audio.cpp
src/platform/sdl/audio.h
src/platform/sdl/sdl_audio.cpp
src/platform/sdl/sdl_audio.h
src/platform/sdl/sdl2_ui.cpp
src/platform/sdl/sdl2_ui.h)
target_compile_definitions(${PROJECT_NAME} PUBLIC USE_SDL=2)
Expand All @@ -574,8 +574,8 @@ if(${PLAYER_TARGET_PLATFORM} STREQUAL "SDL2")
endif()
elseif(${PLAYER_TARGET_PLATFORM} STREQUAL "SDL1")
target_sources(${PROJECT_NAME} PRIVATE
src/platform/sdl/audio.cpp
src/platform/sdl/audio.h
src/platform/sdl/sdl_audio.cpp
src/platform/sdl/sdl_audio.h
src/platform/sdl/axis.h
src/platform/sdl/sdl_ui.cpp
src/platform/sdl/sdl_ui.h)
Expand Down Expand Up @@ -668,8 +668,8 @@ elseif(${PLAYER_TARGET_PLATFORM} STREQUAL "wii")
src/platform/wii/clock.cpp
src/platform/wii/clock.h
src/platform/wii/input_buttons.cpp
src/platform/sdl/audio.cpp
src/platform/sdl/audio.h
src/platform/sdl/sdl_audio.cpp
src/platform/sdl/sdl_audio.h
src/platform/sdl/axis.h
src/platform/sdl/sdl_ui.cpp
src/platform/sdl/sdl_ui.h)
Expand All @@ -681,8 +681,8 @@ elseif(${PLAYER_TARGET_PLATFORM} STREQUAL "amigaos4")
target_compile_definitions(${PROJECT_NAME} PUBLIC USE_SDL=1)
target_include_directories(${PROJECT_NAME} PUBLIC ${SDL_INCLUDE_DIR})
target_sources(${PROJECT_NAME} PRIVATE
src/platform/sdl/audio.cpp
src/platform/sdl/audio.h
src/platform/sdl/sdl_audio.cpp
src/platform/sdl/sdl_audio.h
src/platform/sdl/axis.h
src/platform/sdl/sdl_ui.cpp
src/platform/sdl/sdl_ui.h)
Expand Down Expand Up @@ -753,8 +753,8 @@ endif()

if(APPLE)
target_sources(${PROJECT_NAME} PRIVATE
src/platform/macos/utils.mm
src/platform/macos/utils.h
src/platform/macos/macos_utils.mm
src/platform/macos/macos_utils.h
src/platform/macos/midiout_device_coreaudio.cpp
src/platform/macos/midiout_device_coreaudio.h)

Expand Down
12 changes: 6 additions & 6 deletions Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,8 +443,8 @@ libeasyrpg_player_a_SOURCES = \
SOURCEFILES_SDL2 = \
src/platform/sdl/sdl2_ui.cpp \
src/platform/sdl/sdl2_ui.h \
src/platform/sdl/audio.cpp \
src/platform/sdl/audio.h
src/platform/sdl/sdl_audio.cpp \
src/platform/sdl/sdl_audio.h
if HAVE_SDL2
libeasyrpg_player_a_SOURCES += $(SOURCEFILES_SDL2)
else
Expand All @@ -454,8 +454,8 @@ endif
SOURCEFILES_SDL1 = \
src/platform/sdl/sdl_ui.cpp \
src/platform/sdl/sdl_ui.h \
src/platform/sdl/audio.cpp \
src/platform/sdl/audio.h \
src/platform/sdl/sdl_audio.cpp \
src/platform/sdl/sdl_audio.h \
src/platform/sdl/axis.h
if HAVE_SDL1
libeasyrpg_player_a_SOURCES += $(SOURCEFILES_SDL1)
Expand Down Expand Up @@ -493,8 +493,8 @@ EXTRA_DIST += $(SOURCEFILES_ALSA)
endif

SOURCEFILES_MACOS = \
src/platform/macos/utils.h \
src/platform/macos/utils.mm \
src/platform/macos/macos_utils.h \
src/platform/macos/macos_utils.mm \
src/platform/macos/midiout_device_coreaudio.cpp \
src/platform/macos/midiout_device_coreaudio.h
if MACOS
Expand Down
37 changes: 33 additions & 4 deletions src/audio_decoder_midi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ static const uint8_t midi_set_reg_param_upper = 0x6;
static const uint8_t midi_control_volume = 0x7;
static const uint8_t midi_event_control_change = 0xB;
static const uint8_t midi_set_reg_param_lower = 0x26;
//static const uint8_t midi_control_all_sound_off = 0x78;
static const uint8_t midi_control_all_sound_off = 0x78;
static const uint8_t midi_control_reset_all_controller = 0x79;
//static const uint8_t midi_control_all_note_off = 0x7B;
static const uint8_t midi_set_reg_param_number_lower = 0x64;
Expand All @@ -57,11 +57,11 @@ static uint32_t midimsg_make(uint8_t event_type, uint8_t channel, uint8_t value1

/*static uint32_t midimsg_all_note_off(uint8_t channel) {
return midimsg_make(midi_event_control_change, channel, midi_control_all_note_off, 0);
}
}*/

static uint32_t midimsg_all_sound_off(uint8_t channel) {
return midimsg_make(midi_event_control_change, channel, midi_control_all_sound_off, 0);
}*/
}

static uint32_t midimsg_volume(uint8_t channel, uint8_t volume) {
return midimsg_make(midi_event_control_change, channel, midi_control_volume, volume);
Expand All @@ -87,6 +87,28 @@ static inline uint8_t midimsg_get_value2(uint32_t msg) {
return (msg & 0xFF0000) >> 16;
}

static bool midimsg_validate(uint32_t msg) {
// Do some very basic sanity checks on the messages as
// invalid messages can cause the underlying MIDI synth to crash.
uint8_t event_type = midimsg_get_event_type(msg);
uint8_t channel = midimsg_get_channel(msg);
uint8_t value1 = midimsg_get_value1(msg);
uint8_t value2 = midimsg_get_value2(msg);

if ((event_type >= 0 && event_type < 64) ||
(event_type >= 70 && event_type < 84) ||
(event_type == 88) ||
(event_type >= 91 && event_type < 96) ||
(event_type >= 98 && event_type < 102))
{
if (value1 >= 128) {
// These event types must have a value of 0-127
return false;
}
}
return true;
}

AudioDecoderMidi::AudioDecoderMidi(std::unique_ptr<MidiDecoder> mididec)
: mididec(std::move(mididec)) {
seq = std::make_unique<midisequencer::sequencer>();
Expand Down Expand Up @@ -357,7 +379,9 @@ void AudioDecoderMidi::midi_message(int, uint_least32_t message) {
// Send the modified volume to midiout
message = midimsg_volume(channel, static_cast<uint8_t>(value2 * volume));
}
mididec->SendMidiMessage(message);
if (midimsg_validate(message)) {
mididec->SendMidiMessage(message);
}
}

void AudioDecoderMidi::sysex_message(int, const void* data, std::size_t size) {
Expand All @@ -384,6 +408,11 @@ void AudioDecoderMidi::reset() {
const unsigned char gm_reset[] = { 0xF0, 0x7E, 0x7F, 0x09, 0x01, 0xF7 };
mididec->SendSysExMessage(gm_reset, sizeof(gm_reset));

// SoundOff every channel: only necessary for synths like macOS which tend to get stuck
if (mididec->NeedsSoftReset()) {
SendMessageToAllChannels(midimsg_all_sound_off(0));
}

// Set the Pitch bend range to 256
for (int channel = 0; channel < 16; channel++) {
auto midi_msg = midimsg_make(midi_event_control_change, channel, midi_set_reg_param_number_upper, 0);
Expand Down
10 changes: 10 additions & 0 deletions src/audio_midi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,16 @@ class MidiDecoder {
return true;
}

/*
* Does the sequencer need "sound off" messages sent to every channel between
* tracks? NOTE: enabling this can break smooth fade outs between tracks.
*
* @return true only if synth needs "sound off" messages between tracks.
*/
virtual bool NeedsSoftReset() {
return false;
}

/**
* Attempts to initialize a Midi library for processing the Midi data.
*
Expand Down
4 changes: 4 additions & 0 deletions src/decoder_fluidsynth.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,4 +347,8 @@ fluid_synth_t *FluidSynthDecoder::GetSynthInstance() {
}
}

bool FluidSynthDecoder::NeedsSoftReset() {
return true;
}

#endif
2 changes: 2 additions & 0 deletions src/decoder_fluidsynth.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ class FluidSynthDecoder : public MidiDecoder {
#endif
};

bool NeedsSoftReset() override;

private:
#if defined(HAVE_FLUIDSYNTH) || defined(HAVE_FLUIDLITE)
fluid_synth_t* GetSynthInstance();
Expand Down
2 changes: 1 addition & 1 deletion src/main_data.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
# include <SDL_system.h>
#elif defined(__APPLE__) && TARGET_OS_OSX
# include <sys/syslimits.h>
# include "platform/macos/utils.h"
# include "platform/macos/macos_utils.h"
#endif

// Global variables.
Expand Down
0 src/platform/macos/utils.h → src/platform/macos/macos_utils.h
View file
Open in desktop
File renamed without changes.
2 changes: 1 addition & 1 deletion src/platform/macos/utils.mm → src/platform/macos/macos_utils.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
#ifdef __APPLE__
#import <Foundation/Foundation.h>
#include <string>
#include "platform/macos/utils.h"
#include "platform/macos/macos_utils.h"

bool MacOSUtils::IsAppBundle() {
bool is_bundle = false;
Expand Down
4 changes: 4 additions & 0 deletions src/platform/macos/midiout_device_coreaudio.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ std::string CoreAudioMidiOutDevice::GetName() {
return "CoreAudio MIDI";
}

bool CoreAudioMidiOutDevice::NeedsSoftReset() {
return true;
}

bool CoreAudioMidiOutDevice::IsInitialized() const {
return works;
}
Expand Down
1 change: 1 addition & 0 deletions src/platform/macos/midiout_device_coreaudio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class CoreAudioMidiOutDevice : public MidiDecoder {
void SendMidiMessage(uint32_t message) override;
void SendSysExMessage(const uint8_t* data, size_t size) override;
std::string GetName() override;
bool NeedsSoftReset() override;
bool IsInitialized() const;

private:
Expand Down
4 changes: 2 additions & 2 deletions src/platform/sdl/sdl2_ui.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@
#include "lcf/scope_guard.h"

#ifdef SUPPORT_AUDIO
# include "audio.h"
# include "sdl_audio.h"

# if defined(__APPLE__) && TARGET_OS_OSX
# include "platform/macos/utils.h"
# include "platform/macos/macos_utils.h"
# endif


Expand Down
2 changes: 1 addition & 1 deletion src/platform/sdl/audio.cpp → src/platform/sdl/sdl_audio.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
# include <emscripten.h>
#endif

#include "audio.h"
#include "sdl_audio.h"
#include "output.h"

using namespace std::chrono_literals;
Expand Down
0 src/platform/sdl/audio.h → src/platform/sdl/sdl_audio.h
View file
Open in desktop
File renamed without changes.
2 changes: 1 addition & 1 deletion src/platform/sdl/sdl_ui.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
#endif

#ifdef SUPPORT_AUDIO
# include "audio.h"
# include "sdl_audio.h"

AudioInterface& SdlUi::GetAudio() {
return *audio_;
Expand Down
Loading