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

Feature / Enable a new menu in Sound menu to make synths and sounddrums also send midi at the same time as they play a sound #3313

Merged

Conversation

soymonitus
Copy link
Contributor

@soymonitus soymonitus commented Jan 24, 2025

The reason for this feature is for your sample drums or synths to trigger external synths, devices, or effects.
There is a new entry in the SOUND menu called MIDI with options:

  • Channel: OFF, 1, 2, ...16
  • Note: 0, 1, ...127 (defaults to 60, that is, C3) (shown only for kit rows). The note you set is the "base" note sent by the kit row, but if you have the arpeggiator enabled it will send the correct note, based on 'Chord Simulator' parameter and based on 'Octaves' parameter, depending on the post-arp note that triggers.

My use case:
In my case I will use this feature to send a midi note in kick drum rows, that will trigger an external sidechain compressor, so external synths that don't go through the Deluge can also be sidechain compressed individually apart from the internal Deluge sounds.

For synths:
It currently supports regular MIDI with channel or poly aftertouch. MPE is not supported yet and can be done in a different PR

Copy link
Contributor

github-actions bot commented Jan 24, 2025

Test Results

107 tests  ±0   107 ✅ ±0   0s ⏱️ ±0s
 16 suites ±0     0 💤 ±0 
 16 files   ±0     0 ❌ ±0 

Results for commit e2dfd35. ± Comparison against base commit 93333f1.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@m-m-adams m-m-adams left a comment

Choose a reason for hiding this comment

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

I think this should be on all sounds and not just drums - the code is already shared with synths so it would just be moving the menus from drum to sound

@soymonitus
Copy link
Contributor Author

I think this should be on all sounds and not just drums - the code is already shared with synths so it would just be moving the menus from drum to sound

But this aimed to be a simple functionality. Is it ok then to only support regular midi and not MPE? I wanted to make drums still light, like the mididrum class is, which is as simple as a channel and a note.

Is there any way to make synths and drums behave differently? Kit rows have been always simpler as much as they can, but i agree that eventually a synth should be able to also output midi. But synths are a different beast cause MPE comes into play

@m-m-adams
Copy link
Collaborator

MPE comes into play for drums as well!

@soymonitus
Copy link
Contributor Author

soymonitus commented Jan 26, 2025

MPE comes into play for drums as well!

Not for the existing midi drum rows though.

That's why i expected this feature just as if you merge a sound row with a midi row into one. I don't expect anything else from it

@soymonitus
Copy link
Contributor Author

I support my argument in for example the CV drums, they don't even support pitch or velocity, they are just triggers, if tou want that you have the CV synth clips. Same goes for midi drums, just a note. I think they do have polyAT though (but not MPE) so i should maybe add that too to this PR

# Conflicts:
#	docs/community_features.md
@soymonitus
Copy link
Contributor Author

soymonitus commented Jan 27, 2025

I think I can move all the implementation from Sound to SoundDrum so each type of sound has their own implementation. I think putting everything into the sound class is not the right move as the midi_instrument class has a LOT of code for midi, so it might be better to refactor it into a MidiProcessor class (or something like that), so midiinstruments and soundinstruments both can have a MidiProcessor in charge of doing all the midi and mpe stuff.

For soundDrums, as the goal is to make them super simple and light on resources, I can move all my implementation from Sound to SoundDrum, so it feels similar to MidiDrum in terms of simplicity. Then each can have their own PR, this only for drums, and the other big refactor for making synths also send the full spec of midi/mpe

@soymonitus
Copy link
Contributor Author

soymonitus commented Jan 27, 2025

@m-m-adams I moved all my code to SoundDrum class as per the explanation from my previous messages. Now the midi out for SounDrums is kept damn simple (light on resources), while in the future another implementation can be done for SoundInstruments, which will be far more complicated (will require a big refactoring of MidiInstrument to decouple MIDI related code from it to make it usable by SoundInstrument, or even make SoundInstrument inherit from MidiInstrument directly to avoid all that, who knows)

@m-m-adams
Copy link
Collaborator

MPE comes into play for drums as well!

Not for the existing midi drum rows though.

That's why i expected this feature just as if you merge a sound row with a midi row into one. I don't expect anything else from it

Existing midi drum rows can have mpe data recorded in them, it's been around since official 4.0.

Since the implementation is based on sound and not drum it already supports synths. It basically just needs the menus moved from drum to sound

@m-m-adams
Copy link
Collaborator

I don't think moving the code to drum was a good idea. It was fine in sound and reduces code duplication

@soymonitus soymonitus changed the title Feature / Enable a new menu in SoundDrum to make rows also send midi at the same time as they play a sound Feature / Enable a new menu in Sound menu to make synths and sounddrums also send midi at the same time as they play a sound Jan 30, 2025
@soymonitus
Copy link
Contributor Author

soymonitus commented Jan 30, 2025

@m-m-adams I had to do several changes to avoid sending duplicated noteOffs, and to send the correct polyphonic expression events. Now it is fully working for every case: sequenced notes, auditioned notes, notes with no tails (ONCE samples or synths with no sustain), arpeggiator off and on, allNoteOff events (when changing arp preset for example), etc. Meaning there is always a note off for every note on.

Ready for review

# Conflicts:
#	docs/community_features.md
@m-m-adams m-m-adams added this pull request to the merge queue Feb 1, 2025
Merged via the queue into SynthstromAudible:community with commit 3b718c0 Feb 1, 2025
6 checks passed
@soymonitus soymonitus deleted the monitus/kit_sound_send_midi branch February 5, 2025 08:04
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.

2 participants