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 MIDI Note Off Triggers Scene behaviour #1428

Closed

Conversation

Timebutt
Copy link
Contributor

@Timebutt Timebutt commented May 24, 2023

Hi,

This PR introduces a new MIDI Plugin setting. As it stands right now, when QLC+ receives a Note Off command, it gets auto-mapped to value 0. Because of this, a Note Off can never toggle a switch, which can be very much desired.

The specific use case for this is as follows: effects and chases cannot be flashed, and as such have to be toggled. When programming lights through MIDI in Cubase, I want to make sure that whenever I point anywhere in the track, all of the correct animations immediately start playing, and not for those animations to be triggered at specific points in the song only. To do this, I use very long MIDI notes (for the length of the chorus for instance). To make sure the animations stop running, I always need a second note at the very end of that note (see screenshot below).

Screenshot 2023-05-24 at 19 46 54

This is very counter intuitive for me and has lead to many cases of animations still going on when they shouldn't have (not nice to see moving heads go crazy when a quiet bridge sets in ...). Having the ability to toggle a scene whenever a MIDI note ends is a great feature in my opinion.

This PR doesn't affect existing behaviour, only provides users the ability to configure the plugin in a way that might better suit their needs. It has drastically improved my own flow for sure!

The PR is an initial proposal and I'm very welcome to feedback. I'm not a C++ or Qt developer and just try to adhere to the existing code style. Let me know!

Edit: the name noteOffTriggersScene is probably not the best yet, does anyone have a better suggestion?

Edit2: I also updated the documentation to reflect this new configuration option

@@ -440,7 +440,7 @@ void EnttecDMXUSBPro::slotDataReceived(QByteArray data, bool isMidi)
uchar value = 0;
if (QLCMIDIProtocol::midiToInput(midiCmd, midiData1, midiData2,
MAX_MIDI_CHANNELS, // always listen in OMNI mode
&channel, &value) == true)
&channel, &value, false) == true)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will need device->noteOffTriggersScene() as well, but I'm not sure yet how to get the MIDI device in this context. Also unsure as to why there is a MIDI implementation here whatsoever, the DMX USB Pro is a DMX only device.

Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

This code might refer to the MKII

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, I didn't know about that specific device and the fact that it had MIDI ports. I'll try and figure out how I can fix this then!

@coveralls
Copy link

coveralls commented May 24, 2023

Coverage Status

Coverage: 28.062%. Remained the same when pulling b379117 on Timebutt:add-midi-note-off-behaviour into 3be5b34 on mcallegari:master.

@mcallegari
Copy link
Owner

mcallegari commented Sep 30, 2023

Hi, sorry for the huge delay.
I am reviewing this PR but I'm not convinced this is the proper solution to the problem.
If you have a look at the Generic MIDI input profile, you will see that every control is set to "Slider".
This is to handle notes velocity when you want to attach notes to a fader rather than a button.
This is nice to control the intensity of a QLC+ function.

With your solution, note off would be equal to 64 which is a magic number and doesn't really represent an "off" state.
I am investigating if acting on an input profile could be a better place.
For example, a MIDI input profile has an option called "MIDISendNoteOff" which is near the topic we're discussing here.

Would it work for you if considering the note off event would be handled in a input profile rather than plugin level?

@Timebutt
Copy link
Contributor Author

Timebutt commented Sep 30, 2023

Hey @mcallegari! It's been a while for me too, so had to dig up my own code for a sec to be up to date ;)

The 64 I am currently using is the default value the MIDI specification defines when a MIDI device can generate Note-Off messages, but does not implement the velocity feature.

From the spec (see http://midi.teragonaudio.com/tech/midispec.htm and section Note-Off):

The second data byte is the velocity, a value from 0 to 127. This indicates how quickly the note should be released (where 127 is the fastest). It's up to a MIDI device how it uses velocity information. Often velocity will be used to tailor the VCA release time. MIDI devices that can generate Note Off messages, but don't implement velocity features, will transmit Note Off messages with a preset velocity of 64.

Thinking about this more, that specific line of code should basically be rewritten as *value = MIDI2DMX(data2);, identical to the Note-On and other cases, as the velocity of the original message should simply be retained IMO. In that case someone could still send Note-Off with value 0 and retain the existing behaviour.

Implementing this using an input profile is definitely also a way to go, but wouldn't that be more complex than the proposed solution, both in implementation as user experience? I can definitely think up a scenario in which a user would like per note control over this setting, but at the same time think in real life this is a per MIDI input on/off type of thing.

I do see what you mean with the fader and setting values through a MIDI note velocity, pressing and releasing a note would basically always result in the fader receiving the value 64 in the currently proposed solution (and even after refactoring to *value = MIDI2DMX(data2); as most devices will send 64). This would probably break other people's flows when using a global control indeed.

Curious to hear your thoughts, I'll give it some more thought myself as I hadn't considered the slider case. An ideal scenario would be to retain the event type all the way up to the control. A slider could opt to not consider Note-Off events for instance, but that information gets stripped after midiToInput() of course. That would blow up the scope tremendously though I fear ;)

@mcallegari
Copy link
Owner

The setting I'm referring to in input profile is global and only refers to MIDI profiles:
Screenshot_20231001_100428

Handling one setting per note would definitely be more cumbersome, especially for users.

One additional note: the setting you proposed is stored in the QLC+ configuration file, which is not portable across platforms. An input profile instead can easily be copied as a file and is referenced in the project file IO section.

@Timebutt
Copy link
Contributor Author

Timebutt commented Oct 1, 2023

In that case the Input Profile seems the right way to go. Moreover because it's very closely related to the existing setting indeed. I'll try and find the time to rework my PR here and will get back to you!

@mcallegari mcallegari marked this pull request as draft March 6, 2024 09:57
@Timebutt Timebutt closed this Sep 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants