From 080b14b292251e46b43aac9e1cf7807dc2d3eb2c Mon Sep 17 00:00:00 2001
From: hathach <thach@tinyusb.org>
Date: Fri, 2 Apr 2021 13:26:55 +0700
Subject: [PATCH] fix midi tx fifo overflow cause data corruption
rename
---
.../device/dynamic_configuration/src/main.c | 2 +-
examples/device/midi_test/src/main.c | 8 +-
examples/make.mk | 2 +-
src/class/midi/midi.h | 45 ++++++++
src/class/midi/midi_device.c | 103 +++++++++++-------
src/class/midi/midi_device.h | 16 ++-
6 files changed, 127 insertions(+), 49 deletions(-)
diff --git a/examples/device/dynamic_configuration/src/main.c b/examples/device/dynamic_configuration/src/main.c
index acee295f6..ce1f752d0 100644
--- a/examples/device/dynamic_configuration/src/main.c
+++ b/examples/device/dynamic_configuration/src/main.c
@@ -172,7 +172,7 @@ void midi_task(void)
// regardless of these being used or not. Therefore incoming traffic should be read
// (possibly just discarded) to avoid the sender blocking in IO
uint8_t packet[4];
- while(tud_midi_available()) tud_midi_receive(packet);
+ while( tud_midi_available() ) tud_midi_packet_read(packet);
// send note every 1000 ms
if (board_millis() - start_ms < 286) return; // not enough time
diff --git a/examples/device/midi_test/src/main.c b/examples/device/midi_test/src/main.c
index 858a096b6..af10350df 100644
--- a/examples/device/midi_test/src/main.c
+++ b/examples/device/midi_test/src/main.c
@@ -132,7 +132,7 @@ void midi_task(void)
// regardless of these being used or not. Therefore incoming traffic should be read
// (possibly just discarded) to avoid the sender blocking in IO
uint8_t packet[4];
- while(tud_midi_available()) tud_midi_receive(packet);
+ while ( tud_midi_available() ) tud_midi_packet_read(packet);
// send note every 1000 ms
if (board_millis() - start_ms < 286) return; // not enough time
@@ -146,10 +146,12 @@ void midi_task(void)
if (previous < 0) previous = sizeof(note_sequence) - 1;
// Send Note On for current position at full velocity (127) on channel 1.
- tud_midi_write24(cable_num, 0x90 | channel, note_sequence[note_pos], 127);
+ uint8_t note_on[3] = { 0x90 | channel, note_sequence[note_pos], 127 };
+ tud_midi_write(cable_num, note_on, 3);
// Send Note Off for previous note.
- tud_midi_write24(cable_num, 0x80 | channel, note_sequence[previous], 0);
+ uint8_t note_off[3] = { 0x80 | channel, note_sequence[previous], 0};
+ tud_midi_write(cable_num, note_off, 3);
// Increment position
note_pos++;
diff --git a/examples/make.mk b/examples/make.mk
index e04a2592a..ffcb8ee5c 100644
--- a/examples/make.mk
+++ b/examples/make.mk
@@ -103,7 +103,7 @@ CFLAGS += \
# Debugging/Optimization
ifeq ($(DEBUG), 1)
- CFLAGS += -Og
+ CFLAGS += -O0
else
CFLAGS += -Os
endif
diff --git a/src/class/midi/midi.h b/src/class/midi/midi.h
index f758b6e1c..74dc41749 100644
--- a/src/class/midi/midi.h
+++ b/src/class/midi/midi.h
@@ -61,6 +61,51 @@ typedef enum
MIDI_JACK_EXTERNAL = 0x02
} midi_jack_type_t;
+typedef enum
+{
+ MIDI_CIN_MISC = 0,
+ MIDI_CIN_CABLE_EVENT = 1,
+ MIDI_CIN_SYSCOM_2BYTE = 2, // 2 byte system common message e.g MTC, SongSelect
+ MIDI_CIN_SYSCOM_3BYTE = 3, // 3 byte system common message e.g SPP
+ MIDI_CIN_SYSEX_START = 4, // SysEx starts or continue
+ MIDI_CIN_SYSEX_END_1BYTE = 5, // SysEx ends with 1 data, or 1 byte system common message
+ MIDI_CIN_SYSEX_END_2BYTE = 6, // SysEx ends with 2 data
+ MIDI_CIN_SYSEX_END_3BYTE = 7, // SysEx ends with 3 data
+ MIDI_CIN_NOTE_ON = 8,
+ MIDI_CIN_NOTE_OFF = 9,
+ MIDI_CIN_POLY_KEYPRESS = 10,
+ MIDI_CIN_CONTROL_CHANGE = 11,
+ MIDI_CIN_PROGRAM_CHANGE = 12,
+ MIDI_CIN_CHANNEL_PRESSURE = 13,
+ MIDI_CIN_PITCH_BEND_CHANGE = 14,
+ MIDI_CIN_1BYTE_DATA = 15
+} midi_code_index_number_t;
+
+// MIDI 1.0 status byte
+enum
+{
+ //------------- System Exclusive -------------//
+ MIDI_STATUS_SYSEX_START = 0xF0,
+ MIDI_STATUS_SYSEX_END = 0xF7,
+
+ //------------- System Common -------------//
+ MIDI_STATUS_SYSCOM_TIME_CODE_QUARTER_FRAME = 0xF1,
+ MIDI_STATUS_SYSCOM_SONG_POSITION_POINTER = 0xF2,
+ MIDI_STATUS_SYSCOM_SONG_SELECT = 0xF3,
+ // F4, F5 is undefined
+ MIDI_STATUS_SYSCOM_TUNE_REQUEST = 0xF6,
+
+ //------------- System RealTime -------------//
+ MIDI_STATUS_SYSREAL_TIMING_CLOCK = 0xF8,
+ // 0xF9 is undefined
+ MIDI_STATUS_SYSREAL_START = 0xFA,
+ MIDI_STATUS_SYSREAL_CONTINUE = 0xFB,
+ MIDI_STATUS_SYSREAL_STOP = 0xFC,
+ // 0xFD is undefined
+ MIDI_STATUS_SYSREAL_ACTIVE_SENSING = 0xFE,
+ MIDI_STATUS_SYSREAL_SYSTEM_RESET = 0xFF,
+};
+
/// MIDI Interface Header Descriptor
typedef struct TU_ATTR_PACKED
{
diff --git a/src/class/midi/midi_device.c b/src/class/midi/midi_device.c
index 1ff81c39a..cba1dcca0 100644
--- a/src/class/midi/midi_device.c
+++ b/src/class/midi/midi_device.c
@@ -38,6 +38,7 @@
//--------------------------------------------------------------------+
// MACRO CONSTANT TYPEDEF
//--------------------------------------------------------------------+
+
typedef struct
{
uint8_t itf_num;
@@ -126,16 +127,19 @@ uint32_t tud_midi_n_read(uint8_t itf, uint8_t cable_num, void* buffer, uint32_t
midid_interface_t* midi = &_midid_itf[itf];
// Fill empty buffer
- if (midi->read_buffer_length == 0) {
- if (!tud_midi_n_packet_read(itf, midi->read_buffer)) return 0;
+ if ( midi->read_buffer_length == 0 )
+ {
+ if ( !tud_midi_n_packet_read(itf, midi->read_buffer) ) return 0;
uint8_t code_index = midi->read_buffer[0] & 0x0f;
// We always copy over the first byte.
uint8_t count = 1;
// Ignore subsequent bytes based on the code.
- if (code_index != 0x5 && code_index != 0xf) {
+ if ( code_index != 0x5 && code_index != 0xf )
+ {
count = 2;
- if (code_index != 0x2 && code_index != 0x6 && code_index != 0xc && code_index != 0xd) {
+ if ( code_index != 0x2 && code_index != 0x6 && code_index != 0xc && code_index != 0xd )
+ {
count = 3;
}
}
@@ -150,7 +154,8 @@ uint32_t tud_midi_n_read(uint8_t itf, uint8_t cable_num, void* buffer, uint32_t
memcpy(buffer, midi->read_buffer + 1 + midi->read_target_length, n);
midi->read_target_length += n;
- if (midi->read_target_length == midi->read_buffer_length) {
+ if ( midi->read_target_length == midi->read_buffer_length )
+ {
midi->read_buffer_length = 0;
midi->read_target_length = 0;
}
@@ -166,10 +171,6 @@ bool tud_midi_n_packet_read (uint8_t itf, uint8_t packet[4])
return (num_read == 4);
}
-void midi_rx_done_cb(midid_interface_t* midi, uint8_t const* buffer, uint32_t bufsize) {
- tu_fifo_write_n(&midi->rx_ff, buffer, bufsize);
-}
-
//--------------------------------------------------------------------+
// WRITE API
//--------------------------------------------------------------------+
@@ -185,7 +186,8 @@ static uint32_t write_flush(midid_interface_t* midi)
TU_VERIFY( usbd_edpt_claim(rhport, midi->ep_in), 0 );
uint16_t count = tu_fifo_read_n(&midi->tx_ff, midi->epin_buf, CFG_TUD_MIDI_EP_BUFSIZE);
- if (count > 0)
+
+ if (count)
{
TU_ASSERT( usbd_edpt_xfer(rhport, midi->ep_in, midi->epin_buf, count), 0 );
return count;
@@ -202,21 +204,26 @@ uint32_t tud_midi_n_write(uint8_t itf, uint8_t cable_num, uint8_t const* buffer,
midid_interface_t* midi = &_midid_itf[itf];
TU_VERIFY(midi->itf_num, 0);
+ uint32_t written = 0;
uint32_t i = 0;
while ( i < bufsize )
{
- uint8_t data = buffer[i];
+ uint8_t const data = buffer[i];
+
if ( midi->write_buffer_length == 0 )
{
- uint8_t msg = data >> 4;
+ // new packet
+
+ uint8_t const msg = data >> 4;
midi->write_buffer[1] = data;
midi->write_buffer_length = 2;
+
// Check to see if we're still in a SysEx transmit.
- if ( midi->write_buffer[0] == 0x4 )
+ if ( midi->write_buffer[0] == MIDI_CIN_SYSEX_START )
{
- if ( data == 0xf7 )
+ if ( data == MIDI_STATUS_SYSEX_END )
{
- midi->write_buffer[0] = 0x5;
+ midi->write_buffer[0] = MIDI_CIN_SYSEX_END_1BYTE;
midi->write_target_length = 2;
}
else
@@ -226,29 +233,31 @@ uint32_t tud_midi_n_write(uint8_t itf, uint8_t cable_num, uint8_t const* buffer,
}
else if ( (msg >= 0x8 && msg <= 0xB) || msg == 0xE )
{
- midi->write_buffer[0] = cable_num << 4 | msg;
+ // Channel Voice Messages
+ midi->write_buffer[0] = (cable_num << 4) | msg;
midi->write_target_length = 4;
}
else if ( msg == 0xf )
{
- if ( data == 0xf0 )
+ // System message
+ if ( data == MIDI_STATUS_SYSEX_START )
{
- midi->write_buffer[0] = 0x4;
+ midi->write_buffer[0] = MIDI_CIN_SYSEX_START;
midi->write_target_length = 4;
}
- else if ( data == 0xf1 || data == 0xf3 )
+ else if ( data == MIDI_STATUS_SYSCOM_TIME_CODE_QUARTER_FRAME || data == MIDI_STATUS_SYSCOM_SONG_SELECT )
{
- midi->write_buffer[0] = 0x2;
+ midi->write_buffer[0] = MIDI_CIN_SYSCOM_2BYTE;
midi->write_target_length = 3;
}
- else if ( data == 0xf2 )
+ else if ( data == MIDI_STATUS_SYSCOM_SONG_POSITION_POINTER )
{
- midi->write_buffer[0] = 0x3;
+ midi->write_buffer[0] = MIDI_CIN_SYSCOM_3BYTE;
midi->write_target_length = 4;
}
else
{
- midi->write_buffer[0] = 0x5;
+ midi->write_buffer[0] = MIDI_CIN_SYSEX_END_1BYTE;
midi->write_target_length = 2;
}
}
@@ -264,33 +273,44 @@ uint32_t tud_midi_n_write(uint8_t itf, uint8_t cable_num, uint8_t const* buffer,
}
else
{
+ // On-going packet
+
TU_ASSERT(midi->write_buffer_length < 4, 0);
midi->write_buffer[midi->write_buffer_length] = data;
- midi->write_buffer_length += 1;
+ midi->write_buffer_length++;
+
// See if this byte ends a SysEx.
- if ( midi->write_buffer[0] == 0x4 && data == 0xf7 )
+ if ( midi->write_buffer[0] == MIDI_CIN_SYSEX_START && data == MIDI_STATUS_SYSEX_END )
{
- midi->write_buffer[0] = 0x4 + (midi->write_buffer_length - 1);
+ midi->write_buffer[0] = MIDI_CIN_SYSEX_START + (midi->write_buffer_length - 1);
midi->write_target_length = midi->write_buffer_length;
}
}
+ // Send out packet
if ( midi->write_buffer_length == midi->write_target_length )
{
- uint16_t written = tu_fifo_write_n(&midi->tx_ff, midi->write_buffer, 4);
- if ( written < 4 )
- {
- TU_ASSERT(written == 0);
- break;
- }
- midi->write_buffer_length = 0;
+ // zeroes unused bytes
+ for(uint8_t idx = midi->write_target_length; idx < 4; idx++) midi->write_buffer[idx] = 0;
+
+ uint16_t const count = tu_fifo_write_n(&midi->tx_ff, midi->write_buffer, 4);
+
+ // reset buffer
+ midi->write_buffer_length = midi->write_target_length = 0;
+
+ // fifo overflow, here we assume FIFO is multiple of 4 and didn't check remaining before writing
+ if ( count != 4 ) break;
+
+ // updated written if succeeded
+ written = i;
}
+
i++;
}
write_flush(midi);
- return i;
+ return written;
}
bool tud_midi_n_packet_write (uint8_t itf, uint8_t const packet[4])
@@ -300,8 +320,7 @@ bool tud_midi_n_packet_write (uint8_t itf, uint8_t const packet[4])
return 0;
}
- if (tu_fifo_remaining(&midi->tx_ff) < 4)
- return false;
+ if (tu_fifo_remaining(&midi->tx_ff) < 4) return false;
tu_fifo_write_n(&midi->tx_ff, packet, 4);
write_flush(midi);
@@ -347,9 +366,9 @@ void midid_reset(uint8_t rhport)
uint16_t midid_open(uint8_t rhport, tusb_desc_interface_t const * desc_itf, uint16_t max_len)
{
// 1st Interface is Audio Control v1
- TU_VERIFY(TUSB_CLASS_AUDIO == desc_itf->bInterfaceClass &&
- AUDIO_SUBCLASS_CONTROL == desc_itf->bInterfaceSubClass &&
- AUDIO_FUNC_PROTOCOL_CODE_UNDEF == desc_itf->bInterfaceProtocol, 0);
+ TU_VERIFY(TUSB_CLASS_AUDIO == desc_itf->bInterfaceClass &&
+ AUDIO_SUBCLASS_CONTROL == desc_itf->bInterfaceSubClass &&
+ AUDIO_FUNC_PROTOCOL_CODE_UNDEF == desc_itf->bInterfaceProtocol, 0);
uint16_t drv_len = tu_desc_len(desc_itf);
uint8_t const * p_desc = tu_desc_next(desc_itf);
@@ -365,9 +384,9 @@ uint16_t midid_open(uint8_t rhport, tusb_desc_interface_t const * desc_itf, uint
TU_VERIFY(TUSB_DESC_INTERFACE == tu_desc_type(p_desc), 0);
tusb_desc_interface_t const * desc_midi = (tusb_desc_interface_t const *) p_desc;
- TU_VERIFY(TUSB_CLASS_AUDIO == desc_midi->bInterfaceClass &&
- AUDIO_SUBCLASS_MIDI_STREAMING == desc_midi->bInterfaceSubClass &&
- AUDIO_FUNC_PROTOCOL_CODE_UNDEF == desc_midi->bInterfaceProtocol, 0);
+ TU_VERIFY(TUSB_CLASS_AUDIO == desc_midi->bInterfaceClass &&
+ AUDIO_SUBCLASS_MIDI_STREAMING == desc_midi->bInterfaceSubClass &&
+ AUDIO_FUNC_PROTOCOL_CODE_UNDEF == desc_midi->bInterfaceProtocol, 0);
// Find available interface
midid_interface_t * p_midi = NULL;
diff --git a/src/class/midi/midi_device.h b/src/class/midi/midi_device.h
index 1060fd77a..aaceaf62a 100644
--- a/src/class/midi/midi_device.h
+++ b/src/class/midi/midi_device.h
@@ -59,16 +59,28 @@
// Application API (Multiple Interfaces)
// CFG_TUD_MIDI > 1
//--------------------------------------------------------------------+
+
+// Check if midi interface is mounted
bool tud_midi_n_mounted (uint8_t itf);
+
+// Get the number of bytes available for reading
uint32_t tud_midi_n_available (uint8_t itf, uint8_t cable_num);
+
+// Read byte stream (legacy)
uint32_t tud_midi_n_read (uint8_t itf, uint8_t cable_num, void* buffer, uint32_t bufsize);
+
+// Write byte Stream (legacy)
uint32_t tud_midi_n_write (uint8_t itf, uint8_t cable_num, uint8_t const* buffer, uint32_t bufsize);
+// Write good-old 3 bytes data, this use write packet
static inline
uint32_t tud_midi_n_write24 (uint8_t itf, uint8_t cable_num, uint8_t b1, uint8_t b2, uint8_t b3);
-bool tud_midi_n_packet_read (uint8_t itf, uint8_t packet[4]);
-bool tud_midi_n_packet_write (uint8_t itf, uint8_t const packet[4]);
+// Read event packet (4 bytes)
+bool tud_midi_n_packet_read (uint8_t itf, uint8_t packet[4]);
+
+// Write event packet (4 bytes)
+bool tud_midi_n_packet_write (uint8_t itf, uint8_t const packet[4]);
//--------------------------------------------------------------------+
// Application API (Single Interface)