microsoft / MIDI

Windows MIDI Services
MIT License
301 stars 24 forks source link

[BUG] MIDI 2.0 driver (and/or MIDI 2.0 service) accepts data from invalid cable/groups #376

Closed MusicMaker closed 1 month ago

MusicMaker commented 1 month ago

Describe the bug From a legcay USB MIDI 1.0 device sending data to the host by setting it's cable number to a group number not matching its number of ports, result in a valid UMP message in the service.

To Reproduce

  1. Example: Create a device with a MIDI USB descriptor with 2 in and 2 out ports. the cable number is 0 or 1 in the USB message 2, set the cable number to 2 in the USB messages
  2. Assign the MIDI 2.0 driver
  3. run midi.exe ep monitor
  4. Watch the UMP message come in, the group is set to 2.

Expected behavior Driver/service should not accept the message as it's invalid for the device

Screenshots Device legacy ports are mapped to group 0 and 1 image

Group is set to 2 image

Installer Name or Version Developer preview 6

Desktop (please complete the following information):

Device information, if this is with an external MIDI device:

Driver MIDI 2.0 driver from DP2

Application Information N/A

Additional context N/A

Psychlist1972 commented 1 month ago

If you try another higher number, does the same thing happen?

I'm guessing this is a mismatch between KS Pin #s and Cable #s, but would need more data to confirm.

AmeNote-Michael commented 1 month ago

@Psychlist1972 This was design decision - the USB Driver is just passing data along, it is not processing and filtering out data based on group IDs.

Driver provides all information needed by upper layers to manage the groups, etc.

If this is to change, we need to indicate as a feature. This design decision was made some time ago.

Psychlist1972 commented 1 month ago

@Psychlist1972 This was design decision - the USB Driver is just passing data along, it is not processing and filtering out data based on group IDs.

Driver provides all information needed by upper layers to manage the groups, etc.

If this is to change, we need to indicate as a feature. This design decision was made some time ago.

These are MIDI 1.0 devices, though. You generate the group information in the driver during enumeration.

I take it the cable # is just a field in the packet you receive from the device? If so, how are you mapping that to a group index?

I'm not ready for this to be a feature request given that I'd call the device implementation broken if it uses cable numbers that aren't declared up-front. I do need to understand how this is possible, though.

AmeNote-Michael commented 1 month ago

@Psychlist1972 the virtual cable number is used as the group number in both directions.

AmeNote-MikeKent commented 1 month ago

I believe passing such data is the best implementation. A USB MIDI 1.0 device should not send messages with a cable # which is not associated with a declared Embedded MIDI Out. In a MIDI 1.0 environment, the data goes to nowhere. So if this error occurs, we need to decide how to handle it in a MIDI 2.0 environment. Specifications do not address how to handle this specific error. However, we do have the general principle that transports should generally try to be transparent. The OS API Working Group of the MIDI Association recently discussed a different, specific case where filtering data can have negative implications. I don't think the USB transport (which includes the driver) should try to fix such errors.

Psychlist1972 commented 1 month ago

Good to have this reported here in case it comes up in the future. I agree this is a device issue, not a driver/service issue. Closing. Thanks @MusicMaker for reporting

MusicMaker commented 1 month ago

Thank you for the clarification on the behavior. Wonder though how higher level apps are going to handle this and the other way around (from host to device). Could this lead to more cases of some devices working on one OS and not on another. (like "class compliant" ... but only Windows/OSX class compliant drivers. just joking...