-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Add MidiExport support for different track channels, default SF2 patches and drum mapping #5516
base: master
Are you sure you want to change the base?
Conversation
🤖 Hey, I'm @LmmsBot from github.com/lmms/bot and I made downloads for this pull request, click me to make them magically appear! 🎩
Linux
Windows
macOS
🤖{"platform_name_to_artifacts": {"Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://6697-15778896-gh.circle-artifacts.com/0/lmms-1.2.1.692-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/6697?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "Windows": [{"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://6700-15778896-gh.circle-artifacts.com/0/lmms-1.2.1.692-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/6700?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://6698-15778896-gh.circle-artifacts.com/0/lmms-1.2.1.692-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/6698?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/kwckitf0oyhv6ey5/artifacts/build/lmms-1.2.1-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/33084677"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/q833njtmd4k9vagp/artifacts/build/lmms-1.2.1-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/33084677"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://6701-15778896-gh.circle-artifacts.com/0/lmms-1.2.1.692-mac10.13.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/6701?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "086bc2484bb5a7a50c0b1ca7c717e85f66ffc8f4"} |
Based on the PR description (which is pretty well written!) this looks like a nice improvement. However, I don't believe BB tracks should export to channel 10, at least not unconditionally. For starters, even if we assume that the BB editor only contains simple patterns and drums, we can't know what each instrument in the editor is supposed to be (which track is the snare? Which is the kick?). Secondly the BB editor can contain melodies, just like any other track. Channel 10 isn't suitable to export patterns in that case. If the BB -> Channel 10 conversion is kept I think it ought to be as an option set at export time (similar to how render settings are set now). |
@Spekular Thanks! And indeed, you've got a point. The PR does make it possible to retain drum info if you use solely Sf2 Player instruments in BB editor (and by changing the center note in each one so it matches the desired drum sound to be repeated). The good part is that MIDI file specification allows saving more than one track per channel, so MidiImport could be tweaked in a further PR to retain all the original separated BB tracks info from channel 10. Yet, I totally agree that drums shouldn't be solely attributed to BB tracks and vice versa. An easy fix would be simply picking up all bank=128 / patch=0 tracks (be them from song or BB editor), merging them to channel 10, and leaving the other BB tracks untouched. This way the MIDI import/export cycle would lose no info, and this PR would be somewhat complete even without the extra MidiImport tweaks I mentioned earlier. I think this way should be straightforward and intuitive enough to be implemented without an explicit export option, but do feel free to discuss further. |
- Remaining tracks in BB editor are exported normally to other channels
// Drum Sf2 track, so set its channel to 10 | ||
// (and reverse counter increment) | ||
midiTrack.m_channel = 9; | ||
m_channel--; |
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.
If m_channel
was 9 before calling this function, this line will change it to 10 instead of going back to 9.
QString instName = instTrack->instrumentName(); | ||
if (instName == "Sf2 Player") |
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.
I think this check should belong to Instrument
or InstrumentTrack
in the future. Could you add a comment which addresses it?
@EmoonX Pinging you in case you've missed this one. |
I have compiled this branch on Debian Buster using clang (without VST and Carla). P.S. Program Change inserting sometimes is good (MuseScore importing), but mostly after importing mid-file with Cubase I have to manually delete Program Changes. Some "configuration check box" would be needed in this case ... [I am new in testing ...] |
Any news? |
I believe this PR is dead, not only bc the original author isn't responding, but also because, with the recent cascade of internally breaking changes, this would be a nightmare to merge. I think completely redoing this PR wouldn't be a crazy idea. |
I apologize for necroposting, but I was curious on if there as been any more work done towards improving the MIDI stuff? I was wanting to try using LMMS for making custom music for DOOM Mods, but am extremely disappointed to see that everything is exported to just sound like a grand piano. If there hasn't then that's understandable to an extent, I just am interested on any progress or attempts made after the last few posts. |
If you haven't already, try nightly. Midi support has improved quite a bit. Any further improvements are unpredictable as we are having a dev crunch and most active devs including myself are clueless about the midi standard. |
Got taken by surprise, seeing a notif for this after all these years. Long story short, I opened this PR back then as a way to improve my As it appears to be a requested feature still, I'd love to tackle it again at some point, yet most of the code and concepts involved look like magic runes to me at this point, not to mention the obscene number of merge conflicts that are bound to happen. It's worth a shot just for the sake of it, but no guarantees are given. |
I remember getting a nightly build for the slicer plugin but havent updated in a bit. What should I expect from the midi support now?
Completely up to you if you want to do it or not, programming is a really hard thing at times. Would love to see more improvements to LMMS in general, since it's development seems extremely slow (if I knew a thing in CPP or two I would try and chip in myself). In the mean time, I transferred some of my work to MidiEditor. |
The improvements stopped before SlicerT came along, so whatever is there for you rn, that's it. I was comparing nightly against stable. |
Hello, folks! This is my first pull request to LMMS and probably the first serious one to GitHub as a whole. Hope it could be the start of a safe journey.
This should solve issues #5198 and #5287 (which seem like duplicates?).
Issues
MidiExport plugin currently exports every instrument track (be it one from song editor or BB editor) to a single channel (0) with no program (patch) change event. In other words, everything is always merged to a single track/channel with GM Grand Piano (0) patch by default when exporting to the .mid file format.
The loss of information is a barrier to collaborate with non-LMMS using people, or even to work with MIDI between different programs or electronic keyboards (since, even though LMMS correctly imports to different tracks and patches using the default .sf2, all goes wrong if you simply try next to MIDI export everything; which, ultimately, led me to work on this PR).
Enhancement
The PR introduces the following modifications:
I also did some general refactoring and documentation on the MidiFile/MidiExport modules, both to help myself personally understanding the code and to improve readability and reusing for everyone.
What more?
General ideas to further improve results and workflow:
Comments and suggestions are more than welcome!
General MIDI reference: http://www.music.mcgill.ca/~ich/classes/mumt306/StandardMIDIfileformat.html