search

adafruit / Adafruit_TinyUSB_Arduino

Arduino library for TinyUSB
MIT License
469 stars 122 forks source link

Wrong wTotalLength for MIDI Interface Descriptor #223

Closed 0ba-pia closed 1 year ago

0ba-pia commented 1 year ago

Operating System

Others

IDE version

Arduino 1.8.13

Board

RP2040

ArduinoCore version

1.8.3

TinyUSB Library version

1.6.0

Sketch (attached txt file)

/examples/MIDI/midi_test/midi_test.ino

What happened ?

Hi!

Probably a TinyUSB issue, not sure how to fix that but I had a report from a MIDI Host device manufacturer who told me that my Client MIDI device is not recognised because in the USB descriptor of the device, the wTotalLength is wrong:

Jacks: 07 24 01 00 01 25 00 06 24 02 01 01 00 06 24 02 02 02 00 09 24 03 01 03 01 02 01 00 09 24 03 02 04 01 01 01 00

Endpoints 09 05 01 02 40 00 00 00 00 05 25 01 01 01 09 05 81 02 40 00 00 00 00 05 25 01 01 03

The given 0x0025 size is only the MS streaming descriptor and the jacks, but not the endpoints. The correct value for wTotalLength must be 0x0025 +9+5+9+5 = 0x0041. See paragraph B.4.2 in USB MIDI spec.

midi-specs

How to reproduce ?

Use the midi example code to define a USB MIDI device.

Debug Log

No response

Screenshots

No response

0ba-pia commented 1 year ago

Don't know if that can cause side effects but this issue is solved by changing the MS Header size from 7 to 35 in TinyUSB's usbd.h

define TUD_MIDI_DESC_HEAD(_itfnum, _stridx, _numcables) \

/ Audio Control (AC) Interface /\ 9, TUSB_DESC_INTERFACE, _itfnum, 0, 0, TUSB_CLASS_AUDIO, AUDIO_SUBCLASS_CONTROL, AUDIO_FUNC_PROTOCOL_CODE_UNDEF, _stridx,\ / AC Header /\ 9, TUSB_DESC_CS_INTERFACE, AUDIO_CS_AC_INTERFACE_HEADER, U16_TO_U8S_LE(0x0100), U16_TO_U8S_LE(0x0009), 1, (uint8_t)((_itfnum) + 1),\ / MIDI Streaming (MS) Interface /\ 9, TUSB_DESC_INTERFACE, (uint8_t)((_itfnum) + 1), 0, 2, TUSB_CLASS_AUDIO, AUDIO_SUBCLASS_MIDI_STREAMING, AUDIO_FUNC_PROTOCOL_CODE_UNDEF, 0,\ / MS Header /\ 7, TUSB_DESC_CS_INTERFACE, MIDI_CS_INTERFACE_HEADER, U16_TO_U8S_LE(0x0100), U16_TO_U8S_LE(35 + (_numcables) * TUD_MIDI_DESC_JACK_LEN)

hathach commented 1 year ago

ah, you are right on this total length, it should combined every length starting and including the MIDI header, though the value should be 

  /* MS Header */\
  7, TUSB_DESC_CS_INTERFACE, MIDI_CS_INTERFACE_HEADER, U16_TO_U8S_LE(0x0100), U16_TO_U8S_LE(7 + (_numcables) * (TUD_MIDI_DESC_JACK_LEN) + TUD_MIDI_DESC_EP_LEN(_numcables) * 2)

i.e the wTotalLength in MS header should be computed as follows:

7 + (_numcables) * (TUD_MIDI_DESC_JACK_LEN) + TUD_MIDI_DESC_EP_LEN(_numcables) * 2

I may miss something, @kaysievers could you mind confirming the computation.

kaysievers commented 1 year ago 
7 + (_numcables) * (TUD_MIDI_DESC_JACK_LEN) + TUD_MIDI_DESC_EP_LEN(_numcables) * 2

@hathach, yes, this looks good to me

0ba-pia commented 1 year ago

7 + (_numcables) (TUD_MIDI_DESC_JACK_LEN) + TUD_MIDI_DESC_EP_LEN(_numcables) 2

Correct. Tested that and it works fine.

hathach commented 1 year ago

thank you both @0ba-pia and @kaysievers for filing and reviewing and testing the issue. I have update tinyusb core with this fix. This Arduino TinyUSB will get updated/synced later on soon.

hathach commented 1 year ago

fixed by #224