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

WL#21651: Extend MIDI javascript support. #12070

Merged
merged 14 commits into from Jan 9, 2018

Conversation

Projects
None yet
10 participants

1st-BrainStormer added some commits Dec 29, 2017

Add files via upload
add more mmidi message types to be exposed to javascript
@hifi-gustavo

This comment has been minimized.

hifi-gustavo commented Dec 29, 2017

Can one of the admins verify this patch?

@sethalves

This comment has been minimized.

Contributor

sethalves commented Dec 29, 2017

add to whitelist

1st-BrainStormer added some commits Dec 29, 2017

@MiladNazeri

This comment has been minimized.

Contributor

MiladNazeri commented Dec 29, 2017

build this please

@hifi-gustavo

This comment has been minimized.

@highfidelity highfidelity deleted a comment from hifi-gustavo Dec 29, 2017

@highfidelity highfidelity deleted a comment from hifi-gustavo Dec 29, 2017

@highfidelity highfidelity deleted a comment from hifi-gustavo Dec 29, 2017

@hifi-gustavo

This comment has been minimized.

@1st-BrainStormer

This comment has been minimized.

Contributor

1st-BrainStormer commented Dec 29, 2017

Tested. Everything working as expected.

@thoys

This comment has been minimized.

Contributor

thoys commented Dec 29, 2017

build this please

@hifi-gustavo

This comment has been minimized.

@hifi-gustavo

This comment has been minimized.

@1st-BrainStormer 1st-BrainStormer changed the title from Extend MIDI javascript support. to WL#21651: Extend MIDI javascript support. Dec 30, 2017

@MiladNazeri

This comment has been minimized.

Contributor

MiladNazeri commented Jan 2, 2018

build this please

@hifi-gustavo

This comment has been minimized.

@hifi-gustavo

This comment has been minimized.

Revert "Update (#1)"
This reverts commit 7b1ac24.

@highfidelity highfidelity deleted a comment from hifi-gustavo Jan 5, 2018

@highfidelity highfidelity deleted a comment from hifi-gustavo Jan 5, 2018

@highfidelity highfidelity deleted a comment from hifi-gustavo Jan 5, 2018

@highfidelity highfidelity deleted a comment from hifi-gustavo Jan 5, 2018

@highfidelity highfidelity deleted a comment from hifi-gustavo Jan 5, 2018

@hifi-gustavo

This comment has been minimized.

@misslivirose

Please ensure all of the new methods that can be invoked from JS also have JSDocs formatted comments on them.

Some style fixes. I'd like to have someone more familiar with the audio code that's being changed also take a look at this PR.

midihin[i] = NULL;
instance->allNotesOff();
}
case MIM_OPEN:

This comment has been minimized.

@misslivirose

misslivirose Jan 5, 2018

Contributor

Switch statement should follow indentation standards

note = 0;
}
if (typePitchBendEnabled && type == MIDI_PITCH_BEND_CHANGE) {
bend = ((MIDI_BYTE_MASK & (dwParam1 >> MIDI_SHIFT_NOTE)) | (MIDI_PITCH_BEND_MASK & (dwParam1 >> MIDI_SHIFT_PITCH_BEND))) - 8192;

This comment has been minimized.

@misslivirose

misslivirose Jan 5, 2018

Contributor

Line length

@@ -21,48 +22,75 @@
class Midi : public QObject, public Dependency {
Q_OBJECT
SINGLETON_DEPENDENCY
SINGLETON_DEPENDENCY

This comment has been minimized.

@misslivirose

misslivirose Jan 5, 2018

Contributor

Spacing?

public:
void noteReceived(int status, int note, int velocity); // relay a note to Javascript
void sendNote(int status, int note, int vel); // relay a note to MIDI outputs
void rawMidiReceived(int device, int raw); //relay raw midi data to Javascript

This comment has been minimized.

@misslivirose

misslivirose Jan 5, 2018

Contributor

I don't see the noteReceived method anymore - if this is deprecated, it should be noted first and removed in a later release

This comment has been minimized.

@1st-BrainStormer

1st-BrainStormer Jan 6, 2018

Contributor

noteReceived became midiReceieved but it is backwards compatible and on line 283 in Midi.cpp
emit midiNote(eventData);// Legacy (To maintain backwards compatibility)

@themelissabrown themelissabrown added this to the RC63 milestone Jan 6, 2018

@hifi-gustavo

This comment has been minimized.

if (broadcastEnabled) {
for (int i = 0; i < midihout.size(); i++) {
if (midihout[i] != NULL) {
midiOutShortMsg(midihout[i], message + (note << MIDI_SHIFT_NOTE) + (velocity << MIDI_SHIFT_VELOCITY));

This comment has been minimized.

@kencooke

kencooke Jan 8, 2018

Contributor

for inserting bits, its more readable and consistent with message above to use | instead of +

for (int i = 0; i < midihout.size(); i++) {
if (midihout[i] != NULL) {
midiOutShortMsg(midihout[i], status + (note << MIDI_SHIFT_NOTE) + (vel << MIDI_SHIFT_VELOCITY));
midiOutShortMsg(midihout[i], status + (note << MIDI_SHIFT_NOTE) + (velocity << MIDI_SHIFT_VELOCITY));

This comment has been minimized.

@kencooke

kencooke Jan 8, 2018

Contributor

same as above

This comment has been minimized.

@1st-BrainStormer

1st-BrainStormer Jan 9, 2018

Contributor

Done., Thanks!

@hifi-gustavo

This comment has been minimized.

@themelissabrown themelissabrown merged commit 7b50480 into highfidelity:master Jan 9, 2018

2 checks passed

default Build finished.
Details
license/cla Contributor License Agreement is signed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment