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

MIDI Extension #1725

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

MIDI Extension #1725

wants to merge 13 commits into from

Conversation

Brackets-Coder
Copy link

@Brackets-Coder Brackets-Coder commented Oct 15, 2024

This extension manages MIDI input. While support for MIDI output is possible in the future, I'm not planning on it at the moment. Currently, only MIDI note input is supported. Other MIDI input events return a console log. This means things like pitch bend, damper pedals, etc. will not work. These may also be supported in the future, but not as of now.

Note that MIDI compatibility varies depending on which browser you're using. If the browser doesn't have MIDI API support, it will return an error, show an alert, and the extension won't load. The main cause of this is Safari and significantly outdated versions of browsers.

I've made a sample project, but I won't add it until this gets merged because it uses localhost instead of extensions.turbowarp.org.

353662493-3f9327ac-dd48-4dad-8b59-e35de05e76e6

@GarboMuffin GarboMuffin added the pr: new extension Pull requests that add a new extension label Oct 15, 2024
@CubesterYT CubesterYT self-assigned this Oct 15, 2024
@CubesterYT
Copy link
Member

I'll need to remember to handle this PR

Brackets-Coder and others added 3 commits October 16, 2024 15:27
I've ran prettier like a few dozen times with the same version and settings as the repo. If this fails I have no idea why
@Brackets-Coder
Copy link
Author

Brackets-Coder commented Oct 17, 2024

Something to note: I haven't tested the latest commit, but it basically just condenses some blocks into single blocks with inputs, and it passed all checks, so I'm assuming it works. Code looks perfect, but I can't test it right now.

@Brackets-Coder
Copy link
Author

@CubesterYT One thing to note: If you're going to test this extension's functionality while you're reviewing it, you need a MIDI compatible digital keyboard and a MIDI API compatible browser (basically everything but Safari works). However, you probably don't need to do this as I've tested it and it works consistently.

Take your time. I'll wait and be patient.

@CubesterYT
Copy link
Member

I'm just going to look through the code and make any fixes

@Brackets-Coder
Copy link
Author

I'm just going to look through the code and make any fixes

No problem. Take your time

@lselden
Copy link

lselden commented Oct 31, 2024

Hi @Brackets-Coder - nice work so far, and I'm excited someone else wants to get midi into Turbowarp! I've forked your pull request and gotten started in making some changes. In particular I'm looking to add in support for all midi events (CCs, pitchbend, etc.), and possibly MIDI Out as well.

Before I make a open a pull request to your fork (a pull request of a pull request 😁 ) I just wanted to make sure: are you open to collaborating and accepting changes? I'm a professional developer, but new to Scratch and how people use it/expect blocks to look and work, so please push back on any of my changes/suggestions, or let me know if you have questions.

Let me know if you're down. You can review my progress so far here: https://github.com/lselden/extensions/blob/midi/extensions/MasterMath/midi.js , or review the full changelog:

@lselden
Copy link

lselden commented Oct 31, 2024

@CubesterYT or other testers/reviewers: If on windows you can use the free LoopMIDI and virtualKeys tools to test even if you don't have a hardware MIDI device. I don't have a mac handy but I believe you can use the built-in MIDI Studio feature

@Brackets-Coder
Copy link
Author

https://github.com/lselden/extensions/blob/midi/extensions/MasterMath/midi.js

Thanks for asking. I appreciate your excitement and willingness. The code looks great, professional, well-formatted, etc. but I was kind of not expecting such a request. My initial intent for this extension was to be a small and minimalistic MIDI input handling interface, with the possibility of me adding more MIDI events and MIDI output support in a later update. As of right now, I've marked this PR as "finished" and am waiting on it to be merged before I worry about further updates. So we'll see. Not right now though, thanks anyway. I hope you understand.

@Brackets-Coder
Copy link
Author

@CubesterYT or other testers/reviewers: If on windows you can use the free LoopMIDI and virtualKeys tools to test even if you don't have a hardware MIDI device. I don't have a mac handy but I believe you can use the built-in MIDI Studio feature

Yes. I'm not opposed to this, but for the most part I've been using some physical MIDI input devices I have for testing as they're what are meant to be used by the extension and I'm not quite sure how it will handle software interference with the API, but any testing is better than nothing.

@CubesterYT
Copy link
Member

I'll look into it

@Brackets-Coder
Copy link
Author

@CubesterYT or other testers/reviewers: If on windows you can use the free LoopMIDI and virtualKeys tools to test even if you don't have a hardware MIDI device. I don't have a mac handy but I believe you can use the built-in MIDI Studio feature

Speaking of Macs - A quick note about that. The Safari desktop browser is the only major browser that doesn't support the Web MIDI API. What's with that? The only reason I need the compatibility detection code is because of Safari and really outdated browsers

@lselden
Copy link

lselden commented Oct 31, 2024

@CubesterYT or other testers/reviewers: If on windows you can use the free LoopMIDI and virtualKeys tools to test even if you don't have a hardware MIDI device. I don't have a mac handy but I believe you can use the built-in MIDI Studio feature

Speaking of Macs - A quick note about that. The Safari desktop browser is the only major browser that doesn't support the Web MIDI API. What's with that? The only reason I need the compatibility detection code is because of Safari and really outdated browsers

Because of security concerns - SYSEX could (in theory) be used maliciously, and for fingerprinting. This PR doesn't attempt to use sysex (you have to pass { sysex: true } when requesting midi access), so it shouldn't trigger any restrictions in other browsers (Firefox, I believe?) that doesn't like sysex.

@lselden
Copy link

lselden commented Oct 31, 2024

https://github.com/lselden/extensions/blob/midi/extensions/MasterMath/midi.js

Thanks for asking. I appreciate your excitement and willingness. The code looks great, professional, well-formatted, etc. but I was kind of not expecting such a request. My initial intent for this extension was to be a small and minimalistic MIDI input handling interface, with the possibility of me adding more MIDI events and MIDI output support in a later update. As of right now, I've marked this PR as "finished" and am waiting on it to be merged before I worry about further updates. So we'll see. Not right now though, thanks anyway. I hope you understand.

Totally understand! No complaints here.

@Brackets-Coder
Copy link
Author

Right now there's a block that returns all active notes.
Should I add some more reporters that help with managing these, such as getting a specific index of that array, or re-sorting the array by pitch or most recent / least recent?

@Thebloxers998
Copy link

What's MIDI?

@Brackets-Coder
Copy link
Author

What's MIDI?

MIDI stands for Musical Input Digital Interface. It allows electric instruments (in this case a digital piano keyboard) to communicate with computers and other devices.

This extension allows you to use note input from a MIDI device to control your projects. For example, pressing a note on a piano keyboard could play the corresponding note with the music extension, or be used for music visualization.

@Thebloxers998
Copy link

Oh ok thanks

@Thebloxers998

This comment has been minimized.

@Thebloxers998
Copy link

And also, you should add a icon for the extension

Thebloxers998

This comment was marked as off-topic.

@Thebloxers998

This comment has been minimized.

@Brackets-Coder
Copy link
Author

And also, you should add a icon for the extension

I'm talking about the mini circle thing and the block

I'll consider it, but right now I'm more more on functionality than aesthetic

@lselden
Copy link

lselden commented Nov 5, 2024

Right now there's a block that returns all active notes. Should I add some more reporters that help with managing these, such as getting a specific index of that array, or re-sorting the array by pitch or most recent / least recent?

What I would recommend:

  • Use a javascript Map instead of Array to store the list of notes - this will make lookups/removal much more straightforward:
const activeNotes = new Map();

// add an active note to list
function setActiveNote(note, velocity) {
    activeNotes.set(note, velocity);
}

// get velocity of specific note (return 0 if not in active list)
function getActiveNoteVelocity(note) {
    return activeNotes.get(note) || 0;
}

// check if a note is active - if it isn't in list it'll be 0
function isNoteActive(note) {
    return activeNotes.has(note);
}

// remove note from active note list
function removeActiveNote(note) {
    activeNotes.delete(note);
}

// get list of active notes
function getActiveNotesList() {
    return Array.from(activeNotes.keys())
}
  • Consider storing more information than just velocity. You could store the event.timeStamp or time you receive the message (Date.now(), which can be matched up to "Days since 2000" with some math). To do so save Objects instead of just velocity:
function setActiveNote(note, velocity) {
    activeNotes.set(note, { velocity: velocity, when: Date.now() });
}

// get velocity of specific note (return 0 if not in active list)
function getActiveNoteVelocity(note) {
    const data = activeNotes.get(note);
    if (data) {
        return data.velocity;
    } else {
        return 0;
    }
}
  • One way to handle the list of active Notes would be to have a Reporter that returns "# of Active Notes", and then a Reporter that returns "Active Note at index [INDEX] [PROPERTY]" (where INDEX is in the range 1-# of active notes and PROPERTY is the "Note" or "Velocity". That way users can use a Repeat [X] block and increment a variable each time to go through the array.

  • Another option is to create a "Set List to active notes" command that changes the specified LIST to your active notes:

//...
    blocks: [
                {
                    opcode: 'setActiveNoteList',
                    blockType: Scratch.BlockType.COMMAND,
                    text: 'Set [LIST] to Active Notes,
                    arguments: {
                        LIST: {
                            type: Scratch.ArgumentType.STRING,
                            defaultValue: 'activeNotes'
                        }
                    }
                },
//...
// actual function
// adapted from https://github.com/TurboWarp/extensions/blob/fedbe62912a10aa1b0a3748a5f621d8da55d22c0/extensions/Lily/ListTools.js#L16
    setActiveNoteList(args, util) {
        const LIST = Scratch.Cast.toString(args.LIST);

        // don't do anything if no list argument set
        if (!LIST) {
            return;
        }
        // get list of active notes as array
        const notes = getActiveNotesList();

        // try to find list variable named LIST for all sprites, then for just this sprite
        let listObj;
        // find stage
        const stageTarget = Scratch.vm.runtime.getTargetForStage();
        if (stageTarget) {
            listObj = stageTarget?.lookupVariableByNameAndType(LIST, 'list');
        }
        // if not for all sprites try in this sprite
        if (!listObj && util.target) {
            listObj = util.target.lookupVariableByNameAndType(LIST, 'list');
        }
        // if found then set list value to note list
        if (listObj) {
            listObj.value = notes;
        }
    }

@Brackets-Coder
Copy link
Author

Looks interesting, but I'm really busy trying to fix one more bug to get #1732 into beta so I can't really focus on this "finished" extension right now.

I like the map idea though. I might take a look a little bit later

@Brackets-Coder
Copy link
Author

@CubesterYT do we have a timeframe on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: new extension Pull requests that add a new extension
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants