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.

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

Fix regression in WinMIDI driver #1131

Merged
merged 1 commit into from
Jul 28, 2022
Merged

Fix regression in WinMIDI driver #1131

merged 1 commit into from
Jul 28, 2022

Conversation

albedozero
Copy link
Contributor

@albedozero albedozero commented Jul 27, 2022

In #1115 I created a bug in the winmidi driver that causes all incoming voice messages to have channel, param1, and param2 = 0. In fluid_winmidi_callback() the check for msg_param < 0xf0 fails for all MIDI messages (I had the byte order of dwParam1 backwards, status byte is the lowest byte, not the highest), so the channel and params are never set.

Fixed using the msg_type macro to check for voice category message. Also have to mask the status byte with 0xff for system messages to get the type properly.

the lowest two bytes in dwParam1 are the status byte, not the highest; also need to mask with 0xff to get the system message type
@sonarcloud
Copy link

sonarcloud bot commented Jul 27, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@albedozero albedozero marked this pull request as ready for review July 27, 2022 04:10
@derselbst derselbst changed the title fix check for voice category message Fix regression in WinMIDI driver Jul 28, 2022
Copy link
Member

@derselbst derselbst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thanks

@derselbst derselbst merged commit 8a602bc into FluidSynth:2.2.x Jul 28, 2022
@albedozero
Copy link
Contributor Author

Thanks, sorry for breaking stuff

@derselbst
Copy link
Member

No problem. It also escaped my attention when reviewing your original PR. Don't worry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants