Flesh out MIDI support #1112

gabeplaysdrums · 2017-02-20T01:58:46Z

QMK supports MIDI, but the existing implementation is basically just a proof of concept. As part-time musician, the idea of being able to use my keyboard as a MIDI controller is appealing, so I added some features to make MIDI more useful:

"Tone" keycodes for up to 6 octaves of MIDI notes
Keycodes for channel selection, octave shift, and transpose
Keycodes for momentary pedals (sustain, portamento, etc.)
Virtual modulation wheel
Some documentation (e.g. added size)

I've built a few keymaps on top of this for my personal use which you can see in my fork under satan/keymaps/newsboytko. I've used these keymaps in the studio and on stage during live performances :)

Configuration

Config options for MIDI allow some flexibility so the user can opt into the functionality they need (and not pay for stuff they don't). The options are described in the template config.h:

/*
 * MIDI options
 */

/* Prevent use of disabled MIDI features in the keymap */
//#define MIDI_ENABLE_STRICT 1

/* enable basic MIDI features:
   - MIDI notes can be sent when in Music mode is on
*/
//#define MIDI_BASIC

/* enable advanced MIDI features:
   - MIDI notes can be added to the keymap
   - Octave shift and transpose
   - Virtual sustain, portamento, and modulation wheel
   - etc.
*/
//#define MIDI_ADVANCED

/* override number of MIDI tone keycodes (each octave adds 12 keycodes and allocates 12 bytes) */
//#define MIDI_TONE_KEYCODE_OCTAVES 1

MIDI_BASIC is on par with previous functionality, but is now better integrated with music/audio. For example, music recordings can now play back audio, MIDI notes, or both.

MIDI_ADVANCED is more geared toward users who want to use their keyboard as a full-blown MIDI instrument, including custom layouts (keymaps) and expression features (sustain, modulation, etc.)

You can of course turn on both feature sets and get everything, or neither and write custom actions that use the MIDI protocol directly.

keyboards/satan/keymaps/midi is an example keymap that uses all features.

Compatibility

Existing keymaps that are using MI_ON and MI_OFF have been opted into MIDI_BASIC for parity with previous behavior. Though many of them don't appear to be actually using any of MIDI functionality (e.g. they set MIDI_ENABLE = no in the Makefile). If we wanted to root these out, we could do so by changing the default of MIDI_ENABLE_STRICT in quantum_keycodes.h.

…tes on

tone array: text data bss dec hex filename 0 25698 0 25698 6462 satan_newsboytko.hex 0x6480 bytes written into 0x7000 bytes memory (89.73%). note on array: text data bss dec hex filename 0 25802 0 25802 64ca satan_newsboytko.hex 0x6500 bytes written into 0x7000 bytes memory (90.18%).

…ymap

…iments) satan/keymaps/midi MIDI_ENABLE = no text data bss dec hex filename 0 17080 0 17080 42b8 satan_midi.hex MIDI_ENABLE = yes #define MIDI_TONE_KEYCODE_OCTAVES 3 // default text data bss dec hex filename 0 20846 0 20846 516e satan_midi.hex MIDI_ENABLE = yes #define MIDI_TONE_KEYCODE_OCTAVES 2 // fewer octaves text data bss dec hex filename 0 20846 0 20846 516e satan_midi.hex

ItsHarper · 2017-02-20T03:04:39Z

This looks pretty awesome!

It looks like you've removed the ability to turn MIDI on and off at runtime, which breaks keymaps. That's why the CI check failed. What was your reasoning for that?

gabeplaysdrums · 2017-02-20T03:45:24Z

Thank goodness for the CI build. :)

Here was my thinking:

Technically MIDI was always "on" ... the device was initialized and everything but process_midi wouldn't send any notes unless MI_ON was pressed. You could still send MIDI messages directly through midi_send_noteon and other functions in midi.h. MI_ON would put you into a funny mode where each key sends a midi note, but you couldn't customize which keys sent which notes, etc. Furthermore, I couldn't find any way to get out of this mode without power cycling the keyboard (I had MI_OFF in my keymap but it didn't seem to work). Frankly, it looked to me like prototype code that the author didn't intend for regular use (though I can't say for sure).

