Add native MIDI support for Mac OSX via Core MIDI API #395
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File change notes: - CMakeLists.txt: - link required Core API libraries on OSX - src/MusicSrc/MusicDevice.c: - include relevant headers for native MIDI support on OSX - add Core MIDI handles to native MIDI device struct on OSX - change Windows NativeMidiSendMessage() to take a pointer for outHandle so that calling code can stay common between that and OSX - add device name lookup utilities from rtmidi library - implement Core MIDI based NativeMidiSendMessage() function - implement Core MIDI device init/destroy and device query logic - update NativeMidiSendMessage() callers to pass outHandle as pointer for Windows and OSX
MaddTheSane
reviewed
Nov 21, 2022
src/MusicSrc/MusicDevice.c
Outdated
| } | ||
|
|
||
| // some MIDI devices have a leading space in endpoint name. trim | ||
| CFStringRef space = CFStringCreateWithCString(NULL, " ", kCFStringEncodingUTF8); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
src/MusicSrc/MusicDevice.c
Outdated
| if (str) | ||
| { | ||
| CFStringAppend(result, str); | ||
| CFRelease(str); |
There was a problem hiding this comment.
I didn't see anything in MIDIObjectGetStringProperty that suggests that the property should be released, so I'm assuming it follows the "Get" rule.
In other words, it shouldn't be released.
| if (CFStringGetLength(result) == 0 ) | ||
| { | ||
| CFRelease(result); | ||
| return str; |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
src/MusicSrc/MusicDevice.c
Outdated
| #elif defined(__APPLE__) | ||
| // create Core MIDI client | ||
| if (ndev->midiClient) WARN("NativeMidiInit(): midiClient != 0"); | ||
| const CFStringRef clientNameRef = CFStringCreateWithCString(NULL, "systemshockClient", kCFStringEncodingASCII); |
There was a problem hiding this comment.
Another use of CFSTR()
const CFStringRef clientNameRef = CFSTR("systemshockClient");
const OSStatus clientResult = MIDIClientCreate(clientNameRef, NULL, NULL, &(ndev->midiClient));
//CFRelease(clientNameRef); // Has no effect on CFStrings created with CFSTR
src/MusicSrc/MusicDevice.c
Outdated
| } | ||
| // create output port | ||
| if (ndev->outHandle.outputPort) WARN("NativeMidiInit(): outputPort != 0"); | ||
| const CFStringRef portNameRef = CFStringCreateWithCString(NULL, "systemshockPort", kCFStringEncodingASCII); |
Fix possible memory management errors. Rename some functions to better match CoreFoundation naming conventions (this is not needed, just makes me feel better). Replace unneeded calls to CFStringCreateWithCString with just the CFSTR macro.
Fix typo.
|
You can see what I meant with my own pull request to your pull request: HunterZ#2 |
Update MusicDevice.c
Thanks. I don't know much about coremidi and was just going off of what examples I could find, so I'm happy to merge in your suggestions. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A couple of years ago I contributed native MIDI support for Windows and Linux, but was not able to do so for Mac OSX because I lacked a workable development environment. This has been remedied, and I was able to implement and test Core MIDI support in an OSX Mojave VM talking to a real Roland SC-88 via a USB-MIDI interface.
As with Windows, support for this is automatically enabled in OSX builds because it's an OS-provided functionality.
File change notes: