lmms icon indicating copy to clipboard operation
lmms copied to clipboard

Implement MIDI output note setting

Open zynskeywolf opened this issue 4 years ago • 5 comments

Now the fixed output note setting in the MIDI tab actually works and all notes output to the same key if specified.

zynskeywolf avatar Dec 13 '21 16:12 zynskeywolf

:robot: Hey, I'm @LmmsBot from github.com/lmms/bot and I made downloads for this pull request, click me to make them magically appear! :tophat:

Windows

Linux

macOS

:robot:
{"platform_name_to_artifacts": {"Windows": [{"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://15248-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.151%2Bg12d850364-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/15248?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://15246-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.151%2Bg12d850364-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/15246?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/927kg52ku4083v8g/artifacts/build/lmms-1.3.0-alpha-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/41872964"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/okc4d4k53cya9ors/artifacts/build/lmms-1.3.0-alpha-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/41872964"}], "Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://15249-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.151%2Bg12d850364-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/15249?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://15245-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.151%2Bg12d850364-mac10.14.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/15245?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "08d1d744d97641ced9b7717a5a12d4bf6bf2a912"}

LmmsBot avatar Dec 13 '21 16:12 LmmsBot

When output note is set to a value, It randomly skips some notes. It doesn't happen when output note is set to "---". It happens at any bpm, but if set to a high value it's easier to test. When the value is entered by keyboard (double click) the displayed value is plus one. That happened before, but I notice that on #6235 is fixed to velocity and maybe the fix is similar. Values are saved and restored in project file ok. Tested on W10-64b

superpaik avatar Dec 14 '21 10:12 superpaik

@superpaik Thanks for testing. I tried it on several different systems, both Linux and Windows, but couldn't reproduce the problem you had. When I set the tempo high enough for notes to start skipping, they also do when "---" is set, but only at like 600 bpm and 16th notes. All that could waste more time with it enabled is just a simple setter function so not sure what is going on. Gonna dive into this further. Regarding the ui display, that is fixed now along with the others.

zynskeywolf avatar Dec 20 '21 10:12 zynskeywolf

It could be a problem of my device (though it's a W10 i5 - 16Gb) but can also be related to some other part of the code. Midi arpegios also skip notes notably. Anyway, it's not a big problem and as you said and it's more common with high rate bpm. Thanks, though, to look into it.

superpaik avatar Dec 21 '21 08:12 superpaik

I gave it a quick test in SW only (one instrument set for MIDI output, one for MIDI input). Before the fix, the Note spinner had no effect, the receiving instrument got whatever note was triggered on the sender. After the fix, any key pressed on the sender results in the selected note being sent to the receiver, so I would say that works as intended.

As for the skipped notes: I saw some skipped notes if the PC was under heavy load (compiling mesa). Under not as heavy load (but still not idle), I did not see any skipped notes, but once I saw a note that arrived at the output about 30 ms late. When only LMMS was using CPU, I did not see any problems. So in my case it probably works as expected; i.e. notes are skipped to avoid buffer underruns.

I discovered one issue, though: if I change the Note value during playback, there is a high probability that the change occurs while a note is playing. This means the destination never releases the note, since the related "note off" event comes with a new note number.

On the surface, the solution seems to be either a) to send a note-off event for the old note when the spinner value changes, or b) to remember the note used for last note-on event and use it for the note-off event, regardless of the spinner value. But I'm not sure what happens if somebody tries to automate it and somehow ends up having several notes that all need to be released...

he29-net avatar Aug 20 '22 23:08 he29-net

A silly idea: maintain a map of (current note pitch) -> (pitch sent to MIDI)--consult that during the NoteOff, and then clear it. (The map could be a 128-element array if performance is more crucial than space.) That should be invulnerable to most of the issues of automating that control, even with changes across held notes, and theoretically with overlapping notes (if the input pitch is in the map, just send the NoteOn to the MIDI pitch currently registered again). This might be obnoxious to someone expecting the automation to take effect immediately, but it seems like a bad idea to send NoteOn's because an automation value is changing (especially when it can change every period). More importantly, it should work as one expects in the simpler cases.

Should I make a prototype off of this tip to this effect?

Grissess avatar Nov 21 '22 17:11 Grissess

@Veratil To be sure, have you checked #6571 before merging this?

PhysSong avatar Feb 19 '23 01:02 PhysSong

@Veratil To be sure, have you checked #6571 before merging this?

I completely overlooked it, I apologize. Shall we revert?

Veratil avatar Feb 19 '23 02:02 Veratil