Anyway, since MIDI notes can be bound to specific keys in the keymap now (as it appears might have been the original intent but was never fully implemented?) MI_ON makes little sense. MI_OFF has been repurposed for "send CC for all midi notes off".

Options to resolve:

Add back the (prototype?) "bind all keys to a midi note" behavior of MI_ON, assuming people find it useful for something
Remove MI_ON and update all of the keymaps that use it (probably set those keys to KC_TRANS)
Remove support for MI_ON but keep the preprocessor define so that existing keymaps will compile but not do anything. I could probably also make the compiler emit a warning so that people know it is deprecated.

What are your thoughts on these solutions?

jackhumbert · 2017-02-20T04:02:12Z

Nice! I saw your email but hadn't had a chance to respond yet. My thinking with the midi mode as it is currently is the same as the music mode for keyboards with audio enabled - it can be turned on and off (this worked last I checked, but that was a while ago), can play notes (via midi), and doesn't take up extra room in the keymap.

The midi notes as keycodes was possible early on (it might still be if the defines were updated), but I didn't find it very useful - the method I have in there currently allows for remapping to different modes/scales/setups more easily than if they were bound directly to keys.

I'd be happy to rename MIDI_ON/OFF to MIDI_MODE_ON/OFF (and MIMO_ON/OFF), since that's a bit better at describing what's actually going on. I'd like all of the existing functionality to stay though :)

gabeplaysdrums · 2017-02-20T06:12:29Z

Ok, I've done a more careful audit. process_midi.* has a bunch of stuff that I originally assumed wasn't referenced, but I now know I was wrong :)

See below for more details of what I found, and my proposal for how to move forward ...

Specifically, here's what process_midi.h provides today and how existing keymaps are using it (please let me know if there's anything I missed).

Feature	Description	Usage
`MIDI()` macro	For defining MIDI keycodes?	None
`MIDI12` define	Not sure what this is for ... maybe for defining a chromatic scale?	None
`CHNL()` macro	Possibly for sending keycodes on a specific channel. Appears to be unsupported.	None
`SCALE` macro	Appears to define notes for a C-Major scale in the first 5 octaves	None
`N_*` keycodes	Possibly intended for putting MIDI notes in keymap. However I couldn't find anything that would process them into midi notes so it appears they are unsupported?	None
"MIDI Mode"	Turned on by keycode `MI_ON`. All keys send midi based on their offset in the matrix. Supports several different "scales" that change the layout of the notes on the keyboard. Non-default scales can be enabled by uncommenting code, except for a "custom" scale that doesn't appear to compile. MIDI Mode seems loosely related to "Music Mode" (turned on by keycode `MU_ON`) which allows you to play an audio frequency when a key is pressed (and record and play back a sequence of them) and has similar-looking code. It might seem useful to turn on Music Mode and MIDI Mode at the same time, which would an audio frequency and a MIDI note on keypress. However currently they won't be the same note (e.g. the key at [0,0] will play 659 Hz (E5) and send midi 111 (Eb10)), so anyone who is actually doing this will have a weird experience.	`MI_ON`/`OFF` apppears in many keymaps (in particular ones that use music mode), however all of them also set `MIDI_ENABLE = no`, so it isn't actually being used by anyone.

Observations:

MIDI mode seems similar to Music mode (functionally and code wise), except it sends MIDI messages instead of generating audio frequencies. Rather than having separate keycode processors, it might be more natural make a few changes to process_music.c to make it also send MIDI messages (assuming MIDI_ENABLE is defined). It would also make sense to have a MUSIC_SEND_MIDI_TOGGLE (MU_MITG) in the keymap to turn this behavior on and off.
MIDI Mode seems like a nice add-on to Music mode, but I doubt many are using MIDI mode on its own given the "WIP" description in the Wiki (it also references a keymap_midi.c that doesn't appear to exist in the repo), and the fact that it sends different notes than Music mode.
The layouts of Music Mode and MIDI Mode don't seem particularly useful/comfortable for playing music. Most physical instruments use a piano layout or grid of diatonic scales. There are a lot of software MIDI keyboards out there that translate keypresses into MIDI notes (DAWs like ProTools, Reason, Reaper, etc. have them built in and they often useful, especially in mobile setups), and they pretty much all emulate a piano layout, like the satan/keymaps/midi keymap in my pull request. If there is anyone out there who uses MIDI Mode (I couldn't find any keymaps that would exercise it), I'd love to hear about what you actually use it for?

