[CoreMidi] Add missing CoreMIDI bindings and MidiEventList support. - #24738
[CoreMidi] Add missing CoreMIDI bindings and MidiEventList support.#24738rolfbjarne wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive CoreMIDI bindings for missing APIs and implements MIDI 2.0 Universal MIDI Packet (UMP) support, addressing issues #4452 and #12489. The changes enable developers to work with MIDI devices at a lower level, including custom driver implementation, and provide full support for the modern MIDI 2.0 protocol.
Changes:
- Added MidiEventList and MidiEventPacket classes for MIDI 2.0 UMP support, enabling Universal MIDI Packet handling
- Implemented missing CoreMIDI device/entity management APIs (Create, Dispose, AddOrRemoveEndpoints, etc.) and sysex sending functionality
- Introduced experimental MidiDriver abstract class for implementing custom MIDI drivers with full lifecycle management
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/xtro-sharpie/api-annotations-dotnet/*.ignore | Removed API ignore entries for newly bound CoreMIDI APIs across all platforms |
| tests/cecil-tests/Documentation.cs | Added FunctionPointerType handling to return empty doc IDs for unmanaged function pointers |
| tests/cecil-tests/Documentation.KnownFailures.txt | Removed known failures for newly documented CoreMIDI types and fixed function pointer doc IDs |
| tests/monotouch-test/CoreMidi/MidiEventPacketTest.cs | Comprehensive unit tests for MidiEventPacket struct including bounds checking and data manipulation |
| tests/monotouch-test/CoreMidi/MidiEventListTest.cs | Unit tests for MidiEventList covering construction, adding packets, iteration, and edge cases |
| tests/monotouch-test/CoreMidi/MidiEndpointTest.cs | Added tests for GetRefCons/SetRefCons endpoint reference management APIs |
| tests/monotouch-test/CoreMidi/MidiDeviceTest.cs | Tests for external device creation and MidiSetup add/remove operations |
| tests/monotouch-test/CoreMidi/MidiComprehensiveTest.cs | Extensive integration tests including a complete "Happy Birthday" melody demonstration |
| src/frameworks.sources | Registered new CoreMidi and CoreFoundation source files in the build system |
| src/coremidi.cs | Added MidiDriverProperty enum for driver-specific properties |
| src/CoreMidi/MidiThruConnectionParams.cs | Moved enum definitions outside TVOS conditional to fix tvOS build issues |
| src/CoreMidi/MidiStructs.cs | Implemented MIDI 2.0 structs (Midi2DeviceManufacturer, Midi2DeviceRevisionLevel, MidiCIProfileId variants, MidiSysexSendRequest) |
| src/CoreMidi/MidiServices.cs | Added device/entity creation, external device support, sysex async sending, and protocol-aware port/endpoint creation methods |
| src/CoreMidi/MidiEventPacket.cs | Implemented MidiEventPacket struct with 64-word capacity, indexer, and GetSysexBytes method |
| src/CoreMidi/MidiEventList.cs | Implemented MidiEventList as IDisposable wrapper with Add, Send, Receive, and iteration support |
| src/CoreMidi/MidiDriverInterface.cs | Created experimental MidiDriver abstract class with COM-style interface for custom driver implementation |
| src/CoreMidi/MidiBluetoothDriver.cs | Removed TVOS conditional and added XML documentation for Bluetooth driver methods |
| src/CoreFoundation/CFUuidBytes.cs | Added CFUuidBytes struct for CoreMIDI driver interface QueryInterface method |
| docs/preview-apis.md | Documented MidiDriver as experimental (APL0004) until .NET 12 |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Bind the following missing CoreMIDI APIs: - MIDIDeviceCreate/MIDIDeviceDispose - MIDIExternalDeviceCreate - MIDISetupAddDevice/MIDISetupRemoveDevice - MIDISetupAddExternalDevice/MIDISetupRemoveExternalDevice - MIDIEntityAddOrRemoveEndpoints - MIDIDeviceRemoveEntity - MIDIEndPointGetRefCons/MIDIEndPointSetRefCons - MIDIDriverEnableMonitoring - MIDIGetDriverDeviceList/MIDIGetDriverIORunLoop - MIDISendSysex/MIDISendUMPSysex - MIDIDestinationCreateWithProtocol - MIDISourceCreateWithProtocol - MIDIInputPortCreateWithProtocol - MIDIClientCreateWithBlock - MIDIEventPacketSysexBytesForGroup - MIDI 2.0 structs (MIDI2DeviceManufacturer, MIDI2DeviceRevisionLevel, MIDICIProfileID, MIDISysexSendRequest, MIDISysexSendRequestUMP) - MidiDriver abstract class for implementing custom MIDI drivers Add MidiEventList and MidiEventPacket classes for MIDI 2.0 Universal MIDI Packet (UMP) support (MIDIEventList/MIDIEventPacket structs). Add comprehensive tests including a Happy Birthday melody test. There's a sample project in progress here: dotnet/macios-samples#10. Fixes #4452 Fixes #12489
…sage. Add a binding for the native MIDIEventListForEachEvent function, which parses each Universal MIDI Packet (UMP) in a MidiEventList and invokes a callback with the parsed message. This adds: * A faithful managed MidiUniversalMessage struct (and its variant structs) mirroring the native MIDIUniversalMessage union. * A MidiEventList.ForEachEvent method + MidiUniversalMessageVisitor delegate. * Thorough tests, including parsing a MIDI 1.0 note on, a MIDI 2.0 note on, and the Happy Birthday melody. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Explicit struct layout (with overlapping union fields) makes a struct non-blittable, which forces the runtime to generate marshaling code and prevents passing the struct by value through a 'delegate* unmanaged' function pointer without overhead. Rewrite MidiUniversalMessage and its variant structs to use sequential blittable layout with opaque storage fields, exposing the native unions through unsafe accessor properties that reinterpret the storage. Also replace fixed buffers / array fields with named byte fields. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
9db33b7 to
01b1410
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
rolfbjarne
left a comment
There was a problem hiding this comment.
🤖 Code Review — CoreMIDI bindings
Verdict: MidiUniversalMessage/MidiEventPacket avoid explicit layout/arrays to stay blittable for delegate* unmanaged), and the sysex async lifetime management (disposing the CancellationTokenRegistration before freeing native memory) are all sound. A few correctness/polish items inline.
Positive callouts
- No API-breaking changes; removals handled via
[ObsoletedOSPlatform]with actionable "Call 'X' instead." messages. MidiDriverCOM ref-counting (AddRef/Releaseunder lock, weakGCHandle, finalizer releasing the managed ref) is coherent; experimental surface gated behindAPL0004and documented indocs/preview-apis.md..ignoreentries for the now-boundMIDIEventListForEachEvent/Init/Addand the tvOS enums are correctly removed.- Thorough XML docs replacing
"To be added."stubs; Happy Birthday test present. 🎂
Severity counts: 0 ❌ · 1
Reviewed against origin/main (HEAD 01b1410). CI status not evaluated in this review.
* Fix ArgumentOutOfRangeException constructor misuse (string was passed as paramName instead of message) in MidiEventPacket and MidiEventList. * Add MIT license header and #nullable enable to CFUuidBytes.cs. * Fix CreateInputPort <returns> doc and local alias (MidiPort, not MidiEndpoint). * Remove redundant native @struct comment block in MidiStructs.cs. * Use Assert.That/Is.EqualTo with correct argument order in MidiEndpointTest. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…ilot # Conflicts: # src/CoreMidi/MidiServices.cs # tests/cecil-tests/Documentation.KnownFailures.txt
* Fix XML doc typos in MidiEventList.cs ("lis" -> "list", "packates" -> "packets").
* Make the static strongReferences field in MidiDriverInterface.cs readonly.
* Dispose MidiEventList instances in MidiEventListTest.cs using `using` statements.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Several correctness and API-quality issues were identified (null-handling, unsafe interop, malformed doc IDs, and PR-scope mismatch) that should be resolved before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 24/24 changed files
- Comments generated: 10
- Review effort level: Lite
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
🔥 [CI Build #18b055d] Test results 🔥Test results❌ Tests failed on VSTS: test results 0 tests crashed, 2 tests failed, 262 tests passed. Failures❌ cecil tests1 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ dotnettests tests (macOS)1 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download Successes✅ assembly-processing: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Sonoma (14): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
|
Bind the following missing CoreMIDI APIs:
MIDICIProfileID, MIDISysexSendRequest, MIDISysexSendRequestUMP)
Add MidiEventList and MidiEventPacket classes for MIDI 2.0 Universal MIDI
Packet (UMP) support (MIDIEventList/MIDIEventPacket structs).
Add comprehensive tests including a Happy Birthday melody test.
There's a sample project in progress here: dotnet/macios-samples#10.
Fixes #4452
Fixes #12489