From b2fc3e74576fdbbcf417536d8faa3346a218aa2d Mon Sep 17 00:00:00 2001 From: borine <32966433+borine@users.noreply.github.com> Date: Wed, 20 Jul 2022 16:06:20 +0100 Subject: [PATCH] Improve SCO socket transfer size selection and timing Linux btusb module does not report the required message size for SCO sockets, rather the "MTU" it reports is actually the controller SCO buffer size. However, we can obtain the correct size by reading the first incoming message. SCO packet transfers begin, in both directions, as soon as the transport is acquired, so we are guaranteed to have incoming packets even if the remote microphone is not enabled (provided the adapter is routing incoming SCO through the HCI). This commit adjusts the outgoing message size, if necessary, after the first incoming message has been received. For mSBC streams the timing logic is also modified so that no USB slots are missed if the client runs too slowly. --- .github/spellcheck-wordlist.txt | 2 + src/ba-transport.c | 2 +- src/ba-transport.h | 3 + src/hci.c | 42 ++- src/hci.h | 4 +- src/io.c | 3 + src/ofono.c | 6 +- src/sco.c | 478 +++++++++++++++++++++++++++----- test/test-io.c | 7 + 9 files changed, 453 insertions(+), 94 deletions(-) diff --git a/.github/spellcheck-wordlist.txt b/.github/spellcheck-wordlist.txt index 5380092c3..9a9873cbb 100644 --- a/.github/spellcheck-wordlist.txt +++ b/.github/spellcheck-wordlist.txt @@ -51,6 +51,7 @@ UI UUID UUIDs VBR +WBS XQ # Proper Names @@ -84,6 +85,7 @@ endianness enum getter init +isochronous middleware mutex natively diff --git a/src/ba-transport.c b/src/ba-transport.c index ed87d62d8..714dba3a8 100644 --- a/src/ba-transport.c +++ b/src/ba-transport.c @@ -749,7 +749,7 @@ static int transport_acquire_bt_sco(struct ba_transport *t) { debug("New SCO link: %s: %d", batostr_(&d->addr), fd); - t->mtu_read = t->mtu_write = hci_sco_get_mtu(fd, d->a); + t->mtu_read = t->mtu_write = hci_sco_get_mtu(fd); t->bt_fd = fd; return fd; diff --git a/src/ba-transport.h b/src/ba-transport.h index fb630ad2c..6b63a8a52 100644 --- a/src/ba-transport.h +++ b/src/ba-transport.h @@ -17,6 +17,7 @@ #endif #include +#include #include #include #include @@ -256,6 +257,8 @@ struct ba_transport { /* time-stamp when the SCO link has been closed */ struct timespec closed_at; + /* The SCO socket message size may differ from the reported MTU */ + atomic_uint transfer_bytes; } sco; }; diff --git a/src/hci.c b/src/hci.c index b78ae385f..bb0776f33 100644 --- a/src/hci.c +++ b/src/hci.c @@ -23,9 +23,6 @@ #include #include -#include "ba-adapter.h" -#include "bluealsa-config.h" -#include "shared/bluetooth.h" #include "shared/log.h" /** @@ -112,45 +109,40 @@ int hci_sco_connect(int sco_fd, const bdaddr_t *ba, uint16_t voice) { /** * Get read/write MTU for given SCO socket. * + * Note that the SCO socket "MTU" reported by the Linux kernel is actually + * the response of the HCI HCI_Read_Buffer_Size command. The actual transfer + * size is determined by the underlying bus type, and this is not reported by + * the socket options. We use the socket option MTU as an upper bound for + * choosing the initial buffer size, but this is later optimized when we know + * the actual transfer size by reading the first incoming message. + * * @param sco_fd File descriptor of opened SCO socket. - * @param a The adapter associated with sco_fd. * @return On success this function returns MTU value. Otherwise, 0 is returned and * errno is set to indicate the error. */ -unsigned int hci_sco_get_mtu(int sco_fd, struct ba_adapter *a) { +unsigned int hci_sco_get_mtu(int sco_fd) { + int so_error; struct sco_options options = { 0 }; - struct bt_voice voice = { 0 }; socklen_t len; struct pollfd pfd = { sco_fd, POLLOUT, 0 }; if (poll(&pfd, 1, -1) == -1) warn("Couldn't wait for SCO connection: %s", strerror(errno)); + len = sizeof(so_error); + if (getsockopt(sco_fd, SOL_SOCKET, SO_ERROR, &so_error, &len) == -1) + warn("Couldn't get SCO socket error: %s", strerror(errno)); + if (so_error != 0) { + error("SCO socket connect error: %s", strerror(so_error)); + return 0; + } + len = sizeof(options); if (getsockopt(sco_fd, SOL_SCO, SCO_OPTIONS, &options, &len) == -1) warn("Couldn't get SCO socket options: %s", strerror(errno)); - len = sizeof(voice); - if (getsockopt(sco_fd, SOL_BLUETOOTH, BT_VOICE, &voice, &len) == -1) - warn("Couldn't get SCO voice options: %s", strerror(errno)); - debug("SCO link socket MTU: %d: %u", sco_fd, options.mtu); - /* XXX: It seems, that the MTU value returned by kernel btusb driver - * is incorrect. */ - if ((a->hci.type & 0x0F) == HCI_USB) { - options.mtu = 48; - if (voice.setting == BT_VOICE_TRANSPARENT) { - if (a->chip.manufacturer == 0) - hci_get_version(a->hci.dev_id, &a->chip); - if (!config.disable_realtek_usb_fix && a->chip.manufacturer == BT_COMPID_REALTEK) - options.mtu = 72; - else - options.mtu = 24; - } - debug("USB adjusted SCO MTU: %d: %u", sco_fd, options.mtu); - } - return options.mtu; } diff --git a/src/hci.h b/src/hci.h index ec2391ca2..7de7314fa 100644 --- a/src/hci.h +++ b/src/hci.h @@ -22,8 +22,6 @@ #include /* IWYU pragma: keep */ #include -#include "ba-adapter.h" - int hci_get_version(int dev_id, struct hci_version *ver); /** @@ -39,7 +37,7 @@ int hci_get_version(int dev_id, struct hci_version *ver); int hci_sco_open(int dev_id); int hci_sco_connect(int sco_fd, const bdaddr_t *ba, uint16_t voice); -unsigned int hci_sco_get_mtu(int sco_fd, struct ba_adapter *a); +unsigned int hci_sco_get_mtu(int sco_fd); #define BT_BCM_PARAM_ROUTING_PCM 0x0 #define BT_BCM_PARAM_ROUTING_TRANSPORT 0x1 diff --git a/src/io.c b/src/io.c index 6e5c3a6c9..a5d5a3d00 100644 --- a/src/io.c +++ b/src/io.c @@ -298,6 +298,9 @@ ssize_t io_poll_and_read_bt( return -1; } + if (poll_rv == 0) + return errno = ETIME, -1; + if (fds[0].revents & POLLIN) { /* dispatch incoming event */ io_poll_signal_filter *filter = io->signal.filter != NULL ? diff --git a/src/ofono.c b/src/ofono.c index 8b6bb8741..d2e0e2523 100644 --- a/src/ofono.c +++ b/src/ofono.c @@ -33,6 +33,7 @@ #include #include +#include /* IWYU pragma: keep */ #include #include @@ -129,7 +130,7 @@ static int ofono_acquire_bt_sco(struct ba_transport *t) { #endif t->bt_fd = fd; - t->mtu_read = t->mtu_write = hci_sco_get_mtu(fd, t->d->a); + t->mtu_read = t->mtu_write = hci_sco_get_mtu(fd); ba_transport_set_codec(t, codec); debug("New oFono SCO link (codec: %#x): %d", codec, fd); @@ -826,7 +827,8 @@ static void ofono_agent_new_connection(GDBusMethodInvocation *inv, void *userdat debug("New oFono SCO link (codec: %#x): %d", codec, fd); t->bt_fd = fd; - t->mtu_read = t->mtu_write = hci_sco_get_mtu(fd, t->d->a); + t->mtu_read = t->mtu_write = hci_sco_get_mtu(fd); + ba_transport_set_codec(t, codec); pthread_mutex_unlock(&t->bt_fd_mtx); diff --git a/src/sco.c b/src/sco.c index c8ff0bb77..061fd04da 100644 --- a/src/sco.c +++ b/src/sco.c @@ -31,6 +31,7 @@ #include +#include "audio.h" #include "ba-device.h" #include "ba-transport-pcm.h" #include "bluealsa-config.h" @@ -149,7 +150,8 @@ static void *sco_dispatcher_thread(struct ba_adapter *a) { pthread_mutex_lock(&t->bt_fd_mtx); t->bt_fd = fd; - t->mtu_read = t->mtu_write = hci_sco_get_mtu(fd, a); + t->mtu_read = t->mtu_write = hci_sco_get_mtu(fd); + fd = -1; pthread_mutex_unlock(&t->bt_fd_mtx); @@ -235,23 +237,35 @@ static void *sco_cvsd_enc_thread(struct ba_transport_pcm *t_pcm) { struct ba_transport_thread *th = t_pcm->th; struct io_poll io = { .timeout = -1 }; - const size_t mtu_samples = t->mtu_write / sizeof(int16_t); - const size_t mtu_write = t->mtu_write; - ffb_t buffer = { 0 }; pthread_cleanup_push(PTHREAD_CLEANUP(ffb_free), &buffer); - /* define a bigger buffer to enhance read performance */ - if (ffb_init_int16_t(&buffer, mtu_samples * 4) == -1) { + /* The buffer will be resized once the SCO transfer size is known. We + * choose a small value initially (10ms) to ensure that we discard as little + * audio as possible. */ + if (ffb_init_int16_t(&buffer, 80) == -1) { error("Couldn't create data buffer: %s", strerror(errno)); goto fail_init; } debug_transport_pcm_thread_loop(t_pcm, "START"); - for (ba_transport_thread_state_set_running(th);;) { + ba_transport_thread_state_set_running(th); + + /* The message size of the BT SCO socket */ + uint16_t transfer_bytes = 0; - ssize_t samples = ffb_len_in(&buffer); - switch (samples = io_poll_and_read_pcm(&io, t_pcm, buffer.tail, samples)) { + /* The number of PCM samples equivalent to one BT transfer. */ + unsigned int transfer_samples = 0; + + /* use blocking I/O on FIFO until SCO transfer size is known. */ + ssize_t samples = 0; + size_t start_threshold = SIZE_MAX; + bool mtu_optimized = false; + + asrsync_init(&io.asrs, t_pcm->sampling); + + for (;;) { + switch (samples = io_poll_and_read_pcm(&io, t_pcm, buffer.tail, ffb_len_in(&buffer))) { case -1: if (errno == ESTALE) { ffb_rewind(&buffer); @@ -265,32 +279,73 @@ static void *sco_cvsd_enc_thread(struct ba_transport_pcm *t_pcm) { } ffb_seek(&buffer, samples); + + if (!mtu_optimized) { + transfer_bytes = t->sco.transfer_bytes; + if (transfer_bytes > 0) { + debug("SCO message size %d: %d", th->bt_fd, transfer_bytes); + mtu_optimized = true; + transfer_samples = transfer_bytes / sizeof(uint16_t); + start_threshold = transfer_samples + 40; + } + } + + t_pcm->delay = ffb_len_out(&buffer); + + if (ffb_len_out(&buffer) > start_threshold) + break; + + /* Ensure we have room to read at least 10ms on next iteration */ + if (ffb_len_in(&buffer) < 80) + ffb_shift(&buffer, 80); + + /* keep data transfer at a constant bit rate */ + asrsync_sync(&io.asrs, samples); + + } + + /* We can now safely resize the pcm buffer to optimize read performance */ + size_t bufsize = transfer_samples * 4; + if (buffer.nmemb < bufsize) + ffb_init_int16_t(&buffer, bufsize); + + for (;;) { + samples = ffb_len_out(&buffer); const int16_t *input = buffer.data; size_t input_samples = samples; - while (input_samples >= mtu_samples) { + while (input_samples >= transfer_samples) { ssize_t ret; - if ((ret = io_bt_write(th, input, mtu_write)) <= 0) { + if ((ret = io_bt_write(th, input, transfer_bytes)) <= 0) { if (ret == -1) error("BT write error: %s", strerror(errno)); goto exit; } - input += mtu_samples; - input_samples -= mtu_samples; + input += transfer_samples; + input_samples -= transfer_samples; /* keep data transfer at a constant bit rate */ - asrsync_sync(&io.asrs, mtu_samples); - /* update busy delay (encoding overhead) */ - t_pcm->delay = asrsync_get_busy_usec(&io.asrs) / 100; - + asrsync_sync(&io.asrs, transfer_samples); } ffb_shift(&buffer, samples - input_samples); + while ((samples = io_poll_and_read_pcm(&io, t_pcm, buffer.tail, ffb_len_in(&buffer))) <= 0) { + if (samples == -1) { + if (errno == ESTALE) { + ffb_rewind(&buffer); + continue; + } + error("PCM poll and read error: %s", strerror(errno)); + } + ba_transport_stop_if_no_clients(t); + } + + ffb_seek(&buffer, samples); } exit: @@ -308,7 +363,15 @@ static void *sco_cvsd_dec_thread(struct ba_transport_pcm *t_pcm) { struct ba_transport *t = t_pcm->t; struct ba_transport_thread *th = t_pcm->th; - struct io_poll io = { .timeout = -1 }; + + /* SCO transport should deliver audio packets as soon as it is acquired. + * However, if the adapter does not route incoming SCO to the HCI we will + * never see those packets. The encoder thread is blocked until this thread + * signals that the first packet has been received so we set a timeout for + * the first read only to ensure we can still send that signal. HFP/HSP + * specs do not define a maximum latency for CVSD, so we choose an arbitrary + * timeout of 100 ms. */ + struct io_poll io = { .timeout = 100 }; const size_t mtu_samples = t->mtu_read / sizeof(int16_t); const size_t mtu_samples_multiplier = 2; @@ -321,14 +384,40 @@ static void *sco_cvsd_dec_thread(struct ba_transport_pcm *t_pcm) { goto fail_ffb; } + bool mtu_optimized = false; + debug_transport_pcm_thread_loop(t_pcm, "START"); for (ba_transport_thread_state_set_running(th);;) { ssize_t len = ffb_blen_in(&buffer); - if ((len = io_poll_and_read_bt(&io, th, buffer.tail, len)) == -1) + if ((len = io_poll_and_read_bt(&io, th, buffer.tail, len)) == -1) { + if (errno == ETIME) { + debug("SCO decoder timeout"); + io.timeout = -1; + /* Use socket SCO options MTU */ + len = (ssize_t) t->mtu_write; + /* The MTU value returned by kernel btusb driver is incorrect. + * We use the USB Alt-2 setting of 48 bytes which is correct in + * the typical case of a single SCO connection in use at a time. + */ + if ((t->d->a->hci.type & 0x0F) == HCI_USB) { + len = 48; + debug("USB adjusted SCO MTU: %d: %zd", th->bt_fd, len); + } + t->sco.transfer_bytes = (unsigned int) len; + mtu_optimized = true; + continue; + } error("BT poll and read error: %s", strerror(errno)); + + } else if (len == 0) goto exit; + else if (!mtu_optimized) { + t->sco.transfer_bytes = (unsigned int) len; + io.timeout = -1; + mtu_optimized = true; + } if ((size_t)len == buffer.nmemb * buffer.size) { debug("Resizing CVSD read buffer: %zd -> %zd", @@ -366,6 +455,83 @@ static void *sco_cvsd_dec_thread(struct ba_transport_pcm *t_pcm) { } #if ENABLE_MSBC +/** + * Handle all pending transport thread signals (non-blocking). + */ +static enum ba_transport_thread_signal sco_msbc_poll_transport_thread_signals(struct ba_transport_pcm *pcm) { + + struct ba_transport_thread *th = pcm->th; + struct pollfd pfd = { th->pipe[0], POLLIN, 0 }; + enum ba_transport_thread_signal ret = BA_TRANSPORT_THREAD_SIGNAL_PING; + + for (;;) { + int err; + if ((err = poll(&pfd, 1, 0)) == 0) + break; + else if (err == -1) { + if (errno == EINTR) + continue; + break; + } + + if (pfd.revents & POLLIN) { + enum ba_transport_thread_signal signal; + ba_transport_thread_signal_recv(th, &signal); + switch (signal) { + case BA_TRANSPORT_THREAD_SIGNAL_PCM_SYNC: + ret = signal; + break; + case BA_TRANSPORT_THREAD_SIGNAL_PCM_DROP: + return signal; + default: + break; + } + } + } + + return ret; +} +#endif + +#if ENABLE_MSBC + +/** + * Read data from the PCM FIFO (non-blocking). + * + * @return on success number of samples read, 0 if none are available. + * on failure or FIFO closed -1. + */ +static ssize_t sco_msbc_read_pcm( + struct ba_transport_pcm *pcm, + void *buffer, + size_t samples) { + + ssize_t samples_read; + if ((samples_read = io_pcm_read(pcm, buffer, samples)) <= 0) { + if (samples_read == 0) { + samples_read = -1; + errno = EBADF; + } + else if (errno == EAGAIN) + samples_read = 0; + else if (errno != EBADF) + error("PCM read error: %s", strerror(errno)); + } + + return samples_read; +} +#endif + +#if ENABLE_MSBC + +/** + * SCO over a USB HCI requires strict timing of audio data packets to meet the + * demands of a USB isochronous endpoint. This is crucial for mSBC streams + * because the USB packet boundaries do not align with the mSBC frame + * boundaries. So to support USB devices correctly we use the delay timer to + * regulate writes to the BT socket, and make all other I/O non-blocking to + * ensure that we do not miss a USB deadline. + */ static void *sco_msbc_enc_thread(struct ba_transport_pcm *t_pcm) { pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, NULL); @@ -373,8 +539,19 @@ static void *sco_msbc_enc_thread(struct ba_transport_pcm *t_pcm) { struct ba_transport *t = t_pcm->t; struct ba_transport_thread *th = t_pcm->th; + /* use blocking I/O on FIFO until SCO transfer size is known. */ struct io_poll io = { .timeout = -1 }; - const size_t mtu_write = t->mtu_write; + + /* The message size of the BT SCO socket */ + uint16_t transfer_bytes = 0; + /* The number of PCM samples equivalent to one BT transfer. */ + unsigned int transfer_samples = 0; + /* The number of samples required to guarantee that one BT transfer can be + * produced. */ + unsigned int pcm_threshold = 0; + /* The minimum fill level of the PCM buffer to avoid underrun. */ + size_t start_threshold = SIZE_MAX; + ssize_t samples = 0; struct esco_msbc msbc = { .initialized = false }; pthread_cleanup_push(PTHREAD_CLEANUP(msbc_finish), &msbc); @@ -385,62 +562,183 @@ static void *sco_msbc_enc_thread(struct ba_transport_pcm *t_pcm) { } debug_transport_pcm_thread_loop(t_pcm, "START"); - for (ba_transport_thread_state_set_running(th);;) { + ba_transport_thread_state_set_running(th); - ssize_t samples = ffb_len_in(&msbc.pcm); - switch (samples = io_poll_and_read_pcm(&io, t_pcm, msbc.pcm.tail, samples)) { - case -1: - if (errno == ESTALE) { - /* reinitialize mSBC encoder */ - msbc_init(&msbc); - continue; + bool mtu_optimized = false; + + asrsync_init(&io.asrs, t_pcm->sampling); + + for (;;) { + + if ((samples = io_poll_and_read_pcm(&io, t_pcm, + msbc.pcm.tail, ffb_len_in(&msbc.pcm))) <= 0) { + if (samples == -1) { + if (errno == ESTALE) { + /* reinitialize mSBC encoder */ + msbc_init(&msbc); + continue; + } + error("PCM poll and read error: %s", strerror(errno)); } - error("PCM poll and read error: %s", strerror(errno)); - /* fall-through */ - case 0: ba_transport_stop_if_no_clients(t); continue; } ffb_seek(&msbc.pcm, samples); + t_pcm->delay = ffb_len_out(&msbc.pcm) * 10000 / t_pcm->sampling; + + if (!mtu_optimized) { + transfer_bytes = t->sco.transfer_bytes; + if (transfer_bytes > 0) { + debug("SCO message size %d: %d", th->bt_fd, transfer_bytes); + mtu_optimized = true; + transfer_samples = transfer_bytes * MSBC_CODESAMPLES / sizeof(struct esco_msbc_frame); + /* The number of msbc frames required to guarantee that one BT + * transfer is available. */ + unsigned int msbc_frames = 1; + while (msbc_frames * sizeof(struct esco_msbc_frame) < transfer_bytes) + msbc_frames++; + pcm_threshold = msbc_frames * MSBC_CODESAMPLES; + + start_threshold = pcm_threshold + transfer_samples; + } + } + + /* keep data transfer at a constant bit rate */ + asrsync_sync(&io.asrs, samples); + + if ((size_t)(samples = ffb_len_out(&msbc.pcm)) >= start_threshold) { + /* To minimize latency, discard oldest samples to reduce buffered + * quantity to the start_threshold. */ + ffb_shift(&msbc.pcm, samples - start_threshold); + t_pcm->delay = start_threshold * 10000 / t_pcm->sampling; + break; + } + + /* Ensure we have room to read at least 10ms on next iteration */ + if (ffb_len_in(&msbc.pcm) < 160) + ffb_shift(&msbc.pcm, 160); + + } - while (ffb_len_out(&msbc.pcm) >= MSBC_CODESAMPLES) { + /* We now regulate the rate at which transfers to the SCO socket occur, so + * we must reset the timer. */ + asrsync_init(&io.asrs, t_pcm->sampling); + bool stopping = false; + bool draining = false; + size_t drain_samples = 0; + + for (;;) { + + /* Encode sufficient samples to guarantee the next transfer. We have + * guaranteed that enough samples are available before reaching this + * point. */ + while (ffb_len_out(&msbc.data) < transfer_bytes) { int err; if ((err = msbc_encode(&msbc)) < 0) { error("mSBC encoding error: %s", sbc_strerror(err)); - break; + goto exit; } + } - uint8_t *data = msbc.data.data; - size_t data_len = ffb_blen_out(&msbc.data); + /* Write out the next transfer. */ + uint8_t *data = msbc.data.data; + size_t len; + ssize_t written = 0; + for (len = transfer_bytes; len > 0; data += written, len -= written) { + if ((written = io_bt_write(th, data, len)) <= 0) { + if (written == -1) + error("BT write error: %s", strerror(errno)); + goto exit; + } + } - while (data_len >= mtu_write) { + /* Move unprocessed data to the front of our linear buffer. */ + ffb_shift(&msbc.data, transfer_bytes); + msbc.frames = 0; - ssize_t len; - if ((len = io_bt_write(th, data, mtu_write)) <= 0) { - if (len == -1) - error("BT write error: %s", strerror(errno)); - goto exit; - } + pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL); - data += len; - data_len -= len; + /* keep data transfer at a constant bit rate. This is the only thread + * cancellation point within the encoder loop */ + asrsync_sync(&io.asrs, transfer_samples); + pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, NULL); + + /* Check if any control commands were received while we were sleeping */ + switch (sco_msbc_poll_transport_thread_signals(t_pcm)) { + case BA_TRANSPORT_THREAD_SIGNAL_PCM_DROP: + io_pcm_flush(t_pcm); + /* Rewinding the msbc data buffer may result in an incomplete frame + * having been sent. So to maintain the integrity of the stream we + * allow already encoded audio to play out, and only discard the + * un-encoded PCM samples. */ + ffb_rewind(&msbc.pcm); + draining = false; + break; + case BA_TRANSPORT_THREAD_SIGNAL_PCM_SYNC: + if (!draining) { + /* Make a note of how much audio is buffered at the time the + * DRAIN request is received. We will signal the transport + * thread when this number of samples has been written to BT. */ + draining = true; + drain_samples = io.asrs.frames + ffb_len_out(&msbc.pcm) + 2 * ffb_len_out(&msbc.data); } + break; + default: + break; + } - /* keep data transfer at a constant bit rate */ - asrsync_sync(&io.asrs, msbc.frames * MSBC_CODESAMPLES); - /* update busy delay (encoding overhead) */ - t_pcm->delay = asrsync_get_busy_usec(&io.asrs) / 100; + if (ffb_len_in(&msbc.pcm) > 0) { + struct pollfd pfd[] = {{ t_pcm->fd, POLLIN, 0 }}; + if (poll(pfd, 1, 0) == 1) { + if ((samples = sco_msbc_read_pcm(t_pcm, msbc.pcm.tail, ffb_len_in(&msbc.pcm))) < 0) { + if (errno == EBADF) { + if (!stopping) { + ba_transport_stop_if_no_clients(t); + stopping = true; + } + } + samples = 0; + } + else { + stopping = false; + ffb_seek(&msbc.pcm, samples); + + /* It is possible that we received a DRAIN request before we had + * read all samples from the FIFO. So we apply any necessary + * adjustment here. */ + if (draining) + drain_samples += samples; + } + } + } - /* Move unprocessed data to the front of our linear - * buffer and clear the mSBC frame counter. */ - ffb_shift(&msbc.data, ffb_blen_out(&msbc.data) - data_len); - msbc.frames = 0; + size_t pcm_avail = ffb_len_out(&msbc.pcm); + size_t data_avail = ffb_len_out(&msbc.data); + if (data_avail < transfer_bytes && pcm_avail < pcm_threshold) { + + /* client has stopped sending audio data, so first check if we have + * completed the drain of buffered samples. */ + if (draining && io.asrs.frames >= drain_samples) { + pthread_mutex_lock(&t_pcm->mutex); + t_pcm->synced = true; + pthread_mutex_unlock(&t_pcm->mutex); + pthread_cond_signal(&t_pcm->cond); + draining = false; + } + /* We must continue to send valid msbc frames until the transport is + * released, so we insert silence into the stream. We apply just + * enough silence to bring the buffer level up to the + * start_threshold to minimize the interruption in case the client + * later sends more samples. */ + const size_t required_frames = 1 + (transfer_bytes - data_avail) / sizeof(struct esco_msbc); + const size_t padding = pcm_threshold + (required_frames * MSBC_CODESAMPLES) - pcm_avail; + audio_silence_s16_2le((int16_t *)msbc.pcm.tail, padding , 1, true, false); + ffb_seek(&msbc.pcm, padding); } - } exit: @@ -460,7 +758,14 @@ static void *sco_msbc_dec_thread(struct ba_transport_pcm *t_pcm) { struct ba_transport *t = t_pcm->t; struct ba_transport_thread *th = t_pcm->th; - struct io_poll io = { .timeout = -1 }; + + /* SCO transport should deliver audio packets as soon as it is acquired. + * However, if the adapter does not route incoming SCO to the HCI we will + * never see those packets. The encoder thread is blocked until this thread + * signals that the first packet has been received so we set a timeout for + * the first read only to ensure we can still send that signal. We choose an + * arbitrary timeout of 100 ms. */ + struct io_poll io = { .timeout = 100 }; struct esco_msbc msbc = { .initialized = false }; pthread_cleanup_push(PTHREAD_CLEANUP(msbc_finish), &msbc); @@ -470,15 +775,57 @@ static void *sco_msbc_dec_thread(struct ba_transport_pcm *t_pcm) { goto fail_msbc; } + bool mtu_optimized = false; debug_transport_pcm_thread_loop(t_pcm, "START"); for (ba_transport_thread_state_set_running(th);;) { - ssize_t len = ffb_blen_in(&msbc.data); - if ((len = io_poll_and_read_bt(&io, th, msbc.data.tail, len)) == -1) + ssize_t len = io_poll_and_read_bt(&io, th, msbc.data.tail, ffb_blen_in(&msbc.data)); + switch (len) { + case -1: { + if (errno == ETIME) { + debug("SCO decoder read timeout"); + io.timeout = -1; + /* Incoming SCO is not being routed through the HCI. Use socket + * SCO options MTU. */ + size_t mtu = t->mtu_write; + if ((t->d->a->hci.type & 0x0F) == HCI_USB) { + /* The MTU value returned by kernel btusb driver is + * incorrect. We use the USB Alt-1 setting of 24 bytes + * since this is correct for the majority of currently + * available WBS-capable adapters, except for Realtek + * adapters which use Alt-3 (72 bytes). */ + mtu = 24; + struct ba_adapter *a = t->d->a; + if (a->chip.manufacturer == 0) + hci_get_version(a->hci.dev_id, &a->chip); + if (!config.disable_realtek_usb_fix && a->chip.manufacturer == BT_COMPID_REALTEK) + mtu = 72; + debug("USB adjusted SCO MTU: %d: %zd", th->bt_fd, mtu); + } + else { + /* Transfer whole mSBC frames if possible. */ + if (mtu >= sizeof(esco_msbc_frame_t)) + mtu = sizeof(esco_msbc_frame_t); + debug("Adjusted SCO MTU: %d: %zd", th->bt_fd, mtu); + } + t->sco.transfer_bytes = (unsigned int) mtu; + mtu_optimized = true; + continue; + } error("BT poll and read error: %s", strerror(errno)); - else if (len == 0) + + break; + } + case 0: goto exit; + default: + if (!mtu_optimized) { + t->sco.transfer_bytes = (unsigned int) len; + io.timeout = -1; + mtu_optimized = true; + } + } if (!ba_transport_pcm_is_active(t_pcm)) continue; @@ -486,12 +833,8 @@ static void *sco_msbc_dec_thread(struct ba_transport_pcm *t_pcm) { ffb_seek(&msbc.data, len); int err; - /* Process data until there is no more mSBC frames to decode. This loop - * ensures that for MTU values bigger than the mSBC frame size, the input - * buffer will not fill up causing short reads and mSBC frame losses. */ - while ((err = msbc_decode(&msbc)) > 0) - continue; - if (err < 0) { +again: + if ((err = msbc_decode(&msbc)) < 0) { error("mSBC decoding error: %s", sbc_strerror(err)); continue; } @@ -508,6 +851,15 @@ static void *sco_msbc_dec_thread(struct ba_transport_pcm *t_pcm) { ffb_shift(&msbc.pcm, samples); + /* We must ensure that we can always read a full message from the BT + * socket on each call to io_poll_and_read_bt(). Otherwise the unread + * fraction left in the controller may be overwritten by the next + * incoming SCO packet before we have chance to read it. So if our + * decoder buffer has insufficient space we must decode another frame + * before continuing. */ + if (ffb_blen_in(&msbc.data) < t->mtu_read) + goto again; + } exit: diff --git a/test/test-io.c b/test/test-io.c index 77f7a5903..b8479e989 100644 --- a/test/test-io.c +++ b/test/test-io.c @@ -1064,6 +1064,7 @@ CK_START_TEST(test_sco_cvsd) { BA_TRANSPORT_PROFILE_HSP_AG, "/path/sco/cvsd"); t1->mtu_read = t1->mtu_write = t2->mtu_read = t2->mtu_write = 48; + t1->sco.transfer_bytes = 48; test_io(t1, t2, sco_enc_thread, test_io_thread_dump_bt, 600); test_io(t1, t2, test_io_thread_dump_pcm, sco_dec_thread, 600); @@ -1087,6 +1088,12 @@ CK_START_TEST(test_sco_msbc) { ba_transport_set_codec(t2, HFP_CODEC_MSBC); t1->mtu_read = t1->mtu_write = t2->mtu_read = t2->mtu_write = 24; + /* The test PCM source closes as soon as it has written the test samples. + * For mSBC this can be a problem, as the encoder may then stop before it + * has written all encoded data if the SCO message size is less than the + * the size of a single mSBC frame. Therefore we test using a SCO message + * size that is not less than 60. */ + t1->sco.transfer_bytes = 72; test_io(t1, t2, sco_enc_thread, test_io_thread_dump_bt, 600); test_io(t1, t2, test_io_thread_dump_pcm, sco_dec_thread, 600);