Details

Reviewers

markj
emaste
dev_submerge.ch
meka_tilda.center

Commits

rG7224e9f2d4af: mididump(1): Initial revision
rGf57efe95cc25: mididump(1): Initial revision

Summary

A new utility which dumps MIDI 1.0 events in real-time.

Sponsored by: The FreeBSD Foundation
MFC after: 1 week

Test Plan

$ mididump /dev/umidi0.0
Note on                 channel=1, note=48 (C3, 130.81Hz), velocity=87
Note off                channel=1, note=48 (C3, 130.81Hz), velocity=127
Note on                 channel=1, note=50 (D3, 146.83Hz), velocity=72
Note off                channel=1, note=50 (D3, 146.83Hz), velocity=127
Note on                 channel=1, note=65 (F4, 349.23Hz), velocity=95
Note off                channel=1, note=65 (F4, 349.23Hz), velocity=127
Note on                 channel=1, note=67 (G4, 392.00Hz), velocity=29
Note off                channel=1, note=67 (G4, 392.00Hz), velocity=127
Note on                 channel=1, note=69 (A4, 440.00Hz), velocity=46
Note off                channel=1, note=69 (A4, 440.00Hz), velocity=127
Control/Mode change     channel=1, control=64 (Damper Pedal (Sustain)), value=127 (on)
Control/Mode change     channel=1, control=64 (Damper Pedal (Sustain)), value=0 (off)
^C

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 59158
Build 56045: arc lint + arc unit

Event Timeline

christos created this revision.Aug 23 2024, 3:19 PM

Herald added a subscriber: imp. · View Herald TranscriptAug 23 2024, 3:19 PM

christos requested review of this revision.Aug 23 2024, 3:19 PM

Harbormaster completed remote builds in B59158: Diff 142372.Aug 23 2024, 3:19 PM

christos edited the test plan for this revision. (Show Details)Aug 23 2024, 3:21 PM

Inspired by aseqdump and OpenBSD's midicat, but more user-friendly (printing note names, frequencies, control names, ...), and also native so it does not require ALSA or any sound backend. In the future I would like to support MIDI 2.0 events as well, but I think for now we can roll with just MIDI 1.0.

christos added inline comments.Aug 23 2024, 3:34 PM

usr.bin/mididump/mididump.c
115	https://anotherproducer.com/online-tools-for-musicians/midi-cc-list/ states that these values "usually" correspond to reverb, tremolo, phaser, etc, so I went with a more safe/generic ("Effect #") approach.
211	The instrument names are defined in https://www.music.mcgill.ca/~ich/classes/mumt306/StandardMIDIfileformat.html#BMA1_4. Is it worth it to print the instrument name?
224	I do not have a MIDI with a pitch bender ATM, so it would be nice if someone could test this. It should print a change value in the [-8192, 8192] range, with 0 being "no change". I put the TODO there so that in the future we can print a more user-friendly value.
226	I do not have a way to test this. If someone can, that'd be great.

Add bound check for ctls[].
Use b2 instead of b1 for vendorid in SysEx.

Harbormaster completed remote builds in B59159: Diff 142373.Aug 23 2024, 3:47 PM

Minor fix.

Harbormaster completed remote builds in B59160: Diff 142374.Aug 23 2024, 3:50 PM

Finally got around to bring my MIDI controller home - unfortunately I couldn't test anything meaningful, see inline comments.
Also the messages were not printed immediately, do we need to flush the printf buffer somehow?

usr.bin/mididump/mididump.c
169–172	Given the variable length of MIDI messages, we can't just read additional bytes here unconditionally. We will discard some messages and break the message parser in some cases. I'd suggest to read multiple bytes at once, and keep the bytes not yet consumed by the parser for the next iteration. That would also be more economic.
174	I suppose you want to `switch(raw)`? My test runs get caught immediately in an endless SysEx loop because the timing clock message (`0xf8`) is masked to SysEx (`0xf0`) through `EVENT_MASK` above.

christos marked 2 inline comments as done.Sep 2 2024, 4:15 PM

christos added inline comments.

usr.bin/mididump/mididump.c
169–172	I think it's better to keep the implementation simple and just read on-demand (see updated diff). The program is not resource heavy anyway.
174	Good catch.

read(2) on-demand. Introduce read_byte() function.
Get rid of EVENT_MASK.

Harbormaster completed remote builds in B59277: Diff 142738.Sep 2 2024, 4:17 PM

Regarding the approach to read one byte at a time, that's potentially one syscall every 320 microseconds. Probably ok for this tool, but I hope nobody copies this code for something serious ;-)

usr.bin/mididump/mididump.c
240–244	I don't think this parsing of SysEx vendorid is correct. From https://michd.me/jottings/midi-message-format-reference/: A System exclusive dump starts with 0b1111000, and is followed by 1 or 3 bytes to identify the manufacturer this SysEx dump targets. If the first byte of these identifier bytes is 0b00000000, the ID will be 3 bytes long, otherwise it is only one byte long.

christos added inline comments.Sep 3 2024, 12:59 PM

usr.bin/mididump/mididump.c
240–244	According to https://midi.org/expanded-midi-1-0-messages-list: System Exclusive (data dump) 2nd byte= Vendor ID (or Universal Exclusive) followed by more data bytes and ending with EOX. Which means that I should be printing `b1` for VendorID instead of `b2`. The following sources also confirm this. https://www.recordingblogs.com/wiki/midi-system-exclusive-message: The first byte is the status byte and has the hexadecimal value 0xF0. The second byte is the manufacturer ID. https://cmtext.indiana.edu/MIDI/chapter3_system_messages.php: A System Exclusive code set begins with 11110000 (240 decimal or F0 hex), followed by the manufacturer ID#, then by an unspecified number of data bytes of any ranges from 0-127) and ends with 11110111 (decimal 247 or F7 hex), meaning End of SysEx message (EOX). Also, the article you link to states that the SysEx starts with `0b1111000` (`0x78`), instead of `0xf0`. I am quite confused with his article.

christos added inline comments.Sep 3 2024, 1:22 PM

usr.bin/mididump/mididump.c
240–244	So after further research the following article confirms that the vendorid can have variable length (1 or 3 bytes), although it does not explain how we determine this, so I will take the article's approach as the correct one. https://midi.org/summary-of-midi-1-0-messages: The Manufacturer’s ID code (assigned by MMA or AMEI) is either 1 byte (0iiiiiii) or 3 bytes (0iiiiiii 0iiiiiii 0iiiiiii) The first article I linked to is also from midi.org so I wonder whether one of them has a mistake, or the first one just does not include complete information...

christos added inline comments.Sep 3 2024, 1:23 PM

usr.bin/mididump/mididump.c
240–244	Also, the article you link to states that the SysEx starts with 0b1111000 (0x78), instead of 0xf0. Forget this.

Fix SysEx handling. In the case of 3-byte VendorID, I am not sure if we should
read 3 _additional_ bytes (what the patch does), or 2.

Harbormaster completed remote builds in B59288: Diff 142808.Sep 3 2024, 1:39 PM

In D46418#1060399, @christos wrote:

Fix SysEx handling. In the case of 3-byte VendorID, I am not sure if we should
read 3 _additional_ bytes (what the patch does), or 2.

I suppose we have to read 2 additional bytes (after the first byte of the manufacturer ID was read as 0x00).

This seems to be the official table of manufacturer IDs: https://midi.org/SysExIDtable

Read 2 bytes (instead of 3) in case byte1 in SysEx is 0.

Any objections regarding the output interface?

Harbormaster completed remote builds in B59317: Diff 142980.Sep 5 2024, 4:35 PM

For real devices there's a "Timing clock" message coming at a certain frequency, too much noise to see the other MIDI messages. Maybe we could collect them and print them together, or add a flag to enable or disable printing them?

In D46418#1063564, @dev_submerge.ch wrote:

For real devices there's a "Timing clock" message coming at a certain frequency, too much noise to see the other MIDI messages. Maybe we could collect them and print them together, or add a flag to enable or disable printing them?

Unfortunately I cannot find all these cases with my humble and barebones AKAI LPK25... I will add a flag to print them (not disable them, as probably disabling them is probably the most wanted behavior, no?).

Introduce -t option to print "Timing Clock" messages on-demand, in order to
declutter output.

Harbormaster completed remote builds in B59483: Diff 143338.Sep 14 2024, 5:04 PM

I didn't see any more problems while testing. Would be nice to have a MIDI example file for testing that exercises all types of MIDI messages. mididump file.midi works BTW, but I think it's more of a feature than a bug.

This revision is now accepted and ready to land.Sep 16 2024, 1:15 AM

In D46418#1064118, @dev_submerge.ch wrote:

I didn't see any more problems while testing. Would be nice to have a MIDI example file for testing that exercises all types of MIDI messages. mididump file.midi works BTW, but I think it's more of a feature than a bug.

Yeah, it is nice to examine a file this way as well. Are you okay with the user interface?

In D46418#1064182, @christos wrote:

In D46418#1064118, @dev_submerge.ch wrote:

I didn't see any more problems while testing. Would be nice to have a MIDI example file for testing that exercises all types of MIDI messages. mididump file.midi works BTW, but I think it's more of a feature than a bug.

Yeah, it is nice to examine a file this way as well. Are you okay with the user interface?

I think you can leave the interface as is, it's sufficient for debugging and we can expand it if necessary. The next best practical feature I can think of is timing data (like a log file) and replay functionality, but that's a bit more involved.

Closed by commit rGf57efe95cc25: mididump(1): Initial revision (authored by christos). · Explain WhyFri, Oct 18, 8:46 AM

This revision was automatically updated to reflect the committed changes.

christos added a commit: rGf57efe95cc25: mididump(1): Initial revision.

kevans added a subscriber: kevans.Fri, Oct 18, 4:27 PM

kevans added inline comments.