Extend LMMS note range to match MIDI specification

[he29-net]

Hi,

this PR resolves #1857 by increasing the range of internal LMMS note representation to 0..127 (C-1 to G9) to match the MIDI specification and by removing all previously inserted 12 semitone offsets. Apart from the overall MIDI handling and import / export, it also touches PianoRoll and PianoView, which now have to be able to correctly draw the last, incomplete octave.

This is still work in progress because: now ready for review?

• New projects behave as expected, but there is still the question of upgrading old project files. I tried adding el.attribute("basenote") += 12; to upgrade_1_3_0() in DataFile.cpp, but I think my other LMMS versions are not old enough to trigger the update. I.e. I could not test it so I removed it for now, but it probably still needs to be fixed somehow.

• There is 144 presets (and possibly other stuff) that needs to get the base note updated from 57 to 69, or to a different n+12 value if they have another base note set. I didn't yet figure out a way how to batch-update all of these.

Btw. how will project file updates work when the nightly builds start coming out? There could potentially be a need for a separate update method for every single commit. Unless we explicitly say that nightly builds are allowed to break projects made by other nightlies...

[LmmsBot]

🤖 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

• Linux (AppImage): lmms-1.2.1.583-linux-x86_64.AppImage (build link)

Windows

• Windows 64-bit: lmms-1.2.1.583-mingw-win64.exe (build link)
• Windows 32-bit: lmms-1.2.1.583-mingw-win32.exe (build link)
• Windows 32-bit: lmms-1.2.1-msvc2017-win32.exe (build link)
• Windows 64-bit: lmms-1.2.1-msvc2017-win64.exe (build link)

🤖

{"platform_name_to_artifacts": {"Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://5509-15778896-gh.circle-artifacts.com/0/lmms-1.2.1.583-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/5509?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "Windows": [{"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://5505-15778896-gh.circle-artifacts.com/0/lmms-1.2.1.583-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/5505?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://5508-15778896-gh.circle-artifacts.com/0/lmms-1.2.1.583-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/5508?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/35lqo742wpil0s4q/artifacts/build/lmms-1.2.1-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/29683129"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/594mwek0f3lkx16r/artifacts/build/lmms-1.2.1-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/29683129"}]}, "commit_sha": "160b7580d5881d0154d2c0e4542f614db07d2372"}

[Veratil]

Inserting this before Octave_0 could cause problems if someone didn't use the enum. Will need to check this as best we can.

[he29-net]

I tried to go around and grep for the instances of word "Octave" and the number 57 (previous base note) to correct all the code affected by my changes, but this is my first time touching the core (apart from fft_helpers), so I definitely could have missed something. I'm not sure what would be a good, comprehensive test for this, though.

[curlymorphic]

It may be worth considering testing this patch with +- max pitchbend on all instruments, as I have previously fallen foul to this.

[he29-net]

@curlymorphic Could you clarify what was / could be potentially a problem? And what exactly do you mean by max pitch-bend -- turning the knob to both extremes while range is set to maximum (60)?

I tried that (on most instruments, with a few exceptions) and nothing crashed, so there are at least no out of bounds errors. Most instruments seem to be happy to play infrasonic sounds but generally just produce something inaudible or a distorted gurgling sound, while on the high end some instruments just do not produce any output at all (which is likely good, because those that do produce sound tend to have have nasty aliasing).

[curlymorphic]

@he29-net

I feel you understood my intent, but to clarify, play the lowest midi note, and use pitch bend to lower, play the highest note and bend higher. From your description, you appear to have already considered these cases.

For reference, the issue I had was with my own code while implementing wavetables, I initially only tested without pitch bend, when notes were bent outside the range I was expecting, my code was trying to access non-existant wavetable data. This range check is the responsibility of the plugin to check. My concern is that with extending the note range it may be possible instruments are now required to respond to note values not previously experienced.

You have stated you have already tested this on most instruments, It may be worth just testing on the few remaining cases.

[he29-net]

So I downloaded some content for GIG player and PatMan and also tried to abuse the remaining instruments that do not support pitch shifting by at least moving the base note around and changing the global pitch shift, and they all seem to be working all right.

I also found one more bad basenote offset in Hydrogen import, but I haven't been able to test if it works after fixing it, since LMMS segfaults when I try importing a Hydrogen project (not related to this PR, it segfaults on other versions as well).

When searching for other possibly incompatible code, I found several files that included Note.h for no reason, so I'll get rid of those includes as well.

[he29-net]

So, it turns out that for all the instruments except ZynAddSubFX the change in basenote offset and the addition of a new octave at the bottom exactly cancel each other out. I.e. the base note in the old project is kept at the same value (say 57, instead of the new neutral value 69), but all keys in the piano roll are also interpreted as being placed an octave lower -- the increase in pitch by wrongly placed base note is counteracted by the decrease in pitch by wrongly placed notes, if it makes any sense. :))

The result is that all instruments from old projects play exactly the same thing, except for ZynAddSubFX, which now plays an octave higher since the original issue has been fixed. I expect something similar may happen with VSTs, but I wasn't able to load any VST in 1.1.3 which I used to make an old project file.

---

For the presets, the situation is similar: after batch-updating the base note, all the preset previews now sound the same between the current master and the fixed versions -- except for ZynAddSubFX. Zyn presets now sound an octave higher, since they now actually play A4 / 440 Hz.

I don't intend to update the Zyn presets, as they have been made for Zyn and most of them played the wrong note in LMMS all the time -- now they will finally sound as originally intended. On the contrary, the presets for LMMS were made with the bug in place, so some of them may have just played A3 while others compensated for it and played A4. So here I think it is better if they keep soounding the same as before.

[he29-net]

I added upgrade code for the affected instruments; apart from VeSTige and ZynAddSubFX I also included Carla Patchbay and Carla Rack, since it seems they should be affected in the same way.

I tested the upgrade code by making a project in 1.1.3, opening it in the fixed version and checking if a sequence entered in the piano roll plays in the same octave as before, for all the instruments I could get working -- that should be all except GIG Player and Xpressive which did not exist in 1.1.3, and Carla which I did not yet figure out how to install and / or use.

The implementation should be now complete, so any volunteers who could try to break it? ^^

[StoykoDimitrov]

I added upgrade code for the affected instruments; apart from VeSTige and ZynAddSubFX I also included Carla Patchbay and Carla Rack, since it seems they should be affected in the same way.

I tested the upgrade code by making a project in 1.1.3, opening it in the fixed version and checking if a sequence entered in the piano roll plays in the same octave as before, for all the instruments I could get working -- that should be all except GIG Player and Xpressive which did not exist in 1.1.3, and Carla which I did not yet figure out how to install and / or use.

The implementation should be now complete, so any volunteers who could try to break it? ^^

@he29-net, I am testing this for the last few days and the VST Plugins have the right MIDI mapping now, they are no longer one octave down. The build seems pretty solid - I haven't been able to break it so far. The only issue I have found is if you have a saved project from an LMMS version before this fix, there is an issue with the 'GIG Player' plugin - it becomes one octave down, however if you recreate the project in the current build of LMMS - everything works as expected.

Great Work!
Cheers! :)

[he29-net]

Closing because of the starting bit-rot. This branch is already way out of date from my point of view, so I don't want to waste time maintaining it. (Also, I have another branch based on this one, so the squash-merge process would only create merge conflicts for me).

All the changes will be included some time later as a part of another PR I'm working on, so nothing will be lost, it will just take some (more) time.