-
Notifications
You must be signed in to change notification settings - Fork 312
WebMIDI In/Out extension #2022
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
base: master
Are you sure you want to change the base?
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be more human-readable if possible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extensions/midi/midi.js
Outdated
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't fun to try to understand... Also you should use Scratch.Cast instead of Cast
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
extensions/midi/midi.js
Outdated
| // 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
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
…ke tempo/keySignature
|
Hello, This doesn't seem to read my MIDI devices after my app has packaged (at least in Chrome on my Android when packaged from Penguinmod) (To play a note on this, swipe from one clock position toward another (I will be adding more features to the face of this "clock, such as aftertouch, sustain, page turning, and menu)) Thanks for any help you can provide |
@AliceC-W I couldn't get your app to do anything, so can't comment on it besides it looked like it's trying to play a 0 duration note, which might be an issue. That being said, browsers are particular about allowing MIDI access depending on the context - are you trying to open it as a local file? If so then try running it from a https link instead, which has fewer restrictions. You may also want to test if midi playback is supported by using the If you want further help then please make an example that just has the minimum necessary to reproduce (for example click/touch on the screen to play a single note) |
Yes I am trying to run it right on my phone in Android Chrome How do I run it from a https link? Will this require additional hardware (counting codebeautify.org)? Does Penguinmod offer a way to force that change in the address bar? Here is a simple project detailing my only problem at the moment |
Thanks for providing the project. I'm able to see my output devices on my laptop, and 0 devices on my iPhone (which is expected - Apple doesn't support WebMidi). I'd recommend using Github Pages to host your content (which is free) if you don't mind it being publically accessible - I'm sure there's plenty of tutorials out there if you aren't that familiar with github. You could also try testing if you have a computer on the same network using any static web server software, for example fenix, |
|
Thanks I just found out that Opera GX recognizes my MIDI devices even in airplane mode I think it will do, at least until that environment can be simulated in the .html (directory) itself I'm sorry to hear that about iPhone, definitely let me know if that changes The equipment I used was:
|
|
@lselden I've used both macOS and Windows throughout various periods. About the iPhone -- It's not that Apple straight up just doesn't support WebMIDI, it's that all Safari Browsers across Apple devices don't support the API due to security concerns, which Apple seems to care a lot about. I've gotten WebMIDI to work as expected with Google Chrome on macOS |
|
@lselden Is this still being worked on? |
|
Hello @lselden, video.mov |
extension had a WIP addition of the @tonejs/midi library for midi file support. Removing in favor of it being a separate extension
|
@Brackets-Coder No I'm not actively working on this. I had left this off with partial support for midi file in/out, but never finished. I just updated the library to remove the midi file code. If any one is interested in collaborating (@CHCAT1320 ?) I can submit what I have for midi-file handling as a separate PR |
|
@lselden If everything checks out, I can get this approved. Hopefully we can get it merged. Is everything ready for the review process? |
|
@Brackets-Coder sure go ahead and review. |
Alright None of the Todo checkboxes were clicked, were they meant to be completed or are they still being worked on? |
Brackets-Coder
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lselden I did a bit of a hasty review of the functionality of some (not all) of your blocks, let me know if I missed anything or am misunderstanding. Thanks!
- The icon for the block palette illegible in dark mode, a simple white circle behind it should suffice.
- The MIDI input blocks I tested all seem to be working as expected.
- The MIDI [connected/disconnected] hat block did not activate a visual response when I powered on and off my Roland FP-90 (bluetooth MIDI). There were no errors in the console when this happened. I will likely have to test with a MIDI cable later and see if that fixes the issue
- The "send" midi output block returned the following error:
Uncaught TypeError: can't access property "device", event is null
My keyboard is capable of MIDI output and the "number of [output] devices" reported that the extension was aware of its presence.
The error occurred on line 2292:
sendOutputEvent({ EVENT, DEVICE }, util) {
const text = Scratch.Cast.toString(EVENT);
const event = stringToMidi(text);
if (DEVICE) {
event.device = this._getDeviceIndex(DEVICE, "output");
}
this.midi.sendOutputEvent(event);
}I don't know what was causing this issue, but it's likely the "stringToMidi" function that's not parsing something correctly (I haven't gone through it to check) I used the default input.
I've got another thing I have to run off to and I think I forgot to say something that I tested but let me know if I can do anything more for you
| @@ -0,0 +1,2557 @@ | |||
| // Name: MIDI | |||
| // ID: extmidi | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally IDs are supposed to contain the author's username to differentiate it from generic titles (such as many extensions being called "midi" or "3d". @CubesterYT is this ID unique enough to pass or should it include something more?
| // ID: extmidi | ||
| // Description: An extension to use the WebMidi API for midi input/output. | ||
| // By: lselden | ||
| // By: CHCAT1320 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there no scratch accounts that these link to?
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