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 data.MIDINAMES #927

Merged
merged 2 commits into from
Mar 25, 2022
Merged

Fix data.MIDINAMES #927

merged 2 commits into from
Mar 25, 2022

Conversation

rettinghaus
Copy link
Member

This resolves the problematic altIdent in data.MIDINAMES and fixes a wrong value in this range, as Bag pipe should become "Bag_pipe" instead of "Bagpipe".

This is also a bugfix for MEI 4.
closes #812

@github-actions github-actions bot added the Component: Core Schema changes to source/modules/* (assigned automatically) label Mar 14, 2022
@ahankinson
Copy link
Member

ahankinson commented Mar 14, 2022

Is there a reason it's now starting at 1, not 0?

MIDI program changes are 0-indexed, so this should probably be reflected in MEI.

See: https://www.recordingblogs.com/wiki/midi-program-change-message

@rettinghaus
Copy link
Member Author

This reflects the MIDI Program Change given in the original specifications. Otherwise we need to change the given Keys for percussions accordingly.

The remarks state:

MEI uses 0-based program numbers.

If that leads to confusion we should drop this, because the description in data.MIDIVALUE should be clear in this regard.

@ahankinson
Copy link
Member

About this particular change: The original says Program #0, while the proposed change says MIDI Program Change #1. I think both are correct -- it's the first program change in the list, but "Acoustic Grand Piano" is program 0. (message value 0x00).

Given that we're specifically trying to fix an issue with it rendering to RNG correctly, and not actually trying to change the MIDI semantics, I would suggest keeping the original wording and not introducing this change here.

@rettinghaus rettinghaus added this to 2022-04-28 ODD Thursday in ODD Meetings Mar 14, 2022
@musicEnfanthen musicEnfanthen moved this from 2022-04-28 ODD Thursday to 2022-03-25 ODD Friday in ODD Meetings Mar 17, 2022
@musicEnfanthen
Copy link
Member

@rettinghaus thanks for the PR, and thanks for the approval @ahankinson. Will take this PR to the next ODD meeting.

@musicEnfanthen musicEnfanthen added the Status: Ready To Merge indicates that a pull request is ready for merging label Mar 25, 2022
Copy link
Member

@musicEnfanthen musicEnfanthen left a comment

Choose a reason for hiding this comment

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

Thanks a lot. Looks good.

@bwbohl
Copy link
Member

bwbohl commented Mar 25, 2022

thanks!

@bwbohl bwbohl closed this Mar 25, 2022
@bwbohl bwbohl reopened this Mar 25, 2022
@bwbohl bwbohl merged commit 9cd11a6 into music-encoding:develop Mar 25, 2022
@rettinghaus rettinghaus deleted the develop-midi branch March 25, 2022 14:56
@musicEnfanthen musicEnfanthen removed the Status: Ready To Merge indicates that a pull request is ready for merging label Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Core Schema changes to source/modules/* (assigned automatically)
Projects
No open projects
ODD Meetings
  
2022-03-25 ODD Friday
Development

Successfully merging this pull request may close these issues.

MIDINAMES error in MEI 4/5 RelaxNGs
4 participants