-
Notifications
You must be signed in to change notification settings - Fork 112
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
Add dedicated message type for MIDI Show Control messages #67
base: main
Are you sure you want to change the base?
Conversation
Thank you! I’m glad you could add this.
I’m busy for the next few days but will take a look when I can.
… On Dec 23, 2018, at 4:59 AM, Hugo Trippaers ***@***.***> wrote:
Heya,
I've been using show control for some shows recently and found that MidiMonitor doesn't have a dedicated parser for it. As i'm using it fairly frequently to test and diagnose it proved handy for me to add it. If you are interested in having this in the main distribution i'll happily work with you to get this code into shape for inclusion.
Based on MIDI Show Control 1.1 specification
Tests included in a new scheme SnoizeMIDITests
You can view, comment on, or merge this pull request online at:
#67
Commit Summary
Introduce the SMShowControlMessage
Move parsers to a utility file and complete the parameter parsing for most common shocwontrol messages
Refactor cue item parser to not depend on the 0xF7 marker
File Changes
M Frameworks/SnoizeMIDI/SMMessageParser.m (7)
A Frameworks/SnoizeMIDI/SMShowControlMessage.h (19)
A Frameworks/SnoizeMIDI/SMShowControlMessage.m (308)
A Frameworks/SnoizeMIDI/SMShowControlUtilities.h (53)
A Frameworks/SnoizeMIDI/SMShowControlUtilities.m (67)
A Frameworks/SnoizeMIDI/ShowControlCommandNames.plist (74)
M Frameworks/SnoizeMIDI/SnoizeMIDI.xcodeproj/project.pbxproj (215)
A Frameworks/SnoizeMIDI/SnoizeMIDITests/Info.plist (22)
A Frameworks/SnoizeMIDI/SnoizeMIDITests/SMShowControlMessageTest.m (160)
A Frameworks/SnoizeMIDI/SnoizeMIDITests/SMShowControlUtilitiesTest.m (106)
Patch Links:
https://github.com/krevis/MIDIApps/pull/67.patch
https://github.com/krevis/MIDIApps/pull/67.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Sorry for the delay... holidays, and then I got the usual post-holiday cold. This generally looks really good. I'd like to tweak a few things:
But otherwise this fits in nicely. Thanks for doing all the hard work, especially in such an antique codebase. I'll take a closer pass over this when I can, and tweak the PR. |
Hey, happy new year :) Thanks for the feedback. I'll see if i can fix the issue you mentioned on the assumption of data length. I can make some more asserts to check packet length before and while parsing. |
Good point about reading old files. I started off doing what you mentioned but later made it into a separate message. If they can be stored as SysEx messages but parsed on the fly as show control it would be compatible right? |
Yes, that should work. |
Finally got a little time to work on this. Pushed a change that does the following:
Done.
I fixed this in a couple of places, but need to do more. I haven't really tested it, though -- I don't typically use show control and I don't even know where to start. If you have time, could you send me a MIDI Monitor document with some show control messages in it? What software do you use to generate show control messages? |
QLab is a good example for an application that uses show control (https://figure53.com/qlab/) - to use for testing it’s completely free and very straight forward to set up.
…Sent from my iPhone
On 21 Jan 2019, at 10:15, Kurt Revis ***@***.***> wrote:
Finally got a little time to work on this. Pushed a change that does the following:
Perhaps we can just change SMSystemExclusiveMessage to detect when it's a show-control message, and change how it formats itself. May need to cache some things internally.
Done.
Shouldn't assume anything about the length of the sysex data.There are a few places where you do things like (((Byte *)newData.bytes)[4]), which will crash if newData is too small.
I fixed this in a couple of places, but need to do more.
I haven't really tested it, though -- I don't typically use show control and I don't even know where to start. If you have time, could you send me a MIDI Monitor document with some show control messages in it? What software do you use to generate show control messages?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Works like a charm :-) I've attached a mmon file with a few show control commands in it. And a demo movie of how i generated the commands from QLab. |
… most common shocwontrol messages
- Don't use a separate show control message type, just put it all in sysex message - Cache dataForDisplay in sysex messages - rename some stuff - fix leak in parseCueItemsData TODO: - In tests target, use config files instead of ad-hoc settings - test failing in parseCueItemsData, clean that up probably - check this with MIDI Show Control spec, I suspect there is a lot more to show - new TODOs in sysex message, e.g. checking for message lengths
Rebased on latest master / no rebase issues |
Heya,
I've been using show control for some shows recently and found that MidiMonitor doesn't have a dedicated parser for it. As i'm using it fairly frequently to test and diagnose it proved handy for me to add it. If you are interested in having this in the main distribution i'll happily work with you to get this code into shape for inclusion.
Based on MIDI Show Control 1.1 specification
Tests included in a new scheme SnoizeMIDITests