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

MIDI Program Change message support in VeSTige, SF2 Player and GIG Player #5158

Closed

Conversation

artur-twardowski
Copy link
Contributor

Adds the native support for the following MIDI events in VeSTige:

  • Program Change
  • Controller Change for controllers 0 and 32, which are Bank Select MSB and LSB, respectively

The patch adds two toggle switches to VeSTIge GUI:

  • Capture Program Change events
  • Use Bank Select LSB
    which are loadable and saveable. To make the room for new switches it was neccessary to reorganize slightly the existing user interface - "Show/Hide GUI" and "Stop all notes" buttons were reduced to the form of icons. Additionally, the MIDI preset number was moved from preset name line to the "Preset:" caption above it, so that there is more space for the actual preset name.

Description of new features

When "Capture Program Change events" switch is on, the events above are captured directly by VeSTige and used to determine the ID of preset to switch to; the VST plugin will not receive these events. When the switch is off the MIDI events will be passed through to the plugin.

When "Use Bank Select LSB" switch is on, both controllers 0 and 32 are used. The values are combined to obtain the bank number between 0 and 16383 (2^14 - 1). When the switch is off, only controller 0 is used and is treated as LSB. This allows to choose banks between 0 and 127 (2^7 - 1). This allows to work with MIDI devices that use either of the approaches.

The target preset ID is determined from the bank number and the program number. Presets 0 to 127 belong to bank 0, presets 128..255 fall into bank 1, presets 256..383 belong to bank 2, and so on.

Reception of the Program Change and/or Bank Select messages will trigger the update of the VeSTige GUI, so the preset name is visible immediately.

Validation of changes
Preparation

  • Connect any MIDI controller capable of sending subject messages (e. g. Alesis QX49)
  • Add VeSTige to the song editor and load any VST plugin that has its GUI and at least a few presets. I have used SuperWave P8 and SQ8L.
  • Enable the MIDI input. If you use ALSA Sequencer as MIDI interface, pick appropriate MIDI device from the list.
  • Open the VST GUI window

Validating Program Change

  • Having both switches disabled, trigger Program Change event at your controller.
    • if the plugin you loaded accepts Program Change messages (e. g. SuperWave P8), you will see a change in the plugin's GUI, but no change of the preset in VeSTige
    • if the plugin does not handle Program Change (e. g. SQ8L), there will be no change in the GUI
  • Enable "Capture Program Change events" switch and trigger Program Change event.
    • the preset name displayed in VeSTige GUI should change according to the program number received, contents of plugin's GUI should be updated as well

Validating Bank Select
Note: here I recommend using a VST plugin with more than 128 presets.

  • Enable "Use Bank Select LSB" switch

  • Send Program Change with the value of 0

    • the first preset should be selected
  • Send Bank Select MSB with the value of 0 and Bank Select LSB with the value of 1

    • the 129th preset should be selected (bank 1, program 0, meaning Preset ID=128)
  • Send Bank Select LSB with the value of 2

    • the 257th preset should be selected (bank 2, program 0, meaning Preset ID=256)
  • Send Bank Select MSB with the value of 1 and Bank Select LSB with the value of 0

    • the last preset should be selected (bank 128, program 0, meaning Preset ID=16384)
  • Disable "Use Bank Select LSB" switch

  • Send Bank Select MSB with the value of 0 and Bank Select LSB with the value of 1

    • the first preset should be selected (bank 0, program 0, meaning Preset ID=0)
  • Send Bank Select LSB with any value

    • there should be no reaction of the plugin
  • Send Bank Select MSB with the value of 1

    • the 129th preset should be selected (bank 1, program 0, meaning Preset ID=128)

Validating save & load

  • Set any state of the switches in VeSTige GUI, then save the project
  • Open a new project
  • Reopen the previously saved project
  • Open the GUI of the instance of VeSTige loaded into the project
    • the state of switches should be the same as when the project was saved

Notes
Change in scope of #5138 (already closed) which is actually a part of #1472.
The pull request includes a fix for #5139 which has not yet been merged to the master branch, but is required by the new feature.

@Spekular
Copy link
Member

I don't see why these changes should affect the "Show/Hide GUI" and "Stop all notes" buttons. As these are MIDI related settings, wouldn't they go in the MIDI settings tab?

@artur-twardowski
Copy link
Contributor Author

artur-twardowski commented Aug 28, 2019

MIDI settings tab is part of LMMS. Only the plugin tab displays the controls specific to plugins (in this case VeSTige).
I had to remove the buttons since the new controls would not fit in the gap over preset ID. Even if there was only one switch ("Capture Program Change events") it would be placed really tight.
The better, but yet conservative solution would be to move the VST information frame a bit up, closer to the VeSTige banner. Unfortunately the frame is a part of the background image. The image cannot be easily modified without spoiling the appearance of the plugin, since the background texture is not uniform.

@artur-twardowski
Copy link
Contributor Author

artur-twardowski commented Aug 29, 2019

I thought a bit more about @Spekular's suggestion, it seems that "Use Bank Select LSB" switch could actually be moved to the MIDI tab. I started working on implementation of similar functionality in GIG Player and Sf2 Player, it turned out that I need to repeat there a substantial amount of code. The same problem would appear in other plugins when they get the same functionality as a further step.

Moreover, GIG Player has a plenty of room in its GUI, but fitting the switch into GUI of Sf2 Player will be a hard task.

I think that a new message should be added in the abstraction layer between MIDI clients and consumers. Maybe Instrument or Plugin class could be enhanced with a new virtual function, say, changePresets? The common code would combine Bank Selects and Program Change into a proper call with ready-made preset number. The MIDI tab will give the user a choice if the messages should be intercepted, possibly in a form of a drop-down list:

[Preset selection] => {Off | Program Change only | Program Change and Bank Select MSB only | Program Change and Bank Select MSB/LSB}

This way we will not interfere with CCs 0 and 32 should they have a different context on some MIDI controllers, unless explicitly told to do so.

I will work on some prototype later.

Still, something has to be done with "Capture Program Change" switch. It is specific to VeSTige, I believe that the user should have a choice on how the messages should be handled. If we could move the "plugin information" box a few pixels up, then I could restore the original appearance of aforementioned buttons, adding the new switch below "Stop all notes".

plugins/vestige/vestige.cpp Outdated Show resolved Hide resolved
plugins/vestige/vestige.cpp Outdated Show resolved Hide resolved
plugins/vestige/vestige.cpp Outdated Show resolved Hide resolved
artur-twardowski added a commit to artur-twardowski/lmms that referenced this pull request Sep 1, 2019
artur-twardowski added a commit to artur-twardowski/lmms that referenced this pull request Sep 1, 2019
@artur-twardowski artur-twardowski changed the title MIDI Program Change message support in VeSTige MIDI Program Change message support in VeSTige, SF2 Player and GIG Player Sep 1, 2019
@artur-twardowski
Copy link
Contributor Author

I have finally found a reasonable way to move "Capture Program Change" switch to MIDI tab as well. For this reason I restored the original appearance of VeSTige (but still leaving the preset number in the upper row).

Since the change is now in the common part of the code, I decided to take an opportunity and adapt Sf2 Player and GIG Player to the new functionality. Originally I intended to create a separate pull request for them, but since they allow to choose bank independently from preset I decided to re-design a bit the interface. The instruments do not receive the ready-made preset number anymore, but instead they get a separate "nullable" bank number and program number.

There are 3 policies of handling Bank Select CCs:

  • Ignore; CCs 0 and 32 are not used, bank switching is entirely disabled, the plugin will get bank=-1 when changing the preset. The impact depends on the plugin:
    • If a plugin required bank number to expand the range of selectable presets, it will treat the bank number as 0, effectively allowing selection of presets from 0 to 127. For a moment I thought about "guessing" the bank number from currently selected preset - if we are at, let's say, preset 300th (2 x 128 + 44), we can shift the selection range to "virtual" bank 2, exposing presets 256 to 383. This is how SQ8L treats these "high" presets - it will identify it as C044. However, I finally came up to the conclusion that this behavior would be really confusing to the user.
    • If a plugin has a separate control for bank selection (Sf2 Player, Gig Player), it may leave the bank untouched and switch only the preset. This way, we can select manually a bank, then browse the presets inside using MIDI keyboard. This is where the reason from change came from - if we sent just a single number, the bank would always reset to 0.
  • Use MSB only; CC 0 is used and is treated as LSB, CC 32 is ignored. Some MIDI controllers use MSB as the only bank switching message while others (such as Casio CTK-4200) send both Bank Select messages, but the actual bank number is still in MSB.
  • Use both MSB and LSB; CC 0 and 32 are both used and handled according to their names. To be used with MIDI controllers that actually treat MSB as MSB and LSB as LSB, allowing to choose banks from 0 to 16383. Controllers that use MSB only will cause selection of banks being multiples of 128.

Open points:

  • are there any MIDI controllers that treat CC 0 as LSB and CC 32 as MSB? If there were ones that behave this way, a "bank-inversion" policy could be added.
  • to fit the new control into MIDI tab I removed (hid) the explanation of Custom Base Velocity. Otherwise the section was truncated, but still it enlarged a bit the window creating an oddly-looking margin below the plugin's GUI. Any comments or suggestions on that?
  • the API provides a function allowing each plugin to tell if it actually supports preset switching or not. The function is currently unused - my intention is however to hide the "Enable Preset Selection" section or display some placeholder text in case a plugin does not implement preset switching.

