Skip to content
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

[BUG] Messages following undefined System Common/Realtime messages are corrupt. #361

Closed
m-komo opened this issue Jul 17, 2024 · 8 comments
Closed
Assignees
Labels
area-service-or-api 🖥️ Related to the Windows Service, core API, abstractions, etc. bug 🐞 Something isn't working dp7-fixed 🥳 Fixed in developer preview 7

Comments

@m-komo
Copy link
Collaborator

m-komo commented Jul 17, 2024

Describe the bug
Sending undefined System Common/Realtime messages (0xF4, 0xF5, 0xF9, 0xFD) to the USB MIDI device which is working with the USB MIDI 1.0 driver (USBAUDIO.sys) corrupts following messages.

To Reproduce

  1. Set the Roland UM-ONE mk2 to class-compliant mode and short INPUPT and OUTPUT to loop back messages.
  2. Attach the UM-ONE to PC.
  3. Make sure the device is working with the USB MIDI 1.0 driver (USBAUDIO.sys).
  4. Open midi.exe in two windows.
  5. From one, monitor UM-ONE.
  6. From the other, send the attached "TestMT1.txt" to UM-ONE.
    Received messages on the monitor are corrupt (received_messages.txt).

Expected behavior
All MT2 messages in the file are received as they are.

Screenshots
I have attached the captured USB log file (USBLog.zip).
It shows that USB packets sent to the device are already corrupt.

image

Installer Name or Version

  • Windows.MIDI.Services.In-Box.Service.-.Developer.Preview.6.1.0.24194.2233-x64.exe

Desktop (please complete the following information):

  • OS: Windows 11 24H2
  • OS Build: 26100.1150

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

  • Roland UM-ONE mk2 (Any USB MIDI device compatible with the USB MIDI 1.0 driver.)
  • Driver: OS in-box USB MIDI 1.0 driver (USBAUDIO.sys)

Application Information

  • 'midi.exe' console app.
@Psychlist1972
Copy link
Contributor

Those undefined system common messages do not have a defined number of data bytes in the spec. The behavior ends up being undefined.

That said, I've reported this to Andrew because the translation library would end up in a state where it may never recover if something like the UM-One returns back only the status byte and no data bytes for those undefined messages.

@Psychlist1972 Psychlist1972 added the area-service-or-api 🖥️ Related to the Windows Service, core API, abstractions, etc. label Jul 18, 2024
@Psychlist1972
Copy link
Contributor

Note that whatever change is made here to accommodate this must also be made in the USB MIDI 2 Driver @AmeNote-Michael

@Psychlist1972 Psychlist1972 added the area-usb-driver 💻 Related to the USB MIDI 2.0 driver label Jul 18, 2024
@AmeNote-Michael
Copy link
Collaborator

For the USB MIDI 2 Driver (usbmidi2.sys) it is actually not an issue cause in USB MIDI 1.0 it is a single 32bit word always regardless if message is defined or not. When an undefined message, then I just ignore rest of bytes in the 32 bit data.

For the UMP to BS or BS to UMP, should just pass the undefined byte I think when going from UMP to BS or in case of BS to UMP, should be a UMP message with just the one real time message byte, rest NULL.

@Psychlist1972
Copy link
Contributor

Also discussing here:
midi2-dev/AM_MIDI2.0Lib#16

We may need to decline this one, because these are undefined MIDI 1.0 messages. It pains me to see it mess up the entire stream of data, but no one should be using undefined MIDI 1.0 messages.

We're much more forgiving when it comes to MIDI messages that stay in UMP. But when translating to bytes, we're at the mercy of what devices do with that data.

My recommendation to Andrew for libmidi2 is to just drop these undefined messages when translating to MIDI 1.0 byte data format. But that may not be simple to do.

@Psychlist1972 Psychlist1972 removed the area-usb-driver 💻 Related to the USB MIDI 2.0 driver label Jul 31, 2024
@Psychlist1972
Copy link
Contributor

Update: Andrew is looking at options in libmidi2. Rejecting the status byte as well as any data bytes which come after the undefined message is looking to be a reasonable option. This is really about hardening the translator against bad data, because no one should be using these opcodes.

@Psychlist1972
Copy link
Contributor

Decision: Michael and Andrew will change their code to drop these messages in both directions.

@Psychlist1972
Copy link
Contributor

#397 related

@Psychlist1972
Copy link
Contributor

Closed with release of DP7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-service-or-api 🖥️ Related to the Windows Service, core API, abstractions, etc. bug 🐞 Something isn't working dp7-fixed 🥳 Fixed in developer preview 7
Projects
Status: No status
Development

No branches or pull requests

3 participants