Proposed Changes:

Here's how I think we should move the code forward:

Remove unused defines and public functions from process_midi.h, which is pretty much all of them (the current iteration of my pull request already does this)
Move "MIDI Mode" into the Music processor. This amounts to adding code to process_music.c to send MIDI messages. This should be less code overall and easier to maintain, etc. The existing "MIDI Mode" becomes "Send MIDI Messages in Music Mode"
Add new keycodes for enabling/disabling "Send MIDI messages in Music Mode" (MU_MION, MU_MIOFF, MU_MITG) to logically replace MI_ON/OFF.
Update all existing consumers of MI_ON/OFF to use MU_MION/OFF instead
Bind a new keycode MI_ALLOFF to the "all notes off" midi CC message instead of repurposing MI_OFF, to avoid any confusion
Take the other things in my pull request (new MIDI keycodes, midi_config, etc.) as-is

Net result:

People who want to use MIDI in conjunction with Music/Audio (all of the existing keymaps as far as I can tell?) have the same functionality, but the keycode has a different name and the processing code is a bit more organized.
People who want to use their keyboard as a MIDI instrument are able to define custom layouts, etc. and get features one expects from a MIDI controller.

What do you guys think?

jackhumbert · 2017-02-21T01:41:55Z

quantum/process_keycode/process_midi.c

-          return false;
-      }
-      // basic
-      // uint8_t note = (midi_starting_note + SCALE[record->event.key.col + midi_offset])+12*(MATRIX_ROWS - record->event.key.row);


SCALE is used here - these are the 4 modes I have supported right now. They need to be worked into an option to adjust things at runtime.

Ah, okay. I considered commented code unused :) It wasn't clear to me whether it was experimental or something you intend people to use, since there isn't a way to turn it on in the keymap or anything.

jackhumbert · 2017-02-21T02:03:01Z

I turn MIDI off by default to save room on the chip (we're currently very close to being maxed out) - I leave the keycodes in there so all people have to do is compile with the = yes change to their Makefile. I've found the layouts I have in there useful for the stuff I do on my keyboards - the traditional keyroll may work on staggered boards, but it's pretty awkward on ortholinear ones :) I've been meaning to implement an Omnichord/accordion-like layout for both modes as well.

I'm fine with the MIDI mode moving to the audio/music mode section, but I would like to be able to use it without having to enable audio. It might make more sense to keep things in the MIDI section, and have a shared key-to-note mapping function somewhere else, so there's not any duplicated code.

gabeplaysdrums · 2017-02-22T06:19:00Z

Thanks for elaborating on how you use midi mode. It certainly helps me understand the intent.

When you say "I leave the keycodes in there", you mean MI_ON/OFF, right? Are the N_ constants in process_midi.h supposed to be keycodes? Nothing processes them as far as I can tell ...

The fact that any given layout is comfortable on one board but not another is exactly what motivated my changes to have it defined in the keymap rather than some hardcoded default. While a matrix-based layout might make sense for you on your ortholinear board it doesn't work at all for me. I can understand not wanting to bloat the keymap, which is why I didn't define a keycode for every MIDI note (which would also be pretty awkward because it would force you to set up a layer per octave, etc.), but instead defined a smaller number of 'tone' keycodes and octave/transpose shift, just like traditional dedicated MIDI instruments have.

As for moving midi mode into process_music.c, I know I can make this work without requiring AUDIO_ENABLE. In fact, I have an experimental branch in my fork (newsboytko/music-experiments) that does exactly that, because I wanted to run some code in process_music.c but my keyboard doesn't support audio :). It's pretty hacked up right now but could be cleaned up and merged into this PR. Also, when you implement your Omnichord layout (which sounds super neat!) wouldn't it be nice if you didn't have to edit 2 files? ;)

The more I think about it, I wonder if what we are dealing with here are two different but equally valid ways to use MIDI which should be behind different Makefile options. I think of "midi mode" as a more "basic" use case for someone who just wants to bootstrap a MIDI device with minimal cost and is okay with a hardcoded layout, while other "advanced" users want to use the keyboard as a full-on MIDI instrument complete with custom layouts. What if we had MIDI_BASIC_ENABLE that covers the first type of user, and MIDI_ADVANCED_ENABLE that covers the latter? People who just want MIDI mode set MIDI_BASIC_ENABLE = yes in their Makefile and don't have to pay the keymap (and extra processing code) costs. Those who want custom layouts and other advanced features but don't need MIDI mode set MIDI_ADVANCED_ENABLE = yes, and those who want everything set both.

jackhumbert · 2017-02-22T16:09:22Z

Yeah, the N_ were keycodes when the MIDI functionality occupied the 0x6000 space, but it doesn't anymore, and they aren't used like that.

As far as I know, my boards are the only ones to really advertise (and limited at that) the MIDI functionality, so I was planning on people either using it with them, or building something custom, in which case they wouldn't be using my framework at all :) glad to see that change though!

What music code are you pulling from that file? Maybe it makes sense to move things into one of three files categories/naming schemes:

midi
audio
music

music would then contain all of the shared functions that midi and audio use including both "modes", along with some light ifdefing for each mode (if needed).

The basic/advanced split sounds good, but let's make it a flag set in config.h, with wrappers in the .c file - we have quite a few Makefile options as it is :)

gabeplaysdrums · 2017-02-22T17:07:34Z

Here's a direct link to the diff of my music-experiments branch.

The category split and config.h options sound good. I'll give that a shot and push another iteration of this PR.

MIDI_ENABLE = no text data bss dec hex filename 0 17080 0 17080 42b8 satan_midi.hex MIDI_ENABLE = yes MIDI_BASIC undefined MIDI_ADVANCED undefined text data bss dec hex filename 0 19494 0 19494 4c26 satan_midi.hex MIDI_ENABLE = yes #define MIDI_BASIC MIDI_ADVANCED undefined text data bss dec hex filename 0 19788 0 19788 4d4c satan_midi.hex MIDI_ENABLE = yes MIDI_BASIC undefined #define MIDI_ADVANCED text data bss dec hex filename 0 20846 0 20846 516e satan_midi.hex MIDI_ENABLE = yes #define MIDI_BASIC #define MIDI_ADVANCED text data bss dec hex filename 0 21140 0 21140 5294 satan_midi.hex

Update existing keymaps to enable MIDI_BASIC functionality. Also added an option MIDI_ENABLE_STRICT to be strict about keycode use (which also reduces memory footprint at runtime)

gabeplaysdrums · 2017-02-26T05:17:17Z

Pushed new iteration with all changes as discussed. As an added bonus, music recordings now also play MIDI notes, and recordings use 75% less memory :)

MIDI_ENABLE = no text data bss dec hex filename 0 17080 0 17080 42b8 satan_midi.hex MIDI_ENABLE = yes MIDI_BASIC undefined MIDI_ADVANCED undefined text data bss dec hex filename 0 19494 0 19494 4c26 satan_midi.hex +2414 bytes (vs. MIDI_ENABLE = no) MIDI_ENABLE = yes 0 20846 0 20846 516e satan_midi.hex +1352 bytes (vs. MIDI_ENABLE = yes, MIDI_BASIC off, MIDI_ADVANCED off) MIDI_ENABLE = yes #define MIDI_BASIC #define MIDI_ADVANCED text data bss dec hex filename 0 21292 0 21292 532c satan_midi.hex +1798 bytes (vs. MIDI_ENABLE = yes, MIDI_BASIC off, MIDI_ADVANCED off) Conclusion: +2400 to 4200, depending on config