@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 labels Sep 8, 2019
@PhysSong
Copy link
Member

Some comments:

  • Please remove vestige.orig.cpp.
  • Is it okay not checking if there's a patch with the specific number?
  • It seems like patchChanged is emitted even if the patch remains the same.

@artur-twardowski
Copy link
Contributor Author

artur-twardowski commented Oct 9, 2019

Some comments:

  • Please remove vestige.orig.cpp.
    Done.
  • Is it okay not checking if there's a patch with the specific number?
    For SF2/Gig players, they allow to select an inexistent patch anyway via LcdSpinBoxes.
    For VeSTige, selection of a preset with too high number will result in choosing the last preset available.
    I can update the code of all three plugins to ignore the messages containing invalid values, the question is if such behavior will be confusing to the user.
  • It seems like patchChanged is emitted even if the patch remains the same.
    This behavior is not harmful since the handling of the signal consists of UI update. Nevertheless, for sake of consistency with the signal's name, I updated the code of SF2/Gig players to check if the bank and/or preset number have actually changed. For VeSTige there will be a bit more work to do.

@LmmsBot
Copy link

LmmsBot commented Oct 9, 2019

🤖 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

macOS

Windows

🤖
{"platform_name_to_artifacts": {"Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://6549-15778896-gh.circle-artifacts.com/0/lmms-1.2.1.705-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/6549?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://6546-15778896-gh.circle-artifacts.com/0/lmms-1.2.1.704-mac10.13.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/6546?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://ci.appveyor.com/api/buildjobs/0y4j6d3nr7c0bq0s/artifacts/build/lmms-1.2.1-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/32967989"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/ginakdbhd7b71l40/artifacts/build/lmms-1.2.1-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/32967989"}]}, "commit_sha": "4094657e6cb22d7a2e58fbcb028c03e9e6d490ad"}

artur-twardowski added a commit to artur-twardowski/lmms that referenced this pull request Dec 22, 2019
artur-twardowski added a commit to artur-twardowski/lmms that referenced this pull request Dec 22, 2019
@PhysSong PhysSong self-requested a review December 23, 2019 08:32
@PhysSong PhysSong added needs testing This pull request needs more testing and removed needs style review A style review is currently required for this PR labels Dec 23, 2019
plugins/GigPlayer/GigPlayer.h Outdated Show resolved Hide resolved
plugins/sf2_player/sf2_player.h Outdated Show resolved Hide resolved
plugins/vestige/vestige.cpp Outdated Show resolved Hide resolved
@PhysSong PhysSong removed the needs code review A functional code review is currently required for this PR label Apr 28, 2020
@PhysSong
Copy link
Member

The code looks good in general. I think we need some testers for this. Testing PR before merging is not mandatory but strongly recommended.

winniehell and others added 7 commits May 8, 2020 21:35
* Extract RecentProjectsMenu class from MainWindow
* Clean up updateRecentlyOpenedProjectsMenu
* Remove m_recentlyOpenedProjectsMenu from MainWindow
* Replaced large buttons for "Show/hide GUI" and "Stop all notes"
  with icons to fit the new control.
* Added "Capture Program Change" toggle button.
* Moved ID of plugin's patch to the "Preset" label to make more room
  for the actual preset name.
@artur-twardowski artur-twardowski force-pushed the program-change-support branch from 348ce79 to 0a52ea4 Compare May 8, 2020 19:44
@botwhytho
Copy link

Any testers still needed? In Linux as a daily driver so could test SF2, might be able to boot into a Windows environment and test Vestige too.

@zonkmachine
Copy link
Member

Any testers still needed?

Yes please!

@botwhytho
Copy link

@zonkmachine If there are any instructions on how to build feature branches on Linux (on Ubuntu 20.04) or somewhere to get Linux and/or Windows assets that would be great, otherwise any pointers would be of help as I've used LMMS for a couple of years but not really familiar with it's build system or dependencies. Thanks.

@josh-audio
Copy link
Member

Hi @botwhytho, there are links to pre-built .AppImage files for this PR. See the comment above by @LmmsBot, which is regularly edited with the latest build links.

Comment on lines +169 to +171
/* Base velocity help label hidden to preserve the window size.
* Maybe there is a better solution */
#if 0
Copy link
Member

Choose a reason for hiding this comment

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

This should be fixed before merging.

@artur-twardowski
Copy link
Contributor Author

Code ported to the new code base. A new pull request was created (#6676), closing this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs testing This pull request needs more testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants