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

Added RehearsalMark to midi export as Markers #26196

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

heavy-matill
Copy link

@heavy-matill heavy-matill commented Jan 21, 2025

Resolves: Add Marker meta events to MIDI export and Exporting Rehearsal Markers to MIDI files

I want to export Rehearsal Marks to Midi Markers.
I export my sheetmusic as midi into a DAW, where Midi meta events such as Markers are supported.
Usually the sheetmusic contains Rehearsal Marks and I want the same structure to appear in my DAW.
image

This PR only adds export functionality. Import of Midi Markers to Rehearsal Marks is still missing.

I did not unit test the implementation but it works for the few examples I have on my machine.
I was also able to check it with python:

from miditoolkit import MidiFile
midFile = r"path\to\midi.mid"
midi_obj = MidiFile(midFile)
print(midi_obj)
ticks per beat: 480
max tick: 226561
tempo changes: 1
time sig: 1
key sig: 13
markers: 182
lyrics: True
instruments: 13
print(midi_obj.markers)
[Marker(text="A", time=7680), Marker(text="B", time=53760), ...
  • I signed the CLA
  • The title of the PR describes the problem it addresses
  • Each commit's message describes its purpose and effects, and references the issue it resolves
  • If changes are extensive, there is a sequence of easily reviewable commits
  • The code in the PR follows the coding rules
  • There are no unnecessary changes
  • The code compiles and runs on my machine, preferably after each commit individually
  • I created a unit test or vtest to verify the changes I made (if applicable)


int tick = r->segment()->tick().ticks() + tickOffset;
track.insert(CompatMidiRender::tick(context, tick), ev);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

trailing whitespace

Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Jan 22, 2025
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Jan 22, 2025
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Jan 25, 2025
@heavy-matill
Copy link
Author

heavy-matill commented Jan 25, 2025

Regarding the Codestyle check:

In installed uncrustify on my windows machine and ran it in git bash. However other than complaining about an outdated configuration it did not throw any erros for the mentioned file.

> tools/codestyle/uncrustify_run_file.sh  src/importexport/midi/internal/midiexport/exportmidi.cpp
Option<NUM>: at tools/codestyle/uncrustify_musescore.cfg:209: Expected number , for 'indent_comma_paren'; got 'false'
Option<NUM>: at tools/codestyle/uncrustify_musescore.cfg:212: Expected number , for 'indent_bool_paren'; got 'false'
tools/codestyle/uncrustify_musescore.cfg:296: option 'sp_balance_nested_parens' never works; it has been replaced by 'sp_paren_paren'.

tools/codestyle/uncrustify_musescore.cfg:720: option 'sp_before_tr_emb_cmt' is deprecated; did you want to use 'sp_before_tr_cmt' instead?

tools/codestyle/uncrustify_musescore.cfg:723: option 'sp_num_before_tr_emb_cmt' is deprecated; did you want to use 'sp_num_before_tr_cmt' instead?

Option<UNUM>: at tools/codestyle/uncrustify_musescore.cfg:873: Expected unsigned number , for 'align_nl_cont'; got 'false'
tools/codestyle/uncrustify_musescore.cfg:950: option 'nl_func_var_def_blk' is deprecated; it has been replaced by 'nl_var_def_blk_end_func_top'.
You can also use 'nl_var_def_blk_end' for additional functionality.

Option<UNUM>: at tools/codestyle/uncrustify_musescore.cfg:1410: Expected unsigned number , for 'mod_full_brace_if_chain'; got 'false'
tools/codestyle/uncrustify_musescore.cfg:1560: option 'pp_space' is deprecated; it has been replaced by 'pp_space_after'.

do_source_file: Parsing: src/importexport/midi/internal/midiexport/exportmidi.cpp as language CPP

EDIT: had to install 0.73 of uncrustify via choco install uncrustify --version=0.73

@cbjeukendrup
Copy link
Contributor

The problem is that you need specifically version 0.73 of Uncrustify. (We decided to pin the version at some point, because every Uncrustify update required changes to the code, which was quite inconvenient in combination with the fact that updates are rolled out gradually over GitHub Actions runners, so you never know in advance whether you're going to get a the new or old version.)

…icates, because they are the same fore each staff)
…icates, because they are the same fore each staff)
@Jojo-Schmitz
Copy link
Contributor

Don't use merge commits, but rebase (and force push):
git pull --rebase upstream master; git push --force

…icates, because they are the same fore each staff)
@heavy-matill heavy-matill force-pushed the RehearsalMarkers2Midi branch from f03f165 to 1711ad8 Compare January 26, 2025 20:12
@heavy-matill
Copy link
Author

Don't use merge commits, but rebase (and force push): git pull --rebase upstream master; git push --force

Sorry, my vscode git integration keeps doing those in the background. I believe I have removed most of the unnecessary merge commits

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.

3 participants