MacOS Fixes#3148
Merged
3 commits merged intoNov 20, 2023
Merged
Conversation
5a6aac7 to
d9dc94c
Compare
Ghabry
requested changes
Nov 17, 2023
Member
Ghabry
left a comment
There was a problem hiding this comment.
- You inserted space instead of tab for the indent.
- Can you add this
NeedsSoftResetto Decoder Fluidsynth? Has a similiar issue and your fix is more universal.
d9dc94c to
f6d5c0c
Compare
ghost
approved these changes
Nov 18, 2023
Ghabry
reviewed
Nov 18, 2023
| // MIDI reset event | ||
| if (mididec->NeedsSoftReset()) { | ||
| // SoundOff every channel: necessary for synths like macOS which tend to get stuck | ||
| SendMessageToAllChannels(midimsg_all_sound_off(0)); |
Member
There was a problem hiding this comment.
sorry have to ask you for another small change:
Can you move this new code block between the GM reset and the pitch bend block? (I hope this doesn't break macOS?)
Tested this again in fluidsynth and the all sound off will not work properly when executed before the GM reset (same audio glitch remains).
// GM system on (resets most parameters)
const unsigned char gm_reset[] = { 0xF0, 0x7E, 0x7F, 0x09, 0x01, 0xF7 };
mididec->SendSysExMessage(gm_reset, sizeof(gm_reset));
// MIDI reset event
if (mididec->NeedsSoftReset()) {
// SoundOff every channel: necessary for synths like macOS which tend to g>
SendMessageToAllChannels(midimsg_all_sound_off(0));
}
// Set the Pitch bend range to 256
for (int channel = 0; channel < 16; channel++) {f6d5c0c to
77b52c8
Compare
Ghabry
approved these changes
Nov 18, 2023
Member
Ghabry
left a comment
There was a problem hiding this comment.
thanks, works now with fluidsynth 👍
This pull request was closed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes three issues:
5-beatles-lustig.midfrom Unterwegs in Duesterburg causes a crash inMusicDeviceMIDIEventdue to a bogus MIDI message. We're in some good company here, both VLC (at least on macOS) and Sekaiju also crash attempting to play it.midimsg_all_sound_offfrom reset, to be more accurate with Harmony. Harmony never supported macOS, and the macOS synthesizer being prone to stuck notes is why those messages were there in the first place. I'm open to other solutions to fix this, but without it, MIDI is pretty scuffed on Macs.-G Xcodegenerator in cmake will completely fail to build (at least on Xcode 15), due to a fundamental disconnect between cmake and Xcode when you have multiple code files with the same filename (likesrc/audio.cppvssrc/platform/sdl/audio.cpp). Simplest workaround is to remove the duplication by renaming the two files that had duplicated names.Fix #3135