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

Add MidiExport support for different track channels, default SF2 patches and drum mapping #5516

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

Conversation

EmoonX
Copy link

@EmoonX EmoonX commented May 24, 2020

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:

  • Each instrument track from song editor is written to a different incremental MIDI channel (skipping channel 10; see below)
  • In case of the current instrument track being an Sf2 Player one, a program change event valued to the current SF2 patch (0 to 127) is added to the MIDI file track, signaling the GM instrument to be imported further
  • BB editor instrument tracks are saved exclusively to channel 10 with patch 0, using the repeated pattern currently generated from the BB editor. MIDI note pitch (which corresponds to a different drum sound following GM percussion map) is extracted from the BB instrument tracks and will only make sense if using Sf2 Player for these (which will also need some code tweaks in MidiImport to get a decent result)

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:

  • Add a "default instrument / preset" custom mapping for each patch number. This is an interesting standalone idea as many times we end up working with another bank, another .sf2 file or even a preset from another plugin altogether. Every default preset could have a custom patch number, which would then be used both for exporting and importing. For example, suppose a cool TripleOscillator saw wave custom preset could be saved as the default for GM Saw Wave (81): MidiExport would pick up that info and write a 81 program change to the file, and MidiImport would read that 81 and load the designated plugin and preset (instead of default Sf2 Player and patch number). In other words, this work wouldn't be restricted anymore to only soundfonts.
  • Smartly import the channel 10 track as a pure BB track. MidiImport currently just imports the drums track as a normal (repeating) instrument one. It would be nice to implement some algorithm to build a BB track from this repeated info; that way, it could be instantly exported with no loss of drum information (the way it is with the PR, everything would be lost as only the BB editor ones are written to channel 10; another imported drum one would be just exported as piano...)
  • Do a workaround when there's more than 16 (or 15 normal) channels

Comments and suggestions are more than welcome!

General MIDI reference: http://www.music.mcgill.ca/~ich/classes/mumt306/StandardMIDIfileformat.html

@LmmsBot
Copy link

LmmsBot commented May 24, 2020

🤖 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"}

@Spekular
Copy link
Member

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).

@EmoonX
Copy link
Author

EmoonX commented May 24, 2020

@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.

@PhysSong PhysSong added needs code review A functional code review is currently required for this PR needs style review A style review is currently required for this PR needs testing This pull request needs more testing labels May 27, 2020
// Drum Sf2 track, so set its channel to 10
// (and reverse counter increment)
midiTrack.m_channel = 9;
m_channel--;
Copy link
Member

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.

Comment on lines +220 to +221
QString instName = instTrack->instrumentName();
if (instName == "Sf2 Player")
Copy link
Member

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?

@PhysSong
Copy link
Member

@EmoonX Pinging you in case you've missed this one.

@firewall1110
Copy link
Contributor

I have compiled this branch on Debian Buster using clang (without VST and Carla).
I have tested it in simple way (without drums and BB and with <10 channels).
Exported mid-file understands MuseScore2 and CubaseSX 2.0 (see channels and Program Changes; Muse Score provides correct instrument names - it is good improvement).

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 ...]

@dacanizares
Copy link

Any news?

@RiedleroD
Copy link
Contributor

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.

@Spekular Spekular mentioned this pull request Jan 23, 2022
@LMMS LMMS deleted a comment from JakeThe28th May 30, 2024
@xiiixvv
Copy link

xiiixvv commented Dec 7, 2024

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.

@Rossmaxx
Copy link
Contributor

Rossmaxx commented Dec 7, 2024

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.

@EmoonX
Copy link
Author

EmoonX commented Dec 8, 2024

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 LMMS <-> musical keyboard workflow at the time, but... soon disinterest (and some personal issues) kicked in, and I moved to Bitwig Studio a few months later anyway. I deeply apologize to everyone who didn't get to witness this large amount of new code seeing the light of the day.

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.

@xiiixvv
Copy link

xiiixvv commented Dec 9, 2024

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.

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?

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 LMMS <-> musical keyboard workflow at the time, but... soon disinterest (and some personal issues) kicked in, and I moved to Bitwig Studio a few months later anyway. I deeply apologize to everyone who didn't get to witness this large amount of new code seeing the light of the day.

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.

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.

@Rossmaxx
Copy link
Contributor

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.

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?

The improvements stopped before SlicerT came along, so whatever is there for you rn, that's it. I was comparing nightly against stable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs code review A functional code review is currently required for this PR needs style review A style review is currently required for this PR needs testing This pull request needs more testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants