From 4c7068b5fffd827a4802ddcab6546d25d577f86c Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Wed, 18 Sep 2024 00:14:26 +0200 Subject: [PATCH 01/15] Do not lock any mutex when getting transport name --- src/ba-transport.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/ba-transport.c b/src/ba-transport.c index 74df1dd6e..2b627be8d 100644 --- a/src/ba-transport.c +++ b/src/ba-transport.c @@ -681,8 +681,13 @@ struct ba_transport *ba_transport_new_midi( /** * Get BlueALSA transport type debug name. * + * It is guaranteed that this function will not lock/unlock any mutex at a + * cost of potential race condition when retrieving the name. This function + * is intended to be used for debugging purposes only. + * * @param t Transport structure. * @return Human-readable string. */ +__attribute__ ((no_sanitize("thread"))) const char *ba_transport_debug_name( const struct ba_transport *t) { switch (t->profile) { @@ -692,7 +697,7 @@ const char *ba_transport_debug_name( case BA_TRANSPORT_PROFILE_A2DP_SINK: return t->a2dp.sep->name; case BA_TRANSPORT_PROFILE_HFP_HF: - switch (ba_transport_get_codec(t)) { + switch (t->codec_id) { case HFP_CODEC_UNDEFINED: return "HFP Hands-Free (...)"; case HFP_CODEC_CVSD: @@ -703,7 +708,7 @@ const char *ba_transport_debug_name( return "HFP Hands-Free (LC3-SWB)"; } break; case BA_TRANSPORT_PROFILE_HFP_AG: - switch (ba_transport_get_codec(t)) { + switch (t->codec_id) { case HFP_CODEC_UNDEFINED: return "HFP Audio Gateway (...)"; case HFP_CODEC_CVSD: @@ -722,8 +727,7 @@ const char *ba_transport_debug_name( return "MIDI"; #endif } - debug("Unknown transport: profile:%#x codec:%#x", - t->profile, ba_transport_get_codec(t)); + debug("Unknown transport: profile:%#x codec:%#x", t->profile, t->codec_id); return "N/A"; } #endif From cd371d06361d2c56f28d3ab549719b6b2d7e77bc Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Fri, 20 Sep 2024 18:06:59 +0200 Subject: [PATCH 02/15] Fix some warnings reported by strict ISO C check --- src/a2dp-lc3plus.c | 2 +- src/bluealsa-dbus.c | 2 +- src/bluez.c | 4 ++-- src/dbus-codegen.py | 2 +- test/test-io.c | 2 +- utils/a2dpconf.c | 22 ++++++++++++---------- 6 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/a2dp-lc3plus.c b/src/a2dp-lc3plus.c index 98e84489e..b402c2edd 100644 --- a/src/a2dp-lc3plus.c +++ b/src/a2dp-lc3plus.c @@ -192,7 +192,6 @@ void *a2dp_lc3plus_enc_thread(struct ba_transport_pcm *t_pcm) { struct io_poll io = { .timeout = -1 }; const a2dp_lc3plus_t *configuration = &t->a2dp.configuration.lc3plus; - const int lc3plus_frame_dms = a2dp_lc3plus_get_frame_dms(configuration); const unsigned int channels = t_pcm->channels; const unsigned int rate = t_pcm->rate; const unsigned int rtp_ts_clockrate = 96000; @@ -211,6 +210,7 @@ void *a2dp_lc3plus_enc_thread(struct ba_transport_pcm *t_pcm) { pthread_cleanup_push(PTHREAD_CLEANUP(a2dp_lc3plus_enc_free), handle); + const int lc3plus_frame_dms = a2dp_lc3plus_get_frame_dms(configuration); if ((err = lc3plus_enc_set_frame_dms(handle, lc3plus_frame_dms)) != LC3PLUS_OK) { error("Couldn't set frame length: %s", lc3plus_strerror(err)); goto fail_setup; diff --git a/src/bluealsa-dbus.c b/src/bluealsa-dbus.c index 14f8a5bef..dd3cb4073 100644 --- a/src/bluealsa-dbus.c +++ b/src/bluealsa-dbus.c @@ -727,7 +727,7 @@ static void bluealsa_pcm_select_codec(GDBusMethodInvocation *inv, void *userdata * selection on the transport level. */ pthread_mutex_lock(&t->codec_select_client_mtx); - a2dp_t a2dp_configuration = {}; + a2dp_t a2dp_configuration = { 0 }; size_t a2dp_configuration_size = 0; unsigned int channels = 0; unsigned int rate = 0; diff --git a/src/bluez.c b/src/bluez.c index e82fd4df5..eaaca0c36 100644 --- a/src/bluez.c +++ b/src/bluez.c @@ -418,7 +418,7 @@ static void bluez_endpoint_select_configuration(GDBusMethodInvocation *inv, void const struct a2dp_sep *sep = dbus_obj->sep; const void *data; - a2dp_t capabilities = {}; + a2dp_t capabilities = { 0 }; size_t size = 0; params = g_variant_get_child_value(params, 0); @@ -454,7 +454,7 @@ static void bluez_endpoint_set_configuration(GDBusMethodInvocation *inv, void *u enum bluez_a2dp_transport_state state = 0xFFFF; char *device_path = NULL; - a2dp_t configuration = {}; + a2dp_t configuration = { 0 }; uint16_t volume = 127; uint16_t delay = 150; diff --git a/src/dbus-codegen.py b/src/dbus-codegen.py index c5f03c5b4..bb1763c7b 100755 --- a/src/dbus-codegen.py +++ b/src/dbus-codegen.py @@ -63,7 +63,7 @@ }} {struct}SkeletonClass; G_DEFINE_TYPE({struct}Skeleton, {func}_skeleton, - G_TYPE_DBUS_INTERFACE_SKELETON); + G_TYPE_DBUS_INTERFACE_SKELETON) static void {func}_skeleton_class_init({struct}SkeletonClass *ifc) {{ GDBusInterfaceSkeletonClass *ifc_ = G_DBUS_INTERFACE_SKELETON_CLASS(ifc); diff --git a/test/test-io.c b/test/test-io.c index 115ce6f35..99d6617cb 100644 --- a/test/test-io.c +++ b/test/test-io.c @@ -239,7 +239,7 @@ static void *pcm_write_frames_sndfile_async(void *userdata) { struct pollfd pfds[] = {{ pcm->fd, POLLOUT, 0 }}; SNDFILE *sf = NULL; - SF_INFO sf_info = {}; + SF_INFO sf_info = { 0 }; if ((sf = sf_open(input_pcm_file, SFM_READ, &sf_info)) == NULL) { error("Couldn't load input audio file: %s", sf_strerror(NULL)); ck_assert_ptr_ne(sf, NULL); diff --git a/utils/a2dpconf.c b/utils/a2dpconf.c index e00e40cff..c43502aa3 100644 --- a/utils/a2dpconf.c +++ b/utils/a2dpconf.c @@ -363,21 +363,21 @@ static void dump_aptx_ll(const void *blob, size_t size) { } static void dump_faststream(const void *blob, size_t size) { - const a2dp_faststream_t *faststream = blob; - if (check_blob_size(sizeof(*faststream), size) == -1) + const a2dp_faststream_t *fs = blob; + if (check_blob_size(sizeof(*fs), size) == -1) return; printf("FastStream {\n", bintohex(blob, size)); - printf_vendor(&faststream->info); + printf_vendor(&fs->info); printf("" " direction:8 =%s%s\n" " sample-rate-voice:8 =%s\n" " sample-rate-music:8 =%s%s\n" "}\n", - faststream->direction & FASTSTREAM_DIRECTION_MUSIC ? " Music" : "", - faststream->direction & FASTSTREAM_DIRECTION_VOICE ? " Voice" : "", - faststream->sampling_freq_voice & FASTSTREAM_SAMPLING_FREQ_VOICE_16000 ? " 16000" : "", - faststream->sampling_freq_music & FASTSTREAM_SAMPLING_FREQ_MUSIC_48000 ? " 48000" : "", - faststream->sampling_freq_music & FASTSTREAM_SAMPLING_FREQ_MUSIC_44100 ? " 44100" : ""); + fs->direction & FASTSTREAM_DIRECTION_MUSIC ? " Music" : "", + fs->direction & FASTSTREAM_DIRECTION_VOICE ? " Voice" : "", + fs->sampling_freq_voice & FASTSTREAM_SAMPLING_FREQ_VOICE_16000 ? " 16000" : "", + fs->sampling_freq_music & FASTSTREAM_SAMPLING_FREQ_MUSIC_48000 ? " 48000" : "", + fs->sampling_freq_music & FASTSTREAM_SAMPLING_FREQ_MUSIC_44100 ? " 44100" : ""); } static void dump_lc3plus(const void *blob, size_t size) { @@ -537,11 +537,13 @@ static void dump_lhdc_v5(const void *blob, size_t size) { return; printf("LHDC v5 {\n", bintohex(blob, size)); printf_vendor(&lhdc->info); + const void *data = (uint8_t *)lhdc + sizeof(lhdc->info); + size_t data_size = sizeof(*lhdc) - sizeof(lhdc->info); printf("" " data:%zu = hex:%s\n" "}\n", - sizeof(*lhdc) - sizeof(lhdc->info), - bintohex(blob + sizeof(lhdc->info), sizeof(*lhdc) - sizeof(lhdc->info))); + data_size, + bintohex(data, data_size)); } static void dump_opus(const void *blob, size_t size) { From 3e2e3453f3bf2ea92563f330373421444a22e711 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Tue, 24 Sep 2024 20:12:36 +0200 Subject: [PATCH 03/15] Enhance generated audio dumps in the IO test This commit adds sndalign tool which finds the best alignment between two audio files. It can be used to check codec intrinsic delay by aligning source and encoded/decoded audio files. --- configure.ac | 2 + test/Makefile.am | 4 + test/inc/btd.inc | 15 ++- test/inc/check.inc | 2 - test/inc/mock.inc | 6 -- test/inc/sine.inc | 31 ++++--- test/mock/mock-bluealsa.c | 4 +- test/sndalign.c | 111 +++++++++++++++++++++++ test/test-alsa-pcm.c | 2 +- test/test-at.c | 18 ++-- test/test-audio.c | 32 +++---- test/test-io.c | 186 ++++++++++++++++++++++++-------------- test/test-lc3-swb.c | 4 +- test/test-msbc.c | 4 +- test/test-utils.c | 8 +- 15 files changed, 298 insertions(+), 131 deletions(-) create mode 100644 test/sndalign.c diff --git a/configure.ac b/configure.ac index 6af51cde4..e222cb544 100644 --- a/configure.ac +++ b/configure.ac @@ -339,12 +339,14 @@ AM_COND_IF([ENABLE_MANPAGES], [ AC_ARG_ENABLE([test], [AS_HELP_STRING([--enable-test], [enable unit test])]) +AM_CONDITIONAL([HAVE_SNDFILE], [false]) AM_CONDITIONAL([ENABLE_TEST], [test "x$enable_test" = "xyes"]) AM_COND_IF([ENABLE_TEST], [ AC_SEARCH_LIBS([dlsym], [dl], [], [AC_MSG_ERROR([unable to find dlsym() function])]) PKG_CHECK_MODULES([CHECK], [check >= 0.11.0]) PKG_CHECK_MODULES([SNDFILE], [sndfile >= 1.0], + AM_CONDITIONAL([HAVE_SNDFILE], [true]) AC_DEFINE([HAVE_SNDFILE], [1], [Define to 1 if you have the sndfile library.]), [:]) ]) diff --git a/test/Makefile.am b/test/Makefile.am index 493ca42ef..40980cdf7 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -56,6 +56,10 @@ TESTS += test-msbc check_PROGRAMS += test-msbc endif +if HAVE_SNDFILE +check_PROGRAMS += sndalign +endif + check_LTLIBRARIES = \ libaloader.la libaloader_la_LDFLAGS = \ diff --git a/test/inc/btd.inc b/test/inc/btd.inc index f26d4aab7..7840841ef 100644 --- a/test/inc/btd.inc +++ b/test/inc/btd.inc @@ -10,8 +10,6 @@ * */ -#pragma once - #if HAVE_CONFIG_H # include #endif @@ -203,7 +201,7 @@ fail: return NULL; } -static const char *transport_to_fname(struct ba_transport *t) { +static const char *transport_to_fname(const struct ba_transport *t) { const char *profile = NULL; const char *codec = NULL; @@ -245,6 +243,13 @@ static const char *transport_to_fname(struct ba_transport *t) { return buffer; } +static const char *transport_pcm_to_fname(const struct ba_transport_pcm *t_pcm) { + static char buffer[64]; + snprintf(buffer, sizeof(buffer), "s%u-%u-%uc", + BA_TRANSPORT_PCM_FORMAT_WIDTH(t_pcm->format), t_pcm->rate, t_pcm->channels); + return buffer; +} + /** * Dump incoming BT data to a BT dump file. */ void *bt_dump_io_thread(struct ba_transport_pcm *t_pcm) { @@ -259,8 +264,8 @@ void *bt_dump_io_thread(struct ba_transport_pcm *t_pcm) { struct bt_dump *btd = NULL; char fname[256]; - snprintf(fname, sizeof(fname), "/tmp/bluealsa-%s.btd", - transport_to_fname(t)); + snprintf(fname, sizeof(fname), "/tmp/bluealsa-%s-%s.btd", + transport_to_fname(t), transport_pcm_to_fname(t_pcm)); debug("Creating BT dump file: %s", fname); if ((btd = bt_dump_create(fname, t)) == NULL) { diff --git a/test/inc/check.inc b/test/inc/check.inc index ed2b331a2..99dbd2857 100644 --- a/test/inc/check.inc +++ b/test/inc/check.inc @@ -10,8 +10,6 @@ * */ -#pragma once - #include #include diff --git a/test/inc/mock.inc b/test/inc/mock.inc index dc02db8bd..b787bf694 100644 --- a/test/inc/mock.inc +++ b/test/inc/mock.inc @@ -10,10 +10,6 @@ * */ -#pragma once -#ifndef BLUEALSA_TEST_INC_MOCK_H_ -#define BLUEALSA_TEST_INC_MOCK_H_ - #include #include #include @@ -203,5 +199,3 @@ int spawn_bluealsa_mock(struct spawn_process *sp, const char *service, return 0; } - -#endif diff --git a/test/inc/sine.inc b/test/inc/sine.inc index e38effc70..e3074f979 100644 --- a/test/inc/sine.inc +++ b/test/inc/sine.inc @@ -18,39 +18,40 @@ /** * Generate sine S16_2LE PCM signal. * - * @param buffer Address of the PCM buffer, where the data will be stored. - This buffer has to be big enough to store frames * channels number of + * @param dest Address of the PCM buffer, where the data will be stored. + This buffer has to be big enough to store channels * frames number of PCM samples. + * @param channels Number of channels per PCM frame. The sine wave for each + * channel is phase shifted by PI/3.3. * @param frames The number of PCM frames to generate. - * @param channels Number of channels per PCM frame. - * @param x Sampling argument of the sine function. * @param f Required sine frequency divided by the PCM sample rate. + * @param x Sample counter. * @return Updated x parameter. One may use this value for a next call, in * order to generate smooth sine curve. */ -int snd_pcm_sine_s16_2le(int16_t *buffer, size_t frames, - unsigned int channels, int x, float f) { +size_t snd_pcm_sine_s16_2le(int16_t *dest, unsigned int channels, size_t frames, + float f, size_t x) { for (size_t i = 0; i < frames; x++, i++) for (size_t c = 0; c < channels; c++) - buffer[i * channels + c] = sin(2 * M_PI * f * x + c * M_PI / 3.3) * SHRT_MAX; + dest[i * channels + c] = sin(2 * M_PI * f * x + c * M_PI / 3.3) * SHRT_MAX; return x; } /** * Generate sine S32_4LE PCM signal. */ -int snd_pcm_sine_s32_4le(int32_t *buffer, size_t frames, - unsigned int channels, int x, float f) { +size_t snd_pcm_sine_s32_4le(int32_t *dest, unsigned int channels, size_t frames, + float f, size_t x) { for (size_t i = 0; i < frames; x++, i++) for (size_t c = 0; c < channels; c++) - buffer[i * channels + c] = sin(2 * M_PI * f * x + c * M_PI / 3.3) * INT_MAX; + dest[i * channels + c] = sin(2 * M_PI * f * x + c * M_PI / 3.3) * INT_MAX; return x; } /** * Generate sine S24_4LE PCM signal. */ -int snd_pcm_sine_s24_4le(int32_t *buffer, size_t frames, - unsigned int channels, int x, float f) { - x = snd_pcm_sine_s32_4le(buffer, frames, channels, x, f); - for (size_t i = 0; i < frames * channels; i++) - buffer[i] >>= 8; +size_t snd_pcm_sine_s24_4le(int32_t *dest, unsigned int channels, size_t frames, + float f, size_t x) { + x = snd_pcm_sine_s32_4le(dest, channels, frames, f, x); + for (size_t i = 0; i < channels * frames; i++) + dest[i] >>= 8; return x; } diff --git a/test/mock/mock-bluealsa.c b/test/mock/mock-bluealsa.c index 62058a328..1c06784a3 100644 --- a/test/mock/mock-bluealsa.c +++ b/test/mock/mock-bluealsa.c @@ -106,7 +106,7 @@ static void *mock_dec(struct ba_transport_pcm *t_pcm) { struct pollfd fds[1] = {{ t_pcm->pipe[0], POLLIN, 0 }}; struct asrsync asrs = { .frames = 0 }; int16_t buffer[1024 * 2]; - int x = 0; + size_t x = 0; debug_transport_pcm_thread_loop(t_pcm, "START"); for (ba_transport_pcm_state_set_running(t_pcm);;) { @@ -138,7 +138,7 @@ static void *mock_dec(struct ba_transport_pcm *t_pcm) { const size_t samples = ARRAYSIZE(buffer); const size_t frames = samples / channels; - x = snd_pcm_sine_s16_2le(buffer, frames, channels, x, 146.83 / rate); + x = snd_pcm_sine_s16_2le(buffer, channels, frames, 146.83 / rate, x); io_pcm_scale(t_pcm, buffer, samples); if (io_pcm_write(t_pcm, buffer, samples) == -1) diff --git a/test/sndalign.c b/test/sndalign.c new file mode 100644 index 000000000..2910cbf27 --- /dev/null +++ b/test/sndalign.c @@ -0,0 +1,111 @@ +/* + * sndalign.c + * Copyright (c) 2016-2024 Arkadiusz Bokowy + * + * This file is a part of bluez-alsa. + * + * This project is licensed under the terms of the MIT license. + * + */ + +#include +#include +#include +#include + +#include + +int main(int argc, char *argv[]) { + + if (argc != 3) { + fprintf(stderr, "Usage: %s \n", argv[0]); + return 1; + } + + SNDFILE *sf1; + SF_INFO sf1_info = { 0 }; + if ((sf1 = sf_open(argv[1], SFM_READ, &sf1_info)) == NULL) { + fprintf(stderr, "Couldn't open audio file: %s: %s\n", argv[1], sf_strerror(NULL)); + return EXIT_FAILURE; + } + + printf("Source 1: %s\n", argv[1]); + printf(" Frames: %ld\n", sf1_info.frames); + printf(" Rate: %d\n", sf1_info.samplerate); + printf(" Channels: %d\n", sf1_info.channels); + + SNDFILE *sf2; + SF_INFO sf2_info = { 0 }; + if ((sf2 = sf_open(argv[2], SFM_READ, &sf2_info)) == NULL) { + fprintf(stderr, "Couldn't open audio file: %s: %s\n", argv[2], sf_strerror(NULL)); + return EXIT_FAILURE; + } + + printf("Source 2: %s\n", argv[2]); + printf(" Frames: %ld\n", sf2_info.frames); + printf(" Rate: %d\n", sf2_info.samplerate); + printf(" Channels: %d\n", sf2_info.channels); + + if (sf1_info.channels != sf2_info.channels) { + fprintf(stderr, "Channels mismatch: %d != %d\n", sf1_info.channels, sf2_info.channels); + return EXIT_FAILURE; + } + + if (sf1_info.samplerate != sf2_info.samplerate) { + fprintf(stderr, "Sample rate mismatch: %d != %d\n", sf1_info.samplerate, sf2_info.samplerate); + return EXIT_FAILURE; + } + + short *sf1_data = malloc(sf1_info.frames * sf1_info.channels * sizeof(short)); + if (sf_readf_short(sf1, sf1_data, sf1_info.frames) != sf1_info.frames) { + fprintf(stderr, "Couldn't read audio data: %s\n", sf_strerror(sf1)); + return EXIT_FAILURE; + } + + short *sf2_data = malloc(sf2_info.frames * sf2_info.channels * sizeof(short)); + if (sf_readf_short(sf2, sf2_data, sf2_info.frames) != sf2_info.frames) { + fprintf(stderr, "Couldn't read audio data: %s\n", sf_strerror(sf2)); + return EXIT_FAILURE; + } + + /* Calculate cross-correlation between two audio streams by applying + * different offsets while keeping the defined minimal overlap. */ + + const size_t sf1_frames = sf1_info.frames; + const size_t sf2_frames = sf2_info.frames; + const size_t cross_correlation_frames = sf1_frames + sf2_frames; + long long *cross_correlation = calloc(cross_correlation_frames, sizeof(long long)); + size_t min_overlap = MIN(512, MIN(sf1_frames, sf2_frames)); + + for (size_t i = min_overlap; i < cross_correlation_frames; i++) { + + size_t sf1_begin = i < sf2_frames ? 0 : i - sf2_frames; + size_t sf2_begin = i < sf2_frames ? sf2_frames - i : 0; + size_t sf1_end = i < sf1_frames ? i : sf1_frames; + + const size_t overlap = sf1_end - sf1_begin; + if (overlap < min_overlap) + break; + + for (size_t j = 0; j < overlap; j++) + cross_correlation[i] += sf1_data[sf1_begin + j] * sf2_data[sf2_begin + j]; + + } + + ssize_t max_i = 0; + long long max_v = cross_correlation[0]; + /* Find the maximum value in the cross-correlation array. */ + for (size_t i = 1; i < cross_correlation_frames; i++) + if (cross_correlation[i] > max_v) + max_v = cross_correlation[max_i = i]; + + printf("Best alignment: %zd\n", max_i - sf2_frames); + + sf_close(sf1); + sf_close(sf2); + free(cross_correlation); + free(sf1_data); + free(sf2_data); + + return 0; +} diff --git a/test/test-alsa-pcm.c b/test/test-alsa-pcm.c index c1dfeae5b..eab34da1f 100644 --- a/test/test-alsa-pcm.c +++ b/test/test-alsa-pcm.c @@ -158,7 +158,7 @@ static int test_pcm_close(struct spawn_process *sp_ba_mock, snd_pcm_t *pcm) { static int16_t *test_sine_s16le(snd_pcm_uframes_t size) { static size_t x = 0; assert(ARRAYSIZE(pcm_buffer) >= size * pcm_channels); - x = snd_pcm_sine_s16_2le(pcm_buffer, size, pcm_channels, x, 441.0 / pcm_rate); + x = snd_pcm_sine_s16_2le(pcm_buffer, pcm_channels, size, 441.0 / pcm_rate, x); return pcm_buffer; } diff --git a/test/test-at.c b/test/test-at.c index 14e20aee8..fa015dc1c 100644 --- a/test/test-at.c +++ b/test/test-at.c @@ -143,18 +143,18 @@ CK_START_TEST(test_at_parse_set_bia) { bool state[__HFP_IND_MAX] = { 0 }; ck_assert_int_eq(at_parse_set_bia("1,1,1,1,1,1,1", state), 0); - ck_assert_int_eq(memcmp(state, state_ok1, sizeof(state)), 0); + ck_assert_mem_eq(state, state_ok1, sizeof(state)); ck_assert_int_eq(at_parse_set_bia("1,0,1,1,1,1,0", state), 0); - ck_assert_int_eq(memcmp(state, state_ok2, sizeof(state)), 0); + ck_assert_mem_eq(state, state_ok2, sizeof(state)); /* omitted values shall not be changed */ ck_assert_int_eq(at_parse_set_bia(",,0,0,,,1", state), 0); - ck_assert_int_eq(memcmp(state, state_ok3, sizeof(state)), 0); + ck_assert_mem_eq(state, state_ok3, sizeof(state)); /* truncated values shall not be changed */ ck_assert_int_eq(at_parse_set_bia("1,1", state), 0); - ck_assert_int_eq(memcmp(state, state_ok4, sizeof(state)), 0); + ck_assert_mem_eq(state, state_ok4, sizeof(state)); } CK_END_TEST @@ -168,7 +168,7 @@ CK_START_TEST(test_at_parse_get_cind) { memset(indmap_ok, HFP_IND_NULL, sizeof(indmap_ok)); indmap_ok[0] = HFP_IND_CALL; indmap_ok[2] = HFP_IND_SIGNAL; - ck_assert_int_eq(memcmp(indmap, indmap_ok, sizeof(indmap)), 0); + ck_assert_mem_eq(indmap, indmap_ok, sizeof(indmap)); /* parse +CIND response with white-spaces */ ck_assert_int_eq(at_parse_get_cind(" ( \"call\", ( 0, 1 ) ), ( \"signal\", ( 0-3 ) )", indmap), 0); @@ -186,19 +186,19 @@ CK_START_TEST(test_at_parse_set_cmer) { /* parse +CMER value */ ck_assert_int_eq(at_parse_set_cmer("3,0,0,1,0", cmer), 0); - ck_assert_int_eq(memcmp(cmer, cmer_ok1, sizeof(cmer)), 0); + ck_assert_mem_eq(cmer, cmer_ok1, sizeof(cmer)); /* parse +CMER value with white-spaces */ ck_assert_int_eq(at_parse_set_cmer("3, 0, 0 , 1 , 0", cmer), 0); - ck_assert_int_eq(memcmp(cmer, cmer_ok1, sizeof(cmer)), 0); + ck_assert_mem_eq(cmer, cmer_ok1, sizeof(cmer)); /* parse +CMER value with less elements */ ck_assert_int_eq(at_parse_set_cmer("2,0", cmer), 0); - ck_assert_int_eq(memcmp(cmer, cmer_ok2, sizeof(cmer)), 0); + ck_assert_mem_eq(cmer, cmer_ok2, sizeof(cmer)); /* parse +CMER empty value */ ck_assert_int_eq(at_parse_set_cmer("", cmer), 0); - ck_assert_int_eq(memcmp(cmer, cmer_ok2, sizeof(cmer)), 0); + ck_assert_mem_eq(cmer, cmer_ok2, sizeof(cmer)); /* parse +CMER invalid value */ ck_assert_int_eq(at_parse_set_cmer("3,error", cmer), -1); diff --git a/test/test-audio.c b/test/test-audio.c index 032fef21f..ffff8aecc 100644 --- a/test/test-audio.c +++ b/test/test-audio.c @@ -36,8 +36,8 @@ CK_START_TEST(test_audio_interleave_deinterleave_s16_2le) { int16_t *dest[] = { dest_ch1, dest_ch2 }; audio_deinterleave_s16_2le(dest, interleaved, ARRAYSIZE(dest), ARRAYSIZE(ch1)); - ck_assert_int_eq(memcmp(dest_ch1, ch1, sizeof(ch1)), 0); - ck_assert_int_eq(memcmp(dest_ch2, ch2, sizeof(ch2)), 0); + ck_assert_mem_eq(dest_ch1, ch1, sizeof(ch1)); + ck_assert_mem_eq(dest_ch2, ch2, sizeof(ch2)); } CK_END_TEST @@ -62,9 +62,9 @@ CK_START_TEST(test_audio_interleave_deinterleave_s32_4le) { int32_t *dest[] = { dest_ch1, dest_ch2, dest_ch3 }; audio_deinterleave_s32_4le(dest, interleaved, ARRAYSIZE(dest), ARRAYSIZE(ch1)); - ck_assert_int_eq(memcmp(dest_ch1, ch1, sizeof(ch1)), 0); - ck_assert_int_eq(memcmp(dest_ch2, ch2, sizeof(ch2)), 0); - ck_assert_int_eq(memcmp(dest_ch3, ch3, sizeof(ch3)), 0); + ck_assert_mem_eq(dest_ch1, ch1, sizeof(ch1)); + ck_assert_mem_eq(dest_ch2, ch2, sizeof(ch2)); + ck_assert_mem_eq(dest_ch3, ch3, sizeof(ch3)); } CK_END_TEST @@ -82,37 +82,37 @@ CK_START_TEST(test_audio_scale_s16_2le) { memcpy(tmp, in, sizeof(tmp)); const double scale_mute[] = { 0.0 }; audio_scale_s16_2le(tmp, scale_mute, 1, ARRAYSIZE(tmp)); - ck_assert_int_eq(memcmp(tmp, mute, sizeof(mute)), 0); + ck_assert_mem_eq(tmp, mute, sizeof(mute)); memcpy(tmp, in, sizeof(tmp)); const double scale_none[] = { 1.0 }; audio_scale_s16_2le(tmp, scale_none, 1, ARRAYSIZE(tmp)); - ck_assert_int_eq(memcmp(tmp, in, sizeof(in)), 0); + ck_assert_mem_eq(tmp, in, sizeof(in)); memcpy(tmp, in, sizeof(tmp)); const double scale_half[] = { 0.5 }; audio_scale_s16_2le(tmp, scale_half, 1, ARRAYSIZE(tmp)); - ck_assert_int_eq(memcmp(tmp, half, sizeof(half)), 0); + ck_assert_mem_eq(tmp, half, sizeof(half)); memcpy(tmp, in, sizeof(tmp)); const double scale_mute_l[] = { 0.0, 1.0 }; audio_scale_s16_2le(tmp, scale_mute_l, 2, ARRAYSIZE(tmp) / 2); - ck_assert_int_eq(memcmp(tmp, mute_l, sizeof(mute_l)), 0); + ck_assert_mem_eq(tmp, mute_l, sizeof(mute_l)); memcpy(tmp, in, sizeof(tmp)); const double scale_mute_r[] = { 1.0, 0.0 }; audio_scale_s16_2le(tmp, scale_mute_r, 2, ARRAYSIZE(tmp) / 2); - ck_assert_int_eq(memcmp(tmp, mute_r, sizeof(mute_r)), 0); + ck_assert_mem_eq(tmp, mute_r, sizeof(mute_r)); memcpy(tmp, in, sizeof(tmp)); const double scale_half_l[] = { 0.5, 1.0 }; audio_scale_s16_2le(tmp, scale_half_l, 2, ARRAYSIZE(tmp) / 2); - ck_assert_int_eq(memcmp(tmp, half_l, sizeof(half_l)), 0); + ck_assert_mem_eq(tmp, half_l, sizeof(half_l)); memcpy(tmp, in, sizeof(tmp)); const double scale_half_r[] = { 1.0, 0.5 }; audio_scale_s16_2le(tmp, scale_half_r, 2, ARRAYSIZE(tmp) / 2); - ck_assert_int_eq(memcmp(tmp, half_r, sizeof(half_r)), 0); + ck_assert_mem_eq(tmp, half_r, sizeof(half_r)); } CK_END_TEST @@ -128,22 +128,22 @@ CK_START_TEST(test_audio_scale_s32_4le) { memcpy(tmp, in, sizeof(tmp)); const double scale_mute[] = { 0.0 }; audio_scale_s32_4le(tmp, scale_mute, 1, ARRAYSIZE(tmp)); - ck_assert_int_eq(memcmp(tmp, mute, sizeof(mute)), 0); + ck_assert_mem_eq(tmp, mute, sizeof(mute)); memcpy(tmp, in, sizeof(tmp)); const double scale_mute_l[] = { 0.0, 1.0 }; audio_scale_s32_4le(tmp, scale_mute_l, 2, ARRAYSIZE(tmp) / 2); - ck_assert_int_eq(memcmp(tmp, mute_l, sizeof(mute_l)), 0); + ck_assert_mem_eq(tmp, mute_l, sizeof(mute_l)); memcpy(tmp, in, sizeof(tmp)); const double scale_half[] = { 0.5 }; audio_scale_s32_4le(tmp, scale_half, 1, ARRAYSIZE(tmp)); - ck_assert_int_eq(memcmp(tmp, half, sizeof(half)), 0); + ck_assert_mem_eq(tmp, half, sizeof(half)); memcpy(tmp, in, sizeof(tmp)); const double scale_half_r[] = { 1.0, 0.5 }; audio_scale_s32_4le(tmp, scale_half_r, 2, ARRAYSIZE(tmp) / 2); - ck_assert_int_eq(memcmp(tmp, half_r, sizeof(half_r)), 0); + ck_assert_mem_eq(tmp, half_r, sizeof(half_r)); } CK_END_TEST diff --git a/test/test-io.c b/test/test-io.c index 99d6617cb..8c131ace5 100644 --- a/test/test-io.c +++ b/test/test-io.c @@ -232,6 +232,54 @@ static bool packet_loss = false; /* input BT dump file */ static struct bt_dump *btdin = NULL; +#if HAVE_SNDFILE +static SNDFILE *sf_open_format(const char *path, unsigned int rate, + unsigned int channels, uint16_t format) { + + SF_INFO sf_info = { + .format = SF_FORMAT_WAV, + .channels = channels, + .samplerate = rate, + }; + + switch (BA_TRANSPORT_PCM_FORMAT_WIDTH(format)) { + case 8: + sf_info.format |= SF_FORMAT_PCM_S8; + break; + case 16: + sf_info.format |= SF_FORMAT_PCM_16; + break; + case 24: + sf_info.format |= SF_FORMAT_PCM_24; + break; + case 32: + sf_info.format |= SF_FORMAT_PCM_32; + break; + default: + g_assert_not_reached(); + } + + return sf_open(path, SFM_WRITE, &sf_info); +} +#endif + +#if HAVE_SNDFILE +static sf_count_t sf_write_format(SNDFILE *sf, const void *buffer, + sf_count_t samples, uint16_t format) { + switch (BA_TRANSPORT_PCM_FORMAT_BYTES(format)) { + case 2: + return sf_write_short(sf, buffer, samples); + case 4: + if (BA_TRANSPORT_PCM_FORMAT_WIDTH(format) == 24) + for (size_t i = 0; i < (size_t)samples; i++) + ((int *)buffer)[i] <<= 8; + return sf_write_int(sf, buffer, samples); + default: + g_assert_not_reached(); + } +} +#endif + #if HAVE_SNDFILE static void *pcm_write_frames_sndfile_async(void *userdata) { @@ -242,7 +290,7 @@ static void *pcm_write_frames_sndfile_async(void *userdata) { SF_INFO sf_info = { 0 }; if ((sf = sf_open(input_pcm_file, SFM_READ, &sf_info)) == NULL) { error("Couldn't load input audio file: %s", sf_strerror(NULL)); - ck_assert_ptr_ne(sf, NULL); + ck_assert_ptr_nonnull(sf); } for (;;) { @@ -311,43 +359,76 @@ static void pcm_write_frames(struct ba_transport_pcm *pcm, size_t frames) { } union { - int16_t s16[2 * 1024]; - int32_t s32[2 * 1024]; - } pcm_sine_buffer; - size_t pcm_sine_bytes = 0; + int16_t s16[2 /* max channels */ * 1024]; + int32_t s32[2 /* max channels */ * 1024]; + } pcm_sine; + + const size_t pcm_format_bytes = BA_TRANSPORT_PCM_FORMAT_BYTES(pcm->format); + const size_t pcm_sine_frames = 1024; + const size_t pcm_sine_samples = pcm->channels * pcm_sine_frames; + const size_t pcm_sine_bytes = pcm_sine_samples * pcm_format_bytes; +#if HAVE_SNDFILE + SNDFILE *sf = NULL; +#endif switch (pcm->format) { case BA_TRANSPORT_PCM_FORMAT_S16_2LE: - snd_pcm_sine_s16_2le(pcm_sine_buffer.s16, 1024, pcm->channels, 0, 1.0 / 128); - pcm_sine_bytes = 1024 * pcm->channels * sizeof(int16_t); + g_assert_cmpuint(pcm_sine_bytes, <=, sizeof(pcm_sine.s16)); + snd_pcm_sine_s16_2le(pcm_sine.s16, pcm->channels, pcm_sine_frames, 1.0 / 128, 0); break; case BA_TRANSPORT_PCM_FORMAT_S24_4LE: - snd_pcm_sine_s24_4le(pcm_sine_buffer.s32, 1024, pcm->channels, 0, 1.0 / 128); - pcm_sine_bytes = 1024 * pcm->channels * sizeof(int32_t); + g_assert_cmpuint(pcm_sine_bytes, <=, sizeof(pcm_sine.s32)); + snd_pcm_sine_s24_4le(pcm_sine.s32, pcm->channels, pcm_sine_frames, 1.0 / 128, 0); break; case BA_TRANSPORT_PCM_FORMAT_S32_4LE: - snd_pcm_sine_s32_4le(pcm_sine_buffer.s32, 1024, pcm->channels, 0, 1.0 / 128); - pcm_sine_bytes = 1024 * pcm->channels * sizeof(int32_t); + g_assert_cmpuint(pcm_sine_bytes, <=, sizeof(pcm_sine.s32)); + snd_pcm_sine_s32_4le(pcm_sine.s32, pcm->channels, pcm_sine_frames, 1.0 / 128, 0); break; default: g_assert_not_reached(); } size_t samples = pcm->channels * frames; - size_t bytes = BA_TRANSPORT_PCM_FORMAT_BYTES(pcm->format) * samples; - debug("PCM write samples: %zu (%zu bytes)", samples, bytes); + debug("PCM write samples: %zu", samples); if (dump_data) { - FILE *f; - ck_assert_ptr_ne(f = fopen("sample-sine.pcm", "w"), NULL); - ck_assert_int_eq(fwrite(&pcm_sine_buffer, pcm_sine_bytes, 1, f), 1); - ck_assert_int_eq(fclose(f), 0); +#if HAVE_SNDFILE + char fname[128]; + sprintf(fname, "sample-sine-%s.wav", transport_pcm_to_fname(pcm)); + ck_assert_ptr_nonnull(sf = sf_open_format(fname, pcm->rate, pcm->channels, pcm->format)); +#else + error("Dumping audio files requires sndfile library!"); +#endif } - while (bytes > 0) { - size_t len = pcm_sine_bytes <= bytes ? pcm_sine_bytes : bytes; - ck_assert_int_eq(write(pcm->fd, &pcm_sine_buffer, len), len); - bytes -= len; + int8_t buffer[sizeof(pcm_sine.s32)]; + float fade = 1.0; + + while (samples > 0) { + size_t x = samples > pcm_sine_samples ? pcm_sine_samples : samples; + size_t x_bytes = x * pcm_format_bytes; + samples -= x; + + /* Fade-out the audio signal, so it will be easier to compare + * the original signal with the processed one. */ + switch (pcm_format_bytes) { + case 2: + for (size_t i = 0; i < x; i++) + ((int16_t *)buffer)[i] = pcm_sine.s16[i] * (fade *= 0.9995); + break; + case 4: + for (size_t i = 0; i < x; i++) + ((int32_t *)buffer)[i] = pcm_sine.s32[i] * (fade *= 0.9995); + break; + } + + ck_assert_int_eq(write(pcm->fd, buffer, x_bytes), x_bytes); + +#if HAVE_SNDFILE + if (sf != NULL) + ck_assert_int_eq(sf_write_format(sf, buffer, x, pcm->format), x); +#endif + } if (aging_duration == 0) { @@ -358,6 +439,11 @@ static void pcm_write_frames(struct ba_transport_pcm *pcm, size_t frames) { pthread_mutex_unlock(&pcm->mutex); } +#if HAVE_SNDFILE + if (sf != NULL) + sf_close(sf); +#endif + } struct bt_data { @@ -454,9 +540,9 @@ static void *test_io_thread_dump_bt(struct ba_transport_pcm *t_pcm) { ssize_t len; if (dump_data) { - char fname[64]; - sprintf(fname, "encoded-%s.btd", transport_to_fname(t)); - ck_assert_ptr_ne(btd = bt_dump_create(fname, t), NULL); + char fname[128]; + sprintf(fname, "encoded-%s-%s.btd", transport_to_fname(t), transport_pcm_to_fname(t_pcm)); + ck_assert_ptr_nonnull(btd = bt_dump_create(fname, t)); } debug_transport_pcm_thread_loop(t_pcm, "START"); @@ -492,38 +578,18 @@ static void *test_io_thread_dump_pcm(struct ba_transport_pcm *t_pcm) { pthread_cleanup_push(PTHREAD_CLEANUP(ba_transport_pcm_thread_cleanup), t_pcm); + const size_t pcm_format_bytes = BA_TRANSPORT_PCM_FORMAT_BYTES(t_pcm->format); size_t decoded_samples_total = 0; #if HAVE_SNDFILE SNDFILE *sf = NULL; - SF_INFO sf_info = { - .format = SF_FORMAT_WAV, - .channels = t_pcm->channels, - .samplerate = t_pcm->rate, - }; - switch (BA_TRANSPORT_PCM_FORMAT_WIDTH(t_pcm->format)) { - case 8: - sf_info.format |= SF_FORMAT_PCM_S8; - break; - case 16: - sf_info.format |= SF_FORMAT_PCM_16; - break; - case 24: - sf_info.format |= SF_FORMAT_PCM_24; - break; - case 32: - sf_info.format |= SF_FORMAT_PCM_32; - break; - default: - g_assert_not_reached(); - } #endif if (dump_data) { #if HAVE_SNDFILE - char fname[64]; - sprintf(fname, "decoded-%s.wav", transport_to_fname(t_pcm->t)); - ck_assert_ptr_ne(sf = sf_open(fname, SFM_WRITE, &sf_info), NULL); + char fname[128]; + sprintf(fname, "decoded-%s-%s.wav", transport_to_fname(t_pcm->t), transport_pcm_to_fname(t_pcm)); + ck_assert_ptr_nonnull(sf = sf_open_format(fname, t_pcm->rate, t_pcm->channels, t_pcm->format)); #else error("Dumping audio files requires sndfile library!"); #endif @@ -548,34 +614,20 @@ static void *test_io_thread_dump_pcm(struct ba_transport_pcm *t_pcm) { continue; } - size_t sample_size = BA_TRANSPORT_PCM_FORMAT_BYTES(t_pcm->format); - size_t samples = len / sample_size; + size_t samples = len / pcm_format_bytes; debug("Decoded samples: %zd", samples); decoded_samples_total += samples; #if HAVE_SNDFILE - if (sf != NULL) { - switch (sample_size) { - case 2: - sf_write_short(sf, (short *)buffer, samples); - break; - case 4: - if (sf_info.format & SF_FORMAT_PCM_24) - for (size_t i = 0; i < samples; i++) - ((int *)buffer)[i] <<= 8; - sf_write_int(sf, (int *)buffer, samples); - break; - default: - g_assert_not_reached(); - } - } + if (sf != NULL) + sf_write_format(sf, buffer, samples, t_pcm->format); #endif } debug("Decoded samples total [%zu frames]: %zu", decoded_samples_total / t_pcm->channels, decoded_samples_total); - ck_assert_int_gt(decoded_samples_total, 0); + ck_assert_uint_gt(decoded_samples_total, 0); #if HAVE_SNDFILE if (sf != NULL) @@ -1445,7 +1497,7 @@ int main(int argc, char *argv[]) { return 1; } - unsigned int enabled_codecs = 0xFFFF; + unsigned int enabled_codecs = 0xFFFFFFFF; if (optind != argc) enabled_codecs = 0; diff --git a/test/test-lc3-swb.c b/test/test-lc3-swb.c index 1981ed511..de0a0bbcb 100644 --- a/test/test-lc3-swb.c +++ b/test/test-lc3-swb.c @@ -42,7 +42,7 @@ CK_START_TEST(test_lc3_swb_init) { CK_START_TEST(test_lc3_swb_encode_decode) { int16_t sine[8 * LC3_SWB_CODESAMPLES]; - snd_pcm_sine_s16_2le(sine, ARRAYSIZE(sine), 1, 0, 1.0 / 128); + snd_pcm_sine_s16_2le(sine, 1, ARRAYSIZE(sine), 1.0 / 128, 0); uint8_t data[sizeof(sine)]; uint8_t *data_tail = data; @@ -98,7 +98,7 @@ CK_START_TEST(test_lc3_swb_encode_decode) { CK_START_TEST(test_lc3_swb_decode_plc) { int16_t sine[18 * LC3_SWB_CODESAMPLES]; - snd_pcm_sine_s16_2le(sine, ARRAYSIZE(sine), 1, 0, 1.0 / 128); + snd_pcm_sine_s16_2le(sine, 1, ARRAYSIZE(sine), 1.0 / 128, 0); struct esco_lc3_swb lc3_swb; lc3_swb_init(&lc3_swb); diff --git a/test/test-msbc.c b/test/test-msbc.c index 325b35eec..9d0345be1 100644 --- a/test/test-msbc.c +++ b/test/test-msbc.c @@ -46,7 +46,7 @@ CK_START_TEST(test_msbc_init) { CK_START_TEST(test_msbc_encode_decode) { int16_t sine[8 * MSBC_CODESAMPLES]; - snd_pcm_sine_s16_2le(sine, ARRAYSIZE(sine), 1, 0, 1.0 / 128); + snd_pcm_sine_s16_2le(sine, 1, ARRAYSIZE(sine), 1.0 / 128, 0); uint8_t data[sizeof(sine)]; uint8_t *data_tail = data; @@ -108,7 +108,7 @@ CK_START_TEST(test_msbc_encode_decode) { CK_START_TEST(test_msbc_decode_plc) { int16_t sine[18 * MSBC_CODESAMPLES]; - snd_pcm_sine_s16_2le(sine, ARRAYSIZE(sine), 1, 0, 1.0 / 128); + snd_pcm_sine_s16_2le(sine, 1, ARRAYSIZE(sine), 1.0 / 128, 0); struct esco_msbc msbc = { .initialized = false }; ck_assert_int_eq(msbc_init(&msbc), 0); diff --git a/test/test-utils.c b/test/test-utils.c index bb000c568..b7297ceb4 100644 --- a/test/test-utils.c +++ b/test/test-utils.c @@ -209,7 +209,7 @@ CK_START_TEST(test_ffb) { ck_assert_int_eq(ffb_shift(&ffb_u8, 33), 33); ck_assert_int_eq(ffb_len_in(&ffb_u8), 64 - (36 - 33)); ck_assert_int_eq(ffb_len_out(&ffb_u8), 36 - 33); - ck_assert_int_eq(memcmp(ffb_u8.data, "XYZ", ffb_len_out(&ffb_u8)), 0); + ck_assert_mem_eq(ffb_u8.data, "XYZ", ffb_len_out(&ffb_u8)); ck_assert_int_eq(((uint8_t *)ffb_u8.tail)[-1], 'Z'); ck_assert_int_eq(ffb_shift(&ffb_u8, 100), 36 - 33); @@ -256,13 +256,13 @@ CK_START_TEST(test_ffb_resize) { ck_assert_int_eq(ffb_len_out(&ffb), data_len); ck_assert_int_eq(ffb_len_in(&ffb), 64 - data_len); - ck_assert_int_eq(memcmp(ffb.data, data, data_len), 0); + ck_assert_mem_eq(ffb.data, data, data_len); ck_assert_int_eq(ffb_init_uint8_t(&ffb, 128), 0); ck_assert_int_eq(ffb_len_out(&ffb), data_len); ck_assert_int_eq(ffb_len_in(&ffb), 128 - data_len); - ck_assert_int_eq(memcmp(ffb.data, data, data_len), 0); + ck_assert_mem_eq(ffb.data, data, data_len); ffb_free(&ffb); @@ -285,7 +285,7 @@ CK_START_TEST(test_hex2bin) { uint8_t bin[sizeof(bin_ok)]; ck_assert_int_eq(hex2bin(hex, bin, sizeof(hex) - 1), 5); - ck_assert_int_eq(memcmp(bin, bin_ok, sizeof(bin)), 0); + ck_assert_mem_eq(bin, bin_ok, sizeof(bin)); ck_assert_int_eq(hex2bin(hex, bin, 3), -1); ck_assert_int_eq(errno, EINVAL); From 31fc89ea4a3037bf5e7c6ced16d8a8c74a1a323f Mon Sep 17 00:00:00 2001 From: borine <32966433+borine@users.noreply.github.com> Date: Sun, 15 Sep 2024 21:21:21 +0200 Subject: [PATCH 04/15] Fix bluealsactl monitor after changes in 0d488c7 Closes #729 --- doc/bluealsactl.1.rst | 78 ++++++++++++++++++++++++++++------- src/bluealsa-dbus.c | 2 +- src/bluealsactl/cmd-monitor.c | 19 +++++++-- test/mock/mock-bluez.c | 13 ++++++ test/test-alsa-ctl.c | 5 ++- test/test-utils-ctl.c | 6 ++- 6 files changed, 100 insertions(+), 23 deletions(-) diff --git a/doc/bluealsactl.1.rst b/doc/bluealsactl.1.rst index bbb5bc7fe..5c02f803d 100644 --- a/doc/bluealsactl.1.rst +++ b/doc/bluealsactl.1.rst @@ -82,10 +82,30 @@ list-pcms info *PCM_PATH* Print the properties and available codecs of the given PCM. The properties are printed one per line, in the format - 'PropertyName: Value'. Values are presented in human-readable format - for - example the Volume property is printed as: + "PropertyName: Value". Values are presented in human-readable format, and + for a property with multiple values they are printed as a space-separated + list. + + The Volume property is presented as two lines; "Volume:" indicating + the loudness component of each channel and "Mute:" indicating the mute + component of each channel. Both components are presented with their channel + values in the same order as the ChannelMap. For example: + :: + + ChannelMap: FC FL FR RL RR LFE + Volume: 127 127 127 127 127 127 + Mute: off off off off off off + + If the **--verbose** option is given then "Available codecs:" values include + the codec capabilities as a hexadecimal string suffix, separated by a colon. + Similarly, the "Selected codec:" value includes the current codec + configuration as a hexadecimal string separated by a colon. For example: + :: - ``Volume: 127 127`` + Available codecs: SBC:ffff02fa AAC:c0ffff035b60 + Selected codec: AAC:400084035b60 + + A tool such as ``a2dpconf`` can be used to decode the hex string. The list of available A2DP codecs requires BlueZ SEP support (BlueZ >= 5.52) @@ -93,13 +113,14 @@ info *PCM_PATH* codec [-c NUM] [-r NUM] [--force] *PCM_PATH* [*CODEC*\ [:*CONFIG*]] Get or set the Bluetooth codec used by the given PCM. + If *CODEC* is not given, print a list of additional codecs supported by the + given PCM and the currently selected codec. With the option **--verbose** + the codec capabilities and current configuration are shown in the same + format as for the **info** command. + If *CODEC* is given, change the codec to be used by the given PCM. This command will terminate the PCM if it is currently running. - If *CODEC* is not given, print a list of additional codecs supported by the - given PCM and the currently selected codec. The level of detail in the - output depends on the verbosity level. - Optionally, for A2DP codecs, one can specify A2DP codec configuration which should be selected. The *CONFIG* shall be given as a hexadecimal string. If this parameter is omitted, BlueALSA will select default configuration based @@ -112,6 +133,16 @@ codec [-c NUM] [-r NUM] [--force] *PCM_PATH* [*CODEC*\ [:*CONFIG*]] the configuration. However, this may result in a non-working connection and in the worst case it may crash remote Bluetooth device! + The options **-c NUM** and **-r NUM** may be used to select a specific + channel count and/or sample rate, provided that the given values are + supported by the codec. For example, if the device at path PCM_PATH + supports 5.1 surround sound and 96000 rate with the AAC codec, but is + currently configured as AAC with stereo at 48000, then the configuration + can be changed with: + :: + + bluealsactl codec -c6 -r96000 PCM_PATH aac + Selecting an A2DP codec and listing available A2DP codecs requires BlueZ SEP support (BlueZ >= 5.52). @@ -123,23 +154,31 @@ codec [-c NUM] [-r NUM] [--force] *PCM_PATH* [*CODEC*\ [:*CONFIG*]] Selecting the HFP codec when using oFono is not supported. volume *PCM_PATH* [*VOLUME* [*VOLUME*]...] - Get or set the volume value of the given PCM. + Get or set the volume loudness value(s) of the given PCM. If *VOLUME* is given, set the loudness component of the volume property of the given PCM. If only one value *VOLUME* is given it is applied to all channels. - For stereo (2-channel) PCMs the first value *VOLUME* is applied to channel - 1 (Left), and the second value *VOLUME* is applied to channel 2 (Right). + + For multi-channel PCMs, if multiple *VOLUME* values are given, then each + given value is applied to the corresponding channel of the ChannelMap (see + the **info** command). If the number of values given is less than the + number of channels, then the remaining channels are set to the first given + value. Valid A2DP values for *VOLUME* are 0-127, valid HFP/HSP values are 0-15. + Note that A2DP does not support independent channel volumes, so such a + setting is better suited to use with soft-volume enabled. See + ``bluealsad(8)`` for more details. + mute *PCM_PATH* [*STATE* [*STATE*]...] Get or set the mute switch of the given PCM. If *STATE* argument(s) are given, set mute component of the volume property - of the given PCM. The second *STATE* argument is used for stereo PCMs as - described for the **volume** command. + of the given PCM. Multiple *STATE* arguments are used for multi-channel + PCMs as described for the **volume** command. The *STATE* value can be one of **on**, **yes**, **true**, **y** or **1** for mute on, or **off**, **no**, **false**, **n** or **0** for mute off. @@ -207,12 +246,19 @@ monitor [-p[PROPS] | --properties[=PROPS]] ``PropertyChanged PCM_PATH PROPERTY_NAME VALUE`` - Property names than can be monitored are **Codec**, **Running**, + Property names that can be monitored are **Codec**, **Running**, **SoftVolume** and **Volume**. - The value for Volume is a hexadecimal 16-bit encoding where data for - channel 1 is stored in the upper byte, channel 2 is stored in the lower - byte. The highest bit of both bytes determines whether channel is muted. + Volume is an array of values, each showing the loudness and mute components + of a channel. The order of the values corresponds to the ChannelMap + property (see the **info** command). The loudness is shown as a decimal + integer value, with an optional suffix ``[M]`` indicating that the channel + is muted. For example, for a 2-channel (stereo) A2DP PCM at path PCM_PATH + with both channels at full volume and the right channel muted, the event + would be displayed as: + :: + + PropertyChanged PCM_PATH Volume 127 127[M] *PROPS* is an optional comma-separated list of property names to be monitored. If given, only changes to those properties listed will be diff --git a/src/bluealsa-dbus.c b/src/bluealsa-dbus.c index dd3cb4073..de3a3af5c 100644 --- a/src/bluealsa-dbus.c +++ b/src/bluealsa-dbus.c @@ -1072,7 +1072,7 @@ static bool bluealsa_pcm_set_property(const char *property, GVariant *value, const int level = ba_transport_pcm_volume_range_to_level(volume[i] & 0x7F, volume_max); debug("Setting volume [ch=%zu]: %u [%.2f dB] [%c]", - i, volume[i] & 0x7F, 0.01 * level, muted ? 'x' : ' '); + i, volume[i] & 0x7F, 0.01 * level, muted ? 'M' : ' '); ba_transport_pcm_volume_set(&pcm->volume[i], &level, &muted, NULL); } diff --git a/src/bluealsactl/cmd-monitor.c b/src/bluealsactl/cmd-monitor.c index 1cdeb3419..2bae99a42 100644 --- a/src/bluealsactl/cmd-monitor.c +++ b/src/bluealsactl/cmd-monitor.c @@ -10,6 +10,7 @@ #include #include +#include #include #include #include @@ -95,11 +96,21 @@ static dbus_bool_t monitor_dbus_message_iter_get_pcm_props_cb(const char *key, } else if (monitor_properties_set[PROPERTY_VOLUME].enabled && strcmp(key, monitor_properties_set[PROPERTY_VOLUME].name) == 0) { - if (type != (type_expected = DBUS_TYPE_UINT16)) + if (type != (type_expected = DBUS_TYPE_ARRAY)) goto fail; - dbus_uint16_t volume; - dbus_message_iter_get_basic(&variant, &volume); - printf("PropertyChanged %s Volume 0x%.4X\n", path, volume); + + DBusMessageIter iter; + uint8_t *data; + int len; + + dbus_message_iter_recurse(&variant, &iter); + dbus_message_iter_get_fixed_array(&iter, &data, &len); + + printf("PropertyChanged %s Volume", path); + for (size_t i = 0; i < (size_t)len; i++) + printf(" %u%s", data[i] & 0x7f, data[i] & 0x80 ? "[M]" : ""); + printf("\n"); + } return TRUE; diff --git a/test/mock/mock-bluez.c b/test/mock/mock-bluez.c index d8a17be3c..0c63db041 100644 --- a/test/mock/mock-bluez.c +++ b/test/mock/mock-bluez.c @@ -297,6 +297,15 @@ int mock_bluez_device_profile_new_connection(const char *device_path, return 0; } +static gboolean mock_bluez_media_transport_fuzz(void *userdata) { + MockBluezMediaTransport1 *transport = userdata; + /* Pseudo-random hash based on the device path to simulate different values. */ + unsigned int hash = g_str_hash(mock_bluez_media_transport1_get_device(transport)); + mock_bluez_media_transport1_set_delay(transport, hash % 2777); + mock_bluez_media_transport1_set_volume(transport, hash % 101); + return G_SOURCE_REMOVE; +} + static void mock_bluez_media_endpoint_set_configuration_finish(GObject *source, GAsyncResult *result, G_GNUC_UNUSED void *userdata) { MockBluezMediaEndpoint1 *endpoint = MOCK_BLUEZ_MEDIA_ENDPOINT1(source); @@ -343,6 +352,10 @@ int mock_bluez_device_media_set_configuration(const char *device_path, if (strcmp(uuid, BT_UUID_A2DP_SINK) == 0) mock_bluez_media_transport1_set_state(transport, "pending"); + /* If fuzzing is enabled, update some properties after slight delay. */ + if (mock_fuzzing_ms) + g_timeout_add(mock_fuzzing_ms, mock_bluez_media_transport_fuzz, transport); + rv = 0; break; } diff --git a/test/test-alsa-ctl.c b/test/test-alsa-ctl.c index 0aadb5b3f..5ef1c1bd4 100644 --- a/test/test-alsa-ctl.c +++ b/test/test-alsa-ctl.c @@ -535,12 +535,15 @@ CK_START_TEST(test_notifications) { /* Processed events: * - 0 removes; 2 new elems (12:34:... A2DP) + * - 4 updates per new A2DP (updated delay and volume) * - 2 removes; 4 new elems (12:34:... A2DP, 23:45:... A2DP) + * - 4 updates per new A2DP (updated delay and volume) * - 4 removes; 7 new elems (2x A2DP, SCO playback, battery) * - 7 removes; 9 new elems (2x A2DP, SCO playback/capture, battery) * - 4 updates per codec (SCO codec updates if codec selection is supported) */ - size_t expected_events = (0 + 2) + (2 + 4) + (4 + 7) + (7 + 9) + events_update_codec; + size_t expected_events = + (0 + 2) + 4 + (2 + 4) + 4 + (4 + 7) + (7 + 9) + events_update_codec; /* XXX: It is possible that the battery element (RFCOMM D-Bus path) will not * be exported in time. In such case, the number of events will be less diff --git a/test/test-utils-ctl.c b/test/test-utils-ctl.c index b81429141..f57733bd1 100644 --- a/test/test-utils-ctl.c +++ b/test/test-utils-ctl.c @@ -408,8 +408,12 @@ CK_START_TEST(test_monitor) { ck_assert_ptr_ne(strstr(output, "Device: /org/bluez/hci11/dev_23_45_67_89_AB_CD"), NULL); -#if ENABLE_MSBC /* notifications for property changed */ + ck_assert_ptr_ne(strstr(output, + "PropertyChanged /org/bluealsa/hci11/dev_12_34_56_78_9A_BC/a2dpsrc/sink Volume 54 54"), NULL); + ck_assert_ptr_ne(strstr(output, + "PropertyChanged /org/bluealsa/hci11/dev_23_45_67_89_AB_CD/a2dpsrc/sink Volume 84 84"), NULL); +#if ENABLE_MSBC ck_assert_ptr_ne(strstr(output, "PropertyChanged /org/bluealsa/hci11/dev_12_34_56_78_9A_BC/hfpag/sink Codec CVSD"), NULL); ck_assert_ptr_ne(strstr(output, From ea33342e9ba78f3669e3138359ec33fbe86d002a Mon Sep 17 00:00:00 2001 From: borine <32966433+borine@users.noreply.github.com> Date: Mon, 16 Sep 2024 14:38:29 +0100 Subject: [PATCH 05/15] Manual page for a2dpconf utility --- .github/spellcheck-wordlist.txt | 2 + doc/Makefile.am | 4 ++ doc/a2dpconf.1.rst | 96 +++++++++++++++++++++++++++++++++ doc/bluealsactl.1.rst | 7 +-- 4 files changed, 106 insertions(+), 3 deletions(-) create mode 100644 doc/a2dpconf.1.rst diff --git a/.github/spellcheck-wordlist.txt b/.github/spellcheck-wordlist.txt index f9d690922..abcd4a342 100644 --- a/.github/spellcheck-wordlist.txt +++ b/.github/spellcheck-wordlist.txt @@ -123,6 +123,7 @@ unregister utils # Others +a2dpconf AABBCCDDEEFFGGHHIIJJKKLLMMNNOOPPQQRRSSTTUUVVWWXXYYZZ aas ABCD @@ -174,6 +175,7 @@ dlopen dmix dmx docutils +dpconf dpsnk dpsrc DYN diff --git a/doc/Makefile.am b/doc/Makefile.am index 4937bca7c..13eda61b2 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -23,6 +23,10 @@ if ENABLE_RFCOMM man1_MANS += bluealsa-rfcomm.1 endif +if ENABLE_A2DPCONF +man1_MANS += a2dpconf.1 +endif + if ENABLE_HCITOP man1_MANS += hcitop.1 endif diff --git a/doc/a2dpconf.1.rst b/doc/a2dpconf.1.rst new file mode 100644 index 000000000..5d7fb98e3 --- /dev/null +++ b/doc/a2dpconf.1.rst @@ -0,0 +1,96 @@ +======== +a2dpconf +======== + +---------------------------------------- +Decode A2DP codec capability hex strings +---------------------------------------- + +:Date: September 2024 +:Manual section: 1 +:Manual group: General Commands Manual +:Version: $VERSION$ + +SYNOPSIS +======== + +**a2dpconf** [*OPTION*]... [*CODEC*:]\ *CONFIG*... + +DESCRIPTION +=========== + +**a2dpconf** presents the fields of the given A2DP codec *CONFIG* in a +human-readable format. *CODEC* is the name of the relevant codec, and *CONFIG* +is the hexadecimal encoding of the configuration or capabilities binary "blob" +as reported by tools such as ``bluealsactl(1)`` or the debug output of +``bluealsad(8)``. +(see `EXAMPLES`_ below). + +OPTIONS +======= + +-h, --help + Print this help and exit. + +-V, --version + Print version and exit. + +-x, --auto-detect + Try to auto-detect the codec. If the name of the codec associated with the + configuration string is not known, then give this option and the + configuration string without the codec name prefix. The output is then a + list of all possible known codec configurations for which the given string + is valid. + +EXAMPLES +======== +:: + + $ a2dpconf sbc:ffff0235 + SBC { + sample-rate:4 = 48000 44100 32000 16000 + channel-mode:4 = JointStereo Stereo DualChannel Mono + block-length:4 = 16 12 8 4 + sub-bands:2 = 8 4 + allocation-method:2 = Loudness SNR + min-bit-pool-value:8 = 2 + max-bit-pool-value:8 = 53 + } + +:: + + $ a2dpconf -x ffff0235 + SBC { + sample-rate:4 = 48000 44100 32000 16000 + channel-mode:4 = JointStereo Stereo DualChannel Mono + block-length:4 = 16 12 8 4 + sub-bands:2 = 8 4 + allocation-method:2 = Loudness SNR + min-bit-pool-value:8 = 2 + max-bit-pool-value:8 = 53 + } + MPEG-1,2 Audio { + layer:3 = MP3 MP2 MP1 + crc:1 = true + channel-mode:4 = JointStereo Stereo DualChannel Mono + :1 + media-payload-format:1 = MPF-1 MPF-2 + sample-rate:6 = 48000 44100 32000 24000 22050 16000 + vbr:1 = false + bitrate-index:15 = 0x235 + } + +COPYRIGHT +========= + +Copyright (c) 2016-2024 Arkadiusz Bokowy. + +The bluez-alsa project is licensed under the terms of the MIT license. + +SEE ALSO +======== + +``bluealsactl(1)``, ``bluealsad(8)`` + +Project web site + https://github.com/arkq/bluez-alsa diff --git a/doc/bluealsactl.1.rst b/doc/bluealsactl.1.rst index 5c02f803d..463a86790 100644 --- a/doc/bluealsactl.1.rst +++ b/doc/bluealsactl.1.rst @@ -6,7 +6,7 @@ bluealsactl a simple command line interface for the BlueALSA D-Bus API ---------------------------------------------------------- -:Date: August 2024 +:Date: September 2024 :Manual section: 1 :Manual group: General Commands Manual :Version: $VERSION$ @@ -105,7 +105,7 @@ info *PCM_PATH* Available codecs: SBC:ffff02fa AAC:c0ffff035b60 Selected codec: AAC:400084035b60 - A tool such as ``a2dpconf`` can be used to decode the hex string. + A tool such as ``a2dpconf(1)`` can be used to decode the hex string. The list of available A2DP codecs requires BlueZ SEP support (BlueZ >= 5.52) @@ -285,7 +285,8 @@ The bluez-alsa project is licensed under the terms of the MIT license. SEE ALSO ======== -``bluealsad(8)``, ``bluealsa-aplay(1)``, ``bluealsa-rfcomm(1)`` +``a2dpconf(1)``, ``bluealsa-aplay(1)``, ``bluealsa-rfcomm(1)``, +``bluealsad(8)`` Project web site https://github.com/arkq/bluez-alsa From 5a859d62bdb5e9cf91b92986684e0aa81e295fbc Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Sun, 22 Sep 2024 22:41:29 +0200 Subject: [PATCH 06/15] Account for codec delay when calculating PCM delay --- src/a2dp-aac.c | 34 ++++++++++++++++++-------------- src/a2dp-aptx-hd.c | 2 +- src/a2dp-aptx.c | 2 +- src/a2dp-faststream.c | 9 +++++++-- src/a2dp-lc3plus.c | 9 ++++++++- src/a2dp-ldac.c | 6 +++++- src/a2dp-mpeg.c | 6 +++++- src/a2dp-opus.c | 12 +++++++++++- src/a2dp-sbc.c | 6 +++++- src/ba-transport-pcm.c | 4 +++- src/ba-transport-pcm.h | 10 +++++++--- src/codec-lc3-swb.c | 7 +++++++ src/codec-lc3-swb.h | 1 + src/sco-cvsd.c | 2 +- src/sco-lc3-swb.c | 6 +++++- src/sco-msbc.c | 6 +++++- test/sndalign.c | 44 +++++++++++++++++++++++++++++++----------- test/test-io.c | 2 +- 18 files changed, 126 insertions(+), 42 deletions(-) diff --git a/src/a2dp-aac.c b/src/a2dp-aac.c index 2aed30cca..78e707a80 100644 --- a/src/a2dp-aac.c +++ b/src/a2dp-aac.c @@ -179,7 +179,7 @@ void *a2dp_aac_enc_thread(struct ba_transport_pcm *t_pcm) { struct io_poll io = { .timeout = -1 }; HANDLE_AACENCODER handle; - AACENC_InfoStruct aacinf; + AACENC_InfoStruct info; AACENC_ERROR err; const a2dp_aac_t *configuration = &t->a2dp.configuration.aac; @@ -293,7 +293,7 @@ void *a2dp_aac_enc_thread(struct ba_transport_pcm *t_pcm) { error("Couldn't initialize AAC encoder: %s", aacenc_strerror(err)); goto fail_init; } - if ((err = aacEncInfo(handle, &aacinf)) != AACENC_OK) { + if ((err = aacEncInfo(handle, &info)) != AACENC_OK) { error("Couldn't get encoder info: %s", aacenc_strerror(err)); goto fail_init; } @@ -303,14 +303,17 @@ void *a2dp_aac_enc_thread(struct ba_transport_pcm *t_pcm) { pthread_cleanup_push(PTHREAD_CLEANUP(ffb_free), &bt); pthread_cleanup_push(PTHREAD_CLEANUP(ffb_free), &pcm); - const unsigned int aac_frame_size = aacinf.inputChannels * aacinf.frameLength; + const unsigned int aac_frame_size = info.inputChannels * info.frameLength; const size_t sample_size = BA_TRANSPORT_PCM_FORMAT_BYTES(t_pcm->format); if (ffb_init(&pcm, aac_frame_size, sample_size) == -1 || - ffb_init_uint8_t(&bt, RTP_HEADER_LEN + aacinf.maxOutBufBytes) == -1) { + ffb_init_uint8_t(&bt, RTP_HEADER_LEN + info.maxOutBufBytes) == -1) { error("Couldn't create data buffers: %s", strerror(errno)); goto fail_ffb; } + /* Get the delay introduced by the encoder. */ + t_pcm->codec_delay_dms = info.nDelay * 10000 / rate; + rtp_header_t *rtp_header; /* initialize RTP header and get anchor for payload */ uint8_t *rtp_payload = rtp_a2dp_init(bt.data, &rtp_header, NULL, 0); @@ -322,7 +325,7 @@ void *a2dp_aac_enc_thread(struct ba_transport_pcm *t_pcm) { int in_bufferIdentifiers[] = { IN_AUDIO_DATA }; int out_bufferIdentifiers[] = { OUT_BITSTREAM_DATA }; int in_bufSizes[] = { pcm.nmemb * pcm.size }; - int out_bufSizes[] = { aacinf.maxOutBufBytes }; + int out_bufSizes[] = { info.maxOutBufBytes }; int in_bufElSizes[] = { pcm.size }; int out_bufElSizes[] = { bt.size }; @@ -363,7 +366,7 @@ void *a2dp_aac_enc_thread(struct ba_transport_pcm *t_pcm) { continue; } - while ((in_args.numInSamples = ffb_len_out(&pcm)) > (int)aacinf.inputChannels) { + while ((in_args.numInSamples = ffb_len_out(&pcm)) > (int)info.inputChannels) { if ((err = aacEncEncode(handle, &in_buf, &out_buf, &in_args, &out_args)) != AACENC_OK) error("AAC encoding error: %s", aacenc_strerror(err)); @@ -409,14 +412,14 @@ void *a2dp_aac_enc_thread(struct ba_transport_pcm *t_pcm) { } - unsigned int pcm_frames = out_args.numInSamples / aacinf.inputChannels; + unsigned int pcm_frames = out_args.numInSamples / info.inputChannels; /* keep data transfer at a constant bit rate */ asrsync_sync(&io.asrs, pcm_frames); /* move forward RTP timestamp clock */ rtp_state_update(&rtp, pcm_frames); /* update busy delay (encoding overhead) */ - t_pcm->delay = asrsync_get_busy_usec(&io.asrs) / 100; + t_pcm->processing_delay_dms = asrsync_get_busy_usec(&io.asrs) / 100; /* If the input buffer was not consumed, we have to append new data to * the existing one. Since we do not use ring buffer, we will simply @@ -553,26 +556,29 @@ void *a2dp_aac_dec_thread(struct ba_transport_pcm *t_pcm) { unsigned int data_len = ffb_len_out(&latm); unsigned int valid = ffb_len_out(&latm); - CStreamInfo *aacinf; + CStreamInfo *info; if ((err = aacDecoder_Fill(handle, (uint8_t **)&latm.data, &data_len, &valid)) != AAC_DEC_OK) error("AAC buffer fill error: %s", aacdec_strerror(err)); else if ((err = aacDecoder_DecodeFrame(handle, pcm.tail, ffb_blen_in(&pcm), 0)) != AAC_DEC_OK) error("AAC decode frame error: %s", aacdec_strerror(err)); - else if ((aacinf = aacDecoder_GetStreamInfo(handle)) == NULL) + else if ((info = aacDecoder_GetStreamInfo(handle)) == NULL) error("Couldn't get AAC stream info"); else { - if ((unsigned int)aacinf->numChannels != channels) - warn("AAC channels mismatch: %u != %u", aacinf->numChannels, channels); + if ((unsigned int)info->numChannels != channels) + warn("AAC channels mismatch: %u != %u", info->numChannels, channels); - const size_t samples = (size_t)aacinf->frameSize * channels; + const size_t samples = (size_t)info->frameSize * channels; io_pcm_scale(t_pcm, pcm.data, samples); if (io_pcm_write(t_pcm, pcm.data, samples) == -1) error("PCM write error: %s", strerror(errno)); + /* Update the delay introduced by the decoder. */ + t_pcm->codec_delay_dms = info->outputDelay * 10000 / rate; + /* update local state with decoded PCM frames */ - rtp_state_update(&rtp, aacinf->frameSize); + rtp_state_update(&rtp, info->frameSize); } diff --git a/src/a2dp-aptx-hd.c b/src/a2dp-aptx-hd.c index 56c43c8cd..99115963d 100644 --- a/src/a2dp-aptx-hd.c +++ b/src/a2dp-aptx-hd.c @@ -220,7 +220,7 @@ void *a2dp_aptx_hd_enc_thread(struct ba_transport_pcm *t_pcm) { rtp.ts_pcm_frames += pcm_frames; /* update busy delay (encoding overhead) */ - t_pcm->delay = asrsync_get_busy_usec(&io.asrs) / 100; + t_pcm->processing_delay_dms = asrsync_get_busy_usec(&io.asrs) / 100; /* reinitialize output buffer */ ffb_rewind(&bt); diff --git a/src/a2dp-aptx.c b/src/a2dp-aptx.c index f02572908..9a100d213 100644 --- a/src/a2dp-aptx.c +++ b/src/a2dp-aptx.c @@ -201,7 +201,7 @@ void *a2dp_aptx_enc_thread(struct ba_transport_pcm *t_pcm) { asrsync_sync(&io.asrs, pcm_samples / channels); /* update busy delay (encoding overhead) */ - t_pcm->delay = asrsync_get_busy_usec(&io.asrs) / 100; + t_pcm->processing_delay_dms = asrsync_get_busy_usec(&io.asrs) / 100; /* reinitialize output buffer */ ffb_rewind(&bt); diff --git a/src/a2dp-faststream.c b/src/a2dp-faststream.c index 1234ec67e..5d089907c 100644 --- a/src/a2dp-faststream.c +++ b/src/a2dp-faststream.c @@ -150,9 +150,10 @@ void *a2dp_fs_enc_thread(struct ba_transport_pcm *t_pcm) { pthread_cleanup_push(PTHREAD_CLEANUP(ffb_free), &pcm); pthread_cleanup_push(PTHREAD_CLEANUP(sbc_finish), &sbc); - const unsigned int channels = t_pcm->channels; const size_t sbc_frame_len = sbc_get_frame_length(&sbc); const size_t sbc_frame_samples = sbc_get_codesize(&sbc) / sizeof(int16_t); + const unsigned int channels = t_pcm->channels; + const unsigned int rate = t_pcm->rate; if (ffb_init_int16_t(&pcm, sbc_frame_samples * 3) == -1 || ffb_init_uint8_t(&bt, t->mtu_write) == -1) { @@ -160,6 +161,10 @@ void *a2dp_fs_enc_thread(struct ba_transport_pcm *t_pcm) { goto fail_ffb; } + const unsigned int sbc_delay_frames = 73; + /* Get the total delay introduced by the codec. */ + t_pcm->codec_delay_dms = sbc_delay_frames * 10000 / rate; + debug_transport_pcm_thread_loop(t_pcm, "START"); for (ba_transport_pcm_state_set_running(t_pcm);;) { @@ -223,7 +228,7 @@ void *a2dp_fs_enc_thread(struct ba_transport_pcm *t_pcm) { asrsync_sync(&io.asrs, pcm_frames); /* update busy delay (encoding overhead) */ - t_pcm->delay = asrsync_get_busy_usec(&io.asrs) / 100; + t_pcm->processing_delay_dms = asrsync_get_busy_usec(&io.asrs) / 100; /* If the input buffer was not consumed (due to codesize limit), we * have to append new data to the existing one. Since we do not use diff --git a/src/a2dp-lc3plus.c b/src/a2dp-lc3plus.c index b402c2edd..58fa251fe 100644 --- a/src/a2dp-lc3plus.c +++ b/src/a2dp-lc3plus.c @@ -255,6 +255,13 @@ void *a2dp_lc3plus_enc_thread(struct ba_transport_pcm *t_pcm) { goto fail_ffb; } + /* Get the total delay introduced by the codec. The LC3plus library + * reports total codec delay in case of both encoder and decoder API. + * In order not to overestimate the delay, we are not going to report + * delay in the decoder thread. */ + const int lc3plus_delay_frames = lc3plus_enc_get_delay(handle); + t_pcm->codec_delay_dms = lc3plus_delay_frames * 10000 / rate; + rtp_header_t *rtp_header; rtp_media_header_t *rtp_media_header; /* initialize RTP headers and get anchor for payload */ @@ -377,7 +384,7 @@ void *a2dp_lc3plus_enc_thread(struct ba_transport_pcm *t_pcm) { rtp_state_update(&rtp, pcm_frames); /* update busy delay (encoding overhead) */ - t_pcm->delay = asrsync_get_busy_usec(&io.asrs) / 100; + t_pcm->processing_delay_dms = asrsync_get_busy_usec(&io.asrs) / 100; /* If the input buffer was not consumed (due to codesize limit), we * have to append new data to the existing one. Since we do not use diff --git a/src/a2dp-ldac.c b/src/a2dp-ldac.c index 0c450fefa..9a4446b5e 100644 --- a/src/a2dp-ldac.c +++ b/src/a2dp-ldac.c @@ -174,6 +174,10 @@ void *a2dp_ldac_enc_thread(struct ba_transport_pcm *t_pcm) { goto fail_ffb; } + const unsigned int ldac_delay_frames = 128; + /* Get the total delay introduced by the codec. */ + t_pcm->codec_delay_dms = ldac_delay_frames * 10000 / rate; + rtp_header_t *rtp_header; rtp_media_header_t *rtp_media_header; /* initialize RTP headers and get anchor for payload */ @@ -266,7 +270,7 @@ void *a2dp_ldac_enc_thread(struct ba_transport_pcm *t_pcm) { rtp_state_update(&rtp, pcm_frames); /* update busy delay (encoding overhead) */ - t_pcm->delay = asrsync_get_busy_usec(&io.asrs) / 100; + t_pcm->processing_delay_dms = asrsync_get_busy_usec(&io.asrs) / 100; } diff --git a/src/a2dp-mpeg.c b/src/a2dp-mpeg.c index 7a9bfa957..fd0266b38 100644 --- a/src/a2dp-mpeg.c +++ b/src/a2dp-mpeg.c @@ -234,6 +234,10 @@ void *a2dp_mp3_enc_thread(struct ba_transport_pcm *t_pcm) { goto fail_ffb; } + /* Get the total delay introduced by the codec. */ + const int mpeg_delay_frames = lame_get_encoder_delay(handle); + t_pcm->codec_delay_dms = mpeg_delay_frames * 10000 / rate; + rtp_header_t *rtp_header; rtp_mpeg_audio_header_t *rtp_mpeg_audio_header; /* initialize RTP headers and get anchor for payload */ @@ -319,7 +323,7 @@ void *a2dp_mp3_enc_thread(struct ba_transport_pcm *t_pcm) { rtp_state_update(&rtp, pcm_frames); /* update busy delay (encoding overhead) */ - t_pcm->delay = asrsync_get_busy_usec(&io.asrs) / 100; + t_pcm->processing_delay_dms = asrsync_get_busy_usec(&io.asrs) / 100; /* If the input buffer was not consumed (due to frame alignment), we * have to append new data to the existing one. Since we do not use diff --git a/src/a2dp-opus.c b/src/a2dp-opus.c index 32c2933ec..4631bcb8b 100644 --- a/src/a2dp-opus.c +++ b/src/a2dp-opus.c @@ -174,6 +174,11 @@ void *a2dp_opus_enc_thread(struct ba_transport_pcm *t_pcm) { goto fail_ffb; } + int32_t opus_delay_frames = 0; + /* Get the delay introduced by the encoder. */ + opus_encoder_ctl(opus, OPUS_GET_LOOKAHEAD(&opus_delay_frames)); + t_pcm->codec_delay_dms = opus_delay_frames * 10000 / rate; + rtp_header_t *rtp_header; rtp_media_header_t *rtp_media_header; /* initialize RTP headers and get anchor for payload */ @@ -237,7 +242,7 @@ void *a2dp_opus_enc_thread(struct ba_transport_pcm *t_pcm) { rtp_state_update(&rtp, opus_frame_pcm_frames); /* update busy delay (encoding overhead) */ - t_pcm->delay = asrsync_get_busy_usec(&io.asrs) / 100; + t_pcm->processing_delay_dms = asrsync_get_busy_usec(&io.asrs) / 100; /* If the input buffer was not consumed (due to encoder frame * constraint), we have to append new data to the existing one. @@ -300,6 +305,11 @@ void *a2dp_opus_dec_thread(struct ba_transport_pcm *t_pcm) { goto fail_ffb; } + int32_t opus_delay_frames = 0; + /* Get the delay introduced by the decoder. */ + opus_decoder_ctl(opus, OPUS_GET_LOOKAHEAD(&opus_delay_frames)); + t_pcm->codec_delay_dms = opus_delay_frames * 10000 / rate; + struct rtp_state rtp = { .synced = false }; /* RTP clock frequency equal to PCM sample rate */ rtp_state_init(&rtp, rate, rate); diff --git a/src/a2dp-sbc.c b/src/a2dp-sbc.c index 1ffd27585..696ac1378 100644 --- a/src/a2dp-sbc.c +++ b/src/a2dp-sbc.c @@ -185,6 +185,10 @@ void *a2dp_sbc_enc_thread(struct ba_transport_pcm *t_pcm) { goto fail_ffb; } + const unsigned int sbc_delay_frames = 73; + /* Get the total delay introduced by the codec. */ + t_pcm->codec_delay_dms = sbc_delay_frames * 10000 / rate; + rtp_header_t *rtp_header; rtp_media_header_t *rtp_media_header; @@ -269,7 +273,7 @@ void *a2dp_sbc_enc_thread(struct ba_transport_pcm *t_pcm) { rtp_state_update(&rtp, pcm_frames); /* update busy delay (encoding overhead) */ - t_pcm->delay = asrsync_get_busy_usec(&io.asrs) / 100; + t_pcm->processing_delay_dms = asrsync_get_busy_usec(&io.asrs) / 100; /* If the input buffer was not consumed (due to codesize limit), we * have to append new data to the existing one. Since we do not use diff --git a/src/ba-transport-pcm.c b/src/ba-transport-pcm.c index adbb944d2..af9be01c7 100644 --- a/src/ba-transport-pcm.c +++ b/src/ba-transport-pcm.c @@ -724,7 +724,9 @@ int ba_transport_pcm_get_hardware_volume( int ba_transport_pcm_get_delay(const struct ba_transport_pcm *pcm) { const struct ba_transport *t = pcm->t; - int delay = pcm->delay + ba_transport_pcm_delay_adjustment_get(pcm); + + int delay = pcm->codec_delay_dms + pcm->processing_delay_dms; + delay += ba_transport_pcm_delay_adjustment_get(pcm); if (t->profile & BA_TRANSPORT_PROFILE_MASK_A2DP) delay += t->a2dp.delay; diff --git a/src/ba-transport-pcm.h b/src/ba-transport-pcm.h index 40d7a0c2d..adc4836d6 100644 --- a/src/ba-transport-pcm.h +++ b/src/ba-transport-pcm.h @@ -122,9 +122,13 @@ struct ba_transport_pcm { /* PCM sample rate */ unsigned int rate; - /* Overall PCM delay in 1/10 of millisecond, caused by - * audio encoding or decoding and data transfer. */ - unsigned int delay; + /* Delay caused by the codec due to internal buffering. The delay is + * expressed in 1/10 of millisecond. */ + unsigned int codec_delay_dms; + /* Delay caused by data processing. This delay component depends on + * the host computational power. It is used to compensate for the time + * required to encode or decode audio. */ + unsigned int processing_delay_dms; /* guard delay adjustments access */ pthread_mutex_t delay_adjustments_mtx; diff --git a/src/codec-lc3-swb.c b/src/codec-lc3-swb.c index ba59d843b..01eba66d4 100644 --- a/src/codec-lc3-swb.c +++ b/src/codec-lc3-swb.c @@ -43,6 +43,13 @@ void lc3_swb_init(struct esco_lc3_swb *lc3_swb) { } +/** + * Get LC3-SWB delay in number of samples. */ +ssize_t lc3_swb_get_delay(struct esco_lc3_swb *lc3_swb) { + (void)lc3_swb; + return lc3_delay_samples(7500, 32000); +} + /** * Encode single eSCO LC3-SWB frame. */ ssize_t lc3_swb_encode(struct esco_lc3_swb *lc3_swb) { diff --git a/src/codec-lc3-swb.h b/src/codec-lc3-swb.h index 30da8732e..1658fdf53 100644 --- a/src/codec-lc3-swb.h +++ b/src/codec-lc3-swb.h @@ -74,6 +74,7 @@ struct esco_lc3_swb { void lc3_swb_init(struct esco_lc3_swb *lc3_swb); +ssize_t lc3_swb_get_delay(struct esco_lc3_swb *lc3_swb); ssize_t lc3_swb_encode(struct esco_lc3_swb *lc3_swb); ssize_t lc3_swb_decode(struct esco_lc3_swb *lc3_swb); diff --git a/src/sco-cvsd.c b/src/sco-cvsd.c index a61d24b1f..dea164c16 100644 --- a/src/sco-cvsd.c +++ b/src/sco-cvsd.c @@ -79,7 +79,7 @@ void *sco_cvsd_enc_thread(struct ba_transport_pcm *t_pcm) { /* 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; + t_pcm->processing_delay_dms = asrsync_get_busy_usec(&io.asrs) / 100; } diff --git a/src/sco-lc3-swb.c b/src/sco-lc3-swb.c index 507f6ef29..b9efb590f 100644 --- a/src/sco-lc3-swb.c +++ b/src/sco-lc3-swb.c @@ -41,6 +41,10 @@ void *sco_lc3_swb_enc_thread(struct ba_transport_pcm *t_pcm) { struct esco_lc3_swb codec; lc3_swb_init(&codec); + /* Get the total delay introduced by the codec. */ + const ssize_t lc3_swb_delay_frames = lc3_swb_get_delay(&codec); + t_pcm->codec_delay_dms = lc3_swb_delay_frames * 10000 / t_pcm->rate; + debug_transport_pcm_thread_loop(t_pcm, "START"); for (ba_transport_pcm_state_set_running(t_pcm);;) { @@ -81,7 +85,7 @@ void *sco_lc3_swb_enc_thread(struct ba_transport_pcm *t_pcm) { /* keep data transfer at a constant bit rate */ asrsync_sync(&io.asrs, codec.frames * LC3_SWB_CODESAMPLES); /* update busy delay (encoding overhead) */ - t_pcm->delay = asrsync_get_busy_usec(&io.asrs) / 100; + t_pcm->processing_delay_dms = asrsync_get_busy_usec(&io.asrs) / 100; /* Move unprocessed data to the front of our linear * buffer and clear the LC3-SWB frame counter. */ diff --git a/src/sco-msbc.c b/src/sco-msbc.c index c83eb8045..3c9ceaae7 100644 --- a/src/sco-msbc.c +++ b/src/sco-msbc.c @@ -43,6 +43,10 @@ void *sco_msbc_enc_thread(struct ba_transport_pcm *t_pcm) { goto fail_msbc; } + const unsigned int sbc_delay_frames = 73; + /* Get the total delay introduced by the codec. */ + t_pcm->codec_delay_dms = sbc_delay_frames * 10000 / t_pcm->rate; + debug_transport_pcm_thread_loop(t_pcm, "START"); for (ba_transport_pcm_state_set_running(t_pcm);;) { @@ -88,7 +92,7 @@ void *sco_msbc_enc_thread(struct ba_transport_pcm *t_pcm) { /* 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; + t_pcm->processing_delay_dms = asrsync_get_busy_usec(&io.asrs) / 100; /* Move unprocessed data to the front of our linear * buffer and clear the mSBC frame counter. */ diff --git a/test/sndalign.c b/test/sndalign.c index 2910cbf27..692d0de15 100644 --- a/test/sndalign.c +++ b/test/sndalign.c @@ -9,23 +9,31 @@ */ #include +#include #include #include #include #include +static bool is_silence(const short *src, unsigned int channels, size_t frames) { + for (size_t i = 0; i < channels * frames; i++) + if (src[i] != 0) + return false; + return true; +} + int main(int argc, char *argv[]) { if (argc != 3) { - fprintf(stderr, "Usage: %s \n", argv[0]); - return 1; + printf("Usage: %s \n", argv[0]); + return EXIT_FAILURE; } SNDFILE *sf1; SF_INFO sf1_info = { 0 }; if ((sf1 = sf_open(argv[1], SFM_READ, &sf1_info)) == NULL) { - fprintf(stderr, "Couldn't open audio file: %s: %s\n", argv[1], sf_strerror(NULL)); + fprintf(stderr, "ERR: Couldn't open audio file: %s: %s\n", argv[1], sf_strerror(NULL)); return EXIT_FAILURE; } @@ -37,7 +45,7 @@ int main(int argc, char *argv[]) { SNDFILE *sf2; SF_INFO sf2_info = { 0 }; if ((sf2 = sf_open(argv[2], SFM_READ, &sf2_info)) == NULL) { - fprintf(stderr, "Couldn't open audio file: %s: %s\n", argv[2], sf_strerror(NULL)); + fprintf(stderr, "ERR: Couldn't open audio file: %s: %s\n", argv[2], sf_strerror(NULL)); return EXIT_FAILURE; } @@ -47,30 +55,41 @@ int main(int argc, char *argv[]) { printf(" Channels: %d\n", sf2_info.channels); if (sf1_info.channels != sf2_info.channels) { - fprintf(stderr, "Channels mismatch: %d != %d\n", sf1_info.channels, sf2_info.channels); + fprintf(stderr, "ERR: Channels mismatch: %d != %d\n", sf1_info.channels, sf2_info.channels); return EXIT_FAILURE; } if (sf1_info.samplerate != sf2_info.samplerate) { - fprintf(stderr, "Sample rate mismatch: %d != %d\n", sf1_info.samplerate, sf2_info.samplerate); + fprintf(stderr, "ERR: Sample rate mismatch: %d != %d\n", sf1_info.samplerate, sf2_info.samplerate); return EXIT_FAILURE; } short *sf1_data = malloc(sf1_info.frames * sf1_info.channels * sizeof(short)); if (sf_readf_short(sf1, sf1_data, sf1_info.frames) != sf1_info.frames) { - fprintf(stderr, "Couldn't read audio data: %s\n", sf_strerror(sf1)); + fprintf(stderr, "ERR: Couldn't read audio data: %s\n", sf_strerror(sf1)); + return EXIT_FAILURE; + } + + if (is_silence(sf1_data, sf1_info.channels, sf1_info.frames)) { + fprintf(stderr, "ERR: Source 1 is all silence\n"); return EXIT_FAILURE; } short *sf2_data = malloc(sf2_info.frames * sf2_info.channels * sizeof(short)); if (sf_readf_short(sf2, sf2_data, sf2_info.frames) != sf2_info.frames) { - fprintf(stderr, "Couldn't read audio data: %s\n", sf_strerror(sf2)); + fprintf(stderr, "ERR: Couldn't read audio data: %s\n", sf_strerror(sf2)); + return EXIT_FAILURE; + } + + if (is_silence(sf2_data, sf2_info.channels, sf2_info.frames)) { + fprintf(stderr, "ERR: Source 2 is all silence\n"); return EXIT_FAILURE; } /* Calculate cross-correlation between two audio streams by applying * different offsets while keeping the defined minimal overlap. */ + const size_t channels = sf1_info.channels; const size_t sf1_frames = sf1_info.frames; const size_t sf2_frames = sf2_info.frames; const size_t cross_correlation_frames = sf1_frames + sf2_frames; @@ -88,14 +107,17 @@ int main(int argc, char *argv[]) { break; for (size_t j = 0; j < overlap; j++) - cross_correlation[i] += sf1_data[sf1_begin + j] * sf2_data[sf2_begin + j]; + for (size_t k = 0; k < channels; k++) + cross_correlation[i] += + sf1_data[(sf1_begin + j) * channels + k] * + sf2_data[(sf2_begin + j) * channels + k]; } ssize_t max_i = 0; long long max_v = cross_correlation[0]; /* Find the maximum value in the cross-correlation array. */ - for (size_t i = 1; i < cross_correlation_frames; i++) + for (size_t i = min_overlap; i < cross_correlation_frames; i++) if (cross_correlation[i] > max_v) max_v = cross_correlation[max_i = i]; @@ -107,5 +129,5 @@ int main(int argc, char *argv[]) { free(sf1_data); free(sf2_data); - return 0; + return EXIT_SUCCESS; } diff --git a/test/test-io.c b/test/test-io.c index 8c131ace5..158b6738b 100644 --- a/test/test-io.c +++ b/test/test-io.c @@ -394,7 +394,7 @@ static void pcm_write_frames(struct ba_transport_pcm *pcm, size_t frames) { if (dump_data) { #if HAVE_SNDFILE char fname[128]; - sprintf(fname, "sample-sine-%s.wav", transport_pcm_to_fname(pcm)); + sprintf(fname, "sine-%s.wav", transport_pcm_to_fname(pcm)); ck_assert_ptr_nonnull(sf = sf_open_format(fname, pcm->rate, pcm->channels, pcm->format)); #else error("Dumping audio files requires sndfile library!"); From d19da25e55da33afaf7184efa27313bd8f2f822d Mon Sep 17 00:00:00 2001 From: borine <32966433+borine@users.noreply.github.com> Date: Sat, 28 Sep 2024 18:45:35 +0100 Subject: [PATCH 07/15] Fix temporary deadlock in A2DP codec selection If a D-Bus client thread locks the codec_id_mtx mutex while BlueZ is processing a SetConfiguration request, then that can lead to the main thread being blocked if a client calls any method that needs to read the current codec. However, the SetConfiguration request needs the main thread to be active to respond to the ClearConfiguration call that BlueZ makes during its own handling of SetConfiguration. So, there is a risk of deadlock if the client attempts to read the codec ID while SetConfiguration is in progress. This deadlock is temporary because the D-Bus calls have a timeout - but BlueZ handles the timeout by closing the A2DP profile connection. There does not appear to be any clear reason why SetConfiguration needs to lock the client_id_mtx mutex, since it does not actually modify the transport codec_id (a new transport instance is created instead). So this commit avoids this deadlock simply by removing the codec_id_mtx mutex lock from ba_transport_select_codec_a2dp(). Fixes #725 --- src/ba-rfcomm.c | 4 ++-- src/ba-transport.c | 56 +++++++++++++++++++++++----------------------- test/test-rfcomm.c | 4 +++- 3 files changed, 33 insertions(+), 31 deletions(-) diff --git a/src/ba-rfcomm.c b/src/ba-rfcomm.c index 8313f75ce..a1763b3ab 100644 --- a/src/ba-rfcomm.c +++ b/src/ba-rfcomm.c @@ -138,9 +138,9 @@ static void rfcomm_set_hfp_state(struct ba_rfcomm *r, enum hfp_slc_state state) * Finalize HFP codec selection - signal other threads. */ static void rfcomm_finalize_codec_selection(struct ba_rfcomm *r) { - pthread_mutex_lock(&r->sco->codec_id_mtx); + pthread_mutex_lock(&r->sco->codec_select_client_mtx); r->codec_selection_done = true; - pthread_mutex_unlock(&r->sco->codec_id_mtx); + pthread_mutex_unlock(&r->sco->codec_select_client_mtx); pthread_cond_signal(&r->codec_selection_cond); diff --git a/src/ba-transport.c b/src/ba-transport.c index 2b627be8d..884133923 100644 --- a/src/ba-transport.c +++ b/src/ba-transport.c @@ -413,6 +413,7 @@ struct ba_transport *ba_transport_new_a2dp( return NULL; t->profile = profile; + t->codec_id = sep->config.codec_id; pthread_cond_init(&t->a2dp.state_changed_cond, NULL); t->a2dp.state = BLUEZ_A2DP_TRANSPORT_STATE_IDLE; @@ -440,7 +441,7 @@ struct ba_transport *ba_transport_new_a2dp( t->acquire = transport_acquire_bt_a2dp; t->release = transport_release_bt_a2dp; - ba_transport_set_codec(t, sep->config.codec_id); + a2dp_transport_init(t); storage_pcm_data_sync(&t->a2dp.pcm); storage_pcm_data_sync(&t->a2dp.pcm_bc); @@ -547,6 +548,9 @@ struct ba_transport *ba_transport_new_sco( return NULL; t->profile = profile; + /* In case of the HSP and HFP without codec selection support, + * there is no other option than the CVSD codec. */ + t->codec_id = HFP_CODEC_CVSD; err |= transport_pcm_init(&t->sco.pcm_spk, is_ag ? BA_TRANSPORT_PCM_MODE_SINK : BA_TRANSPORT_PCM_MODE_SOURCE, @@ -568,10 +572,6 @@ struct ba_transport *ba_transport_new_sco( t->acquire = transport_acquire_bt_sco; t->release = transport_release_bt_sco; - /* In case of the HSP and HFP without codec selection support, - * there is no other option than the CVSD codec. */ - uint32_t codec_id = HFP_CODEC_CVSD; - #if ENABLE_HFP_CODEC_SELECTION /* Only HFP supports codec selection. */ if (profile & BA_TRANSPORT_PROFILE_MASK_HFP && @@ -580,16 +580,16 @@ struct ba_transport *ba_transport_new_sco( BA_TEST_ESCO_SUPPORT(device->a)) { # if ENABLE_MSBC if (config.hfp.codecs.msbc) - codec_id = HFP_CODEC_UNDEFINED; + t->codec_id = HFP_CODEC_UNDEFINED; # endif # if ENABLE_LC3_SWB if (config.hfp.codecs.lc3_swb) - codec_id = HFP_CODEC_UNDEFINED; + t->codec_id = HFP_CODEC_UNDEFINED; # endif } #endif - ba_transport_set_codec(t, codec_id); + sco_transport_init(t); storage_pcm_data_sync(&t->sco.pcm_spk); storage_pcm_data_sync(&t->sco.pcm_mic); @@ -898,10 +898,10 @@ int ba_transport_select_codec_a2dp( const struct a2dp_sep_config *remote_sep_cfg, const void *configuration) { - if (!(t->profile & BA_TRANSPORT_PROFILE_MASK_A2DP)) - return errno = ENOTSUP, -1; - - pthread_mutex_lock(&t->codec_id_mtx); +#if DEBUG + /* Assert that we were called with the codec selection mutex locked. */ + g_assert_cmpint(pthread_mutex_trylock(&t->codec_select_client_mtx), !=, 0); +#endif /* the same codec with the same configuration already selected */ if (remote_sep_cfg->codec_id == t->codec_id && @@ -920,19 +920,23 @@ int ba_transport_select_codec_a2dp( if (!bluez_a2dp_set_configuration(t->a2dp.bluez_dbus_sep_path, remote_sep_cfg, configuration, &err)) { error("Couldn't set A2DP configuration: %s", err->message); - pthread_mutex_unlock(&t->codec_id_mtx); g_error_free(err); return errno = EIO, -1; } final: - pthread_mutex_unlock(&t->codec_id_mtx); return 0; } int ba_transport_select_codec_sco( struct ba_transport *t, uint8_t codec_id) { + +#if DEBUG + /* Assert that we were called with the codec selection mutex locked. */ + g_assert_cmpint(pthread_mutex_trylock(&t->codec_select_client_mtx), !=, 0); +#endif + switch (t->profile) { case BA_TRANSPORT_PROFILE_HFP_HF: case BA_TRANSPORT_PROFILE_HFP_AG: @@ -942,18 +946,16 @@ int ba_transport_select_codec_sco( if (t->sco.rfcomm == NULL) return errno = ENOTSUP, -1; - /* Lock the mutex because we are about to change the codec ID. The codec - * ID itself will be set by the RFCOMM thread. The RFCOMM thread and the - * current one will be synchronized by the RFCOMM codec selection - * condition variable. */ - pthread_mutex_lock(&t->codec_id_mtx); - struct ba_rfcomm * const r = t->sco.rfcomm; enum ba_rfcomm_signal rfcomm_signal; /* codec already selected, skip switching */ if (t->codec_id == codec_id) - goto final; + return 0; + + /* The codec ID itself will be set by the RFCOMM thread. The + * RFCOMM thread and the current one will be synchronized by + * the RFCOMM codec selection condition variable. */ switch (codec_id) { case HFP_CODEC_CVSD: @@ -983,24 +985,22 @@ int ba_transport_select_codec_sco( ba_rfcomm_send_signal(r, rfcomm_signal); while (!r->codec_selection_done) - pthread_cond_wait(&r->codec_selection_cond, &t->codec_id_mtx); + pthread_cond_wait(&r->codec_selection_cond, &t->codec_select_client_mtx); - if (t->codec_id != codec_id) { - pthread_mutex_unlock(&t->codec_id_mtx); + if (t->codec_id != codec_id) return errno = EIO, -1; - } -final: - pthread_mutex_unlock(&t->codec_id_mtx); #else (void)codec_id; #endif return 0; case BA_TRANSPORT_PROFILE_HSP_HS: case BA_TRANSPORT_PROFILE_HSP_AG: - default: return errno = ENOTSUP, -1; + default: + g_assert_not_reached(); } + } uint32_t ba_transport_get_codec( diff --git a/test/test-rfcomm.c b/test/test-rfcomm.c index 4df1c1239..4374dd9fb 100644 --- a/test/test-rfcomm.c +++ b/test/test-rfcomm.c @@ -187,10 +187,12 @@ CK_START_TEST(test_rfcomm_hsp_hs) { #if ENABLE_HFP_CODEC_SELECTION static void *test_rfcomm_hfp_ag_switch_codecs(void *userdata) { struct ba_transport *sco = userdata; - /* the test code rejects first codec selection request for mSBC */ + pthread_mutex_lock(&sco->codec_select_client_mtx); ck_assert_int_eq(ba_transport_select_codec_sco(sco, HFP_CODEC_CVSD), 0); + /* The test code rejects first codec selection request for mSBC. */ ck_assert_int_eq(ba_transport_select_codec_sco(sco, HFP_CODEC_MSBC), -1); ck_assert_int_eq(ba_transport_select_codec_sco(sco, HFP_CODEC_MSBC), 0); + pthread_mutex_unlock(&sco->codec_select_client_mtx); return NULL; } #endif From f40fe1cbeeddca98e129cda14c4f3316fcfcc56f Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Thu, 26 Sep 2024 22:48:07 +0200 Subject: [PATCH 08/15] Read-write client delay instead of adjustments --- doc/bluealsa-plugins.7.rst | 6 +- doc/bluealsactl.1.rst | 11 +-- doc/org.bluealsa.PCM1.7.rst | 28 ++---- misc/bash-completion/bluealsad | 2 +- src/asound/bluealsa-ctl.c | 16 ++-- src/asound/bluealsa-pcm.c | 2 + src/ba-transport-pcm.c | 33 ------- src/ba-transport-pcm.h | 15 +-- src/ba-transport.c | 12 +-- src/bluealsa-dbus.c | 92 +++---------------- src/bluealsa-dbus.h | 2 +- src/bluealsa-iface.xml | 9 +- src/bluealsactl/Makefile.am | 2 +- src/bluealsactl/bluealsactl.h | 2 + ...-delay-adjustment.c => cmd-client-delay.c} | 26 +++--- src/bluealsactl/main.c | 16 +++- src/sco.c | 4 +- src/shared/dbus-client-pcm.c | 50 ++-------- src/shared/dbus-client-pcm.h | 12 +-- src/storage.c | 72 ++++++++++----- test/test-ba.c | 10 +- test/test-utils-ctl.c | 23 +++-- 22 files changed, 161 insertions(+), 284 deletions(-) rename src/bluealsactl/{cmd-delay-adjustment.c => cmd-client-delay.c} (73%) diff --git a/doc/bluealsa-plugins.7.rst b/doc/bluealsa-plugins.7.rst index 62bad6472..c7f420349 100644 --- a/doc/bluealsa-plugins.7.rst +++ b/doc/bluealsa-plugins.7.rst @@ -429,7 +429,7 @@ CTL Parameters EXT Causes the plugin to include extra controls. These are the controls for - Bluetooth codec selection, volume mode selection, delay adjustment (sync) + Bluetooth codec selection, volume mode selection, client delay (sync) and/or battery level indicator. If the value is **yes** then all of these additional controls are included; if the value is **no** then none of them are included. The default is @@ -452,13 +452,13 @@ CTL Parameters See the `Volume control` section in the ``bluealsad(8)`` for more information on the software volume setting. - The delay adjustment controls are called "Sync". They can be used to apply + The client delay controls are called "Sync". They can be used to apply a fixed adjustment to the delay reported by the associated PCM to the application, and may be useful with applications that need to synchronize the bluetooth audio stream with some some other stream, such as a video. The values are in milliseconds from ``-3275 ms`` to ``+3275 ms`` in steps of ``25 ms``. The playback control has index 0 and the capture control has - index 1. Each codec supported by a PCM has its own delay adjustment value. + index 1. Each codec supported by a PCM has its own client delay value. Note that this control changes only the delay value reported to the application by ALSA, it does not affect the actual delay (latency) of the PCM stream. Values set by this control type are saved in the BlueALSA diff --git a/doc/bluealsactl.1.rst b/doc/bluealsactl.1.rst index 463a86790..be76fefbf 100644 --- a/doc/bluealsactl.1.rst +++ b/doc/bluealsactl.1.rst @@ -195,16 +195,15 @@ soft-volume *PCM_PATH* [*STATE*] for soft-volume on, or **off**, **no**, **false**, **n** or **0** for soft-volume off. -delay-adjustment *PCM_PATH* [*ADJUSTMENT*] - Get or set the DelayAdjustment property of the given PCM for the current - codec. +client-delay *PCM_PATH* [[-]\ *DELAY*] + Get or set the ClientDelay property of the given PCM. - If the *ADJUSTMENT* argument is given, set the DelayAdjustment property for - the current codec in the given PCM. This property may be used by clients to + If the *DELAY* argument is given, set the ClientDelay property for the + given PCM. This property may be used by clients to adjust the reported audio delay and may be useful with PCM devices that do not report an accurate Delay property. - The *ADJUSTMENT* value is in milliseconds and must be a decimal number with + The *DELAY* value is in milliseconds and must be a decimal number with optional sign prefix (e.g. **250**, **-500**, **+360.4**). The permitted range is [-3276.8, 3276.7]. diff --git a/doc/org.bluealsa.PCM1.7.rst b/doc/org.bluealsa.PCM1.7.rst index 45eb5564b..85a126db7 100644 --- a/doc/org.bluealsa.PCM1.7.rst +++ b/doc/org.bluealsa.PCM1.7.rst @@ -80,22 +80,6 @@ void SelectCodec(string codec, dict props) dbus.Error.NotSupported dbus.Error.Failed -void SetDelayAdjustment(string codec, int16 adjustment) - Set an arbitrary adjustment (+/-) to the reported Delay in 1/10 of - millisecond for a specific codec. This adjustment is applied to the Delay - property when that codec is selected, and can be used to compensate for - devices that do not report accurate Delay values. - - Possible Errors: - :: - - dbus.Error.InvalidArguments - -array{string, int16} GetDelayAdjustments() - Return the array of currently set delay adjustments. Each entry of the - array gives the name of a codec and the adjustment that the PCM will apply - to the Delay property when that codec is selected. - Properties ---------- @@ -165,10 +149,14 @@ array{byte} CodecConfiguration [readonly] uint16 Delay [readonly] Approximate PCM delay in 1/10 of millisecond. -int16 DelayAdjustment [readonly] - An adjustment (+/-) included within the reported Delay in 1/10 of - millisecond to compensate for devices that do not report accurate delay - values. +int16 ClientDelay [readwrite] + Positive (or negative) client side delay in 1/10 of millisecond. + + This property shall be set by the client in order to account for the client + side delay. In case of PCM source it shall be set to a value reported by a + playback subsystem to account for playback delay. In case of PCM sink it + can be used to adjust the Delay property to compensate for devices that do + not report accurate delay values. boolean SoftVolume [readwrite] This property determines whether BlueALSA will make volume control diff --git a/misc/bash-completion/bluealsad b/misc/bash-completion/bluealsad index 5be952742..64bf8e4e6 100644 --- a/misc/bash-completion/bluealsad +++ b/misc/bash-completion/bluealsad @@ -291,7 +291,7 @@ _bluealsactl() { # the command names supported by this version of bluealsactl local simple_commands="list-pcms list-services monitor status" - local path_commands="codec delay-adjustment info mute open soft-volume volume" + local path_commands="codec client-delay info mute open soft-volume volume" # options that may appear before or after the command local global_shortopts="-h" diff --git a/src/asound/bluealsa-ctl.c b/src/asound/bluealsa-ctl.c index 4e1bc6dbf..3e5018a16 100644 --- a/src/asound/bluealsa-ctl.c +++ b/src/asound/bluealsa-ctl.c @@ -131,7 +131,7 @@ struct bluealsa_ctl { bool show_codec; /* if true, show volume mode control */ bool show_vol_mode; - /* if true, show delay adjustment sync control */ + /* if true, show client delay sync control */ bool show_delay_sync; /* if true, show battery level indicator */ bool show_battery; @@ -707,7 +707,7 @@ static size_t bluealsa_elem_list_add_pcm_elems(struct bluealsa_ctl *ctl, n++; } - /* add special delay adjustment "sync" element */ + /* add special client delay "sync" element */ if (ctl->show_delay_sync) { elem_list[n].type = CTL_ELEM_TYPE_DELAY_SYNC; elem_list[n].dev = dev; @@ -1243,7 +1243,7 @@ static int bluealsa_read_enumerated(snd_ctl_ext_t *ext, snd_ctl_ext_key_t key, items[0] = pcm->soft_volume ? 1 : 0; break; case CTL_ELEM_TYPE_DELAY_SYNC: - items[0] = DIV_ROUND(pcm->delay_adjustment - INT16_MIN, DELAY_SYNC_STEP); + items[0] = DIV_ROUND(pcm->client_delay - INT16_MIN, DELAY_SYNC_STEP); break; case CTL_ELEM_TYPE_BATTERY: case CTL_ELEM_TYPE_SWITCH: @@ -1296,16 +1296,16 @@ static int bluealsa_write_enumerated(snd_ctl_ext_t *ext, snd_ctl_ext_key_t key, return 0; pcm->soft_volume = soft_volume; if (!ba_dbus_pcm_update(&ctl->dbus_ctx, pcm, BLUEALSA_PCM_SOFT_VOLUME, NULL)) - return -ENOMEM; + return -EIO; break; case CTL_ELEM_TYPE_DELAY_SYNC: if (items[0] >= DELAY_SYNC_NUM_VALUES) return -EINVAL; - const int16_t delay_adjustment = items[0] * DELAY_SYNC_STEP + DELAY_SYNC_MIN_VALUE; - if (pcm->delay_adjustment == delay_adjustment) + const int16_t delay = items[0] * DELAY_SYNC_STEP + DELAY_SYNC_MIN_VALUE; + if (pcm->client_delay == delay) return 0; - if (!ba_dbus_pcm_set_delay_adjustment(&ctl->dbus_ctx, pcm->pcm_path, - pcm->codec.name, delay_adjustment, NULL)) + pcm->client_delay = delay; + if (!ba_dbus_pcm_update(&ctl->dbus_ctx, pcm, BLUEALSA_PCM_CLIENT_DELAY, NULL)) return -EIO; process_events(&ctl->ext); break; diff --git a/src/asound/bluealsa-pcm.c b/src/asound/bluealsa-pcm.c index 2ad702883..124cb4a2f 100644 --- a/src/asound/bluealsa-pcm.c +++ b/src/asound/bluealsa-pcm.c @@ -934,6 +934,8 @@ static snd_pcm_sframes_t bluealsa_calculate_delay(snd_pcm_ioplug_t *io) { /* data transfer (communication) and encoding/decoding */ delay += (io->rate / 100) * pcm->ba_pcm.delay / 100; + /* additional delay specified by the client */ + delay += (io->rate / 100) * pcm->ba_pcm.client_delay / 100; delay += pcm->delay_ex; diff --git a/src/ba-transport-pcm.c b/src/ba-transport-pcm.c index af9be01c7..3c7e19ff1 100644 --- a/src/ba-transport-pcm.c +++ b/src/ba-transport-pcm.c @@ -85,15 +85,12 @@ int transport_pcm_init( pthread_mutex_init(&pcm->mutex, NULL); pthread_mutex_init(&pcm->state_mtx, NULL); - pthread_mutex_init(&pcm->delay_adjustments_mtx, NULL); pthread_mutex_init(&pcm->client_mtx, NULL); pthread_cond_init(&pcm->cond, NULL); if (pipe(pcm->pipe) == -1) return -1; - pcm->delay_adjustments = g_hash_table_new(NULL, NULL); - pcm->ba_dbus_path = g_strdup_printf("%s/%s/%s", t->d->ba_dbus_path, transport_get_dbus_path_type(t->profile), mode == BA_TRANSPORT_PCM_MODE_SOURCE ? "source" : "sink"); @@ -110,7 +107,6 @@ void transport_pcm_free( pthread_mutex_destroy(&pcm->mutex); pthread_mutex_destroy(&pcm->state_mtx); - pthread_mutex_destroy(&pcm->delay_adjustments_mtx); pthread_mutex_destroy(&pcm->client_mtx); pthread_cond_destroy(&pcm->cond); @@ -119,7 +115,6 @@ void transport_pcm_free( if (pcm->pipe[1] != -1) close(pcm->pipe[1]); - g_hash_table_unref(pcm->delay_adjustments); g_free(pcm->ba_dbus_path); } @@ -726,7 +721,6 @@ int ba_transport_pcm_get_delay(const struct ba_transport_pcm *pcm) { const struct ba_transport *t = pcm->t; int delay = pcm->codec_delay_dms + pcm->processing_delay_dms; - delay += ba_transport_pcm_delay_adjustment_get(pcm); if (t->profile & BA_TRANSPORT_PROFILE_MASK_A2DP) delay += t->a2dp.delay; @@ -736,33 +730,6 @@ int ba_transport_pcm_get_delay(const struct ba_transport_pcm *pcm) { return delay; } -int16_t ba_transport_pcm_delay_adjustment_get( - const struct ba_transport_pcm *pcm) { - - struct ba_transport *t = pcm->t; - uint32_t codec_id = ba_transport_get_codec(t); - int16_t adjustment = 0; - - pthread_mutex_lock(MUTABLE(&pcm->delay_adjustments_mtx)); - void *val = g_hash_table_lookup(pcm->delay_adjustments, GINT_TO_POINTER(codec_id)); - pthread_mutex_unlock(MUTABLE(&pcm->delay_adjustments_mtx)); - - if (val != NULL) - adjustment = GPOINTER_TO_INT(val); - - return adjustment; -} - -void ba_transport_pcm_delay_adjustment_set( - struct ba_transport_pcm *pcm, - uint32_t codec_id, - int16_t adjustment) { - pthread_mutex_lock(&pcm->delay_adjustments_mtx); - g_hash_table_insert(pcm->delay_adjustments, - GINT_TO_POINTER(codec_id), GINT_TO_POINTER(adjustment)); - pthread_mutex_unlock(&pcm->delay_adjustments_mtx); -} - const char *ba_transport_pcm_channel_to_string( enum ba_transport_pcm_channel channel) { switch (channel) { diff --git a/src/ba-transport-pcm.h b/src/ba-transport-pcm.h index adc4836d6..73d7fbe14 100644 --- a/src/ba-transport-pcm.h +++ b/src/ba-transport-pcm.h @@ -129,12 +129,8 @@ struct ba_transport_pcm { * the host computational power. It is used to compensate for the time * required to encode or decode audio. */ unsigned int processing_delay_dms; - - /* guard delay adjustments access */ - pthread_mutex_t delay_adjustments_mtx; - /* PCM delay adjustments in 1/10 of millisecond, set by client API to allow - * user correction of delay reporting inaccuracy. */ - GHashTable *delay_adjustments; + /* Positive (or negative) delay reported by the client. */ + int client_delay_dms; /* indicates whether FIFO buffer was synchronized */ bool synced; @@ -260,13 +256,6 @@ int ba_transport_pcm_get_hardware_volume( int ba_transport_pcm_get_delay( const struct ba_transport_pcm *pcm); -int16_t ba_transport_pcm_delay_adjustment_get( - const struct ba_transport_pcm *pcm); -void ba_transport_pcm_delay_adjustment_set( - struct ba_transport_pcm *pcm, - uint32_t codec_id, - int16_t adjustment); - const char *ba_transport_pcm_channel_to_string( enum ba_transport_pcm_channel channel); diff --git a/src/ba-transport.c b/src/ba-transport.c index 884133923..1c5a628eb 100644 --- a/src/ba-transport.c +++ b/src/ba-transport.c @@ -816,6 +816,12 @@ void ba_transport_unref(struct ba_transport *t) { if (ref_count > 0) return; + debug("Freeing transport: %s", ba_transport_debug_name(t)); + g_assert_cmpint(ref_count, ==, 0); + + if (t->bt_fd != -1) + close(t->bt_fd); + if (t->profile & BA_TRANSPORT_PROFILE_MASK_A2DP) { storage_pcm_data_update(&t->a2dp.pcm); storage_pcm_data_update(&t->a2dp.pcm_bc); @@ -825,12 +831,6 @@ void ba_transport_unref(struct ba_transport *t) { storage_pcm_data_update(&t->sco.pcm_mic); } - debug("Freeing transport: %s", ba_transport_debug_name(t)); - g_assert_cmpint(ref_count, ==, 0); - - if (t->bt_fd != -1) - close(t->bt_fd); - ba_device_unref(d); #if DEBUG diff --git a/src/bluealsa-dbus.c b/src/bluealsa-dbus.c index de3a3af5c..27269a3f7 100644 --- a/src/bluealsa-dbus.c +++ b/src/bluealsa-dbus.c @@ -284,8 +284,8 @@ static GVariant *ba_variant_new_pcm_delay(const struct ba_transport_pcm *pcm) { return g_variant_new_uint16(ba_transport_pcm_get_delay(pcm)); } -static GVariant *ba_variant_new_pcm_delay_adjustment(const struct ba_transport_pcm *pcm) { - return g_variant_new_int16(ba_transport_pcm_delay_adjustment_get(pcm)); +static GVariant *ba_variant_new_pcm_client_delay(const struct ba_transport_pcm *pcm) { + return g_variant_new_int16(pcm->client_delay_dms); } static GVariant *ba_variant_new_pcm_soft_volume(const struct ba_transport_pcm *pcm) { @@ -871,74 +871,6 @@ static void bluealsa_pcm_select_codec(GDBusMethodInvocation *inv, void *userdata g_variant_unref(value); } -static void bluealsa_pcm_set_delay_adjustment(GDBusMethodInvocation *inv, void *userdata) { - - GVariant *params = g_dbus_method_invocation_get_parameters(inv); - struct ba_transport_pcm *pcm = userdata; - const struct ba_transport *t = pcm->t; - - const char *codec; - int16_t adjustment; - - g_variant_get(params, "(&sn)", &codec, &adjustment); - - uint32_t codec_id = 0; - bool is_valid = false; - if (t->profile & BA_TRANSPORT_PROFILE_MASK_A2DP) { - codec_id = a2dp_codecs_codec_id_from_string(codec); - is_valid = codec_id != 0xFFFFFFFF; - } - if (t->profile & BA_TRANSPORT_PROFILE_MASK_SCO) { - codec_id = hfp_codec_id_from_string(codec); - is_valid = codec_id != HFP_CODEC_UNDEFINED; - } - - if (!is_valid) { - error("Invalid codec name: %s", codec); - g_dbus_method_invocation_return_error(inv, G_DBUS_ERROR, - G_DBUS_ERROR_INVALID_ARGS, "Invalid codec name: %s", codec); - return; - } - - ba_transport_pcm_delay_adjustment_set(pcm, codec_id, adjustment); - bluealsa_dbus_pcm_update(pcm, BA_DBUS_PCM_UPDATE_DELAY_ADJUSTMENT); - g_dbus_method_invocation_return_value(inv, NULL); - -} - -static void bluealsa_pcm_get_delay_adjustments(GDBusMethodInvocation *inv, void *userdata) { - - struct ba_transport_pcm *pcm = userdata; - const struct ba_transport *t = pcm->t; - - GVariantBuilder adjustments; - g_variant_builder_init(&adjustments, G_VARIANT_TYPE("a{sn}")); - - pthread_mutex_lock(&pcm->delay_adjustments_mtx); - - GHashTableIter iter; - g_hash_table_iter_init(&iter, pcm->delay_adjustments); - - void *key, *value; - while (g_hash_table_iter_next(&iter, &key, &value)) { - const char *codec = NULL; - if (t->profile & BA_TRANSPORT_PROFILE_MASK_A2DP) - codec = a2dp_codecs_codec_id_to_string(GPOINTER_TO_INT(key)); - if (t->profile & BA_TRANSPORT_PROFILE_MASK_SCO) - codec = hfp_codec_id_to_string(GPOINTER_TO_INT(key)); - if (codec != NULL) { - int16_t adjustment = GPOINTER_TO_INT(value); - g_variant_builder_add(&adjustments, "{sn}", codec, adjustment); - } - } - - pthread_mutex_unlock(&pcm->delay_adjustments_mtx); - - g_dbus_method_invocation_return_value(inv, g_variant_new("(a{sn})", &adjustments)); - g_variant_builder_clear(&adjustments); - -} - static void bluealsa_rfcomm_open(GDBusMethodInvocation *inv, void *userdata) { struct ba_rfcomm *r = userdata; @@ -1002,8 +934,8 @@ static GVariant *bluealsa_pcm_get_property(const char *property, } if (strcmp(property, "Delay") == 0) return ba_variant_new_pcm_delay(pcm); - if (strcmp(property, "DelayAdjustment") == 0) - return ba_variant_new_pcm_delay_adjustment(pcm); + if (strcmp(property, "ClientDelay") == 0) + return ba_variant_new_pcm_client_delay(pcm); if (strcmp(property, "SoftVolume") == 0) return ba_variant_new_pcm_soft_volume(pcm); if (strcmp(property, "Volume") == 0) @@ -1028,6 +960,12 @@ static bool bluealsa_pcm_set_property(const char *property, GVariant *value, const bool is_sco = t->profile & BA_TRANSPORT_PROFILE_MASK_SCO; const int volume_max = is_sco ? HFP_VOLUME_GAIN_MAX : BLUEZ_A2DP_VOLUME_MAX; + if (strcmp(property, "ClientDelay") == 0) { + pcm->client_delay_dms = g_variant_get_int16(value); + bluealsa_dbus_pcm_update(pcm, BA_DBUS_PCM_UPDATE_CLIENT_DELAY); + return TRUE; + } + if (strcmp(property, "SoftVolume") == 0) { const bool soft_volume = g_variant_get_boolean(value); @@ -1098,10 +1036,6 @@ int bluealsa_dbus_pcm_register(struct ba_transport_pcm *pcm) { .handler = bluealsa_pcm_get_codecs }, { .method = "SelectCodec", .handler = bluealsa_pcm_select_codec }, - { .method = "SetDelayAdjustment", - .handler = bluealsa_pcm_set_delay_adjustment }, - { .method = "GetDelayAdjustments", - .handler = bluealsa_pcm_get_delay_adjustments }, { 0 }, }; @@ -1159,10 +1093,10 @@ void bluealsa_dbus_pcm_update(struct ba_transport_pcm *pcm, unsigned int mask) { g_variant_builder_add(&props, "{sv}", "Codec", ba_variant_new_pcm_codec(pcm)); if (mask & BA_DBUS_PCM_UPDATE_CODEC_CONFIG) g_variant_builder_add(&props, "{sv}", "CodecConfiguration", ba_variant_new_pcm_codec_config(pcm)); - if (mask & (BA_DBUS_PCM_UPDATE_DELAY | BA_DBUS_PCM_UPDATE_DELAY_ADJUSTMENT)) + if (mask & BA_DBUS_PCM_UPDATE_DELAY) g_variant_builder_add(&props, "{sv}", "Delay", ba_variant_new_pcm_delay(pcm)); - if (mask & BA_DBUS_PCM_UPDATE_DELAY_ADJUSTMENT) - g_variant_builder_add(&props, "{sv}", "DelayAdjustment", ba_variant_new_pcm_delay_adjustment(pcm)); + if (mask & BA_DBUS_PCM_UPDATE_CLIENT_DELAY) + g_variant_builder_add(&props, "{sv}", "ClientDelay", ba_variant_new_pcm_client_delay(pcm)); if (mask & BA_DBUS_PCM_UPDATE_SOFT_VOLUME) g_variant_builder_add(&props, "{sv}", "SoftVolume", ba_variant_new_pcm_soft_volume(pcm)); if (mask & BA_DBUS_PCM_UPDATE_VOLUME) diff --git a/src/bluealsa-dbus.h b/src/bluealsa-dbus.h index c8edfee3c..1eac5f420 100644 --- a/src/bluealsa-dbus.h +++ b/src/bluealsa-dbus.h @@ -29,7 +29,7 @@ #define BA_DBUS_PCM_UPDATE_CODEC (1 << 4) #define BA_DBUS_PCM_UPDATE_CODEC_CONFIG (1 << 5) #define BA_DBUS_PCM_UPDATE_DELAY (1 << 6) -#define BA_DBUS_PCM_UPDATE_DELAY_ADJUSTMENT (1 << 7) +#define BA_DBUS_PCM_UPDATE_CLIENT_DELAY (1 << 7) #define BA_DBUS_PCM_UPDATE_SOFT_VOLUME (1 << 8) #define BA_DBUS_PCM_UPDATE_VOLUME (1 << 9) #define BA_DBUS_PCM_UPDATE_RUNNING (1 << 10) diff --git a/src/bluealsa-iface.xml b/src/bluealsa-iface.xml index 1c14356c9..34fc1cf3d 100644 --- a/src/bluealsa-iface.xml +++ b/src/bluealsa-iface.xml @@ -23,13 +23,6 @@ - - - - - - - @@ -42,7 +35,7 @@ - + diff --git a/src/bluealsactl/Makefile.am b/src/bluealsactl/Makefile.am index 9b11893e9..beba81cba 100644 --- a/src/bluealsactl/Makefile.am +++ b/src/bluealsactl/Makefile.am @@ -11,8 +11,8 @@ bluealsactl_SOURCES = \ ../../src/shared/dbus-client-pcm.c \ ../../src/shared/hex.c \ ../../src/shared/log.c \ + cmd-client-delay.c \ cmd-codec.c \ - cmd-delay-adjustment.c \ cmd-info.c \ cmd-list-pcms.c \ cmd-list-services.c \ diff --git a/src/bluealsactl/bluealsactl.h b/src/bluealsactl/bluealsactl.h index 778a01d5f..6d9d63a3a 100644 --- a/src/bluealsactl/bluealsactl.h +++ b/src/bluealsactl/bluealsactl.h @@ -48,6 +48,8 @@ void bactl_print_adapters(const struct ba_service_props *props); void bactl_print_profiles_and_codecs(const struct ba_service_props *props); void bactl_print_pcm_available_codecs(const struct ba_pcm *pcm, DBusError *err); void bactl_print_pcm_selected_codec(const struct ba_pcm *pcm); +void bactl_print_pcm_delay(const struct ba_pcm *pcm); +void bactl_print_pcm_client_delay(const struct ba_pcm *pcm); void bactl_print_pcm_soft_volume(const struct ba_pcm *pcm); void bactl_print_pcm_volume(const struct ba_pcm *pcm); void bactl_print_pcm_mute(const struct ba_pcm *pcm); diff --git a/src/bluealsactl/cmd-delay-adjustment.c b/src/bluealsactl/cmd-client-delay.c similarity index 73% rename from src/bluealsactl/cmd-delay-adjustment.c rename to src/bluealsactl/cmd-client-delay.c index 6ba198232..053f8837a 100644 --- a/src/bluealsactl/cmd-delay-adjustment.c +++ b/src/bluealsactl/cmd-client-delay.c @@ -1,5 +1,5 @@ /* - * BlueALSA - bluealsactl/cmd-delay-adjustment.c + * BlueALSA - bluealsactl/cmd-client-delay.c * Copyright (c) 2016-2024 Arkadiusz Bokowy * * This file is a part of bluez-alsa. @@ -21,17 +21,17 @@ #include "shared/dbus-client-pcm.h" static void usage(const char *command) { - printf("Get or set the delay adjustment of the given PCM.\n\n"); - bactl_print_usage("%s [OPTION]... PCM-PATH [ADJUSTMENT]", command); + printf("Get or set the client delay of the given PCM.\n\n"); + bactl_print_usage("%s [OPTION]... PCM-PATH [[-]DELAY]", command); printf("\nOptions:\n" " -h, --help\t\tShow this message and exit\n" "\nPositional arguments:\n" " PCM-PATH\tBlueALSA PCM D-Bus object path\n" - " ADJUSTMENT\tAdjustment value (+/-), in milliseconds\n" + " DELAY\tValue (+/-), in milliseconds\n" ); } -static int cmd_delay_adjustment_func(int argc, char *argv[]) { +static int cmd_client_delay_func(int argc, char *argv[]) { int opt; const char *opts = "+hqv"; @@ -71,7 +71,7 @@ static int cmd_delay_adjustment_func(int argc, char *argv[]) { } if (argc == 2) { - printf("DelayAdjustment: %#.1f ms\n", (double)pcm.delay_adjustment/ 10); + bactl_print_pcm_client_delay(&pcm); return EXIT_SUCCESS; } @@ -90,17 +90,17 @@ static int cmd_delay_adjustment_func(int argc, char *argv[]) { return EXIT_FAILURE; } - if (!ba_dbus_pcm_set_delay_adjustment(&config.dbus, pcm.pcm_path, - pcm.codec.name, adjustment, &err)) { - cmd_print_error("DelayAdjustment update failed: %s", err.message); + pcm.client_delay = adjustment; + if (!ba_dbus_pcm_update(&config.dbus, &pcm, BLUEALSA_PCM_CLIENT_DELAY, &err)) { + cmd_print_error("ClientDelay update failed: %s", err.message); return EXIT_FAILURE; } return EXIT_SUCCESS; } -const struct bactl_command cmd_delay_adjustment = { - "delay-adjustment", - "Get or set PCM delay adjustment", - cmd_delay_adjustment_func, +const struct bactl_command cmd_client_delay = { + "client-delay", + "Get or set PCM client delay", + cmd_client_delay_func, }; diff --git a/src/bluealsactl/main.c b/src/bluealsactl/main.c index 8552311a3..90eaf0710 100644 --- a/src/bluealsactl/main.c +++ b/src/bluealsactl/main.c @@ -277,6 +277,14 @@ void bactl_print_pcm_selected_codec(const struct ba_pcm *pcm) { printf("\n"); } +void bactl_print_pcm_delay(const struct ba_pcm *pcm) { + printf("Delay: %#.1f ms\n", pcm->delay / 10.0); +} + +void bactl_print_pcm_client_delay(const struct ba_pcm *pcm) { + printf("ClientDelay: %#.1f ms\n", pcm->client_delay / 10.0); +} + void bactl_print_pcm_soft_volume(const struct ba_pcm *pcm) { printf("SoftVolume: %s\n", pcm->soft_volume ? "true" : "false"); } @@ -314,8 +322,8 @@ void bactl_print_pcm_properties(const struct ba_pcm *pcm, DBusError *err) { printf("Rate: %d Hz\n", pcm->rate); bactl_print_pcm_available_codecs(pcm, err); bactl_print_pcm_selected_codec(pcm); - printf("Delay: %#.1f ms\n", (double)pcm->delay / 10); - printf("DelayAdjustment: %#.1f ms\n", (double)pcm->delay_adjustment / 10); + bactl_print_pcm_delay(pcm); + bactl_print_pcm_client_delay(pcm); bactl_print_pcm_soft_volume(pcm); bactl_print_pcm_volume(pcm); bactl_print_pcm_mute(pcm); @@ -339,7 +347,7 @@ extern const struct bactl_command cmd_list_pcms; extern const struct bactl_command cmd_status; extern const struct bactl_command cmd_info; extern const struct bactl_command cmd_codec; -extern const struct bactl_command cmd_delay_adjustment; +extern const struct bactl_command cmd_client_delay; extern const struct bactl_command cmd_monitor; extern const struct bactl_command cmd_mute; extern const struct bactl_command cmd_open; @@ -352,7 +360,7 @@ static const struct bactl_command *commands[] = { &cmd_status, &cmd_info, &cmd_codec, - &cmd_delay_adjustment, + &cmd_client_delay, &cmd_volume, &cmd_mute, &cmd_softvol, diff --git a/src/sco.c b/src/sco.c index 532985940..a0421aa45 100644 --- a/src/sco.c +++ b/src/sco.c @@ -297,13 +297,13 @@ void sco_transport_init(struct ba_transport *t) { bluealsa_dbus_pcm_update(&t->sco.pcm_spk, BA_DBUS_PCM_UPDATE_RATE | BA_DBUS_PCM_UPDATE_CODEC | - BA_DBUS_PCM_UPDATE_DELAY_ADJUSTMENT); + BA_DBUS_PCM_UPDATE_CLIENT_DELAY); if (t->sco.pcm_mic.ba_dbus_exported) bluealsa_dbus_pcm_update(&t->sco.pcm_mic, BA_DBUS_PCM_UPDATE_RATE | BA_DBUS_PCM_UPDATE_CODEC | - BA_DBUS_PCM_UPDATE_DELAY_ADJUSTMENT); + BA_DBUS_PCM_UPDATE_CLIENT_DELAY); } diff --git a/src/shared/dbus-client-pcm.c b/src/shared/dbus-client-pcm.c index 4728c2235..8171ebada 100644 --- a/src/shared/dbus-client-pcm.c +++ b/src/shared/dbus-client-pcm.c @@ -508,44 +508,6 @@ dbus_bool_t ba_dbus_pcm_select_codec( return rv; } -dbus_bool_t ba_dbus_pcm_set_delay_adjustment( - struct ba_dbus_ctx *ctx, - const char *pcm_path, - const char *codec, - int16_t adjustment, - DBusError *error) { - - DBusMessage *msg = NULL, *rep = NULL; - dbus_bool_t rv = FALSE; - - if ((msg = dbus_message_new_method_call(ctx->ba_service, pcm_path, - BLUEALSA_INTERFACE_PCM, "SetDelayAdjustment")) == NULL) { - dbus_set_error_const(error, DBUS_ERROR_NO_MEMORY, NULL); - goto fail; - } - - DBusMessageIter iter; - dbus_message_iter_init_append(msg, &iter); - if (!dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING, &codec) || - !dbus_message_iter_append_basic(&iter, DBUS_TYPE_INT16, &adjustment)) { - dbus_set_error_const(error, DBUS_ERROR_NO_MEMORY, NULL); - goto fail; - } - - if ((rep = dbus_connection_send_with_reply_and_block(ctx->conn, - msg, DBUS_TIMEOUT_USE_DEFAULT, error)) == NULL) - goto fail; - - rv = TRUE; - -fail: - if (msg != NULL) - dbus_message_unref(msg); - if (rep != NULL) - dbus_message_unref(rep); - return rv; -} - /** * Update BlueALSA PCM property. */ dbus_bool_t ba_dbus_pcm_update( @@ -559,6 +521,10 @@ dbus_bool_t ba_dbus_pcm_update( const char *type = NULL; switch (property) { + case BLUEALSA_PCM_CLIENT_DELAY: + _property = "ClientDelay"; + type = DBUS_TYPE_INT16_AS_STRING; + break; case BLUEALSA_PCM_SOFT_VOLUME: _property = "SoftVolume"; type = DBUS_TYPE_BOOLEAN_AS_STRING; @@ -584,6 +550,10 @@ dbus_bool_t ba_dbus_pcm_update( goto fail; switch (property) { + case BLUEALSA_PCM_CLIENT_DELAY: + if (!dbus_message_iter_append_basic(&variant, DBUS_TYPE_INT16, &pcm->client_delay)) + goto fail; + break; case BLUEALSA_PCM_SOFT_VOLUME: if (!dbus_message_iter_append_basic(&variant, DBUS_TYPE_BOOLEAN, &pcm->soft_volume)) goto fail; @@ -833,10 +803,10 @@ static dbus_bool_t dbus_message_iter_get_ba_pcm_props_cb(const char *key, goto fail; dbus_message_iter_get_basic(&variant, &pcm->delay); } - else if (strcmp(key, "DelayAdjustment") == 0) { + else if (strcmp(key, "ClientDelay") == 0) { if (type != (type_expected = DBUS_TYPE_INT16)) goto fail; - dbus_message_iter_get_basic(&variant, &pcm->delay_adjustment); + dbus_message_iter_get_basic(&variant, &pcm->client_delay); } else if (strcmp(key, "SoftVolume") == 0) { if (type != (type_expected = DBUS_TYPE_BOOLEAN)) diff --git a/src/shared/dbus-client-pcm.h b/src/shared/dbus-client-pcm.h index 29007eaf0..ce52d99a4 100644 --- a/src/shared/dbus-client-pcm.h +++ b/src/shared/dbus-client-pcm.h @@ -71,6 +71,7 @@ /** * BlueALSA PCM object property. */ enum ba_pcm_property { + BLUEALSA_PCM_CLIENT_DELAY, BLUEALSA_PCM_SOFT_VOLUME, BLUEALSA_PCM_VOLUME, }; @@ -134,8 +135,8 @@ struct ba_pcm { struct ba_pcm_codec codec; /* approximate PCM delay */ dbus_uint16_t delay; - /* manual delay adjustment */ - dbus_int16_t delay_adjustment; + /* client delay */ + dbus_int16_t client_delay; /* software volume */ dbus_bool_t soft_volume; @@ -204,13 +205,6 @@ dbus_bool_t ba_dbus_pcm_select_codec( unsigned int flags, DBusError *error); -dbus_bool_t ba_dbus_pcm_set_delay_adjustment( - struct ba_dbus_ctx *ctx, - const char *pcm_path, - const char *codec, - int16_t adjustment, - DBusError *error); - dbus_bool_t ba_dbus_pcm_update( struct ba_dbus_ctx *ctx, const struct ba_pcm *pcm, diff --git a/src/storage.c b/src/storage.c index 9807905d9..b85bf62d4 100644 --- a/src/storage.c +++ b/src/storage.c @@ -30,7 +30,7 @@ #include "shared/defs.h" #include "shared/log.h" -#define BA_STORAGE_KEY_DELAY_ADJUSTMENT "DelayAdjustments" +#define BA_STORAGE_KEY_CLIENT_DELAYS "ClientDelays" #define BA_STORAGE_KEY_SOFT_VOLUME "SoftVolume" #define BA_STORAGE_KEY_VOLUME "Volume" #define BA_STORAGE_KEY_MUTE "Mute" @@ -198,22 +198,24 @@ int storage_device_clear(const struct ba_device *d) { return rv; } -static int storage_pcm_data_sync_delay(GKeyFile *db, const char *group, - struct ba_transport_pcm *pcm) { +/** + * Load PCM client delays to the hash table. */ +static GHashTable *storage_pcm_data_load_delays(GKeyFile *db, const char *group, + const struct ba_transport_pcm *pcm) { const struct ba_transport *t = pcm->t; - char **adjustments; - gsize length; + GHashTable *delays = g_hash_table_new(NULL, NULL); - if ((adjustments = g_key_file_get_string_list(db, group, - BA_STORAGE_KEY_DELAY_ADJUSTMENT, &length, NULL)) == NULL) - return 0; + size_t length = 0; + char **list = g_key_file_get_string_list(db, group, + BA_STORAGE_KEY_CLIENT_DELAYS, &length, NULL); - for (gsize index = 0; index < length; index++) { - char *codec_name = adjustments[index]; - char *value = strchr(adjustments[index], ':'); + for (size_t i = 0; i < length; i++) { + char *codec_name = list[i]; + char *value = strchr(list[i], ':'); if (value == NULL) continue; + /* Split string into a codec name and delay value. */ *value++ = '\0'; uint32_t codec_id = 0xFFFFFFFF; if (t->profile & BA_TRANSPORT_PROFILE_MASK_A2DP && @@ -222,12 +224,32 @@ static int storage_pcm_data_sync_delay(GKeyFile *db, const char *group, if (t->profile & BA_TRANSPORT_PROFILE_MASK_SCO && (codec_id = hfp_codec_id_from_string(codec_name)) == HFP_CODEC_UNDEFINED) continue; - int16_t adjustment = atoi(value); - ba_transport_pcm_delay_adjustment_set(pcm, codec_id, adjustment); + const int16_t delay = atoi(value); + g_hash_table_insert(delays, GINT_TO_POINTER(codec_id), GINT_TO_POINTER(delay)); + } + + g_strfreev(list); + return delays; +} + +static int storage_pcm_data_sync_delay(GKeyFile *db, const char *group, + struct ba_transport_pcm *pcm) { + + const struct ba_transport *t = pcm->t; + int rv = 0; + + GHashTable *delays = storage_pcm_data_load_delays(db, group, pcm); + + void *value = NULL; + const uint32_t codec_id = t->codec_id; + if (g_hash_table_lookup_extended(delays, GINT_TO_POINTER(codec_id), NULL, &value)) { + /* If the right codec was found, sync the client delay. */ + pcm->client_delay_dms = GPOINTER_TO_INT(value); + rv = 1; } - g_strfreev(adjustments); - return 1; + g_hash_table_unref(delays); + return rv; } static int storage_pcm_data_sync_volume(GKeyFile *db, const char *group, @@ -313,17 +335,24 @@ static void storage_pcm_data_update_delay(GKeyFile *db, const char *group, const struct ba_transport_pcm *pcm) { const struct ba_transport *t = pcm->t; - const size_t num_codecs = g_hash_table_size(pcm->delay_adjustments); - char **list = calloc(num_codecs + 1, sizeof(char *)); - pthread_mutex_lock(MUTABLE(&pcm->delay_adjustments_mtx)); + GHashTable *delays = storage_pcm_data_load_delays(db, group, pcm); + /* Update the delay value for the current codec. */ + g_hash_table_insert(delays, GINT_TO_POINTER(t->codec_id), + GINT_TO_POINTER(pcm->client_delay_dms)); + + const size_t num_codecs = g_hash_table_size(delays); + char **list = calloc(num_codecs + 1, sizeof(char *)); GHashTableIter iter; - g_hash_table_iter_init(&iter, pcm->delay_adjustments); + g_hash_table_iter_init(&iter, delays); size_t index = 0; void *key, *value; while (g_hash_table_iter_next(&iter, &key, &value)) { + if (GPOINTER_TO_INT(value) == 0) + /* Do not store the delay if it is zero. */ + continue; const char *codec = NULL; if (t->profile & BA_TRANSPORT_PROFILE_MASK_A2DP) codec = a2dp_codecs_codec_id_to_string(GPOINTER_TO_INT(key)); @@ -336,11 +365,10 @@ static void storage_pcm_data_update_delay(GKeyFile *db, const char *group, index++; } - pthread_mutex_unlock(MUTABLE(&pcm->delay_adjustments_mtx)); - - g_key_file_set_string_list(db, group, BA_STORAGE_KEY_DELAY_ADJUSTMENT, + g_key_file_set_string_list(db, group, BA_STORAGE_KEY_CLIENT_DELAYS, (const char * const *)list, num_codecs); + g_hash_table_unref(delays); for (size_t i = 0; i < num_codecs; i++) free(list[i]); free(list); diff --git a/test/test-ba.c b/test/test-ba.c index 5a9c36d0d..f629a9c60 100644 --- a/test/test-ba.c +++ b/test/test-ba.c @@ -390,7 +390,7 @@ CK_START_TEST(test_storage) { const char *storage_path = TEST_BLUEALSA_STORAGE_DIR "/00:11:22:33:44:55"; const char *storage_data = "[/org/bluealsa/hci0/dev_00_11_22_33_44_55/a2dpsnk/source]\n" - "DelayAdjustments=SBC:-200\n" + "ClientDelays=SBC:-200\n" "SoftVolume=false\n" "Volume=-5600;-4800;\n" "Mute=false;true;\n"; @@ -426,13 +426,13 @@ CK_START_TEST(test_storage) { ck_assert_int_eq(t->a2dp.pcm.volume[0].soft_mute, false); ck_assert_int_eq(t->a2dp.pcm.volume[1].level, -4800); ck_assert_int_eq(t->a2dp.pcm.volume[1].soft_mute, true); - ck_assert_int_eq(ba_transport_pcm_delay_adjustment_get(&t->a2dp.pcm), -200); + ck_assert_int_eq(t->a2dp.pcm.client_delay_dms, -200); bool muted = true; int level = ba_transport_pcm_volume_range_to_level(100, BLUEZ_A2DP_VOLUME_MAX); ba_transport_pcm_volume_set(&t->a2dp.pcm.volume[0], &level, &muted, NULL); ba_transport_pcm_volume_set(&t->a2dp.pcm.volume[1], &level, &muted, NULL); - ba_transport_pcm_delay_adjustment_set(&t->a2dp.pcm, A2DP_CODEC_SBC, 140); + t->a2dp.pcm.client_delay_dms = 140; ba_adapter_unref(a); ba_device_unref(d); @@ -446,13 +446,13 @@ CK_START_TEST(test_storage) { const char *storage_data_new = "[/org/bluealsa/hci0/dev_00_11_22_33_44_55/a2dpsnk/source]\n" - "DelayAdjustments=SBC:140;\n" + "ClientDelays=SBC:140;\n" "SoftVolume=false\n" "Volume=-344;-344;\n" "Mute=true;true;\n" "\n" "[/org/bluealsa/hci0/dev_00_11_22_33_44_55/a2dpsnk/sink]\n" - "DelayAdjustments=\n" + "ClientDelays=\n" "SoftVolume=false\n" "Volume=\n" "Mute=\n"; diff --git a/test/test-utils-ctl.c b/test/test-utils-ctl.c index f57733bd1..9d6d327a7 100644 --- a/test/test-utils-ctl.c +++ b/test/test-utils-ctl.c @@ -260,7 +260,7 @@ CK_START_TEST(test_codec) { } CK_END_TEST -CK_START_TEST(test_delay_adjustment) { +CK_START_TEST(test_client_delay) { struct spawn_process sp_ba_mock; ck_assert_int_ne(spawn_bluealsa_mock(&sp_ba_mock, NULL, true, @@ -271,23 +271,26 @@ CK_START_TEST(test_delay_adjustment) { /* check printing help text */ ck_assert_int_eq(run_bluealsactl(output, sizeof(output), - "delay-adjustment", "--help", NULL), 0); + "client-delay", "--help", NULL), 0); ck_assert_ptr_ne(strstr(output, "-h, --help"), NULL); - /* check default delay adjustment */ + /* check default client delay */ ck_assert_int_eq(run_bluealsactl(output, sizeof(output), - "delay-adjustment", "/org/bluealsa/hci11/dev_12_34_56_78_9A_BC/a2dpsrc/sink", + "client-delay", "/org/bluealsa/hci11/dev_12_34_56_78_9A_BC/a2dpsrc/sink", NULL), 0); - ck_assert_ptr_ne(strstr(output, "DelayAdjustment: 0.0 ms"), NULL); + ck_assert_ptr_ne(strstr(output, "ClientDelay: 0.0 ms"), NULL); - /* check setting delay adjustment */ + /* check setting client delay */ ck_assert_int_eq(run_bluealsactl(output, sizeof(output), - "delay-adjustment", "/org/bluealsa/hci11/dev_12_34_56_78_9A_BC/a2dpsrc/sink", "-7.5", + "client-delay", "/org/bluealsa/hci11/dev_12_34_56_78_9A_BC/a2dpsrc/sink", "-7.5", NULL), 0); + + /* check that setting client delay does not affect delay */ ck_assert_int_eq(run_bluealsactl(output, sizeof(output), - "delay-adjustment", "/org/bluealsa/hci11/dev_12_34_56_78_9A_BC/a2dpsrc/sink", + "info", "/org/bluealsa/hci11/dev_12_34_56_78_9A_BC/a2dpsrc/sink", NULL), 0); - ck_assert_ptr_ne(strstr(output, "DelayAdjustment: -7.5 ms"), NULL); + ck_assert_ptr_ne(strstr(output, "ClientDelay: -7.5 ms"), NULL); + ck_assert_ptr_ne(strstr(output, "Delay: 10.0 ms"), NULL); spawn_terminate(&sp_ba_mock, 0); spawn_close(&sp_ba_mock, NULL); @@ -501,7 +504,7 @@ int main(int argc, char *argv[]) { tcase_add_test(tc, test_list_pcms); tcase_add_test(tc, test_info); tcase_add_test(tc, test_codec); - tcase_add_test(tc, test_delay_adjustment); + tcase_add_test(tc, test_client_delay); tcase_add_test(tc, test_volume); tcase_add_test(tc, test_monitor); tcase_add_test(tc, test_open); From e640aa4c57d35e213e14833b6525de620232f599 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Fri, 27 Sep 2024 21:01:12 +0200 Subject: [PATCH 09/15] PCM delay and client delay properties monitoring --- doc/bluealsactl.1.rst | 4 ++-- src/bluealsactl/cmd-monitor.c | 36 +++++++++++++++++++++++++++-------- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/doc/bluealsactl.1.rst b/doc/bluealsactl.1.rst index be76fefbf..5f5dfe204 100644 --- a/doc/bluealsactl.1.rst +++ b/doc/bluealsactl.1.rst @@ -245,8 +245,8 @@ monitor [-p[PROPS] | --properties[=PROPS]] ``PropertyChanged PCM_PATH PROPERTY_NAME VALUE`` - Property names that can be monitored are **Codec**, **Running**, - **SoftVolume** and **Volume**. + Property names that can be monitored are **Codec**, **Delay**, + **ClientDelay**, **Running**, **SoftVolume** and **Volume**. Volume is an array of values, each showing the loudness and mute components of a channel. The order of the values corresponds to the ChannelMap diff --git a/src/bluealsactl/cmd-monitor.c b/src/bluealsactl/cmd-monitor.c index 2bae99a42..b4f2032b7 100644 --- a/src/bluealsactl/cmd-monitor.c +++ b/src/bluealsactl/cmd-monitor.c @@ -26,8 +26,10 @@ enum { PROPERTY_CODEC, + PROPERTY_DELAY, + PROPERTY_CLIENT_DELAY, PROPERTY_RUNNING, - PROPERTY_SOFTVOL, + PROPERTY_SOFT_VOLUME, PROPERTY_VOLUME, }; @@ -39,8 +41,10 @@ struct property { static bool monitor_properties = false; static struct property monitor_properties_set[] = { [PROPERTY_CODEC] = { "Codec", false }, + [PROPERTY_DELAY] = { "Delay", false }, + [PROPERTY_CLIENT_DELAY] = { "ClientDelay", false }, [PROPERTY_RUNNING] = { "Running", false }, - [PROPERTY_SOFTVOL] = { "SoftVolume", false }, + [PROPERTY_SOFT_VOLUME] = { "SoftVolume", false }, [PROPERTY_VOLUME] = { "Volume", false }, }; @@ -76,7 +80,23 @@ static dbus_bool_t monitor_dbus_message_iter_get_pcm_props_cb(const char *key, goto fail; const char *codec; dbus_message_iter_get_basic(&variant, &codec); - printf("PropertyChanged %s Codec %s\n", path, codec); + printf("PropertyChanged %s %s %s\n", path, key, codec); + } + else if (monitor_properties_set[PROPERTY_DELAY].enabled && + strcmp(key, monitor_properties_set[PROPERTY_DELAY].name) == 0) { + if (type != (type_expected = DBUS_TYPE_UINT16)) + goto fail; + dbus_uint16_t delay; + dbus_message_iter_get_basic(&variant, &delay); + printf("PropertyChanged %s %s %#.1f\n", path, key, delay / 10.0); + } + else if (monitor_properties_set[PROPERTY_CLIENT_DELAY].enabled && + strcmp(key, monitor_properties_set[PROPERTY_CLIENT_DELAY].name) == 0) { + if (type != (type_expected = DBUS_TYPE_INT16)) + goto fail; + dbus_int16_t delay; + dbus_message_iter_get_basic(&variant, &delay); + printf("PropertyChanged %s %s %#.1f\n", path, key, delay / 10.0); } else if (monitor_properties_set[PROPERTY_RUNNING].enabled && strcmp(key, monitor_properties_set[PROPERTY_RUNNING].name) == 0) { @@ -84,15 +104,15 @@ static dbus_bool_t monitor_dbus_message_iter_get_pcm_props_cb(const char *key, goto fail; dbus_bool_t running; dbus_message_iter_get_basic(&variant, &running); - printf("PropertyChanged %s Running %s\n", path, running ? "true" : "false"); + printf("PropertyChanged %s %s %s\n", path, key, running ? "true" : "false"); } - else if (monitor_properties_set[PROPERTY_SOFTVOL].enabled && - strcmp(key, monitor_properties_set[PROPERTY_SOFTVOL].name) == 0) { + else if (monitor_properties_set[PROPERTY_SOFT_VOLUME].enabled && + strcmp(key, monitor_properties_set[PROPERTY_SOFT_VOLUME].name) == 0) { if (type != (type_expected = DBUS_TYPE_BOOLEAN)) goto fail; dbus_bool_t softvol; dbus_message_iter_get_basic(&variant, &softvol); - printf("PropertyChanged %s SoftVolume %s\n", path, softvol ? "true" : "false"); + printf("PropertyChanged %s %s %s\n", path, key, softvol ? "true" : "false"); } else if (monitor_properties_set[PROPERTY_VOLUME].enabled && strcmp(key, monitor_properties_set[PROPERTY_VOLUME].name) == 0) { @@ -106,7 +126,7 @@ static dbus_bool_t monitor_dbus_message_iter_get_pcm_props_cb(const char *key, dbus_message_iter_recurse(&variant, &iter); dbus_message_iter_get_fixed_array(&iter, &data, &len); - printf("PropertyChanged %s Volume", path); + printf("PropertyChanged %s %s", path, key); for (size_t i = 0; i < (size_t)len; i++) printf(" %u%s", data[i] & 0x7f, data[i] & 0x80 ? "[M]" : ""); printf("\n"); From 4b7f1ce40996f59e3c270d52e561c500ccc96d0d Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Sun, 13 Oct 2024 20:51:01 +0200 Subject: [PATCH 10/15] Check for SEP init failure when creating transport --- src/a2dp.c | 11 -------- src/a2dp.h | 5 ---- src/ba-transport.c | 63 +++++++++++++++++++++++++--------------------- src/sco.c | 3 ++- src/sco.h | 4 +-- test/test-ba.c | 15 ++++++++--- test/test-io.c | 9 ++++--- test/test-rfcomm.c | 2 -- 8 files changed, 56 insertions(+), 56 deletions(-) diff --git a/src/a2dp.c b/src/a2dp.c index f3dbb7be5..34c5bf103 100644 --- a/src/a2dp.c +++ b/src/a2dp.c @@ -43,7 +43,6 @@ #endif #include "a2dp-sbc.h" #include "ba-config.h" -#include "ba-transport.h" #include "shared/a2dp-codecs.h" #include "shared/log.h" @@ -390,13 +389,3 @@ const char *a2dp_check_strerror( debug("Unknown error code: %#x", err); return "Check error"; } - -int a2dp_transport_init( - struct ba_transport *t) { - return t->a2dp.sep->transport_init(t); -} - -int a2dp_transport_start( - struct ba_transport *t) { - return t->a2dp.sep->transport_start(t); -} diff --git a/src/a2dp.h b/src/a2dp.h index d451310f8..0018133ed 100644 --- a/src/a2dp.h +++ b/src/a2dp.h @@ -240,9 +240,4 @@ enum a2dp_check_err a2dp_check_configuration( const char *a2dp_check_strerror( enum a2dp_check_err err); -int a2dp_transport_init( - struct ba_transport *t); -int a2dp_transport_start( - struct ba_transport *t); - #endif diff --git a/src/ba-transport.c b/src/ba-transport.c index 1c5a628eb..f0ace4b63 100644 --- a/src/ba-transport.c +++ b/src/ba-transport.c @@ -421,6 +421,9 @@ struct ba_transport *ba_transport_new_a2dp( t->a2dp.sep = sep; memcpy(&t->a2dp.configuration, configuration, sep->config.caps_size); + t->acquire = transport_acquire_bt_a2dp; + t->release = transport_release_bt_a2dp; + err |= transport_pcm_init(&t->a2dp.pcm, is_sink ? BA_TRANSPORT_PCM_MODE_SOURCE : BA_TRANSPORT_PCM_MODE_SINK, t, true); @@ -432,17 +435,18 @@ struct ba_transport *ba_transport_new_a2dp( if (err != 0) goto fail; + /* do codec-specific initialization */ + if (sep->transport_init(t) != 0) { + errno = EINVAL; + goto fail; + } + if ((errno = pthread_create(&t->thread_manager_thread_id, NULL, PTHREAD_FUNC(transport_thread_manager), t)) != 0) { t->thread_manager_thread_id = config.main_thread; goto fail; } - t->acquire = transport_acquire_bt_a2dp; - t->release = transport_release_bt_a2dp; - - a2dp_transport_init(t); - storage_pcm_data_sync(&t->a2dp.pcm); storage_pcm_data_sync(&t->a2dp.pcm_bc); @@ -552,26 +556,6 @@ struct ba_transport *ba_transport_new_sco( * there is no other option than the CVSD codec. */ t->codec_id = HFP_CODEC_CVSD; - err |= transport_pcm_init(&t->sco.pcm_spk, - is_ag ? BA_TRANSPORT_PCM_MODE_SINK : BA_TRANSPORT_PCM_MODE_SOURCE, - t, true); - - err |= transport_pcm_init(&t->sco.pcm_mic, - is_ag ? BA_TRANSPORT_PCM_MODE_SOURCE : BA_TRANSPORT_PCM_MODE_SINK, - t, false); - - if (err != 0) - goto fail; - - if ((errno = pthread_create(&t->thread_manager_thread_id, - NULL, PTHREAD_FUNC(transport_thread_manager), t)) != 0) { - t->thread_manager_thread_id = config.main_thread; - goto fail; - } - - t->acquire = transport_acquire_bt_sco; - t->release = transport_release_bt_sco; - #if ENABLE_HFP_CODEC_SELECTION /* Only HFP supports codec selection. */ if (profile & BA_TRANSPORT_PROFILE_MASK_HFP && @@ -589,7 +573,30 @@ struct ba_transport *ba_transport_new_sco( } #endif - sco_transport_init(t); + t->acquire = transport_acquire_bt_sco; + t->release = transport_release_bt_sco; + + err |= transport_pcm_init(&t->sco.pcm_spk, + is_ag ? BA_TRANSPORT_PCM_MODE_SINK : BA_TRANSPORT_PCM_MODE_SOURCE, + t, true); + + err |= transport_pcm_init(&t->sco.pcm_mic, + is_ag ? BA_TRANSPORT_PCM_MODE_SOURCE : BA_TRANSPORT_PCM_MODE_SINK, + t, false); + + if (err != 0) + goto fail; + + if (sco_transport_init(t) != 0) { + errno = EINVAL; + goto fail; + } + + if ((errno = pthread_create(&t->thread_manager_thread_id, + NULL, PTHREAD_FUNC(transport_thread_manager), t)) != 0) { + t->thread_manager_thread_id = config.main_thread; + goto fail; + } storage_pcm_data_sync(&t->sco.pcm_spk); storage_pcm_data_sync(&t->sco.pcm_mic); @@ -1026,7 +1033,7 @@ void ba_transport_set_codec( return; if (t->profile & BA_TRANSPORT_PROFILE_MASK_A2DP) - a2dp_transport_init(t); + t->a2dp.sep->transport_init(t); else if (t->profile & BA_TRANSPORT_PROFILE_MASK_SCO) sco_transport_init(t); @@ -1077,7 +1084,7 @@ int ba_transport_start(struct ba_transport *t) { debug("Starting transport: %s", ba_transport_debug_name(t)); if (t->profile & BA_TRANSPORT_PROFILE_MASK_A2DP) - return a2dp_transport_start(t); + return t->a2dp.sep->transport_start(t); if (t->profile & BA_TRANSPORT_PROFILE_MASK_SCO) return sco_transport_start(t); diff --git a/src/sco.c b/src/sco.c index a0421aa45..e839cbfb7 100644 --- a/src/sco.c +++ b/src/sco.c @@ -256,7 +256,7 @@ void *sco_dec_thread(struct ba_transport_pcm *pcm) { } } -void sco_transport_init(struct ba_transport *t) { +int sco_transport_init(struct ba_transport *t) { t->sco.pcm_spk.format = BA_TRANSPORT_PCM_FORMAT_S16_2LE; t->sco.pcm_spk.channels = 1; @@ -305,6 +305,7 @@ void sco_transport_init(struct ba_transport *t) { BA_DBUS_PCM_UPDATE_CODEC | BA_DBUS_PCM_UPDATE_CLIENT_DELAY); + return 0; } int sco_transport_start(struct ba_transport *t) { diff --git a/src/sco.h b/src/sco.h index a32db9391..c9cc23c8a 100644 --- a/src/sco.h +++ b/src/sco.h @@ -1,6 +1,6 @@ /* * BlueALSA - sco.h - * Copyright (c) 2016-2023 Arkadiusz Bokowy + * Copyright (c) 2016-2024 Arkadiusz Bokowy * * This file is a part of bluez-alsa. * @@ -20,7 +20,7 @@ #include "ba-transport.h" int sco_setup_connection_dispatcher(struct ba_adapter *a); -void sco_transport_init(struct ba_transport *t); +int sco_transport_init(struct ba_transport *t); int sco_transport_start(struct ba_transport *t); #endif diff --git a/test/test-ba.c b/test/test-ba.c index f629a9c60..c456564e2 100644 --- a/test/test-ba.c +++ b/test/test-ba.c @@ -49,8 +49,6 @@ /* Keep persistent storage in the current directory. */ #define TEST_BLUEALSA_STORAGE_DIR "storage-test-ba" -int a2dp_transport_init(struct ba_transport *t) { (void)t; return 0; } -int a2dp_transport_start(struct ba_transport *t) { (void)t; return 0; } int midi_transport_alsa_seq_create(struct ba_transport *t) { (void)t; return 0; } int midi_transport_alsa_seq_delete(struct ba_transport *t) { (void)t; return 0; } int midi_transport_start(struct ba_transport *t) { (void)t; return 0; } @@ -313,6 +311,11 @@ CK_START_TEST(test_ba_transport_pcm_format) { } CK_END_TEST +static int sep_transport_init(struct ba_transport *t) { + (void)t; + return 0; +} + CK_START_TEST(test_ba_transport_pcm_volume) { struct ba_adapter *a; @@ -325,7 +328,9 @@ CK_START_TEST(test_ba_transport_pcm_volume) { ck_assert_ptr_ne(d = ba_device_new(a, &addr), NULL); ck_assert_int_eq(storage_device_clear(d), 0); - struct a2dp_sep sep = { .config = { .type = A2DP_SINK, .codec_id = A2DP_CODEC_SBC } }; + struct a2dp_sep sep = { + .config = { .type = A2DP_SINK, .codec_id = A2DP_CODEC_SBC }, + .transport_init = sep_transport_init }; a2dp_sbc_t configuration = { .channel_mode = SBC_CHANNEL_MODE_STEREO }; ck_assert_ptr_ne(t_a2dp = ba_transport_new_a2dp(d, BA_TRANSPORT_PROFILE_A2DP_SINK, "/owner", "/path/a2dp", &sep, @@ -410,7 +415,9 @@ CK_START_TEST(test_storage) { ck_assert_ptr_ne(a = ba_adapter_new(0), NULL); ck_assert_ptr_ne(d = ba_device_new(a, &addr), NULL); - struct a2dp_sep sep = { .config = { .type = A2DP_SINK, .codec_id = A2DP_CODEC_SBC } }; + struct a2dp_sep sep = { + .config = { .type = A2DP_SINK, .codec_id = A2DP_CODEC_SBC }, + .transport_init = sep_transport_init }; a2dp_sbc_t configuration = { .channel_mode = SBC_CHANNEL_MODE_STEREO }; ck_assert_ptr_ne(t = ba_transport_new_a2dp(d, BA_TRANSPORT_PROFILE_A2DP_SINK, "/owner", "/path", &sep, diff --git a/test/test-io.c b/test/test-io.c index 158b6738b..2eea60f9b 100644 --- a/test/test-io.c +++ b/test/test-io.c @@ -726,8 +726,9 @@ static struct ba_transport *test_transport_new_a2dp( if (input_bt_file != NULL) configuration = &btdin->a2dp_configuration; #endif - struct ba_transport *t = ba_transport_new_a2dp(device, profile, ":test", - dbus_path, sep, configuration); + struct ba_transport *t; + ck_assert_ptr_nonnull(t = ba_transport_new_a2dp(device, profile, ":test", + dbus_path, sep, configuration)); t->acquire = test_transport_acquire; t->release = test_transport_release_bt_a2dp; return t; @@ -772,7 +773,9 @@ CK_START_TEST(test_a2dp_sbc) { CK_START_TEST(test_a2dp_sbc_invalid_config) { - const a2dp_sbc_t config_sbc_invalid = { 0 }; + const a2dp_sbc_t config_sbc_invalid = { + .sampling_freq = SBC_SAMPLING_FREQ_44100, + .channel_mode = SBC_CHANNEL_MODE_STEREO }; struct ba_transport *t = test_transport_new_a2dp(device1, BA_TRANSPORT_PROFILE_A2DP_SOURCE, "/path/sbc", &a2dp_sbc_source, &config_sbc_invalid); diff --git a/test/test-rfcomm.c b/test/test-rfcomm.c index 4374dd9fb..8b050d738 100644 --- a/test/test-rfcomm.c +++ b/test/test-rfcomm.c @@ -58,8 +58,6 @@ static void dbus_update_counters_wait(unsigned int *counter, unsigned int value) pthread_mutex_unlock(&dbus_update_mtx); } -int a2dp_transport_init(struct ba_transport *t) { (void)t; return 0; } -int a2dp_transport_start(struct ba_transport *t) { (void)t; return 0; } int midi_transport_alsa_seq_create(struct ba_transport *t) { (void)t; return 0; } int midi_transport_alsa_seq_delete(struct ba_transport *t) { (void)t; return 0; } int midi_transport_start(struct ba_transport *t) { (void)t; return 0; } From f93a6e816777c88719271c1adfc99e6f22e64d4a Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Sun, 13 Oct 2024 21:11:44 +0200 Subject: [PATCH 11/15] Fix configuration for Android 13 A2DP Opus codec --- NEWS | 1 + src/a2dp-opus.c | 10 ++++++++-- src/bluez.c | 3 +++ src/shared/a2dp-codecs.h | 10 +++++----- utils/a2dpconf.c | 5 +++-- 5 files changed, 20 insertions(+), 9 deletions(-) diff --git a/NEWS b/NEWS index 13187f16e..44f16cf18 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,7 @@ unreleased - renamed bluealsa-cli to bluealsactl (no backward compatibility) - channel map and volume control for surround sound (5.1, 7.1) audio - native A2DP volume control by default (dropped --a2dp-volume option) +- fix configuration for Android 13 A2DP Opus codec bluez-alsa v4.3.1 (2024-08-30) ============================== diff --git a/src/a2dp-opus.c b/src/a2dp-opus.c index 4631bcb8b..030f1d659 100644 --- a/src/a2dp-opus.c +++ b/src/a2dp-opus.c @@ -52,6 +52,8 @@ static const struct a2dp_bit_mapping a2dp_opus_channels[] = { }; static const struct a2dp_bit_mapping a2dp_opus_rates[] = { + { OPUS_SAMPLING_FREQ_16000, { 16000 } }, + { OPUS_SAMPLING_FREQ_24000, { 24000 } }, { OPUS_SAMPLING_FREQ_48000, { 48000 } }, { 0 } }; @@ -481,7 +483,9 @@ struct a2dp_sep a2dp_opus_source = { .capabilities.opus = { .info = A2DP_VENDOR_INFO_INIT(OPUS_VENDOR_ID, OPUS_CODEC_ID), .sampling_freq = - OPUS_SAMPLING_FREQ_48000, + OPUS_SAMPLING_FREQ_48000 | + OPUS_SAMPLING_FREQ_24000 | + OPUS_SAMPLING_FREQ_16000, .frame_duration = OPUS_FRAME_DURATION_100 | OPUS_FRAME_DURATION_200, @@ -511,7 +515,9 @@ struct a2dp_sep a2dp_opus_sink = { .capabilities.opus = { .info = A2DP_VENDOR_INFO_INIT(OPUS_VENDOR_ID, OPUS_CODEC_ID), .sampling_freq = - OPUS_SAMPLING_FREQ_48000, + OPUS_SAMPLING_FREQ_48000 | + OPUS_SAMPLING_FREQ_24000 | + OPUS_SAMPLING_FREQ_16000, .frame_duration = OPUS_FRAME_DURATION_100 | OPUS_FRAME_DURATION_200, diff --git a/src/bluez.c b/src/bluez.c index eaaca0c36..42a9a0b67 100644 --- a/src/bluez.c +++ b/src/bluez.c @@ -1693,6 +1693,9 @@ bool bluez_a2dp_set_configuration( pthread_mutex_unlock(&bluez_mutex); + debug("A2DP requested codec: %s", a2dp_codecs_codec_id_to_string(remote_sep_cfg->codec_id)); + hexdump("A2DP requested configuration blob", configuration, remote_sep_cfg->caps_size); + if ((rep = g_dbus_connection_send_message_with_reply_sync(config.dbus, msg, G_DBUS_SEND_MESSAGE_FLAGS_NONE, -1, NULL, NULL, error)) == NULL || g_dbus_message_to_gerror(rep, error)) diff --git a/src/shared/a2dp-codecs.h b/src/shared/a2dp-codecs.h index 86cb21bca..ece9030fa 100644 --- a/src/shared/a2dp-codecs.h +++ b/src/shared/a2dp-codecs.h @@ -694,7 +694,9 @@ typedef struct a2dp_lhdc_v5 { #define OPUS_VENDOR_ID BT_COMPID_GOOGLE #define OPUS_CODEC_ID 0x0001 -#define OPUS_SAMPLING_FREQ_48000 (1 << 0) +#define OPUS_SAMPLING_FREQ_48000 (1 << 2) +#define OPUS_SAMPLING_FREQ_24000 (1 << 1) +#define OPUS_SAMPLING_FREQ_16000 (1 << 0) #define OPUS_FRAME_DURATION_100 (1 << 0) #define OPUS_FRAME_DURATION_200 (1 << 1) @@ -706,15 +708,13 @@ typedef struct a2dp_lhdc_v5 { typedef struct a2dp_opus { a2dp_vendor_info_t info; #if __BYTE_ORDER == __BIG_ENDIAN - uint8_t rfa:2; - uint8_t sampling_freq:1; + uint8_t sampling_freq:3; uint8_t frame_duration:2; uint8_t channel_mode:3; #else uint8_t channel_mode:3; uint8_t frame_duration:2; - uint8_t sampling_freq:1; - uint8_t rfa:2; + uint8_t sampling_freq:3; #endif } __attribute__ ((packed)) a2dp_opus_t; diff --git a/utils/a2dpconf.c b/utils/a2dpconf.c index c43502aa3..1a80deeca 100644 --- a/utils/a2dpconf.c +++ b/utils/a2dpconf.c @@ -553,12 +553,13 @@ static void dump_opus(const void *blob, size_t size) { printf("Opus {\n", bintohex(blob, size)); printf_vendor(&opus->info); printf("" - " :2\n" - " sample-rate:1 =%s\n" + " sample-rate:3 =%s%s%s\n" " frame-duration:2 =%s%s\n" " channel-mode:3 =%s%s%s\n" "}\n", opus->sampling_freq & OPUS_SAMPLING_FREQ_48000 ? " 48000" : "", + opus->sampling_freq & OPUS_SAMPLING_FREQ_24000 ? " 24000" : "", + opus->sampling_freq & OPUS_SAMPLING_FREQ_16000 ? " 16000" : "", opus->frame_duration & OPUS_FRAME_DURATION_100 ? " 10ms" : "", opus->frame_duration & OPUS_FRAME_DURATION_200 ? " 20ms" : "", opus->channel_mode & OPUS_CHANNEL_MODE_STEREO ? " Stereo" : "", From 9d37f0e7035952c0225f9e4c956c89619b7385d3 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Sun, 21 Nov 2021 10:33:54 +0100 Subject: [PATCH 12/15] PCM client delay reporting for A2DP sink profile This feature requires BlueZ >= 5.79 --- src/ba-transport-pcm.c | 70 ++++++++++++++++++++++++++++++++++++----- src/ba-transport-pcm.h | 11 +++++-- src/ba-transport.h | 2 ++ src/bluealsa-dbus.c | 8 ++--- src/bluez.c | 7 ++++- test/test-rfcomm.c | 2 +- test/test-utils-ctl.c | 10 +++--- utils/aplay/Makefile.am | 3 +- utils/aplay/aplay.c | 53 ++++++++++++++++++++++++++++--- 9 files changed, 140 insertions(+), 26 deletions(-) diff --git a/src/ba-transport-pcm.c b/src/ba-transport-pcm.c index 3c7e19ff1..d9306aa14 100644 --- a/src/ba-transport-pcm.c +++ b/src/ba-transport-pcm.c @@ -24,6 +24,7 @@ #include #include +#include #include #include "audio.h" @@ -631,7 +632,11 @@ void ba_transport_pcm_volume_set( } -int ba_transport_pcm_volume_update(struct ba_transport_pcm *pcm) { +/** + * Synchronize PCM volume level. + * + * This function notifies remote Bluetooth device and D-Bus clients. */ +int ba_transport_pcm_volume_sync(struct ba_transport_pcm *pcm, unsigned int update_mask) { struct ba_transport *t = pcm->t; @@ -684,8 +689,8 @@ int ba_transport_pcm_volume_update(struct ba_transport_pcm *pcm) { } final: - /* notify connected clients (including requester) */ - bluealsa_dbus_pcm_update(pcm, BA_DBUS_PCM_UPDATE_VOLUME); + /* Notify all connected D-Bus clients. */ + bluealsa_dbus_pcm_update(pcm, update_mask); return 0; } @@ -716,20 +721,71 @@ int ba_transport_pcm_get_hardware_volume( return 0; } -int ba_transport_pcm_get_delay(const struct ba_transport_pcm *pcm) { +/** + * Get PCM playback/capture cumulative delay. */ +int ba_transport_pcm_delay_get(const struct ba_transport_pcm *pcm) { const struct ba_transport *t = pcm->t; + int delay = 0; - int delay = pcm->codec_delay_dms + pcm->processing_delay_dms; + delay += pcm->codec_delay_dms; + delay += pcm->processing_delay_dms; - if (t->profile & BA_TRANSPORT_PROFILE_MASK_A2DP) + /* Add delay reported by BlueZ but only for A2DP Source profile. In case + * of A2DP Sink, the BlueZ delay value is in fact our client delay. */ + if (t->profile & BA_TRANSPORT_PROFILE_A2DP_SOURCE) delay += t->a2dp.delay; - if (t->profile & BA_TRANSPORT_PROFILE_MASK_SCO) + /* HFP/HSP profiles do not provide any delay information. However, we can + * assume some arbitrary value here - for now it will be 10 ms. */ + else if (t->profile & BA_TRANSPORT_PROFILE_MASK_AG) delay += 10; return delay; } +/** + * Synchronize PCM playback delay. + * + * This function notifies remote Bluetooth device and D-Bus clients. */ +int ba_transport_pcm_delay_sync(struct ba_transport_pcm *pcm, unsigned int update_mask) { + + struct ba_transport *t = pcm->t; + int delay = 0; + + delay += pcm->codec_delay_dms; + delay += pcm->processing_delay_dms; + delay += pcm->client_delay_dms; + + /* In case of A2DP Sink, update the delay property of the BlueZ media + * transport interface. BlueZ should forward this value to the remote + * device, so it can adjust audio/video synchronization. */ + if (t->profile == BA_TRANSPORT_PROFILE_A2DP_SINK && + t->a2dp.delay_reporting && + abs(delay - t->a2dp.delay) >= 100 /* 10ms */) { + + GError *err = NULL; + t->a2dp.delay = delay; + g_dbus_set_property(config.dbus, t->bluez_dbus_owner, t->bluez_dbus_path, + BLUEZ_IFACE_MEDIA_TRANSPORT, "Delay", g_variant_new_uint16(delay), &err); + + if (err != NULL) { + if (err->code == G_DBUS_ERROR_PROPERTY_READ_ONLY) + /* Even though BlueZ documentation says that the Delay property is + * read-write, it might not be true. In case when the delay write + * operation fails with "not writable" error, we should not try to + * update the delay report value any more. */ + t->a2dp.delay_reporting = false; + warn("Couldn't set A2DP transport delay: %s", err->message); + g_error_free(err); + } + + } + + /* Notify all connected D-Bus clients. */ + bluealsa_dbus_pcm_update(pcm, update_mask); + return 0; +} + const char *ba_transport_pcm_channel_to_string( enum ba_transport_pcm_channel channel) { switch (channel) { diff --git a/src/ba-transport-pcm.h b/src/ba-transport-pcm.h index 73d7fbe14..acd1e9d85 100644 --- a/src/ba-transport-pcm.h +++ b/src/ba-transport-pcm.h @@ -247,15 +247,20 @@ void ba_transport_pcm_volume_set( const bool *soft_mute, const bool *hard_mute); -int ba_transport_pcm_volume_update( - struct ba_transport_pcm *pcm); +int ba_transport_pcm_volume_sync( + struct ba_transport_pcm *pcm, + unsigned int update_mask); int ba_transport_pcm_get_hardware_volume( const struct ba_transport_pcm *pcm); -int ba_transport_pcm_get_delay( +int ba_transport_pcm_delay_get( const struct ba_transport_pcm *pcm); +int ba_transport_pcm_delay_sync( + struct ba_transport_pcm *pcm, + unsigned int update_mask); + const char *ba_transport_pcm_channel_to_string( enum ba_transport_pcm_channel channel); diff --git a/src/ba-transport.h b/src/ba-transport.h index 18e1dda9a..f1cb42317 100644 --- a/src/ba-transport.h +++ b/src/ba-transport.h @@ -127,6 +127,8 @@ struct ba_transport { /* selected audio codec configuration */ a2dp_t configuration; + /* delay reporting support */ + bool delay_reporting; /* delay reported by BlueZ */ uint16_t delay; /* volume reported by BlueZ */ diff --git a/src/bluealsa-dbus.c b/src/bluealsa-dbus.c index 27269a3f7..431b1199d 100644 --- a/src/bluealsa-dbus.c +++ b/src/bluealsa-dbus.c @@ -281,7 +281,7 @@ static GVariant *ba_variant_new_pcm_codec_config(const struct ba_transport_pcm * } static GVariant *ba_variant_new_pcm_delay(const struct ba_transport_pcm *pcm) { - return g_variant_new_uint16(ba_transport_pcm_get_delay(pcm)); + return g_variant_new_uint16(ba_transport_pcm_delay_get(pcm)); } static GVariant *ba_variant_new_pcm_client_delay(const struct ba_transport_pcm *pcm) { @@ -962,7 +962,7 @@ static bool bluealsa_pcm_set_property(const char *property, GVariant *value, if (strcmp(property, "ClientDelay") == 0) { pcm->client_delay_dms = g_variant_get_int16(value); - bluealsa_dbus_pcm_update(pcm, BA_DBUS_PCM_UPDATE_CLIENT_DELAY); + ba_transport_pcm_delay_sync(pcm, BA_DBUS_PCM_UPDATE_CLIENT_DELAY); return TRUE; } @@ -987,7 +987,7 @@ static bool bluealsa_pcm_set_property(const char *property, GVariant *value, pthread_mutex_unlock(&pcm->mutex); - bluealsa_dbus_pcm_update(pcm, BA_DBUS_PCM_UPDATE_SOFT_VOLUME | BA_DBUS_PCM_UPDATE_VOLUME); + ba_transport_pcm_volume_sync(pcm, BA_DBUS_PCM_UPDATE_SOFT_VOLUME | BA_DBUS_PCM_UPDATE_VOLUME); return true; } @@ -1017,7 +1017,7 @@ static bool bluealsa_pcm_set_property(const char *property, GVariant *value, pthread_mutex_unlock(&pcm->mutex); - ba_transport_pcm_volume_update(pcm); + ba_transport_pcm_volume_sync(pcm, BA_DBUS_PCM_UPDATE_VOLUME); return true; } diff --git a/src/bluez.c b/src/bluez.c index 42a9a0b67..d4a0d6927 100644 --- a/src/bluez.c +++ b/src/bluez.c @@ -455,6 +455,7 @@ static void bluez_endpoint_set_configuration(GDBusMethodInvocation *inv, void *u enum bluez_a2dp_transport_state state = 0xFFFF; char *device_path = NULL; a2dp_t configuration = { 0 }; + bool delay_reporting = false; uint16_t volume = 127; uint16_t delay = 150; @@ -508,6 +509,7 @@ static void bluez_endpoint_set_configuration(GDBusMethodInvocation *inv, void *u else if (strcmp(property, "Delay") == 0 && g_variant_validate_value(value, G_VARIANT_TYPE_UINT16, property)) { delay = g_variant_get_uint16(value); + delay_reporting = true; } else if (strcmp(property, "Volume") == 0 && g_variant_validate_value(value, G_VARIANT_TYPE_UINT16, property)) { @@ -566,6 +568,7 @@ static void bluez_endpoint_set_configuration(GDBusMethodInvocation *inv, void *u } t->a2dp.bluez_dbus_sep_path = dbus_obj->path; + t->a2dp.delay_reporting = delay_reporting; t->a2dp.delay = delay; t->a2dp.volume = volume; @@ -576,6 +579,8 @@ static void bluez_endpoint_set_configuration(GDBusMethodInvocation *inv, void *u &configuration, sep->config.caps_size); debug("PCM configuration: channels=%u rate=%u", t->a2dp.pcm.channels, t->a2dp.pcm.rate); + debug("Delay reporting: %s", + delay_reporting ? "supported" : "unsupported"); ba_transport_set_a2dp_state(t, state); @@ -1331,8 +1336,8 @@ static void bluez_signal_interfaces_added(GDBusConnection *conn, const char *sen } g_variant_unref(value); - } + } g_variant_iter_free(properties); } diff --git a/test/test-rfcomm.c b/test/test-rfcomm.c index 8b050d738..b8cb9df00 100644 --- a/test/test-rfcomm.c +++ b/test/test-rfcomm.c @@ -325,7 +325,7 @@ CK_START_TEST(test_rfcomm_hfp_ag) { ba_transport_pcm_volume_set(&pcm->volume[0], &level, NULL, NULL); pthread_mutex_unlock(&pcm->mutex); /* use internal API to update volume */ - ba_transport_pcm_volume_update(pcm); + ba_transport_pcm_volume_sync(pcm, BA_DBUS_PCM_UPDATE_VOLUME); ck_assert_rfcomm_recv(fd, "\r\n+VGS:7\r\n"); ba_transport_destroy(sco); diff --git a/test/test-utils-ctl.c b/test/test-utils-ctl.c index 9d6d327a7..63a0a4f94 100644 --- a/test/test-utils-ctl.c +++ b/test/test-utils-ctl.c @@ -264,7 +264,7 @@ CK_START_TEST(test_client_delay) { struct spawn_process sp_ba_mock; ck_assert_int_ne(spawn_bluealsa_mock(&sp_ba_mock, NULL, true, - "--profile=a2dp-source", + "--profile=a2dp-sink", NULL), -1); char output[4096]; @@ -276,21 +276,21 @@ CK_START_TEST(test_client_delay) { /* check default client delay */ ck_assert_int_eq(run_bluealsactl(output, sizeof(output), - "client-delay", "/org/bluealsa/hci11/dev_12_34_56_78_9A_BC/a2dpsrc/sink", + "client-delay", "/org/bluealsa/hci11/dev_12_34_56_78_9A_BC/a2dpsnk/source", NULL), 0); ck_assert_ptr_ne(strstr(output, "ClientDelay: 0.0 ms"), NULL); /* check setting client delay */ ck_assert_int_eq(run_bluealsactl(output, sizeof(output), - "client-delay", "/org/bluealsa/hci11/dev_12_34_56_78_9A_BC/a2dpsrc/sink", "-7.5", + "client-delay", "/org/bluealsa/hci11/dev_12_34_56_78_9A_BC/a2dpsnk/source", "-7.5", NULL), 0); /* check that setting client delay does not affect delay */ ck_assert_int_eq(run_bluealsactl(output, sizeof(output), - "info", "/org/bluealsa/hci11/dev_12_34_56_78_9A_BC/a2dpsrc/sink", + "info", "/org/bluealsa/hci11/dev_12_34_56_78_9A_BC/a2dpsnk/source", NULL), 0); ck_assert_ptr_ne(strstr(output, "ClientDelay: -7.5 ms"), NULL); - ck_assert_ptr_ne(strstr(output, "Delay: 10.0 ms"), NULL); + ck_assert_ptr_ne(strstr(output, "Delay: 0.0 ms"), NULL); spawn_terminate(&sp_ba_mock, 0); spawn_close(&sp_ba_mock, NULL); diff --git a/utils/aplay/Makefile.am b/utils/aplay/Makefile.am index 570a4c7c9..b57b71b89 100644 --- a/utils/aplay/Makefile.am +++ b/utils/aplay/Makefile.am @@ -1,5 +1,5 @@ # BlueALSA - Makefile.am -# Copyright (c) 2016-2023 Arkadiusz Bokowy +# Copyright (c) 2016-2024 Arkadiusz Bokowy if ENABLE_APLAY @@ -12,6 +12,7 @@ bluealsa_aplay_SOURCES = \ ../../src/shared/ffb.c \ ../../src/shared/log.c \ ../../src/shared/nv.c \ + ../../src/shared/rt.c \ alsa-mixer.c \ alsa-pcm.c \ dbus.c \ diff --git a/utils/aplay/aplay.c b/utils/aplay/aplay.c index a3bc8aa60..255718561 100644 --- a/utils/aplay/aplay.c +++ b/utils/aplay/aplay.c @@ -28,6 +28,7 @@ #include #include #include +#include #include #include @@ -40,6 +41,7 @@ #include "shared/ffb.h" #include "shared/log.h" #include "shared/nv.h" +#include "shared/rt.h" #include "alsa-mixer.h" #include "alsa-pcm.h" #include "dbus.h" @@ -563,8 +565,7 @@ static void *io_worker_routine(struct io_worker *w) { w->ba_pcm.pcm_path, softvol ? "software" : "pass-through"); if (softvol != w->ba_pcm.soft_volume) { w->ba_pcm.soft_volume = softvol; - if (!ba_dbus_pcm_update(&dbus_ctx, &w->ba_pcm, - BLUEALSA_PCM_SOFT_VOLUME, &err)) { + if (!ba_dbus_pcm_update(&dbus_ctx, &w->ba_pcm, BLUEALSA_PCM_SOFT_VOLUME, &err)) { error("Couldn't set BlueALSA source PCM volume mode: %s", err.message); dbus_error_free(&err); goto fail; @@ -593,6 +594,12 @@ static void *io_worker_routine(struct io_worker *w) { size_t pcm_open_retry_pcm_samples = 0; size_t pcm_open_retries = 0; + /* The time-stamp for delay update rate limiting. */ + struct timespec pcm_delay_update_ts = { 0 }; + /* Window buffer for calculating delay moving average. */ + snd_pcm_sframes_t pcm_delay_frames[64]; + size_t pcm_delay_frames_i = 0; + size_t pause_retry_pcm_samples = pcm_1s_samples; size_t pause_retries = 0; @@ -745,10 +752,13 @@ static void *io_worker_routine(struct io_worker *w) { io_worker_mixer_open(w, mixer_device, mixer_elem_name, mixer_elem_index); io_worker_mixer_volume_sync_setup(w); - /* reset retry counters */ + /* Reset retry counters. */ pcm_open_retry_pcm_samples = 0; pcm_open_retries = 0; + /* Reset moving delay window buffer. */ + memset(pcm_delay_frames, 0, sizeof(pcm_delay_frames)); + if (verbose >= 2) { info("Used configuration for %s:\n" " ALSA PCM buffer time: %u us (%zu bytes)\n" @@ -803,9 +813,44 @@ static void *io_worker_routine(struct io_worker *w) { goto close_alsa; } - /* move leftovers to the beginning and reposition tail */ + /* Move leftovers to the beginning and reposition tail. */ ffb_shift(&buffer, frames * w->ba_pcm.channels); + int ret; + if ((ret = snd_pcm_delay(w->snd_pcm, + &pcm_delay_frames[pcm_delay_frames_i++ % ARRAYSIZE(pcm_delay_frames)])) != 0) + warn("Couldn't get PCM delay: %s", snd_strerror(ret)); + else { + + struct timespec ts_now; + /* Rate limit delay updates to 1 update per second. */ + struct timespec ts_delay = { .tv_sec = 1 }; + + gettimestamp(&ts_now); + timespecadd(&pcm_delay_update_ts, &ts_delay, &ts_delay); + + snd_pcm_sframes_t pcm_delay_frames_avg = 0; + for (size_t i = 0; i < ARRAYSIZE(pcm_delay_frames); i++) + pcm_delay_frames_avg += pcm_delay_frames[i]; + pcm_delay_frames_avg /= ARRAYSIZE(pcm_delay_frames); + + const int delay = pcm_delay_frames_avg * 10000 / w->ba_pcm.rate; + if (difftimespec(&ts_now, &ts_delay, &ts_delay) < 0 && + abs(delay - w->ba_pcm.client_delay) >= 100 /* 10ms */) { + + pcm_delay_update_ts = ts_now; + + w->ba_pcm.client_delay = delay; + if (!ba_dbus_pcm_update(&dbus_ctx, &w->ba_pcm, BLUEALSA_PCM_CLIENT_DELAY, &err)) { + error("Couldn't update BlueALSA PCM client delay: %s", err.message); + dbus_error_free(&err); + goto fail; + } + + } + + } + continue; close_alsa: From 45af1a782464e88baa6b3803916b9725c797d4b6 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Mon, 28 Oct 2024 23:03:47 +0100 Subject: [PATCH 13/15] Hint compiler about memory aliasing restrictions --- src/a2dp.c | 4 ++-- src/a2dp.h | 4 ++-- src/audio.c | 42 ++++++++++++++++-------------------- src/audio.h | 20 ++++++++--------- src/bluealsactl/cmd-open.c | 10 +++++---- src/shared/dbus-client-pcm.c | 20 ++++++++++------- src/shared/hex.c | 4 ++-- src/shared/hex.h | 6 +++--- 8 files changed, 55 insertions(+), 55 deletions(-) diff --git a/src/a2dp.c b/src/a2dp.c index 34c5bf103..649cf4ed9 100644 --- a/src/a2dp.c +++ b/src/a2dp.c @@ -156,8 +156,8 @@ uint32_t a2dp_bit_mapping_lookup_value( * This function performs a simple bitwise AND operation on given capabilities * and mask. */ void a2dp_caps_bitwise_intersect( - void *capabilities, - const void *mask, + void * restrict capabilities, + const void * restrict mask, size_t size) { const uint8_t *caps_mask = mask; diff --git a/src/a2dp.h b/src/a2dp.h index 0018133ed..549d1da51 100644 --- a/src/a2dp.h +++ b/src/a2dp.h @@ -86,8 +86,8 @@ enum a2dp_stream { }; void a2dp_caps_bitwise_intersect( - void *capabilities, - const void *mask, + void * restrict capabilities, + const void * restrict mask, size_t size); bool a2dp_caps_has_main_stream_only( diff --git a/src/audio.c b/src/audio.c index 5d4f337d5..4f812bf0f 100644 --- a/src/audio.c +++ b/src/audio.c @@ -35,8 +35,8 @@ double audio_loudness_to_decibel(double value) { /** * Join channels into interleaved S16 PCM signal. */ -void audio_interleave_s16_2le(int16_t *dest, const int16_t **src, - unsigned int channels, size_t frames) { +void audio_interleave_s16_2le(int16_t * restrict dest, + const int16_t * restrict * restrict src, unsigned int channels, size_t frames) { for (size_t f = 0; f < frames; f++) for (size_t c = 0; c < channels; c++) *dest++ = src[c][f]; @@ -44,8 +44,8 @@ void audio_interleave_s16_2le(int16_t *dest, const int16_t **src, /** * Join channels into interleaved S32 PCM signal. */ -void audio_interleave_s32_4le(int32_t *dest, const int32_t **src, - unsigned int channels, size_t frames) { +void audio_interleave_s32_4le(int32_t * restrict dest, + const int32_t * restrict * restrict src, unsigned int channels, size_t frames) { for (size_t f = 0; f < frames; f++) for (size_t c = 0; c < channels; c++) *dest++ = src[c][f]; @@ -53,8 +53,8 @@ void audio_interleave_s32_4le(int32_t *dest, const int32_t **src, /** * Split interleaved S16 PCM signal into channels. */ -void audio_deinterleave_s16_2le(int16_t **dest, const int16_t *src, - unsigned int channels, size_t frames) { +void audio_deinterleave_s16_2le(int16_t * restrict * restrict dest, + const int16_t * restrict src, unsigned int channels, size_t frames) { for (size_t f = 0; f < frames; f++) for (size_t c = 0; c < channels; c++) dest[c][f] = *src++; @@ -62,8 +62,8 @@ void audio_deinterleave_s16_2le(int16_t **dest, const int16_t *src, /** * Split interleaved S32 PCM signal into channels. */ -void audio_deinterleave_s32_4le(int32_t **dest, const int32_t *src, - unsigned int channels, size_t frames) { +void audio_deinterleave_s32_4le(int32_t * restrict * restrict dest, + const int32_t * restrict src, unsigned int channels, size_t frames) { for (size_t f = 0; f < frames; f++) for (size_t c = 0; c < channels; c++) dest[c][f] = *src++; @@ -80,24 +80,18 @@ void audio_deinterleave_s32_4le(int32_t **dest, const int32_t *src, * @param scale The scaling factor per channel for the PCM signal. * @param channels The number of channels in the buffer. * @param frames The number of PCM frames in the buffer. */ -void audio_scale_s16_2le(int16_t *buffer, const double *scale, - unsigned int channels, size_t frames) { - size_t i = 0; - while (frames--) - for (size_t ii = 0; ii < channels; ii++) { - int16_t s = (int16_t)le16toh(buffer[i]) * scale[ii]; - buffer[i++] = htole16(s); - } +void audio_scale_s16_2le(int16_t * restrict buffer, + const double * restrict scale, unsigned int channels, size_t frames) { + for (size_t i = 0; frames; frames--) + for (size_t c = 0; c < channels; c++, i++) + buffer[i] = htole16(((int16_t)le16toh(buffer[i]) * scale[c])); } /** * Scale S32_4LE PCM signal. */ -void audio_scale_s32_4le(int32_t *buffer, const double *scale, - unsigned int channels, size_t frames) { - size_t i = 0; - while (frames--) - for (size_t ii = 0; ii < channels; ii++) { - int32_t s = (int32_t)le32toh(buffer[i]) * scale[ii]; - buffer[i++] = htole32(s); - } +void audio_scale_s32_4le(int32_t * restrict buffer, + const double * restrict scale, unsigned int channels, size_t frames) { + for (size_t i = 0; frames; frames--) + for (size_t c = 0; c < channels; c++, i++) + buffer[i] = htole32((int32_t)le32toh(buffer[i]) * scale[c]); } diff --git a/src/audio.h b/src/audio.h index 076d34199..2b20998c0 100644 --- a/src/audio.h +++ b/src/audio.h @@ -22,21 +22,21 @@ double audio_decibel_to_loudness(double value); double audio_loudness_to_decibel(double value); -void audio_interleave_s16_2le(int16_t *dest, const int16_t **src, - unsigned int channels, size_t frames); -void audio_interleave_s32_4le(int32_t *dest, const int32_t **src, - unsigned int channels, size_t frames); +void audio_interleave_s16_2le(int16_t * restrict dest, + const int16_t * restrict * restrict src, unsigned int channels, size_t frames); +void audio_interleave_s32_4le(int32_t * restrict dest, + const int32_t * restrict * restrict src, unsigned int channels, size_t frames); #define audio_interleave_s24_4le audio_interleave_s32_4le -void audio_deinterleave_s16_2le(int16_t **dest, const int16_t *src, - unsigned int channels, size_t frames); -void audio_deinterleave_s32_4le(int32_t **dest, const int32_t *src, - unsigned int channels, size_t frames); +void audio_deinterleave_s16_2le(int16_t * restrict * restrict dest, + const int16_t * restrict src, unsigned int channels, size_t frames); +void audio_deinterleave_s32_4le(int32_t * restrict * restrict dest, + const int32_t * restrict src, unsigned int channels, size_t frames); #define audio_deinterleave_s24_4le audio_deinterleave_s32_4le -void audio_scale_s16_2le(int16_t *buffer, const double *scale, +void audio_scale_s16_2le(int16_t * restrict buffer, const double * restrict scale, unsigned int channels, size_t frames); -void audio_scale_s32_4le(int32_t *buffer, const double *scale, +void audio_scale_s32_4le(int32_t * restrict buffer, const double * restrict scale, unsigned int channels, size_t frames); #define audio_scale_s24_4le audio_scale_s32_4le diff --git a/src/bluealsactl/cmd-open.c b/src/bluealsactl/cmd-open.c index 8e9bd3fb1..443e820a9 100644 --- a/src/bluealsactl/cmd-open.c +++ b/src/bluealsactl/cmd-open.c @@ -100,7 +100,8 @@ static int cmd_open_func(int argc, char *argv[]) { } uint8_t buffer[4096]; - uint8_t buffer_hex[sizeof(buffer) * 2 + 1]; + uint8_t buffer_bin[sizeof(buffer) / 2]; + char buffer_hex[sizeof(buffer) * 2 + 1]; ssize_t count; while ((count = read(fd_input, buffer, sizeof(buffer))) > 0) { @@ -111,15 +112,16 @@ static int cmd_open_func(int argc, char *argv[]) { if (hex) { if (fd_input == STDIN_FILENO) { - if ((count = hex2bin((const char *)buffer, buffer, count)) == -1) { + pos = buffer_bin; + if ((count = hex2bin((const char *)buffer, buffer_bin, count)) == -1) { cmd_print_error("Couldn't decode hex string: %s", strerror(errno)); continue; } } if (fd_output == STDOUT_FILENO) { - count = bin2hex(buffer, (char *)buffer_hex, count); - pos = buffer_hex; + pos = (const uint8_t *)buffer_hex; + count = bin2hex(buffer, buffer_hex, count); } } diff --git a/src/shared/dbus-client-pcm.c b/src/shared/dbus-client-pcm.c index 8171ebada..44d0cda53 100644 --- a/src/shared/dbus-client-pcm.c +++ b/src/shared/dbus-client-pcm.c @@ -214,8 +214,9 @@ const char *ba_dbus_pcm_codec_get_canonical_name( return a2dp_codecs_get_canonical_name(alias); } -static void dbus_message_iter_get_codec_data(DBusMessageIter *variant, - struct ba_pcm_codec *codec) { +static void dbus_message_iter_get_codec_data( + DBusMessageIter * restrict variant, + struct ba_pcm_codec * restrict codec) { DBusMessageIter iter; unsigned char *data; @@ -229,8 +230,9 @@ static void dbus_message_iter_get_codec_data(DBusMessageIter *variant, } -static void dbus_message_iter_get_codec_channels(DBusMessageIter *variant, - struct ba_pcm_codec *codec) { +static void dbus_message_iter_get_codec_channels( + DBusMessageIter * restrict variant, + struct ba_pcm_codec * restrict codec) { DBusMessageIter iter; unsigned char *data; @@ -245,8 +247,9 @@ static void dbus_message_iter_get_codec_channels(DBusMessageIter *variant, } -static void dbus_message_iter_get_codec_rates(DBusMessageIter *variant, - struct ba_pcm_codec *codec) { +static void dbus_message_iter_get_codec_rates( + DBusMessageIter * restrict variant, + struct ba_pcm_codec * restrict codec) { DBusMessageIter iter; dbus_uint32_t *data; @@ -261,8 +264,9 @@ static void dbus_message_iter_get_codec_rates(DBusMessageIter *variant, } -static void dbus_message_iter_get_codec_channel_maps(DBusMessageIter *variant, - struct ba_pcm_codec *codec) { +static void dbus_message_iter_get_codec_channel_maps( + DBusMessageIter * restrict variant, + struct ba_pcm_codec * restrict codec) { size_t i; DBusMessageIter iter_array; diff --git a/src/shared/hex.c b/src/shared/hex.c index a702e4d41..3b5a1e8af 100644 --- a/src/shared/hex.c +++ b/src/shared/hex.c @@ -21,7 +21,7 @@ * data. * @param n The length of the binary buffer which shall be encoded. * @return This function returns length of the hex string. */ -ssize_t bin2hex(const void *bin, char *hex, size_t n) { +ssize_t bin2hex(const void * restrict bin, char * restrict hex, size_t n) { static const char map_bin2hex[] = "0123456789abcdef"; @@ -45,7 +45,7 @@ ssize_t bin2hex(const void *bin, char *hex, size_t n) { * @return On success this function returns the size of the binary data. If * an error has occurred, -1 is returned and errno is set to indicate the * error. */ -ssize_t hex2bin(const char *hex, void *bin, size_t n) { +ssize_t hex2bin(const char * restrict hex, void * restrict bin, size_t n) { static const int map_hex2bin[256] = { ['0'] = 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, diff --git a/src/shared/hex.h b/src/shared/hex.h index 74d173844..2310352b3 100644 --- a/src/shared/hex.h +++ b/src/shared/hex.h @@ -1,6 +1,6 @@ /* * BlueALSA - hex.h - * Copyright (c) 2016-2021 Arkadiusz Bokowy + * Copyright (c) 2016-2024 Arkadiusz Bokowy * * This file is a part of bluez-alsa. * @@ -18,7 +18,7 @@ #include -ssize_t bin2hex(const void *bin, char *hex, size_t n); -ssize_t hex2bin(const char *hex, void *bin, size_t n); +ssize_t bin2hex(const void * restrict bin, char * restrict hex, size_t n); +ssize_t hex2bin(const char * restrict hex, void * restrict bin, size_t n); #endif From b9db5b697a40728635377c7e6a662d6cde6b3afa Mon Sep 17 00:00:00 2001 From: borine <32966433+borine@users.noreply.github.com> Date: Sat, 2 Nov 2024 19:34:32 +0100 Subject: [PATCH 14/15] Fix aplay output volume scaling caused by 0d488c7 --- utils/aplay/aplay.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/aplay/aplay.c b/utils/aplay/aplay.c index 255718561..67fc5c7da 100644 --- a/utils/aplay/aplay.c +++ b/utils/aplay/aplay.c @@ -455,7 +455,7 @@ static int io_worker_mixer_volume_sync_snd_mixer_elem( * all channels to the average left-right volume. */ unsigned int volume_sum = 0, muted = 0; - for (size_t i = 1; i < ba_pcm->channels; i++) { + for (size_t i = 0; i < ba_pcm->channels; i++) { volume_sum += ba_pcm->volume[i].volume; muted |= ba_pcm->volume[i].muted; } From 59c953a2cd6ae81cfad9f3bf08b0905c3feb07ef Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Sun, 10 Nov 2024 09:39:19 +0100 Subject: [PATCH 15/15] Fix audio scaling bug introduced in 45af1a7 Fixes #737 --- src/audio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/audio.c b/src/audio.c index 4f812bf0f..6c9a08132 100644 --- a/src/audio.c +++ b/src/audio.c @@ -84,7 +84,7 @@ void audio_scale_s16_2le(int16_t * restrict buffer, const double * restrict scale, unsigned int channels, size_t frames) { for (size_t i = 0; frames; frames--) for (size_t c = 0; c < channels; c++, i++) - buffer[i] = htole16(((int16_t)le16toh(buffer[i]) * scale[c])); + buffer[i] = htole16((int16_t)((int16_t)le16toh(buffer[i]) * scale[c])); } /** @@ -93,5 +93,5 @@ void audio_scale_s32_4le(int32_t * restrict buffer, const double * restrict scale, unsigned int channels, size_t frames) { for (size_t i = 0; frames; frames--) for (size_t c = 0; c < channels; c++, i++) - buffer[i] = htole32((int32_t)le32toh(buffer[i]) * scale[c]); + buffer[i] = htole32((int32_t)((int32_t)le32toh(buffer[i]) * scale[c])); }