WebMIDI In/Out extension#2022
Conversation
Added midi extension for input/output of midi using the WebMidi API. Still pending documentation and peer-review
use "t=<time>" instead of "@<time>". Allow using "type=note pitch=60" instead of "note 60" if need to be verbose. Some bugfixes
|
At a glance, this looks much better than mine. You'll definitely need to add it to the list in |
| */ | ||
| function noteNameToMidiPitch(note, defaultOctave = 4) { | ||
| const parts = | ||
| /(?<pitch>[A-G])(?<flat>[b♭]+)?(?<sharp>[#♯]+)?_?(?<octave>-?\d+)?/i.exec( |
There was a problem hiding this comment.
This should be more human-readable if possible
| function stringToMidi(text, opts = {}) { | ||
| if (typeof text !== "string") text = Cast.toString(text); | ||
| const fullRe = | ||
| /^\s*(?<type>[a-zA-Z]{2,})?\s*((?<pitch>[A-G][#b♯♭_]*-?\d?)|(?<data1>\b-?[0-9a-f]{1,5}\b))?\s*(?<data2>\b[0-9a-f]{1,3}\b)?\s*(?<keyvals>.+)\s*$/; |
There was a problem hiding this comment.
This isn't fun to try to understand... Also you should use Scratch.Cast instead of Cast
There was a problem hiding this comment.
Cast here is a local const of Scratch.Cast. Switched to the verbose Scratch.Cast to avoid confusion in the future
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
| // sanity check - don't let a note be more than 1 minute | ||
| // if anyone ever needs a longer note then they should manually write event string |
There was a problem hiding this comment.
Why?
I know it's a rare occurrence but as a musician (pianist), developer, and Scratch user you shouldn't generally limit the user's capabilities unless necessary. Notes technically can be a minute long and a user shouldn't have that length unexpectedly cut off...
Keep in mind this is just a quick look-over and I haven't fully analyzed it in depth yet
There was a problem hiding this comment.
I hear ya. I checked and the Scratch Music extension clamps to 100 beats max (100 seconds at 60bpm) - updated code accordingly
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
| } | ||
|
|
||
| //#endregion | ||
| Scratch.extensions.register(new MidiExtension()); |
There was a problem hiding this comment.
This class name is a bit overgeneralized?
Also your code style is not exactly like mine but I guess it's just a matter of preference. seems a bit cluttered to me but I guess it's just a personal problem.
Other than that it looks fine
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
|
Just a quick review, nothing in depth. Going over best practices for TW extensions in particular, not necessarily anything related to the functionality of the code. |
Existing code uses "dur" for "duration", and "pitch" for "noteName"
midi precision doesn't go under 2-5 milliseconds, so forcing numbers to be readable 1ms precision
Added midi extension for input/output of midi using the WebMidi API. Still pending documentation and peer-review
|
@SharkPool-SP mind doing a quick review? |
|
I don't have a midi to test with |
That's right, I forgot you told me |
okay |
|
ill do it when they fix the other requested changes |
added extra block for outputting devices as json, allow outputting active notes as json
|
@SharkPool-SP I addressed comments and added some JSON output blocks. I also plan on adding some documentation. |
|
Then I'll wait until documentation is made before merging |
|
Hi @lselden, I'd like to get this extension merged as soon as possible. Is there anything I can do to help you with that? |
|
Hello! I've taken a look at your WebMIDI I/O extension and it looks very good!
But other than that, it's really good. |
|
Btw I think I'm going to do a MIDI Player extension, if anybody here wants to contribute just dm me on discord @ samuelloufcoder. |
Will that extension record/write files too? |
… to "is note active", not just the pitch
return promise to suspend execution for duration of event
@samuellouf I added the input device selection to the when hats - good idea! I didn't get a chance to add the type filter to the input event hat. Sounds like a good idea though, and I'd welcome the contribution |
Hi @Brackets-Coder - thanks for the nudge. I've added some help docs and a sample project, and made a couple of bugfixes. @SharkPool-SP I've added the documentation - hopefully it makes sense and is useful |
|
Okay, I'll probably take a look sometime tomorrow or after |
|
@lselden sorry for the delay, I'll review this weekend if I remember |
Brackets-Coder
left a comment
There was a problem hiding this comment.
This isn't a code review, I'm just looking over the documentation with two quick suggestions. Let me know if you have any questions or need anything.
I haven't tested the sample project or code yet after my previous review, but everything else looks okay
|
p.s. sorry for my tardiness |
|
@Brackets-Coder sorry for my tardiness in seeing your comments. I updated as per your comments. |
|
@lselden Apologies for the delays in getting this merged. I'm currently out of town so things are moving a bit slower but I'll try to help you and review where I can. Thanks for contributing! |
This is an extension that adds MIDI support using the WebMIDI API. It tries to cover most use cases - it supports notes, CC, Pitch Bend, Program Change messages, etc.
Features
Todo