From 3e3e9f8a978b274b8fe1f5a9b5fd41a94f928606 Mon Sep 17 00:00:00 2001 From: Zixun LI Date: Wed, 29 Jul 2026 00:34:59 +0200 Subject: class/mtp: preserve final OUT payload before ZLP --- src/class/mtp/mtp_device.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) (limited to 'src/class') diff --git a/src/class/mtp/mtp_device.c b/src/class/mtp/mtp_device.c index 7657899ec..275c9f858 100644 --- a/src/class/mtp/mtp_device.c +++ b/src/class/mtp/mtp_device.c @@ -437,8 +437,11 @@ bool mtpd_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t event, uint32_t TU_LOG_DRV(" MTP Data %s CB: xferred_bytes=%lu, xferred_len/total_len=%lu/%lu, is_complete=%d\r\n", is_data_in ? "IN" : "OUT", xferred_bytes, p_mtp->xferred_len, p_mtp->total_len, is_complete ? 1 : 0); - // Send/queue ZLP if packet is full-sized but transfer is complete - if (is_complete && xferred_bytes > 0 && !(xferred_bytes & (threshold - 1))) { + // Send/queue ZLP if packet is full-sized but transfer is complete. + // OUT must deliver this final payload to the application before receiving + // its terminating ZLP below. + const bool need_zlp = is_complete && xferred_bytes > 0 && !(xferred_bytes & (threshold - 1)); + if (is_data_in && need_zlp) { TU_LOG_DRV(" queue ZLP\r\n"); TU_VERIFY(usbd_edpt_claim(p_mtp->rhport, ep_addr)); TU_ASSERT(usbd_edpt_xfer(p_mtp->rhport, ep_addr, NULL, 0, false)); @@ -466,9 +469,16 @@ bool mtpd_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t event, uint32_t cb_data.io_container = headerless_packet; cb_data.io_container.payload_bytes = xferred_bytes; } - tud_mtp_data_xfer_cb(&cb_data); + if (xferred_bytes > 0) { + tud_mtp_data_xfer_cb(&cb_data); + } - if (is_complete) { + if (need_zlp) { + TU_LOG_DRV(" queue ZLP\r\n"); + TU_VERIFY(usbd_edpt_claim(p_mtp->rhport, ep_addr)); + TU_ASSERT(usbd_edpt_xfer(p_mtp->rhport, ep_addr, NULL, 0, false)); + return true; + } else if (is_complete) { // back to header + payload for response cb_data.io_container = headered_packet; cb_data.io_container.header->len = sizeof(mtp_container_header_t); -- cgit v1.3.1 From 15dd3120ac4a9dea0d979dc541ef8e0f52f5aa26 Mon Sep 17 00:00:00 2001 From: Javid Khan Date: Thu, 30 Jul 2026 14:13:50 +0530 Subject: clamp committed video payload size to streaming ep buffer Signed-off-by: Javid Khan --- src/class/video/video_device.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'src/class') diff --git a/src/class/video/video_device.c b/src/class/video/video_device.c index 3797e6b2b..390349f13 100644 --- a/src/class/video/video_device.c +++ b/src/class/video/video_device.c @@ -1145,6 +1145,12 @@ static int handle_video_stm_cs_req(uint8_t rhport, uint8_t stage, TU_VERIFY(_update_streaming_parameters(stm, param), VIDEO_ERROR_INVALID_VALUE_WITHIN_RANGE); /* Set the negotiated value */ stm->max_payload_transfer_size = param->dwMaxPayloadTransferSize; + /* A host may commit before the parameters are fully negotiated, in which case + * _update_streaming_parameters returns early without capping the payload size. + * Clamp here so a bulk stream cannot overrun the endpoint buffer. */ + if (CFG_TUD_VIDEO_STREAMING_EP_BUFSIZE < stm->max_payload_transfer_size) { + stm->max_payload_transfer_size = CFG_TUD_VIDEO_STREAMING_EP_BUFSIZE; + } int ret = tud_video_commit_cb(stm->index_vc, stm->index_vs, param); if (VIDEO_ERROR_NONE == ret) { stm->state = VS_STATE_COMMITTED; -- cgit v1.3.1 From 282d46e68d9100af0dfdcc01e7689bb63bbf8419 Mon Sep 17 00:00:00 2001 From: ice458 <85405449+ice458@users.noreply.github.com> Date: Fri, 14 Aug 2026 16:00:06 +0900 Subject: usbtmc: re-arm (or stall) the bulk-OUT endpoint after a USB488 TRIGGER A single USB488 TRIGGER message left the bulk-OUT endpoint un-armed, so the host's next bulk-OUT transfer timed out. The trigger itself succeeded silently, so the failure surfaced on a later, unrelated command; only a USBTMC device clear recovered it. The bundled examples/device/usbtmc reproduced this as shipped. Every other branch of the STATE_IDLE dispatch in usbtmcd_xfer_cb() leaves the endpoint in a defined state: it either transitions out of STATE_IDLE so a later tud_usbtmc_start_bus_read() can re-arm it, or it stalls and lets the CLEAR_FEATURE(ENDPOINT_HALT) handler recover it. USBTMC_MSGID_USB488_TRIGGER did neither, and because the state stayed STATE_IDLE, even an application following the contract documented in usbtmc_device.h got a silent no-op from tud_usbtmc_start_bus_read(). Transition to STATE_NAK so the re-arm can take effect, and stall the endpoint when trigger is unsupported or the application callback rejects it, matching the existing handling for messages the driver cannot process. The callback result is deliberately not wrapped in TU_VERIFY(), which would return before the stall/re-arm and reintroduce the same hang. Since the driver now re-arms after a trigger, drop tud_usbtmc_msg_trigger_cb from the list of callbacks after which the application must do so. Fixes #3821 Co-Authored-By: Claude Opus 5 --- src/class/usbtmc/usbtmc_device.c | 18 +++++++++++++++--- src/class/usbtmc/usbtmc_device.h | 1 - 2 files changed, 15 insertions(+), 4 deletions(-) (limited to 'src/class') diff --git a/src/class/usbtmc/usbtmc_device.c b/src/class/usbtmc/usbtmc_device.c index 07190d89f..e248341ac 100644 --- a/src/class/usbtmc/usbtmc_device.c +++ b/src/class/usbtmc/usbtmc_device.c @@ -497,9 +497,21 @@ bool usbtmcd_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t result, uint #if (CFG_TUD_USBTMC_ENABLE_488) case USBTMC_MSGID_USB488_TRIGGER: - // Spec says we halt the EP if we didn't declare we support it. - TU_VERIFY(usbtmc_state.capabilities->bmIntfcCapabilities488.supportsTrigger); - TU_VERIFY(tud_usbtmc_msg_trigger_cb(msg)); + // Unlike the messages above, TRIGGER is complete on arrival and has no response, so nothing else + // will move us out of STATE_IDLE. Do it here, otherwise the tud_usbtmc_start_bus_read() below (and + // any call the application makes from its callback) is a no-op and the bulk-OUT endpoint is left + // un-armed, silently timing out every subsequent host transfer. + TU_VERIFY(atomicChangeState(STATE_IDLE, STATE_NAK)); + + // Spec says we halt the EP if we didn't declare we support it; do the same when the application + // rejects the trigger. The callback result must not be wrapped in TU_VERIFY() here: returning + // early would skip both the stall and the re-arm below. + if (!usbtmc_state.capabilities->bmIntfcCapabilities488.supportsTrigger || + !tud_usbtmc_msg_trigger_cb(msg)) { + usbd_edpt_stall(rhport, usbtmc_state.ep_bulk_out); + return false; + } + tud_usbtmc_start_bus_read(); break; #endif diff --git a/src/class/usbtmc/usbtmc_device.h b/src/class/usbtmc/usbtmc_device.h index 3dc700876..efda84f16 100644 --- a/src/class/usbtmc/usbtmc_device.h +++ b/src/class/usbtmc/usbtmc_device.h @@ -25,7 +25,6 @@ // * tud_usbtmc_open_cb // * tud_usbtmc_msg_data_cb // * tud_usbtmc_msgBulkIn_complete_cb -// * tud_usbtmc_msg_trigger_cb // * (successful) tud_usbtmc_check_abort_bulk_out_cb // * (successful) tud_usbtmc_check_abort_bulk_in_cb // * (successful) tud_usmtmc_bulkOut_clearFeature_cb -- cgit v1.3.1 From af81f9ef42254301c2239eed657b0adce8b466b0 Mon Sep 17 00:00:00 2001 From: ice458 <85405449+ice458@users.noreply.github.com> Date: Fri, 14 Aug 2026 16:23:38 +0900 Subject: usbtmc: document why the trigger re-arm result is ignored A false return from tud_usbtmc_start_bus_read() here does not mean arming failed: it means the endpoint is already armed, either because the application re-armed it from its trigger callback or because a transfer is still queued (usbd_edpt_xfer() reports failure when the endpoint is busy). Both cases end in STATE_IDLE, so the state cannot disambiguate them either, and stalling on the result would halt a healthy endpoint. Co-Authored-By: Claude Opus 5 --- src/class/usbtmc/usbtmc_device.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src/class') diff --git a/src/class/usbtmc/usbtmc_device.c b/src/class/usbtmc/usbtmc_device.c index e248341ac..0e9978a81 100644 --- a/src/class/usbtmc/usbtmc_device.c +++ b/src/class/usbtmc/usbtmc_device.c @@ -511,6 +511,9 @@ bool usbtmcd_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t result, uint usbd_edpt_stall(rhport, usbtmc_state.ep_bulk_out); return false; } + // Result deliberately ignored: false here means the endpoint is already armed - either the + // application re-armed it from its callback, or a transfer is still queued - not that arming + // failed. Stalling on it would halt a healthy endpoint. tud_usbtmc_start_bus_read(); break; -- cgit v1.3.1 From 16629759cd26973cd8e26bee632b339e718f83ba Mon Sep 17 00:00:00 2001 From: Saulo Veríssimo Date: Fri, 14 Aug 2026 15:21:18 -0300 Subject: feat(midi2): complete the UMP stream discovery responder Adds the Device Identity Notification with an app callback, MIDI-CI version and SysEx8 stream count in FB Info, honors the Endpoint Discovery filter bitmap, and paces discovery replies by TX FIFO room. --- src/class/midi/midi2_device.c | 129 +++++++++++++++++++++++++++++++++++++----- src/class/midi/midi2_device.h | 28 +++++++++ 2 files changed, 143 insertions(+), 14 deletions(-) (limited to 'src/class') diff --git a/src/class/midi/midi2_device.c b/src/class/midi/midi2_device.c index 1d40a2efa..e03717992 100644 --- a/src/class/midi/midi2_device.c +++ b/src/class/midi/midi2_device.c @@ -36,6 +36,9 @@ TU_ATTR_WEAK const char* tud_midi2_fb_name_cb(uint8_t itf, uint8_t fb_idx) { TU_ATTR_WEAK tud_midi2_stream_result_t tud_midi2_stream_msg_cb(uint8_t itf, const uint32_t* ump_words) { (void) itf; (void) ump_words; return MIDI2_STREAM_PASS; } +TU_ATTR_WEAK bool tud_midi2_device_identity_cb(uint8_t itf, tud_midi2_device_identity_t* identity) { + (void) itf; (void) identity; return false; +} //--------------------------------------------------------------------+ // Byte order note @@ -59,6 +62,7 @@ enum { enum { STREAM_ENDPOINT_DISCOVERY = 0x000, STREAM_ENDPOINT_INFO = 0x001, + STREAM_DEVICE_IDENTITY = 0x002, STREAM_EP_NAME = 0x003, STREAM_PROD_INSTANCE_ID = 0x004, STREAM_CONFIG_REQUEST = 0x005, @@ -103,6 +107,12 @@ typedef struct { uint8_t protocol; bool negotiated; + // Discovery reply bits waiting for TX FIFO room, drained on TX complete + uint8_t nego_pending_ep_filter; + uint8_t nego_pending_fb_filter; + uint8_t nego_pending_fb_num; // block requested by the pending discovery, 0xFF = all + uint8_t nego_pending_fb_next; // next block index to reply for + /*------------- From this point, data is not cleared by bus reset -------------*/ struct { midi2d_tx_t tx; @@ -380,6 +390,33 @@ static void _nego_send_config_notify(midi2d_interface_t* p_midi, uint8_t protoco _nego_send_ump(p_midi, msg, 4); } +static void _nego_send_device_identity(midi2d_interface_t* p_midi) { + tud_midi2_device_identity_t id; + tu_memclr(&id, sizeof(id)); + if (!tud_midi2_device_identity_cb(_itf_idx(p_midi), &id)) return; + + // Every field is a run of bytes, each carrying 7 bits, laid out in the same + // order as the MIDI 1.0 Device Inquiry reply this message mirrors. A 1-byte + // manufacturer ID occupies the first of the three bytes, the other two stay + // zero, so the caller passes it as 0x7D0000 and not 0x00007D. + uint32_t msg[4] = {0}; + msg[0] = ((uint32_t) MT_STREAM << 28) + | ((uint32_t) STREAM_DEVICE_IDENTITY << 16); + msg[1] = id.manufacturer & UINT32_C(0x7F7F7F); + // Family and model are 14-bit numbers sent least significant byte first, + // as in the Device Inquiry reply. Manufacturer above is a byte sequence + // rather than a number, so it keeps its own order. + msg[2] = ((uint32_t) (id.family & 0x7F) << 24) + | ((uint32_t) ((id.family >> 7) & 0x7F) << 16) + | ((uint32_t) (id.model & 0x7F) << 8) + | ((uint32_t) ((id.model >> 7) & 0x7F)); + msg[3] = ((uint32_t) ((id.sw_revision >> 24) & 0x7F) << 24) + | ((uint32_t) ((id.sw_revision >> 16) & 0x7F) << 16) + | ((uint32_t) ((id.sw_revision >> 8) & 0x7F) << 8) + | ((uint32_t) (id.sw_revision & 0x7F)); + _nego_send_ump(p_midi, msg, 4); +} + static void _nego_send_fb_info(midi2d_interface_t* p_midi, uint8_t fb_idx) { // Derive direction and group span for this block from the GTB descriptor. uint16_t gtb_len = 0; @@ -395,10 +432,75 @@ static void _nego_send_fb_info(midi2d_interface_t* p_midi, uint8_t fb_idx) { | ((uint32_t) fb_idx << 8) | _fb_dir_byte(type); // UI hint + bDirection from the GTB block type msg[1] = ((uint32_t) first_group << 24) - | ((uint32_t) num_groups << 16); + | ((uint32_t) num_groups << 16) + | ((uint32_t) (CFG_TUD_MIDI2_FB_CI_VERSION & 0xFF) << 8) + | ((uint32_t) (CFG_TUD_MIDI2_FB_SYSEX8_STREAMS & 0xFF)); _nego_send_ump(p_midi, msg, 4); } +// Byte cost of one stream text reply (name or product id), all packets included. +static uint16_t _nego_stream_text_bytes(bool has_index, const char* str) { + if (!str || str[0] == '\0') return 0; + const uint8_t per_pkt = has_index ? 13 : 14; + const uint16_t len = (uint16_t) strlen(str); + return (uint16_t)(((len + per_pkt - 1) / per_pkt) * 16); +} + +// Send pending discovery replies, one whole reply at a time and only when the +// TX FIFO can take it. A full-filter Endpoint Discovery asks for more bytes +// than the default FIFO holds; replies that do not fit stay pending and are +// retried from the TX complete path, paced by the transfer flow. +static void _nego_send_pending(midi2d_interface_t* p_midi) { + tu_fifo_t* tx_ff = &p_midi->ep_stream.tx.ff; + const uint16_t depth = tu_fifo_depth(tx_ff); + const uint8_t itf = _itf_idx(p_midi); + + while (p_midi->nego_pending_ep_filter) { + const uint8_t bit = (uint8_t)(p_midi->nego_pending_ep_filter & (uint8_t)(-p_midi->nego_pending_ep_filter)); + uint16_t needed; + switch (bit) { + case 0x04: needed = _nego_stream_text_bytes(false, tud_midi2_ep_name_cb(itf)); break; + case 0x08: needed = _nego_stream_text_bytes(false, tud_midi2_product_id_cb(itf)); break; + default: needed = 16; break; // endpoint info, device identity, config notify + } + if (needed > depth) needed = depth; // oversized reply: send best effort, never stall + if (tu_fifo_remaining(tx_ff) < needed) return; + + switch (bit) { + case 0x01: _nego_send_endpoint_info(p_midi); break; + case 0x02: _nego_send_device_identity(p_midi); break; + case 0x04: _nego_send_stream_text(p_midi, STREAM_EP_NAME, false, 0, tud_midi2_ep_name_cb(itf)); break; + case 0x08: _nego_send_stream_text(p_midi, STREAM_PROD_INSTANCE_ID, false, 0, tud_midi2_product_id_cb(itf)); break; + case 0x10: _nego_send_config_notify(p_midi, p_midi->protocol); break; + default: break; + } + p_midi->nego_pending_ep_filter &= (uint8_t) ~bit; + } + + const uint8_t fb_count = _gtb_block_count(p_midi); + while (p_midi->nego_pending_fb_filter && p_midi->nego_pending_fb_next < fb_count) { + const uint8_t f = p_midi->nego_pending_fb_next; + if (p_midi->nego_pending_fb_num != 0xFF && p_midi->nego_pending_fb_num != f) { + p_midi->nego_pending_fb_next++; + continue; + } + // Info and name for one block go out together to keep per-block ordering. + uint16_t needed = (p_midi->nego_pending_fb_filter & 0x01) ? 16 : 0; + if (p_midi->nego_pending_fb_filter & 0x02) { + needed = (uint16_t)(needed + _nego_stream_text_bytes(true, tud_midi2_fb_name_cb(itf, f))); + } + if (needed > depth) needed = depth; + if (tu_fifo_remaining(tx_ff) < needed) return; + + if (p_midi->nego_pending_fb_filter & 0x01) _nego_send_fb_info(p_midi, f); + if (p_midi->nego_pending_fb_filter & 0x02) { + _nego_send_stream_text(p_midi, STREAM_FB_NAME, true, f, tud_midi2_fb_name_cb(itf, f)); + } + p_midi->nego_pending_fb_next++; + } + if (p_midi->nego_pending_fb_next >= fb_count) p_midi->nego_pending_fb_filter = 0; +} + static void _nego_handle_stream_msg(midi2d_interface_t* p_midi, const uint32_t* words) { // Let the application override this message before the built-in responder. switch (tud_midi2_stream_msg_cb(_itf_idx(p_midi), words)) { @@ -421,9 +523,9 @@ static void _nego_handle_stream_msg(midi2d_interface_t* p_midi, const uint32_t* switch (status) { case STREAM_ENDPOINT_DISCOVERY: - _nego_send_endpoint_info(p_midi); - _nego_send_stream_text(p_midi, STREAM_EP_NAME, false, 0, tud_midi2_ep_name_cb(_itf_idx(p_midi))); - _nego_send_stream_text(p_midi, STREAM_PROD_INSTANCE_ID, false, 0, tud_midi2_product_id_cb(_itf_idx(p_midi))); + // Filter bitmap: each bit set asks for one individual reply. + p_midi->nego_pending_ep_filter |= (uint8_t)(words[1] & 0x1F); + _nego_send_pending(p_midi); break; case STREAM_CONFIG_REQUEST: { @@ -436,17 +538,12 @@ static void _nego_handle_stream_msg(midi2d_interface_t* p_midi, const uint32_t* break; } - case STREAM_FB_DISCOVERY: { - uint8_t fb_idx = (words[0] >> 8) & 0xFF; - uint8_t filter = words[0] & 0xFF; // bit 0: FB Info, bit 1: FB Name - uint8_t fb_count = _gtb_block_count(p_midi); - for (uint8_t f = 0; f < fb_count; f++) { - if (fb_idx != 0xFF && fb_idx != f) continue; - if (filter & 0x01) _nego_send_fb_info(p_midi, f); - if (filter & 0x02) _nego_send_stream_text(p_midi, STREAM_FB_NAME, true, f, tud_midi2_fb_name_cb(_itf_idx(p_midi), f)); - } + case STREAM_FB_DISCOVERY: + p_midi->nego_pending_fb_num = (uint8_t)((words[0] >> 8) & 0xFF); + p_midi->nego_pending_fb_filter = (uint8_t)(words[0] & 0x03); // bit 0: FB Info, bit 1: FB Name + p_midi->nego_pending_fb_next = 0; + _nego_send_pending(p_midi); break; - } default: break; @@ -824,6 +921,10 @@ bool midi2d_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t result, uint3 } tu_edpt_stream_read_xfer(ep_rx); } else if (ep_addr == ep_tx->ep_addr && result == XFER_RESULT_SUCCESS) { + // Completed transfer freed FIFO room: flush discovery replies still pending. + if (p_midi->alt_setting == 1) { + _nego_send_pending(p_midi); + } uint16_t queued = _tx_start_xfer(p_midi); // Send ZLP if no more data is queued but the last transfer was exactly mps if (queued == 0 && tu_fifo_count(&ep_tx->ff) == 0 && xferred_bytes > 0 && diff --git a/src/class/midi/midi2_device.h b/src/class/midi/midi2_device.h index 171b404b7..e3eb084d9 100644 --- a/src/class/midi/midi2_device.h +++ b/src/class/midi/midi2_device.h @@ -58,6 +58,17 @@ extern "C" { #define CFG_TUD_MIDI2_PRODUCT_ID "TinyUSB-MIDI2" #endif +// Function Block capabilities reported in Function Block Info Notification. +// The GTB descriptor carries direction and group span, but not these: they +// depend on what the application implements, so they default to "none". +#ifndef CFG_TUD_MIDI2_FB_CI_VERSION + #define CFG_TUD_MIDI2_FB_CI_VERSION 0 // 0: none or unknown, 1 or higher: MIDI-CI version +#endif + +#ifndef CFG_TUD_MIDI2_FB_SYSEX8_STREAMS + #define CFG_TUD_MIDI2_FB_SYSEX8_STREAMS 0 // 0: unsupported, 1: single, 2-255: simultaneous streams +#endif + // String descriptor index for the Group Terminal Block (iBlockItem, Table 5-6). // 0 = no string descriptor (default, spec-allowed). #ifndef CFG_TUD_MIDI2_BLOCK_STRIDX @@ -118,6 +129,17 @@ typedef enum { MIDI2_STREAM_NEGOTIATED_MIDI2, } tud_midi2_stream_result_t; +// Device identity fields, as defined for the MIDI 1.0 Device Inquiry reply and +// reused by the Device Identity Notification. Every byte carries 7 bits. +// A 1-byte System Exclusive ID goes in the first of the three manufacturer +// bytes, so 0x7D is passed as 0x7D0000. +typedef struct { + uint32_t manufacturer; // 3 bytes, first byte is most significant + uint16_t family; // 2 bytes + uint16_t model; // 2 bytes + uint32_t sw_revision; // 4 bytes +} tud_midi2_device_identity_t; + //--------------------------------------------------------------------+ // Application Callback API (weak, optional) //--------------------------------------------------------------------+ @@ -138,6 +160,12 @@ const uint8_t* tud_midi2_gtb_desc_cb(uint8_t itf, uint16_t* len); // discovery. Return NULL or "" for no name. const char* tud_midi2_fb_name_cb(uint8_t itf, uint8_t fb_idx); +// Optional device identity, sent as a Device Identity Notification when the +// host sets the 'd' bit in the Endpoint Discovery filter. Same four fields as +// the MIDI 1.0 Device Inquiry reply. Return false to skip the notification, +// which is the default. All values are 7-bit per byte. +bool tud_midi2_device_identity_cb(uint8_t itf, tud_midi2_device_identity_t* identity); + // Optional: intercept an incoming UMP Stream message (MT 0xF). Return PASS to // let the built-in responder handle it, or HANDLED / NEGOTIATED_* if the app // answered it (e.g. via tud_midi2_n_ump_write). Lets an app override a single -- cgit v1.3.1 From 0504faf29825deb130bfeb88dba46bb1c1bdec75 Mon Sep 17 00:00:00 2001 From: Saulo Veríssimo Date: Fri, 14 Aug 2026 16:43:36 -0300 Subject: fix(midi2): keep discovery replies valid under TX pressure Text replies resume instead of dropping their tail packets, which used to leave a Start/Continue sequence without an End. A new Function Block Discovery now merges with a pending one instead of replacing it. --- src/class/midi/midi2_device.c | 90 ++++++++++++++++++++++++------------------- 1 file changed, 50 insertions(+), 40 deletions(-) (limited to 'src/class') diff --git a/src/class/midi/midi2_device.c b/src/class/midi/midi2_device.c index e03717992..369d380c5 100644 --- a/src/class/midi/midi2_device.c +++ b/src/class/midi/midi2_device.c @@ -112,6 +112,7 @@ typedef struct { uint8_t nego_pending_fb_filter; uint8_t nego_pending_fb_num; // block requested by the pending discovery, 0xFF = all uint8_t nego_pending_fb_next; // next block index to reply for + uint16_t nego_text_offset; // progress into the text reply being sent /*------------- From this point, data is not cleared by bus reset -------------*/ struct { @@ -337,16 +338,20 @@ static void _nego_send_endpoint_info(midi2d_interface_t* p_midi) { // index byte (the Function Block number for FB Name) and 13 chars fit per // packet; otherwise the text starts there and 14 chars fit (Endpoint Name, // Product Instance Id). -static void _nego_send_stream_text(midi2d_interface_t* p_midi, uint16_t status, - bool has_index, uint8_t index, const char* str) { - if (!str || str[0] == '\0') return; +// Sends a stream text from `offset` and returns how far it got. Resuming keeps +// the End packet, which dropping the tail would lose. +static uint16_t _nego_send_stream_text(midi2d_interface_t* p_midi, uint16_t status, + bool has_index, uint8_t index, const char* str, + uint16_t offset) { + if (!str || str[0] == '\0') return 0; - uint16_t total_len = (uint16_t) strlen(str); - uint16_t offset = 0; + const uint16_t total_len = (uint16_t) strlen(str); const uint8_t per_pkt = has_index ? 13 : 14; const uint8_t head_chars = has_index ? 1 : 2; // chars carried in word0 + if (offset >= total_len) return total_len; while (offset < total_len) { + if (tu_fifo_remaining(&p_midi->ep_stream.tx.ff) < 16) break; uint16_t remaining = total_len - offset; uint8_t n = (uint8_t)((remaining > per_pkt) ? per_pkt : remaining); bool is_first = (offset == 0); @@ -380,6 +385,7 @@ static void _nego_send_stream_text(midi2d_interface_t* p_midi, uint16_t status, _nego_send_ump(p_midi, msg, 4); offset += n; } + return offset; } static void _nego_send_config_notify(midi2d_interface_t* p_midi, uint8_t protocol) { @@ -438,41 +444,37 @@ static void _nego_send_fb_info(midi2d_interface_t* p_midi, uint8_t fb_idx) { _nego_send_ump(p_midi, msg, 4); } -// Byte cost of one stream text reply (name or product id), all packets included. -static uint16_t _nego_stream_text_bytes(bool has_index, const char* str) { - if (!str || str[0] == '\0') return 0; - const uint8_t per_pkt = has_index ? 13 : 14; - const uint16_t len = (uint16_t) strlen(str); - return (uint16_t)(((len + per_pkt - 1) / per_pkt) * 16); -} - // Send pending discovery replies, one whole reply at a time and only when the // TX FIFO can take it. A full-filter Endpoint Discovery asks for more bytes // than the default FIFO holds; replies that do not fit stay pending and are // retried from the TX complete path, paced by the transfer flow. static void _nego_send_pending(midi2d_interface_t* p_midi) { tu_fifo_t* tx_ff = &p_midi->ep_stream.tx.ff; - const uint16_t depth = tu_fifo_depth(tx_ff); const uint8_t itf = _itf_idx(p_midi); while (p_midi->nego_pending_ep_filter) { const uint8_t bit = (uint8_t)(p_midi->nego_pending_ep_filter & (uint8_t)(-p_midi->nego_pending_ep_filter)); - uint16_t needed; + const char* text = NULL; + uint16_t status = 0; switch (bit) { - case 0x04: needed = _nego_stream_text_bytes(false, tud_midi2_ep_name_cb(itf)); break; - case 0x08: needed = _nego_stream_text_bytes(false, tud_midi2_product_id_cb(itf)); break; - default: needed = 16; break; // endpoint info, device identity, config notify + case 0x04: text = tud_midi2_ep_name_cb(itf); status = STREAM_EP_NAME; break; + case 0x08: text = tud_midi2_product_id_cb(itf); status = STREAM_PROD_INSTANCE_ID; break; + default: break; } - if (needed > depth) needed = depth; // oversized reply: send best effort, never stall - if (tu_fifo_remaining(tx_ff) < needed) return; - switch (bit) { - case 0x01: _nego_send_endpoint_info(p_midi); break; - case 0x02: _nego_send_device_identity(p_midi); break; - case 0x04: _nego_send_stream_text(p_midi, STREAM_EP_NAME, false, 0, tud_midi2_ep_name_cb(itf)); break; - case 0x08: _nego_send_stream_text(p_midi, STREAM_PROD_INSTANCE_ID, false, 0, tud_midi2_product_id_cb(itf)); break; - case 0x10: _nego_send_config_notify(p_midi, p_midi->protocol); break; - default: break; + if (text != NULL) { + p_midi->nego_text_offset = _nego_send_stream_text(p_midi, status, false, 0, text, + p_midi->nego_text_offset); + if (p_midi->nego_text_offset < (uint16_t) strlen(text)) return; // resume on TX complete + p_midi->nego_text_offset = 0; + } else { + if (tu_fifo_remaining(tx_ff) < 16) return; + switch (bit) { + case 0x01: _nego_send_endpoint_info(p_midi); break; + case 0x02: _nego_send_device_identity(p_midi); break; + case 0x10: _nego_send_config_notify(p_midi, p_midi->protocol); break; + default: break; + } } p_midi->nego_pending_ep_filter &= (uint8_t) ~bit; } @@ -484,17 +486,16 @@ static void _nego_send_pending(midi2d_interface_t* p_midi) { p_midi->nego_pending_fb_next++; continue; } - // Info and name for one block go out together to keep per-block ordering. - uint16_t needed = (p_midi->nego_pending_fb_filter & 0x01) ? 16 : 0; - if (p_midi->nego_pending_fb_filter & 0x02) { - needed = (uint16_t)(needed + _nego_stream_text_bytes(true, tud_midi2_fb_name_cb(itf, f))); + if ((p_midi->nego_pending_fb_filter & 0x01) && p_midi->nego_text_offset == 0) { + if (tu_fifo_remaining(tx_ff) < 16) return; + _nego_send_fb_info(p_midi, f); } - if (needed > depth) needed = depth; - if (tu_fifo_remaining(tx_ff) < needed) return; - - if (p_midi->nego_pending_fb_filter & 0x01) _nego_send_fb_info(p_midi, f); if (p_midi->nego_pending_fb_filter & 0x02) { - _nego_send_stream_text(p_midi, STREAM_FB_NAME, true, f, tud_midi2_fb_name_cb(itf, f)); + const char* name = tud_midi2_fb_name_cb(itf, f); + p_midi->nego_text_offset = _nego_send_stream_text(p_midi, STREAM_FB_NAME, true, f, name, + p_midi->nego_text_offset); + if (name != NULL && p_midi->nego_text_offset < (uint16_t) strlen(name)) return; + p_midi->nego_text_offset = 0; } p_midi->nego_pending_fb_next++; } @@ -538,12 +539,21 @@ static void _nego_handle_stream_msg(midi2d_interface_t* p_midi, const uint32_t* break; } - case STREAM_FB_DISCOVERY: - p_midi->nego_pending_fb_num = (uint8_t)((words[0] >> 8) & 0xFF); - p_midi->nego_pending_fb_filter = (uint8_t)(words[0] & 0x03); // bit 0: FB Info, bit 1: FB Name - p_midi->nego_pending_fb_next = 0; + case STREAM_FB_DISCOVERY: { + const uint8_t req_num = (uint8_t)((words[0] >> 8) & 0xFF); + // Merge with a pending request: repeating a Function Block Info is allowed + // at any time, losing a requested one is not. + if (p_midi->nego_pending_fb_filter && p_midi->nego_pending_fb_num != req_num) { + p_midi->nego_pending_fb_num = 0xFF; + p_midi->nego_pending_fb_next = 0; + } else if (!p_midi->nego_pending_fb_filter) { + p_midi->nego_pending_fb_num = req_num; + p_midi->nego_pending_fb_next = 0; + } + p_midi->nego_pending_fb_filter |= (uint8_t)(words[0] & 0x03); // bit 0: FB Info, bit 1: FB Name _nego_send_pending(p_midi); break; + } default: break; -- cgit v1.3.1 From dfd197ff0c83a01ac55a99b85f2e8f3794ea0a47 Mon Sep 17 00:00:00 2001 From: HiFiPhile Date: Sat, 15 Aug 2026 05:11:55 +0200 Subject: fix(midi2): fix discovery response racing Signed-off-by: HiFiPhile --- src/class/midi/midi2_device.c | 97 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 79 insertions(+), 18 deletions(-) (limited to 'src/class') diff --git a/src/class/midi/midi2_device.c b/src/class/midi/midi2_device.c index 369d380c5..b0a9e2503 100644 --- a/src/class/midi/midi2_device.c +++ b/src/class/midi/midi2_device.c @@ -112,7 +112,10 @@ typedef struct { uint8_t nego_pending_fb_filter; uint8_t nego_pending_fb_num; // block requested by the pending discovery, 0xFF = all uint8_t nego_pending_fb_next; // next block index to reply for + bool nego_pending_fb_restart; // restart after the active FB name when requests merge + uint16_t nego_text_status; // text reply owning nego_text_offset, 0 = none uint16_t nego_text_offset; // progress into the text reply being sent + uint8_t nego_text_index; // Function Block index for an active FB name /*------------- From this point, data is not cleared by bus reset -------------*/ struct { @@ -444,29 +447,85 @@ static void _nego_send_fb_info(midi2d_interface_t* p_midi, uint8_t fb_idx) { _nego_send_ump(p_midi, msg, 4); } +static void _nego_clear_pending(midi2d_interface_t* p_midi) { + p_midi->nego_pending_ep_filter = 0; + p_midi->nego_pending_fb_filter = 0; + p_midi->nego_pending_fb_num = 0; + p_midi->nego_pending_fb_next = 0; + p_midi->nego_pending_fb_restart = false; + p_midi->nego_text_status = 0; + p_midi->nego_text_offset = 0; + p_midi->nego_text_index = 0; +} + +static const char* _nego_text_cb(midi2d_interface_t* p_midi, uint16_t status, uint8_t index) { + const uint8_t itf = _itf_idx(p_midi); + switch (status) { + case STREAM_EP_NAME: return tud_midi2_ep_name_cb(itf); + case STREAM_PROD_INSTANCE_ID: return tud_midi2_product_id_cb(itf); + case STREAM_FB_NAME: return tud_midi2_fb_name_cb(itf, index); + default: return NULL; + } +} + +// Send or resume one text reply. While it is incomplete, its status and index +// identify the sole owner of nego_text_offset so another discovery request +// cannot resume a different string from the same offset. +static bool _nego_send_text(midi2d_interface_t* p_midi, uint16_t status, uint8_t index) { + const char* text = _nego_text_cb(p_midi, status, index); + const uint16_t len = text ? (uint16_t) strlen(text) : 0; + + p_midi->nego_text_status = status; + p_midi->nego_text_index = index; + p_midi->nego_text_offset = _nego_send_stream_text(p_midi, status, status == STREAM_FB_NAME, + index, text, p_midi->nego_text_offset); + if (p_midi->nego_text_offset < len) return false; + + p_midi->nego_text_status = 0; + p_midi->nego_text_offset = 0; + p_midi->nego_text_index = 0; + return true; +} + // Send pending discovery replies, one whole reply at a time and only when the // TX FIFO can take it. A full-filter Endpoint Discovery asks for more bytes // than the default FIFO holds; replies that do not fit stay pending and are // retried from the TX complete path, paced by the transfer flow. static void _nego_send_pending(midi2d_interface_t* p_midi) { tu_fifo_t* tx_ff = &p_midi->ep_stream.tx.ff; - const uint8_t itf = _itf_idx(p_midi); + + // An incomplete text sequence must finish before any newly arrived request + // is serviced; otherwise its Continue/End packets could be attached to a + // different Endpoint or Function Block string. + if (p_midi->nego_text_status) { + const uint16_t status = p_midi->nego_text_status; + const uint8_t index = p_midi->nego_text_index; + if (!_nego_send_text(p_midi, status, index)) return; + + if (status == STREAM_FB_NAME) { + if (p_midi->nego_pending_fb_restart) { + p_midi->nego_pending_fb_next = 0; + p_midi->nego_pending_fb_restart = false; + } else { + p_midi->nego_pending_fb_next++; + } + } else { + const uint8_t bit = (status == STREAM_EP_NAME) ? 0x04 : 0x08; + p_midi->nego_pending_ep_filter &= (uint8_t) ~bit; + } + } while (p_midi->nego_pending_ep_filter) { const uint8_t bit = (uint8_t)(p_midi->nego_pending_ep_filter & (uint8_t)(-p_midi->nego_pending_ep_filter)); - const char* text = NULL; uint16_t status = 0; switch (bit) { - case 0x04: text = tud_midi2_ep_name_cb(itf); status = STREAM_EP_NAME; break; - case 0x08: text = tud_midi2_product_id_cb(itf); status = STREAM_PROD_INSTANCE_ID; break; + case 0x04: status = STREAM_EP_NAME; break; + case 0x08: status = STREAM_PROD_INSTANCE_ID; break; default: break; } - if (text != NULL) { - p_midi->nego_text_offset = _nego_send_stream_text(p_midi, status, false, 0, text, - p_midi->nego_text_offset); - if (p_midi->nego_text_offset < (uint16_t) strlen(text)) return; // resume on TX complete - p_midi->nego_text_offset = 0; + if (status != 0) { + if (!_nego_send_text(p_midi, status, 0)) return; } else { if (tu_fifo_remaining(tx_ff) < 16) return; switch (bit) { @@ -491,11 +550,7 @@ static void _nego_send_pending(midi2d_interface_t* p_midi) { _nego_send_fb_info(p_midi, f); } if (p_midi->nego_pending_fb_filter & 0x02) { - const char* name = tud_midi2_fb_name_cb(itf, f); - p_midi->nego_text_offset = _nego_send_stream_text(p_midi, STREAM_FB_NAME, true, f, name, - p_midi->nego_text_offset); - if (name != NULL && p_midi->nego_text_offset < (uint16_t) strlen(name)) return; - p_midi->nego_text_offset = 0; + if (!_nego_send_text(p_midi, STREAM_FB_NAME, f)) return; } p_midi->nego_pending_fb_next++; } @@ -541,16 +596,21 @@ static void _nego_handle_stream_msg(midi2d_interface_t* p_midi, const uint32_t* case STREAM_FB_DISCOVERY: { const uint8_t req_num = (uint8_t)((words[0] >> 8) & 0xFF); + const uint8_t req_filter = (uint8_t)(words[0] & 0x03); // Merge with a pending request: repeating a Function Block Info is allowed // at any time, losing a requested one is not. - if (p_midi->nego_pending_fb_filter && p_midi->nego_pending_fb_num != req_num) { - p_midi->nego_pending_fb_num = 0xFF; - p_midi->nego_pending_fb_next = 0; + if (req_filter && p_midi->nego_pending_fb_filter) { + if (p_midi->nego_pending_fb_num != req_num) p_midi->nego_pending_fb_num = 0xFF; + if (p_midi->nego_text_status == STREAM_FB_NAME) { + p_midi->nego_pending_fb_restart = true; + } else { + p_midi->nego_pending_fb_next = 0; + } } else if (!p_midi->nego_pending_fb_filter) { p_midi->nego_pending_fb_num = req_num; p_midi->nego_pending_fb_next = 0; } - p_midi->nego_pending_fb_filter |= (uint8_t)(words[0] & 0x03); // bit 0: FB Info, bit 1: FB Name + p_midi->nego_pending_fb_filter |= req_filter; // bit 0: FB Info, bit 1: FB Name _nego_send_pending(p_midi); break; } @@ -861,6 +921,7 @@ bool midi2d_control_xfer_cb(uint8_t rhport, uint8_t stage, const tusb_control_re tu_edpt_stream_clear(&p_midi->ep_stream.rx); tu_fifo_clear(&p_midi->ep_stream.tx.ff); + _nego_clear_pending(p_midi); if (alt == 1) { p_midi->negotiated = false; -- cgit v1.3.1 From 8737c5adfca7e51003e743bcc8bcefed837fb1d8 Mon Sep 17 00:00:00 2001 From: Zixun LI Date: Sat, 15 Aug 2026 05:51:43 +0200 Subject: Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: HiFiPhile --- src/class/video/video_device.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'src/class') diff --git a/src/class/video/video_device.c b/src/class/video/video_device.c index 390349f13..770595178 100644 --- a/src/class/video/video_device.c +++ b/src/class/video/video_device.c @@ -1144,13 +1144,10 @@ static int handle_video_stm_cs_req(uint8_t rhport, uint8_t stage, video_probe_and_commit_control_t *param = &stm->probe_commit_payload; TU_VERIFY(_update_streaming_parameters(stm, param), VIDEO_ERROR_INVALID_VALUE_WITHIN_RANGE); /* Set the negotiated value */ - stm->max_payload_transfer_size = param->dwMaxPayloadTransferSize; - /* A host may commit before the parameters are fully negotiated, in which case - * _update_streaming_parameters returns early without capping the payload size. - * Clamp here so a bulk stream cannot overrun the endpoint buffer. */ - if (CFG_TUD_VIDEO_STREAMING_EP_BUFSIZE < stm->max_payload_transfer_size) { - stm->max_payload_transfer_size = CFG_TUD_VIDEO_STREAMING_EP_BUFSIZE; + if (CFG_TUD_VIDEO_STREAMING_EP_BUFSIZE < param->dwMaxPayloadTransferSize) { + param->dwMaxPayloadTransferSize = CFG_TUD_VIDEO_STREAMING_EP_BUFSIZE; } + stm->max_payload_transfer_size = param->dwMaxPayloadTransferSize; int ret = tud_video_commit_cb(stm->index_vc, stm->index_vs, param); if (VIDEO_ERROR_NONE == ret) { stm->state = VS_STATE_COMMITTED; -- cgit v1.3.1