jackhumbert · 2017-02-28T03:27:05Z

quantum/process_keycode/process_music.c

@@ -8,36 +17,41 @@ int music_offset = 7;
 static bool music_sequence_recording = false;
 static bool music_sequence_recorded = false;
 static bool music_sequence_playing = false;
-static float music_sequence[16] = {0};
+static uint8_t music_sequence[16] = {0};


Thanks - I knew that was a bad idea :)

Yeah, the one advantage I could see to using a float here is you could theoretically record things like pitch bends that are between notes in the chromatic scale, but I think once you go down that road the logical conclusion is recording full samples which will use a lot more memory.

jackhumbert · 2017-02-28T03:30:20Z

Perfect - let me test things out a bit, and we'll get this merged :)

gabeplaysdrums · 2017-02-28T16:52:32Z

Yeah, I've done some testing on my end but it would be good to make sure audio is working as expected. I don't have a keyboard that supports audio (C6 pin is used) so I couldn't test it properly, but I did validate that the frequency is correct with dprintf, so in theory it works.

gabeplaysdrums · 2017-02-28T17:01:37Z

quantum/process_keycode/process_midi.h

+#ifdef MIDI_BASIC
+void process_midi_basic_noteon(uint8_t note);
+void process_midi_basic_noteoff(uint8_t note);
+void process_midi_basic_stop_all_notes(void);


this should probably be named process_midi_basic_all_notes_off to align better with names used in process_music.c

gabeplaysdrums · 2017-02-28T17:02:30Z

quantum/process_keycode/process_audio.h

+bool process_audio(uint16_t keycode, keyrecord_t *record);
+void process_audio_noteon(uint8_t note);
+void process_audio_noteoff(uint8_t note);
+void process_audio_stop_all_notes(void);


this should probably be named process_audio_all_notes_off to align better with names used in process_music.c

gabeplaysdrums · 2017-02-28T17:05:33Z

quantum/template/keymaps/default/Makefile

@@ -9,7 +9,7 @@ CONSOLE_ENABLE = no         # Console for debug(+400)
 COMMAND_ENABLE = yes        # Commands for debug and configuration
 NKRO_ENABLE = yes           # Nkey Rollover - if this doesn't work, see here: https://github.com/tmk/tmk_keyboard/wiki/FAQ#nkro-doesnt-work
 BACKLIGHT_ENABLE = no       # Enable keyboard backlight functionality
-MIDI_ENABLE = no            # MIDI controls
+MIDI_ENABLE = no            # MIDI support (+3800)


Should be updated to reflect new estimates (2400 to 4200)

gabeplaysdrums · 2017-03-11T21:07:30Z

Hey @jackhumbert any update on this?

jackhumbert · 2017-03-12T18:18:51Z

Not yet - hoping to be able to get this merged next week :)

jackhumbert · 2017-03-28T13:26:53Z

Gonna go ahead and merge this :) thanks!

* Update crimsonkeyboards_resume1800_default.json Updated .json so that scroll lock and num lock shows up properly.

Gabriel Young added 12 commits February 19, 2017 14:34

Merge branch 'qmk/master'

40a8a88

Expand MIDI key codes

5ae1411

initial implementation of polyphony using variable length array of no…

e405ab4

…tes on

clean up commented code

f2b2e05

remove disabled code

f67aefc

add support for pedal cc messages

7c5e510

implement modulation

dd8f8e6

add keycodes for transpose range

5e6097f

expose midi_config

ae0752d

Allow customization of the number of tone keycodes and add example ke…

d1fe24a

…ymap

fix typos in keymap

64eecfc

jackhumbert reviewed Feb 21, 2017

View reviewed changes

Gabriel Young added 5 commits February 25, 2017 15:02

Factor basic note processing into respective processors

1000799

Add basic layer to sample MIDI keycap

ea14ed1

Update existing keymaps

a64ae10

