-
Notifications
You must be signed in to change notification settings - Fork 980
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
.midi Export #4319
.midi Export #4319
Conversation
I am close to completing this feature 👍 |
fixes:#4311 @pikurasa I think the PR is ready for review. Please check it. Next, I have to work on adding instruments to the MIDI file. After exporting, you can play the MIDI file in media players like Windows Media Player. Right now, it will only play on the piano. |
@pikurasa The midi export is finally done. Please review this PR. final.mp4For testing we can use : https://pianotify.com/import-midi-file |
I tested https://github.com/sugarlabs/musicblocks/blob/master/examples/Holly-Jolly-Christmas.html, and the output seems to get off track after the third note. This is the MIDI file generated while on your branch, |
@pikurasa, I have fixed the issue. I thought _midiData was filled instantly, but that is not the case. It takes some time to retrieve all the data. Like Lilypond and others, I have created an after-save function to generate a MIDI file when midi data is filled. Here is the result :-> midi2.mp4 |
Yes, it seems to work well. I think it's ready for a code review. |
js/logo.js
Outdated
@@ -1690,7 +1703,11 @@ class Logo { | |||
// console.log("saving mxml output"); | |||
logo.activity.save.afterSaveMxml(); | |||
logo.runningMxml = false; | |||
} else if (tur.singer.suppressOutput) { | |||
} else if (logo.runningMIDI) { | |||
logo.activity.save.afterSaveMIDI(); //save midi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need for this comment
js/SaveInterface.js
Outdated
generateMidi(data); | ||
activity.logo._midiData = {}; | ||
document.body.style.cursor = "default"; | ||
},500); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add a space after the comma
One other small change: please update the Save help string in turtledefs to include MIDI. |
@walterbender Sir I have updated turtledefs.js and other changes you mentioned. |
@@ -566,6 +566,8 @@ const createHelpContent = (activity) => { | |||
_("Save project"), | |||
_("Save project as HTML") + | |||
"<br/>" + | |||
_("Save project as MIDI") + | |||
"<br/>" + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be Save music as MIDI ???
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I thought about that. You can edit MIDI files, so I thought "project" would be appropriate.
Also, since when you import MIDI, you get a MB project, I guess "project" does make sense. |
I can add test cases for MIDI now in SaveInterface |
@Commanderk3 Please add documentation for .MIDI export in |
Sure |
FWIW, I tested our "Romanian Folk Dance" example (in the examples folder), and I'm very impressed with how it was able to handle the complex rhythms in that project. |
Happy to see it working! |
To export .midi this PR uses the library tones/midi.