From b94d6e53a1e6c816dacd4ad195496c79ff4e47c3 Mon Sep 17 00:00:00 2001 From: Pauli Virtanen Date: Sun, 12 May 2024 12:41:36 +0300 Subject: [PATCH] bluez5: use initial SCO RX wait workaround only for USB controllers Kernel-provided MTU does not work for USB controllers and the correct packet size to send can be known currently only from RX. So we are waiting for RX to get it. The known problem is USB-specific, we shouldn't need the workaround for other transport types. Don't wait for POLLIN for non-USB controllers on connect, but ready things on POLLOUT as usual. For non-USB controllers, pick some sensible packet sizes to use initially, before we switch to same size as for RX. --- spa/plugins/bluez5/backend-native.c | 24 +++++++++++----- spa/plugins/bluez5/bluez5-dbus.c | 5 +--- spa/plugins/bluez5/defs.h | 2 +- spa/plugins/bluez5/sco-io.c | 43 +++++++++++++++++++++++------ 4 files changed, 54 insertions(+), 20 deletions(-) diff --git a/spa/plugins/bluez5/backend-native.c b/spa/plugins/bluez5/backend-native.c index ecea26d18..968387618 100644 --- a/spa/plugins/bluez5/backend-native.c +++ b/spa/plugins/bluez5/backend-native.c @@ -1733,8 +1733,8 @@ static void sco_event(struct spa_source *source) } } - if (source->rmask & SPA_IO_IN) { - SPA_FLAG_UPDATE(source->mask, SPA_IO_IN, false); + if (source->rmask & (SPA_IO_OUT | SPA_IO_IN)) { + SPA_FLAG_CLEAR(source->mask, SPA_IO_OUT | SPA_IO_IN); spa_loop_update_source(backend->main_loop, source); sco_ready(t); } @@ -1750,15 +1750,25 @@ static void sco_start_source(struct spa_bt_transport *t) td->err = -EINPROGRESS; - /* - * We on purpose wait for POLLIN when connecting (not POLLOUT as usual), to - * indicate ready only after we are sure the device is sending data. - */ td->sco.func = sco_event; td->sco.data = t; td->sco.fd = t->fd; - td->sco.mask = SPA_IO_HUP | SPA_IO_ERR | SPA_IO_IN; + td->sco.mask = SPA_IO_HUP | SPA_IO_ERR; td->sco.rmask = 0; + + switch (t->device->adapter->bus_type) { + case BUS_TYPE_USB: + /* With USB controllers, we have to determine packet size from incoming + * packets before we can send. Wait for POLLIN when connecting (not + * POLLOUT as usual). + */ + td->sco.mask |= SPA_IO_IN; + break; + default: + td->sco.mask |= SPA_IO_OUT; + break; + } + spa_loop_add_source(backend->main_loop, &td->sco); } diff --git a/spa/plugins/bluez5/bluez5-dbus.c b/spa/plugins/bluez5/bluez5-dbus.c index 47dabfeb8..63df1ff4c 100644 --- a/spa/plugins/bluez5/bluez5-dbus.c +++ b/spa/plugins/bluez5/bluez5-dbus.c @@ -3219,10 +3219,7 @@ static int spa_bt_transport_stop_volume_timer(struct spa_bt_transport *transport int spa_bt_transport_ensure_sco_io(struct spa_bt_transport *t, struct spa_loop *data_loop) { if (t->sco_io == NULL) { - t->sco_io = spa_bt_sco_io_create(data_loop, - t->fd, - t->read_mtu, - t->write_mtu); + t->sco_io = spa_bt_sco_io_create(t, data_loop, t->monitor->log); if (t->sco_io == NULL) return -ENOMEM; } diff --git a/spa/plugins/bluez5/defs.h b/spa/plugins/bluez5/defs.h index a797073ec..ba9e54aca 100644 --- a/spa/plugins/bluez5/defs.h +++ b/spa/plugins/bluez5/defs.h @@ -571,7 +571,7 @@ struct spa_bt_iso_io; struct spa_bt_sco_io; -struct spa_bt_sco_io *spa_bt_sco_io_create(struct spa_loop *data_loop, int fd, uint16_t read_mtu, uint16_t write_mtu); +struct spa_bt_sco_io *spa_bt_sco_io_create(struct spa_bt_transport *transport, struct spa_loop *data_loop, struct spa_log *log); void spa_bt_sco_io_destroy(struct spa_bt_sco_io *io); void spa_bt_sco_io_set_source_cb(struct spa_bt_sco_io *io, int (*source_cb)(void *userdata, uint8_t *data, int size), void *userdata); void spa_bt_sco_io_set_sink_cb(struct spa_bt_sco_io *io, int (*sink_cb)(void *userdata), void *userdata); diff --git a/spa/plugins/bluez5/sco-io.c b/spa/plugins/bluez5/sco-io.c index cb72953d8..4b4b8f96d 100644 --- a/spa/plugins/bluez5/sco-io.c +++ b/spa/plugins/bluez5/sco-io.c @@ -30,6 +30,10 @@ #include "defs.h" +SPA_LOG_TOPIC_DEFINE_STATIC(log_topic, "spa.bluez5.sco-io"); +#undef SPA_LOG_TOPIC_DEFAULT +#define SPA_LOG_TOPIC_DEFAULT &log_topic + /* We'll use the read rx data size to find the correct packet size for writing, * since kernel might not report it as the socket MTU, see @@ -54,6 +58,7 @@ struct spa_bt_sco_io { uint16_t read_mtu; uint16_t write_mtu; + struct spa_log *log; struct spa_loop *data_loop; struct spa_source source; @@ -103,6 +108,9 @@ static void sco_io_on_ready(struct spa_source *source) goto stop; } + if (res != (int)io->read_size) + spa_log_trace(io->log, "%p: packet size:%d", io, res); + io->read_size = res; if (io->source_cb) { @@ -185,23 +193,42 @@ int spa_bt_sco_io_write(struct spa_bt_sco_io *io, uint8_t *buf, int size) } -struct spa_bt_sco_io *spa_bt_sco_io_create(struct spa_loop *data_loop, - int fd, - uint16_t read_mtu, - uint16_t write_mtu) +struct spa_bt_sco_io *spa_bt_sco_io_create(struct spa_bt_transport *transport, struct spa_loop *data_loop, struct spa_log *log) { struct spa_bt_sco_io *io; + spa_log_topic_init(log, &log_topic); + io = calloc(1, sizeof(struct spa_bt_sco_io)); if (io == NULL) return io; - io->fd = fd; - io->read_mtu = read_mtu; - io->write_mtu = write_mtu; + io->fd = transport->fd; + io->read_mtu = transport->read_mtu; + io->write_mtu = transport->write_mtu; io->data_loop = data_loop; + io->log = log; - io->read_size = 0; + if (transport->device->adapter->bus_type == BUS_TYPE_USB) { + /* For USB we need to wait for RX to know it. Using wrong size doesn't + * work anyway, and may result to errors printed to dmesg if too big. + */ + io->read_size = 0; + } else { + /* Set some sensible initial packet size */ + switch (transport->codec) { + case HFP_AUDIO_CODEC_CVSD: + io->read_size = 48; /* 3ms S16_LE 8000 Hz */ + break; + case HFP_AUDIO_CODEC_MSBC: + case HFP_AUDIO_CODEC_LC3_SWB: + default: + io->read_size = HFP_CODEC_PACKET_SIZE; + break; + } + } + + spa_log_debug(io->log, "%p: initial packet size:%d", io, io->read_size); /* Add the ready callback */ io->source.data = io;