Update existing keymaps to enable MIDI_BASIC functionality. Also added an option MIDI_ENABLE_STRICT to be strict about keycode use (which also reduces memory footprint at runtime)

Fix redundant process_music.c in sources

b911e19

jackhumbert reviewed Feb 28, 2017

View reviewed changes

gabeplaysdrums commented Feb 28, 2017

View reviewed changes

Gabriel Young added 2 commits March 2, 2017 11:40

fix 'stop_all_notes' naming to be more consistent

d11962a

add debug messages to audio

0734f56

jackhumbert merged commit 7e37daa into qmk:master Mar 28, 2017

mattpcaswell pushed a commit to mattpcaswell/qmk_firmware that referenced this pull request Jun 7, 2023

update default keymap: crimsonkeyboards/resume1800 (qmk#1112)

b3ef9cc

* Update crimsonkeyboards_resume1800_default.json Updated .json so that scroll lock and num lock shows up properly.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Flesh out MIDI support #1112

Flesh out MIDI support #1112

gabeplaysdrums commented Feb 20, 2017 •

edited

Loading

ItsHarper commented Feb 20, 2017

gabeplaysdrums commented Feb 20, 2017 •

edited

Loading

jackhumbert commented Feb 20, 2017

gabeplaysdrums commented Feb 20, 2017 •

edited

Loading

jackhumbert Feb 21, 2017

gabeplaysdrums Feb 22, 2017

jackhumbert commented Feb 21, 2017

gabeplaysdrums commented Feb 22, 2017 •

edited

Loading

jackhumbert commented Feb 22, 2017

gabeplaysdrums commented Feb 22, 2017

gabeplaysdrums commented Feb 26, 2017 •

edited

Loading

jackhumbert Feb 28, 2017

gabeplaysdrums Feb 28, 2017

jackhumbert commented Feb 28, 2017

gabeplaysdrums commented Feb 28, 2017

gabeplaysdrums Feb 28, 2017

gabeplaysdrums Feb 28, 2017

gabeplaysdrums Feb 28, 2017

gabeplaysdrums commented Mar 11, 2017

jackhumbert commented Mar 12, 2017

jackhumbert commented Mar 28, 2017

Flesh out MIDI support #1112

Flesh out MIDI support #1112

Conversation

gabeplaysdrums commented Feb 20, 2017 • edited Loading

Configuration

Compatibility

ItsHarper commented Feb 20, 2017

gabeplaysdrums commented Feb 20, 2017 • edited Loading

jackhumbert commented Feb 20, 2017

gabeplaysdrums commented Feb 20, 2017 • edited Loading

jackhumbert Feb 21, 2017

Choose a reason for hiding this comment

gabeplaysdrums Feb 22, 2017

Choose a reason for hiding this comment

jackhumbert commented Feb 21, 2017

gabeplaysdrums commented Feb 22, 2017 • edited Loading

jackhumbert commented Feb 22, 2017

gabeplaysdrums commented Feb 22, 2017

gabeplaysdrums commented Feb 26, 2017 • edited Loading

jackhumbert Feb 28, 2017

Choose a reason for hiding this comment

gabeplaysdrums Feb 28, 2017

Choose a reason for hiding this comment

jackhumbert commented Feb 28, 2017

gabeplaysdrums commented Feb 28, 2017

gabeplaysdrums Feb 28, 2017

Choose a reason for hiding this comment

gabeplaysdrums Feb 28, 2017

Choose a reason for hiding this comment

gabeplaysdrums Feb 28, 2017

Choose a reason for hiding this comment

gabeplaysdrums commented Mar 11, 2017

jackhumbert commented Mar 12, 2017

jackhumbert commented Mar 28, 2017

gabeplaysdrums commented Feb 20, 2017 •

edited

Loading

gabeplaysdrums commented Feb 20, 2017 •

edited

Loading

gabeplaysdrums commented Feb 20, 2017 •

edited

Loading

gabeplaysdrums commented Feb 22, 2017 •

edited

Loading

gabeplaysdrums commented Feb 26, 2017 •

edited

Loading