diff --git a/profiles/midi/libmidi.c b/profiles/midi/libmidi.c index 5b77cd3..8b63fac 100644 --- a/profiles/midi/libmidi.c +++ b/profiles/midi/libmidi.c @@ -65,8 +65,9 @@ inline static void append_timestamp_high_maybe(struct midi_write_parser *parser) /* Make sure timesampt_high is assigned a non-zero value */ do { - /* convert µs to ms */ - parser->rtime = g_get_monotonic_time() / 1000; + /* convert µs to ms, shifted into the peer's clock domain */ + parser->rtime = g_get_monotonic_time() / 1000 + + parser->ts_offset; timestamp_high |= (parser->rtime & 0x1F80) >> 7; } while (timestamp_high == 0x80); @@ -85,6 +86,7 @@ int midi_write_init(struct midi_write_parser *parser, size_t buffer_size) int err; parser->rtime = 0; + parser->ts_offset = 0; parser->rstatus = SND_SEQ_EVENT_NONE; parser->stream_size = buffer_size; diff --git a/profiles/midi/libmidi.h b/profiles/midi/libmidi.h index 41f522d..9754b3f 100644 --- a/profiles/midi/libmidi.h +++ b/profiles/midi/libmidi.h @@ -31,6 +31,12 @@ struct midi_buffer { struct midi_write_parser { int64_t rtime; /* last writer's real time */ + int64_t ts_offset; /* ms added to the host clock so that + * outgoing BLE-MIDI timestamps land in + * the peer's clock domain (learned from + * received packets; some devices, e.g. + * Roland FP-E50, silently drop events + * stamped outside their own clock) */ snd_seq_event_type_t rstatus; /* running status event type */ struct midi_buffer midi_stream; /* MIDI I/O byte stream */ size_t stream_size; /* what is the maximum size of the midi_stream array */ diff --git a/profiles/midi/midi.c b/profiles/midi/midi.c index bab309b..5d5e61a 100644 --- a/profiles/midi/midi.c +++ b/profiles/midi/midi.c @@ -13,6 +13,7 @@ #endif #include +#include #include #include "lib/bluetooth.h" @@ -78,6 +79,17 @@ static bool midi_write_cb(struct io *io, void *user_data) midi_read_ev(&midi->midi_out, event, foreach_cb, midi); + /* + * Flush after every event: one MIDI message per BLE packet, + * each with a full header + timestamp. Some devices (Roland + * FP-E50) fail to parse multi-event packets with running + * status and stop voicing notes entirely (cf. bluez#471). + */ + if (midi_write_has_data(&midi->midi_out)) { + foreach_cb(&midi->midi_out, midi); + midi_write_reset(&midi->midi_out); + } + } while (err > 0); if (midi_write_has_data(&midi->midi_out)) @@ -124,6 +136,16 @@ static void midi_io_value_cb(uint16_t value_handle, const uint8_t *value, i += count; } + /* + * Re-sync our outgoing timestamp clock to the peer's: some devices + * (e.g. Roland FP-E50) silently discard events whose timestamps do + * not match their own clock domain. Every received packet carries + * the peer's current 13-bit millisecond clock; track the offset so + * append_timestamp_high_maybe() stamps writes in that domain. + */ + midi->midi_out.ts_offset = (int64_t) (uint16_t) midi->midi_in.timestamp - + g_get_monotonic_time() / 1000; + return; _err: diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c index 50fcb26..3bf14ec 100644 --- a/src/shared/gatt-helpers.c +++ b/src/shared/gatt-helpers.c @@ -1158,6 +1158,117 @@ struct bt_gatt_request *bt_gatt_discover_included_services(struct bt_att *att, return bt_gatt_request_ref(op); } +/* + * The Read By Type Response carrying characteristic declarations is required + * by the spec to hold attribute data records that are all the same length + * (Core spec, Vol 3, Part F, 3.4.4.2). The declared length distinguishes a + * 16-bit UUID record (7 octets) from a 128-bit UUID one (21 octets), so a + * compliant server never mixes the two in a single response. + * + * Some Roland devices (e.g. the FP-E50 / GO:KEYS BLE MIDI interface) violate + * this and pack 7- and 21-octet records together under a single declared + * length. Because the byte count can still be an exact multiple of the + * declared length, the parser silently slices the 128-bit records into bogus + * 16-bit ones, hiding the real BLE MIDI characteristic UUID + * (7772E5DB-3868-4112-A1A9-F2669D106BF3). See bluez/bluez#604. + * + * The GATT model guarantees that a Characteristic Value declaration is the + * attribute immediately following its Characteristic declaration, so the + * value handle is always declaration handle + 1. char_uniform_valid() uses + * that invariant to confirm a response really is uniform; when it is not, + * split_mixed_char_response() walks the list record by record, choosing each + * record's size by which choice keeps the rest of the list consistent, and + * appends each as its own (uniform) result chunk so the existing iterator can + * consume them unchanged. + */ +static bool char_uniform_valid(const uint8_t *buf, size_t buflen, + size_t data_length) +{ + size_t off; + uint16_t prev_handle = 0; + bool first = true; + + if (!buflen || (buflen % data_length)) + return false; + + for (off = 0; off + data_length <= buflen; off += data_length) { + uint16_t handle = get_le16(buf + off); + uint16_t value_handle = get_le16(buf + off + 3); + + if (value_handle != handle + 1) + return false; + if (!first && handle <= prev_handle) + return false; + + prev_handle = handle; + first = false; + } + + return true; +} + +/* + * Return true if a characteristic declaration record of size rec_len starting + * at off is self-consistent and leaves the remainder of the buffer able to + * start another valid record (or end exactly). Used to pick 7 vs 21 octets + * for a mixed-length response. + */ +static bool char_record_consistent(const uint8_t *buf, size_t buflen, + size_t off, size_t rec_len) +{ + uint16_t handle, value_handle, next_handle, next_value_handle; + + if (off + rec_len > buflen) + return false; + + handle = get_le16(buf + off); + value_handle = get_le16(buf + off + 3); + + /* Characteristic value handle must immediately follow declaration */ + if (value_handle != handle + 1) + return false; + + /* This record consumes the rest of the buffer exactly */ + if (off + rec_len == buflen) + return true; + + /* Otherwise the next record header must look like a declaration too */ + if (off + rec_len + 7 > buflen) + return false; + + next_handle = get_le16(buf + off + rec_len); + next_value_handle = get_le16(buf + off + rec_len + 3); + + return next_handle > handle && next_value_handle == next_handle + 1; +} + +static bool split_mixed_char_response(uint8_t opcode, const uint8_t *buf, + size_t buflen, struct bt_gatt_request *op, + uint16_t *last_handle) +{ + size_t off = 0; + + while (off < buflen) { + size_t rec_len; + + /* Prefer the 16-bit UUID interpretation, fall back to 128-bit */ + if (char_record_consistent(buf, buflen, off, 7)) + rec_len = 7; + else if (char_record_consistent(buf, buflen, off, 21)) + rec_len = 21; + else + return false; + + if (!result_append(opcode, buf + off, rec_len, rec_len, op)) + return false; + + *last_handle = get_le16(buf + off); + off += rec_len; + } + + return off == buflen; +} + static void discover_chrcs_cb(uint8_t opcode, const void *pdu, uint16_t length, void *user_data) { @@ -1165,6 +1276,8 @@ static void discover_chrcs_cb(uint8_t opcode, const void *pdu, bool success; uint8_t att_ecode = 0; size_t data_length; + const uint8_t *buf; + size_t buflen; uint16_t last_handle; if (opcode == BT_ATT_OP_ERROR_RSP) { @@ -1188,18 +1301,31 @@ static void discover_chrcs_cb(uint8_t opcode, const void *pdu, data_length = ((uint8_t *) pdu)[0]; - if ((data_length != 7 && data_length != 21) || - ((length - 1) % data_length)) { + if (data_length != 7 && data_length != 21) { success = false; goto done; } - if (!result_append(opcode, pdu + 1, length - 1, - data_length, op)) { + buf = (const uint8_t *) pdu + 1; + buflen = length - 1; + + if (char_uniform_valid(buf, buflen, data_length)) { + /* Spec-compliant, equal-length response: append as one chunk */ + if (!result_append(opcode, buf, buflen, data_length, op)) { + success = false; + goto done; + } + last_handle = get_le16(buf + buflen - data_length); + } else if (!split_mixed_char_response(opcode, buf, buflen, op, + &last_handle)) { + /* + * Neither a clean equal-length list nor a recoverable + * mixed-length one (Roland BLE MIDI quirk); give up rather + * than expose mis-sliced characteristics. + */ success = false; goto done; } - last_handle = get_le16(pdu + length - data_length); /* * If last handle is lower from previous start handle then it is smth