* [PATCH BlueZ] audio: Add support for specific error codes for A2DP configuration
@ 2025-09-11 13:53 Per Waagø
2025-09-11 14:08 ` [BlueZ] " bluez.test.bot
2025-09-11 14:43 ` [PATCH BlueZ] " Luiz Augusto von Dentz
0 siblings, 2 replies; 8+ messages in thread
From: Per Waagø @ 2025-09-11 13:53 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Per Waagø
The A2DP specification defines error codes that shall be used if
the codec capabilities contain improper settings. This change allows
clients to trigger the sending of these specific error codes by
returning the corresponding error messages from
MediaEndpoint1.SetConfiguration.
This update is fully backwards compatible: clients passing other error
messages will continue to receive the default error code as before. On
older BlueZ versions, these new errors will also result in the default
error code, enabling clients to implement support for the new errors
without breaking compatibility.
This change enables passing A2DP/SNK/AVP/* and A2DP/SRC/AVP/*
qualification tests.
---
doc/org.bluez.MediaEndpoint.rst | 37 ++++++++++++++++
profiles/audio/a2dp.c | 78 ++++++++++++++++++++++++++++++---
profiles/audio/a2dp.h | 5 ++-
profiles/audio/avdtp.c | 4 +-
profiles/audio/media.c | 20 +++++----
src/error.h | 38 ++++++++++++++++
6 files changed, 165 insertions(+), 17 deletions(-)
diff --git a/doc/org.bluez.MediaEndpoint.rst b/doc/org.bluez.MediaEndpoint.rst
index 474cc1160..2721d473e 100644
--- a/doc/org.bluez.MediaEndpoint.rst
+++ b/doc/org.bluez.MediaEndpoint.rst
@@ -54,6 +54,43 @@ be configured and the properties must contain the following properties:
See **org.bluez.MediaTransport(5)** QoS property.
+ Possible errors:
+ :org.bluez.Error.A2DP.InvalidCodecType:
+ :org.bluez.Error.A2DP.NotSupportedCodecType:
+ :org.bluez.Error.A2DP.InvalidSamplingFrequency:
+ :org.bluez.Error.A2DP.NotSupportedSamplingFrequency:
+ :org.bluez.Error.A2DP.InvalidChannelMode:
+ :org.bluez.Error.A2DP.NotSupportedChannelMode:
+ :org.bluez.Error.A2DP.InvalidSubbands:
+ :org.bluez.Error.A2DP.NotSupportedSubbands:
+ :org.bluez.Error.A2DP.InvalidAllocationMethod:
+ :org.bluez.Error.A2DP.NotSupportedAllocationMethod:
+ :org.bluez.Error.A2DP.InvalidMinimumBitpoolValue:
+ :org.bluez.Error.A2DP.NotSupportedMinimumBitpoolValue:
+ :org.bluez.Error.A2DP.InvalidMaximumBitpoolValue:
+ :org.bluez.Error.A2DP.NotSupportedMaximumBitpoolValue:
+ :org.bluez.Error.A2DP.InvalidInvalidLayer:
+ :org.bluez.Error.A2DP.NotSupportedLayer:
+ :org.bluez.Error.A2DP.NotSupporterdCRC:
+ :org.bluez.Error.A2DP.NotSupporterdMPF:
+ :org.bluez.Error.A2DP.NotSupporterdVBR:
+ :org.bluez.Error.A2DP.InvalidBitRate:
+ :org.bluez.Error.A2DP.NotSupportedBitRate:
+ :org.bluez.Error.A2DP.InvalidObjectType:
+ :org.bluez.Error.A2DP.NotSupportedObjectType:
+ :org.bluez.Error.A2DP.InvalidChannels:
+ :org.bluez.Error.A2DP.NotSupportedChannels:
+ :org.bluez.Error.A2DP.InvalidVersion:
+ :org.bluez.Error.A2DP.NotSupportedVersion:
+ :org.bluez.Error.A2DP.NotSupportedMaximumSUL:
+ :org.bluez.Error.A2DP.InvalidBlockLength:
+ :org.bluez.Error.A2DP.InvalidCPType:
+ :org.bluez.Error.A2DP.InvalidCPFormat:
+ :org.bluez.Error.A2DP.InvalidCodecParameter:
+ :org.bluez.Error.A2DP.NotSupportedCodecParameter:
+ :org.bluez.Error.A2DP.InvalidDRC:
+ :org.bluez.Error.A2DP.NotSupportedDRC:
+
array{byte} SelectConfiguration(array{byte} capabilities)
`````````````````````````````````````````````````````````
diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index c0a53eae9..661843a89 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -157,6 +157,73 @@ static GSList *servers = NULL;
static GSList *setups = NULL;
static unsigned int cb_id = 0;
+struct a2dp_error {
+ const char *error_name;
+ uint8_t error_code;
+};
+
+#define A2DP_ERROR_PREFIX ERROR_INTERFACE ".A2DP."
+
+static struct a2dp_error config_errors[] = {
+ {"InvalidCodecType", A2DP_INVALID_CODEC_TYPE},
+ {"NotSupportedCodecType", A2DP_NOT_SUPPORTED_CODEC_TYPE},
+ {"InvalidSamplingFrequency", A2DP_INVALID_SAMPLING_FREQUENCY},
+ {"NotSupportedSamplingFrequency",
+ A2DP_NOT_SUPPORTED_SAMPLING_FREQUENCY},
+ {"InvalidChannelMode", A2DP_INVALID_CHANNEL_MODE},
+ {"NotSupportedChannelMode", A2DP_NOT_SUPPORTED_CHANNEL_MODE},
+ {"InvalidSubbands", A2DP_INVALID_SUBBANDS},
+ {"NotSupportedSubbands", A2DP_NOT_SUPPORTED_SUBBANDS},
+ {"InvalidAllocationMethod", A2DP_INVALID_ALLOCATION_METHOD},
+ {"NotSupportedAllocationMethod", A2DP_NOT_SUPPORTED_ALLOCATION_METHOD},
+ {"InvalidMinimumBitpoolValue",
+ A2DP_INVALID_MINIMUM_BITPOOL_VALUE},
+ {"NotSupportedMinimumBitpoolValue",
+ A2DP_NOT_SUPPORTED_MINIMUM_BITPOOL_VALUE},
+ {"InvalidMaximumBitpoolValue", A2DP_INVALID_MAXIMUM_BITPOOL_VALUE},
+ {"NotSupportedMaximumBitpoolValue",
+ A2DP_NOT_SUPPORTED_MAXIMUM_BITPOOL_VALUE},
+ {"InvalidInvalidLayer", A2DP_INVALID_INVALID_LAYER},
+ {"NotSupportedLayer", A2DP_NOT_SUPPORTED_LAYER},
+ {"NotSupporterdCRC", A2DP_NOT_SUPPORTERD_CRC},
+ {"NotSupporterdMPF", A2DP_NOT_SUPPORTERD_MPF},
+ {"NotSupporterdVBR", A2DP_NOT_SUPPORTERD_VBR},
+ {"InvalidBitRate", A2DP_INVALID_BIT_RATE},
+ {"NotSupportedBitRate", A2DP_NOT_SUPPORTED_BIT_RATE},
+ {"InvalidObjectType", A2DP_INVALID_OBJECT_TYPE},
+ {"NotSupportedObjectType", A2DP_NOT_SUPPORTED_OBJECT_TYPE},
+ {"InvalidChannels", A2DP_INVALID_CHANNELS},
+ {"NotSupportedChannels", A2DP_NOT_SUPPORTED_CHANNELS},
+ {"InvalidVersion", A2DP_INVALID_VERSION},
+ {"NotSupportedVersion", A2DP_NOT_SUPPORTED_VERSION},
+ {"NotSupportedMaximumSUL", A2DP_NOT_SUPPORTED_MAXIMUM_SUL},
+ {"InvalidBlockLength", A2DP_INVALID_BLOCK_LENGTH},
+ {"InvalidCPType", A2DP_INVALID_CP_TYPE},
+ {"InvalidCPFormat", A2DP_INVALID_CP_FORMAT},
+ {"InvalidCodecParameter", A2DP_INVALID_CODEC_PARAMETER},
+ {"NotSupportedCodecParameter", A2DP_NOT_SUPPORTED_CODEC_PARAMETER},
+ {"InvalidDRC", A2DP_INVALID_DRC},
+ {"NotSupportedDRC", A2DP_NOT_SUPPORTED_DRC}
+};
+
+uint8_t a2dp_parse_config_error(const char *error_name)
+{
+ size_t prefix_length;
+ size_t i;
+
+ prefix_length = strlen(A2DP_ERROR_PREFIX);
+ if (strncmp(A2DP_ERROR_PREFIX, error_name, prefix_length))
+ return AVDTP_UNSUPPORTED_CONFIGURATION;
+
+ error_name += prefix_length;
+ for (i = 0; i < ARRAY_SIZE(config_errors); i++) {
+ if (strcmp(config_errors[i].error_name, error_name) == 0)
+ return config_errors[i].error_code;
+ }
+
+ return AVDTP_UNSUPPORTED_CONFIGURATION;
+}
+
static struct a2dp_setup *setup_ref(struct a2dp_setup *setup)
{
setup->ref++;
@@ -688,11 +755,10 @@ done:
return FALSE;
}
-static void endpoint_setconf_cb(struct a2dp_setup *setup, gboolean ret)
+static void endpoint_setconf_cb(struct a2dp_setup *setup, uint8_t error_code)
{
- if (ret == FALSE)
- setup_error_init(setup, AVDTP_MEDIA_CODEC,
- AVDTP_UNSUPPORTED_CONFIGURATION);
+ if (error_code != 0)
+ setup_error_init(setup, AVDTP_MEDIA_CODEC, error_code);
auto_config(setup);
setup_unref(setup);
@@ -865,11 +931,11 @@ static gboolean endpoint_getcap_ind(struct avdtp *session,
return TRUE;
}
-static void endpoint_open_cb(struct a2dp_setup *setup, gboolean ret)
+static void endpoint_open_cb(struct a2dp_setup *setup, uint8_t error_code)
{
int err = error_to_errno(setup->err);
- if (ret == FALSE) {
+ if (error_code != 0) {
setup->stream = NULL;
finalize_setup_errno(setup, -EPERM, finalize_config, NULL);
goto done;
diff --git a/profiles/audio/a2dp.h b/profiles/audio/a2dp.h
index c698bc983..afa02c12d 100644
--- a/profiles/audio/a2dp.h
+++ b/profiles/audio/a2dp.h
@@ -15,7 +15,8 @@ struct a2dp_setup;
typedef void (*a2dp_endpoint_select_t) (struct a2dp_setup *setup, void *ret,
int size);
-typedef void (*a2dp_endpoint_config_t) (struct a2dp_setup *setup, gboolean ret);
+typedef void (*a2dp_endpoint_config_t) (struct a2dp_setup *setup,
+ uint8_t error_code);
struct a2dp_endpoint {
const char *(*get_name) (struct a2dp_sep *sep, void *user_data);
@@ -70,6 +71,8 @@ unsigned int a2dp_select_capabilities(struct avdtp *session,
unsigned int a2dp_config(struct avdtp *session, struct a2dp_sep *sep,
a2dp_config_cb_t cb, GSList *caps,
void *user_data);
+uint8_t a2dp_parse_config_error(const char *error_name);
+
unsigned int a2dp_resume(struct avdtp *session, struct a2dp_sep *sep,
a2dp_stream_cb_t cb, void *user_data);
unsigned int a2dp_suspend(struct avdtp *session, struct a2dp_sep *sep,
diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index 30648251f..ed4e22b26 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -1494,8 +1494,8 @@ static void setconf_cb(struct avdtp *session, struct avdtp_stream *stream,
struct conf_rej rej;
if (err != NULL) {
- rej.error = AVDTP_UNSUPPORTED_CONFIGURATION;
- rej.category = err->err.error_code;
+ rej.error = err->err.error_code;
+ rej.category = AVDTP_UNSUPPORTED_CONFIGURATION;
avdtp_send(session, session->in_cmd.transaction,
AVDTP_MSG_TYPE_REJECT, AVDTP_SET_CONFIGURATION,
&rej, sizeof(rej));
diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index 9b3042c18..332f643bb 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -333,7 +333,7 @@ static void endpoint_reply(DBusPendingCall *call, void *user_data)
DBusMessage *reply;
DBusMessageIter args, props;
DBusError err;
- gboolean value;
+ uint8_t error_code;
void *ret = NULL;
int size = -1;
@@ -356,8 +356,12 @@ static void endpoint_reply(DBusPendingCall *call, void *user_data)
if (dbus_message_is_method_call(request->msg,
MEDIA_ENDPOINT_INTERFACE,
- "SetConfiguration"))
+ "SetConfiguration")) {
endpoint_remove_transport(endpoint, request->transport);
+ error_code = a2dp_parse_config_error(err.name);
+ ret = &error_code;
+ size = 1;
+ }
dbus_error_free(&err);
goto done;
@@ -390,8 +394,8 @@ static void endpoint_reply(DBusPendingCall *call, void *user_data)
}
size = 1;
- value = TRUE;
- ret = &value;
+ error_code = 0;
+ ret = &error_code;
done:
dbus_message_unref(reply);
@@ -634,9 +638,9 @@ static void config_cb(struct media_endpoint *endpoint, void *ret, int size,
void *user_data)
{
struct a2dp_config_data *data = user_data;
- gboolean *ret_value = ret;
+ uint8_t *ret_value = ret;
- data->cb(data->setup, ret_value ? *ret_value : FALSE);
+ data->cb(data->setup, ret_value ? *ret_value : 1);
}
static int set_config(struct a2dp_sep *sep, uint8_t *configuration,
@@ -1098,7 +1102,7 @@ static void pac_config_cb(struct media_endpoint *endpoint, void *ret, int size,
void *user_data)
{
struct pac_config_data *data = user_data;
- gboolean *ret_value = ret;
+ uint8_t *error_code = ret;
struct media_transport *transport;
/* If transport was cleared, configuration was cancelled */
@@ -1106,7 +1110,7 @@ static void pac_config_cb(struct media_endpoint *endpoint, void *ret, int size,
if (!transport)
return;
- data->cb(data->stream, ret_value ? 0 : -EINVAL);
+ data->cb(data->stream, error_code == 0 ? 0 : -EINVAL);
}
static struct media_transport *pac_ucast_config(struct bt_bap_stream *stream,
diff --git a/src/error.h b/src/error.h
index 47602d39b..8157795c2 100644
--- a/src/error.h
+++ b/src/error.h
@@ -88,3 +88,41 @@ DBusMessage *btd_error_profile_unavailable(DBusMessage *msg);
DBusMessage *btd_error_failed(DBusMessage *msg, const char *str);
DBusMessage *btd_error_bredr_errno(DBusMessage *msg, int err);
DBusMessage *btd_error_le_errno(DBusMessage *msg, int err);
+
+enum a2dp_error_codes : uint8_t {
+ A2DP_INVALID_CODEC_TYPE = 0xc1,
+ A2DP_NOT_SUPPORTED_CODEC_TYPE = 0xc2,
+ A2DP_INVALID_SAMPLING_FREQUENCY = 0xc3,
+ A2DP_NOT_SUPPORTED_SAMPLING_FREQUENCY = 0xc4,
+ A2DP_INVALID_CHANNEL_MODE = 0xc5,
+ A2DP_NOT_SUPPORTED_CHANNEL_MODE = 0xc6,
+ A2DP_INVALID_SUBBANDS = 0xc7,
+ A2DP_NOT_SUPPORTED_SUBBANDS = 0xc8,
+ A2DP_INVALID_ALLOCATION_METHOD = 0xc9,
+ A2DP_NOT_SUPPORTED_ALLOCATION_METHOD = 0xca,
+ A2DP_INVALID_MINIMUM_BITPOOL_VALUE = 0xcb,
+ A2DP_NOT_SUPPORTED_MINIMUM_BITPOOL_VALUE = 0xcc,
+ A2DP_INVALID_MAXIMUM_BITPOOL_VALUE = 0xcd,
+ A2DP_NOT_SUPPORTED_MAXIMUM_BITPOOL_VALUE = 0xce,
+ A2DP_INVALID_INVALID_LAYER = 0xcf,
+ A2DP_NOT_SUPPORTED_LAYER = 0xd0,
+ A2DP_NOT_SUPPORTERD_CRC = 0xd1,
+ A2DP_NOT_SUPPORTERD_MPF = 0xd2,
+ A2DP_NOT_SUPPORTERD_VBR = 0xd3,
+ A2DP_INVALID_BIT_RATE = 0xd4,
+ A2DP_NOT_SUPPORTED_BIT_RATE = 0xd5,
+ A2DP_INVALID_OBJECT_TYPE = 0xd6,
+ A2DP_NOT_SUPPORTED_OBJECT_TYPE = 0xd7,
+ A2DP_INVALID_CHANNELS = 0xd8,
+ A2DP_NOT_SUPPORTED_CHANNELS = 0xd9,
+ A2DP_INVALID_VERSION = 0xda,
+ A2DP_NOT_SUPPORTED_VERSION = 0xdb,
+ A2DP_NOT_SUPPORTED_MAXIMUM_SUL = 0xdc,
+ A2DP_INVALID_BLOCK_LENGTH = 0xdd,
+ A2DP_INVALID_CP_TYPE = 0xe0,
+ A2DP_INVALID_CP_FORMAT = 0xe1,
+ A2DP_INVALID_CODEC_PARAMETER = 0xe2,
+ A2DP_NOT_SUPPORTED_CODEC_PARAMETER = 0xe3,
+ A2DP_INVALID_DRC = 0xe4,
+ A2DP_NOT_SUPPORTED_DRC = 0xe5,
+};
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* RE: [BlueZ] audio: Add support for specific error codes for A2DP configuration
2025-09-11 13:53 [PATCH BlueZ] audio: Add support for specific error codes for A2DP configuration Per Waagø
@ 2025-09-11 14:08 ` bluez.test.bot
2025-09-11 14:43 ` [PATCH BlueZ] " Luiz Augusto von Dentz
1 sibling, 0 replies; 8+ messages in thread
From: bluez.test.bot @ 2025-09-11 14:08 UTC (permalink / raw)
To: linux-bluetooth, pwaago
[-- Attachment #1: Type: text/plain, Size: 27538 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1001368
---Test result---
Test Summary:
CheckPatch PENDING 0.30 seconds
GitLint PENDING 0.40 seconds
BuildEll PASS 19.86 seconds
BluezMake FAIL 47.94 seconds
MakeCheck FAIL 159.36 seconds
MakeDistcheck FAIL 38.73 seconds
CheckValgrind FAIL 41.27 seconds
CheckSmatch FAIL 97.43 seconds
bluezmakeextell FAIL 23.32 seconds
IncrementalBuild PENDING 1.26 seconds
ScanBuild FAIL 102.41 seconds
Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:
##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:
##############################
Test: BluezMake - FAIL
Desc: Build BlueZ
Output:
In file included from tools/gatt-service.c:28:
./src/error.h:92:23: error: expected identifier or ‘(’ before ‘:’ token
92 | enum a2dp_error_codes : uint8_t {
| ^
make[1]: *** [Makefile:6848: tools/gatt-service.o] Error 1
make[1]: *** Waiting for unfinished jobs....
tools/mgmt-tester.c: In function ‘main’:
tools/mgmt-tester.c:12904:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
12904 | int main(int argc, char *argv[])
| ^~~~
make: *** [Makefile:4039: all] Error 2
##############################
Test: MakeCheck - FAIL
Desc: Run Bluez Make Check
Output:
In file included from tools/gatt-service.c:28:
./src/error.h:92:23: error: expected identifier or ‘(’ before ‘:’ token
92 | enum a2dp_error_codes : uint8_t {
| ^
make[1]: *** [Makefile:6848: tools/gatt-service.o] Error 1
make: *** [Makefile:10469: check] Error 2
##############################
Test: MakeDistcheck - FAIL
Desc: Run Bluez Make Distcheck
Output:
Package cups was not found in the pkg-config search path.
Perhaps you should add the directory containing `cups.pc'
to the PKG_CONFIG_PATH environment variable
No package 'cups' found
In file included from ../../tools/gatt-service.c:28:
../../src/error.h:92:23: error: expected identifier or ‘(’ before ‘:’ token
92 | enum a2dp_error_codes : uint8_t {
| ^
make[2]: *** [Makefile:6848: tools/gatt-service.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [Makefile:4039: all] Error 2
make: *** [Makefile:10390: distcheck] Error 1
##############################
Test: CheckValgrind - FAIL
Desc: Run Bluez Make Check with Valgrind
Output:
In file included from tools/gatt-service.c:28:
./src/error.h:92:23: error: expected identifier or ‘(’ before ‘:’ token
92 | enum a2dp_error_codes : uint8_t {
| ^
make[1]: *** [Makefile:6848: tools/gatt-service.o] Error 1
make[1]: *** Waiting for unfinished jobs....
tools/mgmt-tester.c: In function ‘main’:
tools/mgmt-tester.c:12904:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
12904 | int main(int argc, char *argv[])
| ^~~~
make: *** [Makefile:10469: check] Error 2
##############################
Test: CheckSmatch - FAIL
Desc: Run smatch tool with source
Output:
src/shared/crypto.c:271:21: warning: Variable length array is used.
src/shared/crypto.c:272:23: warning: Variable length array is used.
src/shared/gatt-helpers.c:768:31: warning: Variable length array is used.
src/shared/gatt-helpers.c:830:31: warning: Variable length array is used.
src/shared/gatt-helpers.c:1323:31: warning: Variable length array is used.
src/shared/gatt-helpers.c:1354:23: warning: Variable length array is used.
src/shared/gatt-server.c:278:25: warning: Variable length array is used.
src/shared/gatt-server.c:618:25: warning: Variable length array is used.
src/shared/gatt-server.c:716:25: warning: Variable length array is used.
src/shared/bap.c:317:25: warning: array of flexible structures
src/shared/bap.c: note: in included file:
./src/shared/ascs.h:88:25: warning: array of flexible structures
src/shared/shell.c: note: in included file (through /usr/include/readline/readline.h):
/usr/include/readline/rltypedefs.h:35:23: warning: non-ANSI function declaration of function 'Function'
/usr/include/readline/rltypedefs.h:36:25: warning: non-ANSI function declaration of function 'VFunction'
/usr/include/readline/rltypedefs.h:37:27: warning: non-ANSI function declaration of function 'CPFunction'
/usr/include/readline/rltypedefs.h:38:29: warning: non-ANSI function declaration of function 'CPPFunction'
src/shared/crypto.c:271:21: warning: Variable length array is used.
src/shared/crypto.c:272:23: warning: Variable length array is used.
src/shared/gatt-helpers.c:768:31: warning: Variable length array is used.
src/shared/gatt-helpers.c:830:31: warning: Variable length array is used.
src/shared/gatt-helpers.c:1323:31: warning: Variable length array is used.
src/shared/gatt-helpers.c:1354:23: warning: Variable length array is used.
src/shared/gatt-server.c:278:25: warning: Variable length array is used.
src/shared/gatt-server.c:618:25: warning: Variable length array is used.
src/shared/gatt-server.c:716:25: warning: Variable length array is used.
src/shared/bap.c:317:25: warning: array of flexible structures
src/shared/bap.c: note: in included file:
./src/shared/ascs.h:88:25: warning: array of flexible structures
src/shared/shell.c: note: in included file (through /usr/include/readline/readline.h):
/usr/include/readline/rltypedefs.h:35:23: warning: non-ANSI function declaration of function 'Function'
/usr/include/readline/rltypedefs.h:36:25: warning: non-ANSI function declaration of function 'VFunction'
/usr/include/readline/rltypedefs.h:37:27: warning: non-ANSI function declaration of function 'CPFunction'
/usr/include/readline/rltypedefs.h:38:29: warning: non-ANSI function declaration of function 'CPPFunction'
tools/mesh-cfgtest.c:1453:17: warning: unknown escape sequence: '\%'
tools/sco-tester.c: note: in included file:
./lib/bluetooth/bluetooth.h:232:15: warning: array of flexible structures
./lib/bluetooth/bluetooth.h:237:31: warning: array of flexible structures
tools/bneptest.c:634:39: warning: unknown escape sequence: '\%'
tools/seq2bseq.c:57:26: warning: Variable length array is used.
tools/gatt-service.c: note: in included file:
./src/error.h:92:23: warning: missing identifier in declaration
./src/error.h:92:23: error: Expected ; at the end of type declaration
./src/error.h:92:23: error: got :
In file included from tools/gatt-service.c:28:
./src/error.h:92:23: error: expected identifier or ‘(’ before ‘:’ token
92 | enum a2dp_error_codes : uint8_t {
| ^
make[1]: *** [Makefile:6848: tools/gatt-service.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:4039: all] Error 2
##############################
Test: bluezmakeextell - FAIL
Desc: Build Bluez with External ELL
Output:
In file included from tools/gatt-service.c:28:
./src/error.h:92:23: error: expected identifier or ‘(’ before ‘:’ token
92 | enum a2dp_error_codes : uint8_t {
| ^
make[1]: *** [Makefile:6848: tools/gatt-service.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:4039: all] Error 2
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:
##############################
Test: ScanBuild - FAIL
Desc: Run Scan Build
Output:
src/shared/gatt-client.c:451:21: warning: Use of memory after it is freed
gatt_db_unregister(op->client->db, op->db_id);
^~~~~~~~~~
src/shared/gatt-client.c:696:2: warning: Use of memory after it is freed
discovery_op_complete(op, false, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:996:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1102:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1296:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1361:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1636:6: warning: Use of memory after it is freed
if (read_db_hash(op)) {
^~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1641:2: warning: Use of memory after it is freed
discover_all(op);
^~~~~~~~~~~~~~~~
src/shared/gatt-client.c:2147:6: warning: Use of memory after it is freed
if (read_db_hash(op)) {
^~~~~~~~~~~~~~~~
src/shared/gatt-client.c:2155:8: warning: Use of memory after it is freed
discovery_op_ref(op),
^~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:3180:2: warning: Use of memory after it is freed
complete_write_long_op(req, success, 0, false);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:3202:2: warning: Use of memory after it is freed
request_unref(req);
^~~~~~~~~~~~~~~~~~
12 warnings generated.
src/shared/bap.c:1534:8: warning: Use of memory after it is freed
bap = bt_bap_ref_safe(bap);
^~~~~~~~~~~~~~~~~~~~
src/shared/bap.c:2316:20: warning: Use of memory after it is freed
return queue_find(stream->bap->streams, NULL, stream);
^~~~~~~~~~~~~~~~~~~~
2 warnings generated.
src/shared/gatt-client.c:451:21: warning: Use of memory after it is freed
gatt_db_unregister(op->client->db, op->db_id);
^~~~~~~~~~
src/shared/gatt-client.c:696:2: warning: Use of memory after it is freed
discovery_op_complete(op, false, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:996:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1102:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1296:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1361:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1636:6: warning: Use of memory after it is freed
if (read_db_hash(op)) {
^~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1641:2: warning: Use of memory after it is freed
discover_all(op);
^~~~~~~~~~~~~~~~
src/shared/gatt-client.c:2147:6: warning: Use of memory after it is freed
if (read_db_hash(op)) {
^~~~~~~~~~~~~~~~
src/shared/gatt-client.c:2155:8: warning: Use of memory after it is freed
discovery_op_ref(op),
^~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:3180:2: warning: Use of memory after it is freed
complete_write_long_op(req, success, 0, false);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:3202:2: warning: Use of memory after it is freed
request_unref(req);
^~~~~~~~~~~~~~~~~~
12 warnings generated.
tools/hciattach.c:817:7: warning: Although the value stored to 'n' is used in the enclosing expression, the value is never actually read from 'n'
if ((n = read_hci_event(fd, resp, 10)) < 0) {
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/hciattach.c:865:7: warning: Although the value stored to 'n' is used in the enclosing expression, the value is never actually read from 'n'
if ((n = read_hci_event(fd, resp, 4)) < 0) {
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/hciattach.c:887:8: warning: Although the value stored to 'n' is used in the enclosing expression, the value is never actually read from 'n'
if ((n = read_hci_event(fd, resp, 10)) < 0) {
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/hciattach.c:909:7: warning: Although the value stored to 'n' is used in the enclosing expression, the value is never actually read from 'n'
if ((n = read_hci_event(fd, resp, 4)) < 0) {
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/hciattach.c:930:7: warning: Although the value stored to 'n' is used in the enclosing expression, the value is never actually read from 'n'
if ((n = read_hci_event(fd, resp, 4)) < 0) {
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/hciattach.c:974:7: warning: Although the value stored to 'n' is used in the enclosing expression, the value is never actually read from 'n'
if ((n = read_hci_event(fd, resp, 6)) < 0) {
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
6 warnings generated.
src/shared/bap.c:1534:8: warning: Use of memory after it is freed
bap = bt_bap_ref_safe(bap);
^~~~~~~~~~~~~~~~~~~~
src/shared/bap.c:2316:20: warning: Use of memory after it is freed
return queue_find(stream->bap->streams, NULL, stream);
^~~~~~~~~~~~~~~~~~~~
2 warnings generated.
src/oui.c:50:2: warning: Value stored to 'hwdb' is never read
hwdb = udev_hwdb_unref(hwdb);
^ ~~~~~~~~~~~~~~~~~~~~~
src/oui.c:53:2: warning: Value stored to 'udev' is never read
udev = udev_unref(udev);
^ ~~~~~~~~~~~~~~~~
2 warnings generated.
tools/rfcomm.c:234:3: warning: Value stored to 'i' is never read
i = execvp(cmdargv[0], cmdargv);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/rfcomm.c:234:7: warning: Null pointer passed to 1st parameter expecting 'nonnull'
i = execvp(cmdargv[0], cmdargv);
^~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/rfcomm.c:354:8: warning: Although the value stored to 'fd' is used in the enclosing expression, the value is never actually read from 'fd'
if ((fd = open(devname, O_RDONLY | O_NOCTTY)) < 0) {
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/rfcomm.c:497:14: warning: Assigned value is garbage or undefined
req.channel = raddr.rc_channel;
^ ~~~~~~~~~~~~~~~~
tools/rfcomm.c:515:8: warning: Although the value stored to 'fd' is used in the enclosing expression, the value is never actually read from 'fd'
if ((fd = open(devname, O_RDONLY | O_NOCTTY)) < 0) {
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
5 warnings generated.
tools/hcidump.c:180:9: warning: Potential leak of memory pointed to by 'dp'
if (fds[i].fd == sock)
^~~
tools/hcidump.c:248:17: warning: Assigned value is garbage or undefined
dh->ts_sec = htobl(frm.ts.tv_sec);
^ ~~~~~~~~~~~~~~~~~~~~
tools/hcidump.c:326:9: warning: 1st function call argument is an uninitialized value
if (be32toh(dp.flags) & 0x02) {
^~~~~~~~~~~~~~~~~
/usr/include/endian.h:46:22: note: expanded from macro 'be32toh'
# define be32toh(x) __bswap_32 (x)
^~~~~~~~~~~~~~
tools/hcidump.c:341:20: warning: 1st function call argument is an uninitialized value
frm.data_len = be32toh(dp.len);
^~~~~~~~~~~~~~~
/usr/include/endian.h:46:22: note: expanded from macro 'be32toh'
# define be32toh(x) __bswap_32 (x)
^~~~~~~~~~~~~~
tools/hcidump.c:346:14: warning: 1st function call argument is an uninitialized value
opcode = be32toh(dp.flags) & 0xffff;
^~~~~~~~~~~~~~~~~
/usr/include/endian.h:46:22: note: expanded from macro 'be32toh'
# define be32toh(x) __bswap_32 (x)
^~~~~~~~~~~~~~
tools/hcidump.c:384:17: warning: Assigned value is garbage or undefined
frm.data_len = btohs(dh.len);
^ ~~~~~~~~~~~~~
tools/hcidump.c:394:11: warning: Assigned value is garbage or undefined
frm.len = frm.data_len;
^ ~~~~~~~~~~~~
tools/hcidump.c:398:9: warning: 1st function call argument is an uninitialized value
ts = be64toh(ph.ts);
^~~~~~~~~~~~~~
/usr/include/endian.h:51:22: note: expanded from macro 'be64toh'
# define be64toh(x) __bswap_64 (x)
^~~~~~~~~~~~~~
tools/hcidump.c:403:13: warning: 1st function call argument is an uninitialized value
frm.in = be32toh(dp.flags) & 0x01;
^~~~~~~~~~~~~~~~~
/usr/include/endian.h:46:22: note: expanded from macro 'be32toh'
# define be32toh(x) __bswap_32 (x)
^~~~~~~~~~~~~~
tools/hcidump.c:408:11: warning: Assigned value is garbage or undefined
frm.in = dh.in;
^ ~~~~~
tools/hcidump.c:437:7: warning: Null pointer passed to 1st parameter expecting 'nonnull'
fd = open(file, open_flags, 0644);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
11 warnings generated.
tools/ciptool.c:351:7: warning: 5th function call argument is an uninitialized value
sk = do_connect(ctl, dev_id, &src, &dst, psm, (1 << CMTP_LOOPBACK));
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
src/sdp-xml.c:126:10: warning: Assigned value is garbage or undefined
buf[1] = data[i + 1];
^ ~~~~~~~~~~~
src/sdp-xml.c:306:11: warning: Assigned value is garbage or undefined
buf[1] = data[i + 1];
^ ~~~~~~~~~~~
src/sdp-xml.c:344:11: warning: Assigned value is garbage or undefined
buf[1] = data[i + 1];
^ ~~~~~~~~~~~
3 warnings generated.
tools/sdptool.c:941:26: warning: Result of 'malloc' is converted to a pointer of type 'uint32_t', which is incompatible with sizeof operand type 'int'
uint32_t *value_int = malloc(sizeof(int));
~~~~~~~~~~ ^~~~~~ ~~~~~~~~~~~
tools/sdptool.c:980:4: warning: 1st function call argument is an uninitialized value
free(allocArray[i]);
^~~~~~~~~~~~~~~~~~~
tools/sdptool.c:3777:2: warning: Potential leak of memory pointed to by 'si.name'
return add_service(0, &si);
^~~~~~~~~~~~~~~~~~~~~~~~~~
tools/sdptool.c:4112:4: warning: Potential leak of memory pointed to by 'context.svc'
return -1;
^~~~~~~~~
4 warnings generated.
tools/avtest.c:243:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 3);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:253:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 4);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:262:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 3);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:276:5: warning: Value stored to 'len' is never read
len = write(sk, buf,
^ ~~~~~~~~~~~~~~
tools/avtest.c:283:5: warning: Value stored to 'len' is never read
len = write(sk, buf,
^ ~~~~~~~~~~~~~~
tools/avtest.c:290:5: warning: Value stored to 'len' is never read
len = write(sk, buf,
^ ~~~~~~~~~~~~~~
tools/avtest.c:297:5: warning: Value stored to 'len' is never read
len = write(sk, buf,
^ ~~~~~~~~~~~~~~
tools/avtest.c:309:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 4);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:313:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 2);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:322:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 3);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:326:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 2);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:335:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 3);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:342:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 2);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:364:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 4);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:368:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 2);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:377:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 3);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:381:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 2);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:394:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 4);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:398:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 2);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:405:4: warning: Value stored to 'len' is never read
len = write(sk, buf, 2);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:415:4: warning: Value stored to 'len' is never read
len = write(sk, buf, 2);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:580:3: warning: Value stored to 'len' is never read
len = write(sk, buf, 2);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:588:3: warning: Value stored to 'len' is never read
len = write(sk, buf, invalid ? 2 : 3);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/avtest.c:602:3: warning: Value stored to 'len' is never read
len = write(sk, buf, 4 + media_transport_size);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/avtest.c:615:3: warning: Value stored to 'len' is never read
len = write(sk, buf, 3);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:625:3: warning: Value stored to 'len' is never read
len = write(sk, buf, 3);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:637:3: warning: Value stored to 'len' is never read
len = write(sk, buf, 3);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:652:3: warning: Value stored to 'len' is never read
len = write(sk, buf, 3);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:664:3: warning: Value stored to 'len' is never read
len = write(sk, buf, 3);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:673:3: warning: Value stored to 'len' is never read
len = write(sk, buf, 3);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:680:3: warning: Value stored to 'len' is never read
len = write(sk, buf, 2);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:716:2: warning: Value stored to 'len' is never read
len = write(sk, buf, AVCTP_HEADER_LENGTH + sizeof(play_pressed));
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
32 warnings generated.
tools/btproxy.c:836:15: warning: Null pointer passed to 1st parameter expecting 'nonnull'
tcp_port = atoi(optarg);
^~~~~~~~~~~~
tools/btproxy.c:839:8: warning: Null pointer passed to 1st parameter expecting 'nonnull'
if (strlen(optarg) > 3 && !strncmp(optarg, "hci", 3))
^~~~~~~~~~~~~~
2 warnings generated.
tools/create-image.c:76:3: warning: Value stored to 'fd' is never read
fd = -1;
^ ~~
tools/create-image.c:84:3: warning: Value stored to 'fd' is never read
fd = -1;
^ ~~
tools/create-image.c:92:3: warning: Value stored to 'fd' is never read
fd = -1;
^ ~~
tools/create-image.c:105:2: warning: Value stored to 'fd' is never read
fd = -1;
^ ~~
4 warnings generated.
tools/iso-tester.c:1906:7: warning: Potential leak of memory pointed to by 'addr'
err = bind(sk, (struct sockaddr *) addr, sizeof(*addr) +
~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/iso-tester.c:1914:7: warning: Potential leak of memory pointed to by 'addr'
err = bind(sk, (struct sockaddr *) addr, sizeof(*addr));
~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 warnings generated.
tools/check-selftest.c:42:3: warning: Value stored to 'ptr' is never read
ptr = fgets(result, sizeof(result), fp);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
In file included from tools/gatt-service.c:28:
./src/error.h:92:23: error: expected identifier or ‘(’ before ‘:’ token
92 | enum a2dp_error_codes : uint8_t {
| ^
make[1]: *** [Makefile:6848: tools/gatt-service.o] Error 1
make[1]: *** Waiting for unfinished jobs....
tools/btgatt-client.c:1824:2: warning: Value stored to 'argv' is never read
argv += optind;
^ ~~~~~~
1 warning generated.
tools/btgatt-server.c:1212:2: warning: Value stored to 'argv' is never read
argv -= optind;
^ ~~~~~~
1 warning generated.
make: *** [Makefile:4039: all] Error 2
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH BlueZ] audio: Add support for specific error codes for A2DP configuration
2025-09-11 13:53 [PATCH BlueZ] audio: Add support for specific error codes for A2DP configuration Per Waagø
2025-09-11 14:08 ` [BlueZ] " bluez.test.bot
@ 2025-09-11 14:43 ` Luiz Augusto von Dentz
2025-09-11 15:12 ` Per Waago (pwaago)
2025-09-11 16:25 ` Pauli Virtanen
1 sibling, 2 replies; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2025-09-11 14:43 UTC (permalink / raw)
To: Per Waagø, Pauli Virtanen; +Cc: linux-bluetooth
Hi Per,
On Thu, Sep 11, 2025 at 9:56 AM Per Waagø <pwaago@cisco.com> wrote:
>
> The A2DP specification defines error codes that shall be used if
> the codec capabilities contain improper settings. This change allows
> clients to trigger the sending of these specific error codes by
> returning the corresponding error messages from
> MediaEndpoint1.SetConfiguration.
>
> This update is fully backwards compatible: clients passing other error
> messages will continue to receive the default error code as before. On
> older BlueZ versions, these new errors will also result in the default
> error code, enabling clients to implement support for the new errors
> without breaking compatibility.
While I can see the value for debugging I doubt we could do any
handling of these errors, so the result would be the same regardless
of what error is sent back it is not recoverable.
@Pauli Virtanen Perhaps you can give some feedback regarding these
codes, would pipewire be interested in generating specific error
codes? Some of them seems to be SBC specific like bitpool.
> This change enables passing A2DP/SNK/AVP/* and A2DP/SRC/AVP/*
> qualification tests.
> ---
> doc/org.bluez.MediaEndpoint.rst | 37 ++++++++++++++++
> profiles/audio/a2dp.c | 78 ++++++++++++++++++++++++++++++---
> profiles/audio/a2dp.h | 5 ++-
> profiles/audio/avdtp.c | 4 +-
> profiles/audio/media.c | 20 +++++----
> src/error.h | 38 ++++++++++++++++
> 6 files changed, 165 insertions(+), 17 deletions(-)
>
> diff --git a/doc/org.bluez.MediaEndpoint.rst b/doc/org.bluez.MediaEndpoint.rst
> index 474cc1160..2721d473e 100644
> --- a/doc/org.bluez.MediaEndpoint.rst
> +++ b/doc/org.bluez.MediaEndpoint.rst
> @@ -54,6 +54,43 @@ be configured and the properties must contain the following properties:
>
> See **org.bluez.MediaTransport(5)** QoS property.
>
> + Possible errors:
> + :org.bluez.Error.A2DP.InvalidCodecType:
> + :org.bluez.Error.A2DP.NotSupportedCodecType:
> + :org.bluez.Error.A2DP.InvalidSamplingFrequency:
> + :org.bluez.Error.A2DP.NotSupportedSamplingFrequency:
> + :org.bluez.Error.A2DP.InvalidChannelMode:
> + :org.bluez.Error.A2DP.NotSupportedChannelMode:
> + :org.bluez.Error.A2DP.InvalidSubbands:
> + :org.bluez.Error.A2DP.NotSupportedSubbands:
> + :org.bluez.Error.A2DP.InvalidAllocationMethod:
> + :org.bluez.Error.A2DP.NotSupportedAllocationMethod:
> + :org.bluez.Error.A2DP.InvalidMinimumBitpoolValue:
> + :org.bluez.Error.A2DP.NotSupportedMinimumBitpoolValue:
> + :org.bluez.Error.A2DP.InvalidMaximumBitpoolValue:
> + :org.bluez.Error.A2DP.NotSupportedMaximumBitpoolValue:
> + :org.bluez.Error.A2DP.InvalidInvalidLayer:
> + :org.bluez.Error.A2DP.NotSupportedLayer:
> + :org.bluez.Error.A2DP.NotSupporterdCRC:
> + :org.bluez.Error.A2DP.NotSupporterdMPF:
> + :org.bluez.Error.A2DP.NotSupporterdVBR:
> + :org.bluez.Error.A2DP.InvalidBitRate:
> + :org.bluez.Error.A2DP.NotSupportedBitRate:
> + :org.bluez.Error.A2DP.InvalidObjectType:
> + :org.bluez.Error.A2DP.NotSupportedObjectType:
> + :org.bluez.Error.A2DP.InvalidChannels:
> + :org.bluez.Error.A2DP.NotSupportedChannels:
> + :org.bluez.Error.A2DP.InvalidVersion:
> + :org.bluez.Error.A2DP.NotSupportedVersion:
> + :org.bluez.Error.A2DP.NotSupportedMaximumSUL:
> + :org.bluez.Error.A2DP.InvalidBlockLength:
> + :org.bluez.Error.A2DP.InvalidCPType:
> + :org.bluez.Error.A2DP.InvalidCPFormat:
> + :org.bluez.Error.A2DP.InvalidCodecParameter:
> + :org.bluez.Error.A2DP.NotSupportedCodecParameter:
> + :org.bluez.Error.A2DP.InvalidDRC:
> + :org.bluez.Error.A2DP.NotSupportedDRC:
Introducing a error domain for A2DP is probably a good idea, but this
only applies to endpoints that are A2DP specific, so this need to be
adjusted to e.g.: possible for A2DP or something like that, also I
don't know how the client would be able to tell it can return these
errors? Or the expectation is that the client can start sending them
immediately since the old behavior will convert them to
AVDTP_UNSUPPORTED_CONFIGURATION anyway?
While at it split the commit to have the documentation changes as a
separate change.
> array{byte} SelectConfiguration(array{byte} capabilities)
> `````````````````````````````````````````````````````````
>
> diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> index c0a53eae9..661843a89 100644
> --- a/profiles/audio/a2dp.c
> +++ b/profiles/audio/a2dp.c
> @@ -157,6 +157,73 @@ static GSList *servers = NULL;
> static GSList *setups = NULL;
> static unsigned int cb_id = 0;
>
> +struct a2dp_error {
> + const char *error_name;
> + uint8_t error_code;
> +};
> +
> +#define A2DP_ERROR_PREFIX ERROR_INTERFACE ".A2DP."
> +
> +static struct a2dp_error config_errors[] = {
> + {"InvalidCodecType", A2DP_INVALID_CODEC_TYPE},
> + {"NotSupportedCodecType", A2DP_NOT_SUPPORTED_CODEC_TYPE},
> + {"InvalidSamplingFrequency", A2DP_INVALID_SAMPLING_FREQUENCY},
> + {"NotSupportedSamplingFrequency",
> + A2DP_NOT_SUPPORTED_SAMPLING_FREQUENCY},
> + {"InvalidChannelMode", A2DP_INVALID_CHANNEL_MODE},
> + {"NotSupportedChannelMode", A2DP_NOT_SUPPORTED_CHANNEL_MODE},
> + {"InvalidSubbands", A2DP_INVALID_SUBBANDS},
> + {"NotSupportedSubbands", A2DP_NOT_SUPPORTED_SUBBANDS},
> + {"InvalidAllocationMethod", A2DP_INVALID_ALLOCATION_METHOD},
> + {"NotSupportedAllocationMethod", A2DP_NOT_SUPPORTED_ALLOCATION_METHOD},
> + {"InvalidMinimumBitpoolValue",
> + A2DP_INVALID_MINIMUM_BITPOOL_VALUE},
> + {"NotSupportedMinimumBitpoolValue",
> + A2DP_NOT_SUPPORTED_MINIMUM_BITPOOL_VALUE},
> + {"InvalidMaximumBitpoolValue", A2DP_INVALID_MAXIMUM_BITPOOL_VALUE},
> + {"NotSupportedMaximumBitpoolValue",
> + A2DP_NOT_SUPPORTED_MAXIMUM_BITPOOL_VALUE},
> + {"InvalidInvalidLayer", A2DP_INVALID_INVALID_LAYER},
> + {"NotSupportedLayer", A2DP_NOT_SUPPORTED_LAYER},
> + {"NotSupporterdCRC", A2DP_NOT_SUPPORTERD_CRC},
> + {"NotSupporterdMPF", A2DP_NOT_SUPPORTERD_MPF},
> + {"NotSupporterdVBR", A2DP_NOT_SUPPORTERD_VBR},
> + {"InvalidBitRate", A2DP_INVALID_BIT_RATE},
> + {"NotSupportedBitRate", A2DP_NOT_SUPPORTED_BIT_RATE},
> + {"InvalidObjectType", A2DP_INVALID_OBJECT_TYPE},
> + {"NotSupportedObjectType", A2DP_NOT_SUPPORTED_OBJECT_TYPE},
> + {"InvalidChannels", A2DP_INVALID_CHANNELS},
> + {"NotSupportedChannels", A2DP_NOT_SUPPORTED_CHANNELS},
> + {"InvalidVersion", A2DP_INVALID_VERSION},
> + {"NotSupportedVersion", A2DP_NOT_SUPPORTED_VERSION},
> + {"NotSupportedMaximumSUL", A2DP_NOT_SUPPORTED_MAXIMUM_SUL},
> + {"InvalidBlockLength", A2DP_INVALID_BLOCK_LENGTH},
> + {"InvalidCPType", A2DP_INVALID_CP_TYPE},
> + {"InvalidCPFormat", A2DP_INVALID_CP_FORMAT},
> + {"InvalidCodecParameter", A2DP_INVALID_CODEC_PARAMETER},
> + {"NotSupportedCodecParameter", A2DP_NOT_SUPPORTED_CODEC_PARAMETER},
> + {"InvalidDRC", A2DP_INVALID_DRC},
> + {"NotSupportedDRC", A2DP_NOT_SUPPORTED_DRC}
> +};
> +
> +uint8_t a2dp_parse_config_error(const char *error_name)
> +{
> + size_t prefix_length;
> + size_t i;
> +
> + prefix_length = strlen(A2DP_ERROR_PREFIX);
> + if (strncmp(A2DP_ERROR_PREFIX, error_name, prefix_length))
> + return AVDTP_UNSUPPORTED_CONFIGURATION;
> +
> + error_name += prefix_length;
> + for (i = 0; i < ARRAY_SIZE(config_errors); i++) {
> + if (strcmp(config_errors[i].error_name, error_name) == 0)
> + return config_errors[i].error_code;
> + }
> +
> + return AVDTP_UNSUPPORTED_CONFIGURATION;
> +}
> +
> static struct a2dp_setup *setup_ref(struct a2dp_setup *setup)
> {
> setup->ref++;
> @@ -688,11 +755,10 @@ done:
> return FALSE;
> }
>
> -static void endpoint_setconf_cb(struct a2dp_setup *setup, gboolean ret)
> +static void endpoint_setconf_cb(struct a2dp_setup *setup, uint8_t error_code)
> {
> - if (ret == FALSE)
> - setup_error_init(setup, AVDTP_MEDIA_CODEC,
> - AVDTP_UNSUPPORTED_CONFIGURATION);
> + if (error_code != 0)
> + setup_error_init(setup, AVDTP_MEDIA_CODEC, error_code);
>
> auto_config(setup);
> setup_unref(setup);
> @@ -865,11 +931,11 @@ static gboolean endpoint_getcap_ind(struct avdtp *session,
> return TRUE;
> }
>
> -static void endpoint_open_cb(struct a2dp_setup *setup, gboolean ret)
> +static void endpoint_open_cb(struct a2dp_setup *setup, uint8_t error_code)
> {
> int err = error_to_errno(setup->err);
>
> - if (ret == FALSE) {
> + if (error_code != 0) {
> setup->stream = NULL;
> finalize_setup_errno(setup, -EPERM, finalize_config, NULL);
> goto done;
> diff --git a/profiles/audio/a2dp.h b/profiles/audio/a2dp.h
> index c698bc983..afa02c12d 100644
> --- a/profiles/audio/a2dp.h
> +++ b/profiles/audio/a2dp.h
> @@ -15,7 +15,8 @@ struct a2dp_setup;
>
> typedef void (*a2dp_endpoint_select_t) (struct a2dp_setup *setup, void *ret,
> int size);
> -typedef void (*a2dp_endpoint_config_t) (struct a2dp_setup *setup, gboolean ret);
> +typedef void (*a2dp_endpoint_config_t) (struct a2dp_setup *setup,
> + uint8_t error_code);
>
> struct a2dp_endpoint {
> const char *(*get_name) (struct a2dp_sep *sep, void *user_data);
> @@ -70,6 +71,8 @@ unsigned int a2dp_select_capabilities(struct avdtp *session,
> unsigned int a2dp_config(struct avdtp *session, struct a2dp_sep *sep,
> a2dp_config_cb_t cb, GSList *caps,
> void *user_data);
> +uint8_t a2dp_parse_config_error(const char *error_name);
> +
> unsigned int a2dp_resume(struct avdtp *session, struct a2dp_sep *sep,
> a2dp_stream_cb_t cb, void *user_data);
> unsigned int a2dp_suspend(struct avdtp *session, struct a2dp_sep *sep,
> diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
> index 30648251f..ed4e22b26 100644
> --- a/profiles/audio/avdtp.c
> +++ b/profiles/audio/avdtp.c
> @@ -1494,8 +1494,8 @@ static void setconf_cb(struct avdtp *session, struct avdtp_stream *stream,
> struct conf_rej rej;
>
> if (err != NULL) {
> - rej.error = AVDTP_UNSUPPORTED_CONFIGURATION;
> - rej.category = err->err.error_code;
> + rej.error = err->err.error_code;
> + rej.category = AVDTP_UNSUPPORTED_CONFIGURATION;
> avdtp_send(session, session->in_cmd.transaction,
> AVDTP_MSG_TYPE_REJECT, AVDTP_SET_CONFIGURATION,
> &rej, sizeof(rej));
> diff --git a/profiles/audio/media.c b/profiles/audio/media.c
> index 9b3042c18..332f643bb 100644
> --- a/profiles/audio/media.c
> +++ b/profiles/audio/media.c
> @@ -333,7 +333,7 @@ static void endpoint_reply(DBusPendingCall *call, void *user_data)
> DBusMessage *reply;
> DBusMessageIter args, props;
> DBusError err;
> - gboolean value;
> + uint8_t error_code;
> void *ret = NULL;
> int size = -1;
>
> @@ -356,8 +356,12 @@ static void endpoint_reply(DBusPendingCall *call, void *user_data)
>
> if (dbus_message_is_method_call(request->msg,
> MEDIA_ENDPOINT_INTERFACE,
> - "SetConfiguration"))
> + "SetConfiguration")) {
> endpoint_remove_transport(endpoint, request->transport);
> + error_code = a2dp_parse_config_error(err.name);
> + ret = &error_code;
> + size = 1;
> + }
>
> dbus_error_free(&err);
> goto done;
> @@ -390,8 +394,8 @@ static void endpoint_reply(DBusPendingCall *call, void *user_data)
> }
>
> size = 1;
> - value = TRUE;
> - ret = &value;
> + error_code = 0;
> + ret = &error_code;
>
> done:
> dbus_message_unref(reply);
> @@ -634,9 +638,9 @@ static void config_cb(struct media_endpoint *endpoint, void *ret, int size,
> void *user_data)
> {
> struct a2dp_config_data *data = user_data;
> - gboolean *ret_value = ret;
> + uint8_t *ret_value = ret;
>
> - data->cb(data->setup, ret_value ? *ret_value : FALSE);
> + data->cb(data->setup, ret_value ? *ret_value : 1);
> }
>
> static int set_config(struct a2dp_sep *sep, uint8_t *configuration,
> @@ -1098,7 +1102,7 @@ static void pac_config_cb(struct media_endpoint *endpoint, void *ret, int size,
> void *user_data)
> {
> struct pac_config_data *data = user_data;
> - gboolean *ret_value = ret;
> + uint8_t *error_code = ret;
> struct media_transport *transport;
>
> /* If transport was cleared, configuration was cancelled */
> @@ -1106,7 +1110,7 @@ static void pac_config_cb(struct media_endpoint *endpoint, void *ret, int size,
> if (!transport)
> return;
>
> - data->cb(data->stream, ret_value ? 0 : -EINVAL);
> + data->cb(data->stream, error_code == 0 ? 0 : -EINVAL);
> }
>
> static struct media_transport *pac_ucast_config(struct bt_bap_stream *stream,
> diff --git a/src/error.h b/src/error.h
> index 47602d39b..8157795c2 100644
> --- a/src/error.h
> +++ b/src/error.h
> @@ -88,3 +88,41 @@ DBusMessage *btd_error_profile_unavailable(DBusMessage *msg);
> DBusMessage *btd_error_failed(DBusMessage *msg, const char *str);
> DBusMessage *btd_error_bredr_errno(DBusMessage *msg, int err);
> DBusMessage *btd_error_le_errno(DBusMessage *msg, int err);
> +
> +enum a2dp_error_codes : uint8_t {
> + A2DP_INVALID_CODEC_TYPE = 0xc1,
> + A2DP_NOT_SUPPORTED_CODEC_TYPE = 0xc2,
> + A2DP_INVALID_SAMPLING_FREQUENCY = 0xc3,
> + A2DP_NOT_SUPPORTED_SAMPLING_FREQUENCY = 0xc4,
> + A2DP_INVALID_CHANNEL_MODE = 0xc5,
> + A2DP_NOT_SUPPORTED_CHANNEL_MODE = 0xc6,
> + A2DP_INVALID_SUBBANDS = 0xc7,
> + A2DP_NOT_SUPPORTED_SUBBANDS = 0xc8,
> + A2DP_INVALID_ALLOCATION_METHOD = 0xc9,
> + A2DP_NOT_SUPPORTED_ALLOCATION_METHOD = 0xca,
> + A2DP_INVALID_MINIMUM_BITPOOL_VALUE = 0xcb,
> + A2DP_NOT_SUPPORTED_MINIMUM_BITPOOL_VALUE = 0xcc,
> + A2DP_INVALID_MAXIMUM_BITPOOL_VALUE = 0xcd,
> + A2DP_NOT_SUPPORTED_MAXIMUM_BITPOOL_VALUE = 0xce,
> + A2DP_INVALID_INVALID_LAYER = 0xcf,
> + A2DP_NOT_SUPPORTED_LAYER = 0xd0,
> + A2DP_NOT_SUPPORTERD_CRC = 0xd1,
> + A2DP_NOT_SUPPORTERD_MPF = 0xd2,
> + A2DP_NOT_SUPPORTERD_VBR = 0xd3,
> + A2DP_INVALID_BIT_RATE = 0xd4,
> + A2DP_NOT_SUPPORTED_BIT_RATE = 0xd5,
> + A2DP_INVALID_OBJECT_TYPE = 0xd6,
> + A2DP_NOT_SUPPORTED_OBJECT_TYPE = 0xd7,
> + A2DP_INVALID_CHANNELS = 0xd8,
> + A2DP_NOT_SUPPORTED_CHANNELS = 0xd9,
> + A2DP_INVALID_VERSION = 0xda,
> + A2DP_NOT_SUPPORTED_VERSION = 0xdb,
> + A2DP_NOT_SUPPORTED_MAXIMUM_SUL = 0xdc,
> + A2DP_INVALID_BLOCK_LENGTH = 0xdd,
> + A2DP_INVALID_CP_TYPE = 0xe0,
> + A2DP_INVALID_CP_FORMAT = 0xe1,
> + A2DP_INVALID_CODEC_PARAMETER = 0xe2,
> + A2DP_NOT_SUPPORTED_CODEC_PARAMETER = 0xe3,
> + A2DP_INVALID_DRC = 0xe4,
> + A2DP_NOT_SUPPORTED_DRC = 0xe5,
> +};
Hmm, I guess there should be part of a2dp.h since this error header is
about D-Bus not A2DP codes.
> --
> 2.43.0
>
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH BlueZ] audio: Add support for specific error codes for A2DP configuration
2025-09-11 14:43 ` [PATCH BlueZ] " Luiz Augusto von Dentz
@ 2025-09-11 15:12 ` Per Waago (pwaago)
2025-09-11 15:52 ` Luiz Augusto von Dentz
2025-09-11 16:25 ` Pauli Virtanen
1 sibling, 1 reply; 8+ messages in thread
From: Per Waago (pwaago) @ 2025-09-11 15:12 UTC (permalink / raw)
To: Luiz Augusto von Dentz, Pauli Virtanen; +Cc: linux-bluetooth@vger.kernel.org
Hi Luiz, thanks for reviewing.
> From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
> Sent: Thursday, September 11, 2025 16:43
> To: Per Waago (pwaago); Pauli Virtanen
> Cc: linux-bluetooth@vger.kernel.org
> Subject: Re: [PATCH BlueZ] audio: Add support for specific error codes for A2DP configuration
>
> Hi Per,
>
> On Thu, Sep 11, 2025 at 9:56 AM Per Waagø <pwaago@cisco.com> wrote:
> >
> > The A2DP specification defines error codes that shall be used if
> > the codec capabilities contain improper settings. This change allows
> > clients to trigger the sending of these specific error codes by
> > returning the corresponding error messages from
> > MediaEndpoint1.SetConfiguration.
> >
> > This update is fully backwards compatible: clients passing other error
> > messages will continue to receive the default error code as before. On
> > older BlueZ versions, these new errors will also result in the default
> > error code, enabling clients to implement support for the new errors
> > without breaking compatibility.
>
> While I can see the value for debugging I doubt we could do any
> handling of these errors, so the result would be the same regardless
> of what error is sent back it is not recoverable.
>
The main motivation for adding them is to be able to pass the
mandatory qualification tests, which now checks the errors codes
returned from SetConfiguration in detail. I don't think they are very
useful otherwise.
The errors are specified in table 5.5 in the A2DP spec:
https://www.bluetooth.com/specifications/specs/html/?src=a2dp_v1-4-1_1752513648/A2DP_v1.4.1/out/en/index-en.html#UUID-0ba19ee9-7277-1068-d2dc-b9e638cca568_Table_5.5
I included all of them for completeness. In that table, it is also stated
which codecs they apply to. Some are SBC-specific, some apply to all codecs or
other codecs.
> @Pauli Virtanen Perhaps you can give some feedback regarding these
> codes, would pipewire be interested in generating specific error
> codes? Some of them seems to be SBC specific like bitpool.
>
> > This change enables passing A2DP/SNK/AVP/* and A2DP/SRC/AVP/*
> > qualification tests.
> > ---
> > doc/org.bluez.MediaEndpoint.rst | 37 ++++++++++++++++
> > profiles/audio/a2dp.c | 78 ++++++++++++++++++++++++++++++---
> > profiles/audio/a2dp.h | 5 ++-
> > profiles/audio/avdtp.c | 4 +-
> > profiles/audio/media.c | 20 +++++----
> > src/error.h | 38 ++++++++++++++++
> > 6 files changed, 165 insertions(+), 17 deletions(-)
> >
> > diff --git a/doc/org.bluez.MediaEndpoint.rst b/doc/org.bluez.MediaEndpoint.rst
> > index 474cc1160..2721d473e 100644
> > --- a/doc/org.bluez.MediaEndpoint.rst
> > +++ b/doc/org.bluez.MediaEndpoint.rst
> > @@ -54,6 +54,43 @@ be configured and the properties must contain the following properties:
> >
> > See **org.bluez.MediaTransport(5)** QoS property.
> >
> > + Possible errors:
> > + :org.bluez.Error.A2DP.InvalidCodecType:
> > + :org.bluez.Error.A2DP.NotSupportedCodecType:
> > + :org.bluez.Error.A2DP.InvalidSamplingFrequency:
> > + :org.bluez.Error.A2DP.NotSupportedSamplingFrequency:
> > + :org.bluez.Error.A2DP.InvalidChannelMode:
> > + :org.bluez.Error.A2DP.NotSupportedChannelMode:
> > + :org.bluez.Error.A2DP.InvalidSubbands:
> > + :org.bluez.Error.A2DP.NotSupportedSubbands:
> > + :org.bluez.Error.A2DP.InvalidAllocationMethod:
> > + :org.bluez.Error.A2DP.NotSupportedAllocationMethod:
> > + :org.bluez.Error.A2DP.InvalidMinimumBitpoolValue:
> > + :org.bluez.Error.A2DP.NotSupportedMinimumBitpoolValue:
> > + :org.bluez.Error.A2DP.InvalidMaximumBitpoolValue:
> > + :org.bluez.Error.A2DP.NotSupportedMaximumBitpoolValue:
> > + :org.bluez.Error.A2DP.InvalidInvalidLayer:
> > + :org.bluez.Error.A2DP.NotSupportedLayer:
> > + :org.bluez.Error.A2DP.NotSupporterdCRC:
> > + :org.bluez.Error.A2DP.NotSupporterdMPF:
> > + :org.bluez.Error.A2DP.NotSupporterdVBR:
> > + :org.bluez.Error.A2DP.InvalidBitRate:
> > + :org.bluez.Error.A2DP.NotSupportedBitRate:
> > + :org.bluez.Error.A2DP.InvalidObjectType:
> > + :org.bluez.Error.A2DP.NotSupportedObjectType:
> > + :org.bluez.Error.A2DP.InvalidChannels:
> > + :org.bluez.Error.A2DP.NotSupportedChannels:
> > + :org.bluez.Error.A2DP.InvalidVersion:
> > + :org.bluez.Error.A2DP.NotSupportedVersion:
> > + :org.bluez.Error.A2DP.NotSupportedMaximumSUL:
> > + :org.bluez.Error.A2DP.InvalidBlockLength:
> > + :org.bluez.Error.A2DP.InvalidCPType:
> > + :org.bluez.Error.A2DP.InvalidCPFormat:
> > + :org.bluez.Error.A2DP.InvalidCodecParameter:
> > + :org.bluez.Error.A2DP.NotSupportedCodecParameter:
> > + :org.bluez.Error.A2DP.InvalidDRC:
> > + :org.bluez.Error.A2DP.NotSupportedDRC:
>
> Introducing a error domain for A2DP is probably a good idea, but this
> only applies to endpoints that are A2DP specific, so this need to be
> adjusted to e.g.: possible for A2DP or something like that, also I
> don't know how the client would be able to tell it can return these
> errors? Or the expectation is that the client can start sending them
> immediately since the old behavior will convert them to
> AVDTP_UNSUPPORTED_CONFIGURATION anyway?
>
That was what I thought. The clients can just start sending these, and
they will be converted to the correct error code if bluez supports it
or to AVDTP_UNSUPPORTED_CONFIGURATION otherwise.
> While at it split the commit to have the documentation changes as a
> separate change.
Will adjust text and split into a separate commit.
>
> > array{byte} SelectConfiguration(array{byte} capabilities)
> > `````````````````````````````````````````````````````````
> >
> > diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> > index c0a53eae9..661843a89 100644
> > --- a/profiles/audio/a2dp.c
> > +++ b/profiles/audio/a2dp.c
> > @@ -157,6 +157,73 @@ static GSList *servers = NULL;
> > static GSList *setups = NULL;
> > static unsigned int cb_id = 0;
> >
> > +struct a2dp_error {
> > + const char *error_name;
> > + uint8_t error_code;
> > +};
> > +
> > +#define A2DP_ERROR_PREFIX ERROR_INTERFACE ".A2DP."
> > +
> > +static struct a2dp_error config_errors[] = {
> > + {"InvalidCodecType", A2DP_INVALID_CODEC_TYPE},
> > + {"NotSupportedCodecType", A2DP_NOT_SUPPORTED_CODEC_TYPE},
> > + {"InvalidSamplingFrequency", A2DP_INVALID_SAMPLING_FREQUENCY},
> > + {"NotSupportedSamplingFrequency",
> > + A2DP_NOT_SUPPORTED_SAMPLING_FREQUENCY},
> > + {"InvalidChannelMode", A2DP_INVALID_CHANNEL_MODE},
> > + {"NotSupportedChannelMode", A2DP_NOT_SUPPORTED_CHANNEL_MODE},
> > + {"InvalidSubbands", A2DP_INVALID_SUBBANDS},
> > + {"NotSupportedSubbands", A2DP_NOT_SUPPORTED_SUBBANDS},
> > + {"InvalidAllocationMethod", A2DP_INVALID_ALLOCATION_METHOD},
> > + {"NotSupportedAllocationMethod", A2DP_NOT_SUPPORTED_ALLOCATION_METHOD},
> > + {"InvalidMinimumBitpoolValue",
> > + A2DP_INVALID_MINIMUM_BITPOOL_VALUE},
> > + {"NotSupportedMinimumBitpoolValue",
> > + A2DP_NOT_SUPPORTED_MINIMUM_BITPOOL_VALUE},
> > + {"InvalidMaximumBitpoolValue", A2DP_INVALID_MAXIMUM_BITPOOL_VALUE},
> > + {"NotSupportedMaximumBitpoolValue",
> > + A2DP_NOT_SUPPORTED_MAXIMUM_BITPOOL_VALUE},
> > + {"InvalidInvalidLayer", A2DP_INVALID_INVALID_LAYER},
> > + {"NotSupportedLayer", A2DP_NOT_SUPPORTED_LAYER},
> > + {"NotSupporterdCRC", A2DP_NOT_SUPPORTERD_CRC},
> > + {"NotSupporterdMPF", A2DP_NOT_SUPPORTERD_MPF},
> > + {"NotSupporterdVBR", A2DP_NOT_SUPPORTERD_VBR},
> > + {"InvalidBitRate", A2DP_INVALID_BIT_RATE},
> > + {"NotSupportedBitRate", A2DP_NOT_SUPPORTED_BIT_RATE},
> > + {"InvalidObjectType", A2DP_INVALID_OBJECT_TYPE},
> > + {"NotSupportedObjectType", A2DP_NOT_SUPPORTED_OBJECT_TYPE},
> > + {"InvalidChannels", A2DP_INVALID_CHANNELS},
> > + {"NotSupportedChannels", A2DP_NOT_SUPPORTED_CHANNELS},
> > + {"InvalidVersion", A2DP_INVALID_VERSION},
> > + {"NotSupportedVersion", A2DP_NOT_SUPPORTED_VERSION},
> > + {"NotSupportedMaximumSUL", A2DP_NOT_SUPPORTED_MAXIMUM_SUL},
> > + {"InvalidBlockLength", A2DP_INVALID_BLOCK_LENGTH},
> > + {"InvalidCPType", A2DP_INVALID_CP_TYPE},
> > + {"InvalidCPFormat", A2DP_INVALID_CP_FORMAT},
> > + {"InvalidCodecParameter", A2DP_INVALID_CODEC_PARAMETER},
> > + {"NotSupportedCodecParameter", A2DP_NOT_SUPPORTED_CODEC_PARAMETER},
> > + {"InvalidDRC", A2DP_INVALID_DRC},
> > + {"NotSupportedDRC", A2DP_NOT_SUPPORTED_DRC}
> > +};
> > +
> > +uint8_t a2dp_parse_config_error(const char *error_name)
> > +{
> > + size_t prefix_length;
> > + size_t i;
> > +
> > + prefix_length = strlen(A2DP_ERROR_PREFIX);
> > + if (strncmp(A2DP_ERROR_PREFIX, error_name, prefix_length))
> > + return AVDTP_UNSUPPORTED_CONFIGURATION;
> > +
> > + error_name += prefix_length;
> > + for (i = 0; i < ARRAY_SIZE(config_errors); i++) {
> > + if (strcmp(config_errors[i].error_name, error_name) == 0)
> > + return config_errors[i].error_code;
> > + }
> > +
> > + return AVDTP_UNSUPPORTED_CONFIGURATION;
> > +}
> > +
> > static struct a2dp_setup *setup_ref(struct a2dp_setup *setup)
> > {
> > setup->ref++;
> > @@ -688,11 +755,10 @@ done:
> > return FALSE;
> > }
> >
> > -static void endpoint_setconf_cb(struct a2dp_setup *setup, gboolean ret)
> > +static void endpoint_setconf_cb(struct a2dp_setup *setup, uint8_t error_code)
> > {
> > - if (ret == FALSE)
> > - setup_error_init(setup, AVDTP_MEDIA_CODEC,
> > - AVDTP_UNSUPPORTED_CONFIGURATION);
> > + if (error_code != 0)
> > + setup_error_init(setup, AVDTP_MEDIA_CODEC, error_code);
> >
> > auto_config(setup);
> > setup_unref(setup);
> > @@ -865,11 +931,11 @@ static gboolean endpoint_getcap_ind(struct avdtp *session,
> > return TRUE;
> > }
> >
> > -static void endpoint_open_cb(struct a2dp_setup *setup, gboolean ret)
> > +static void endpoint_open_cb(struct a2dp_setup *setup, uint8_t error_code)
> > {
> > int err = error_to_errno(setup->err);
> >
> > - if (ret == FALSE) {
> > + if (error_code != 0) {
> > setup->stream = NULL;
> > finalize_setup_errno(setup, -EPERM, finalize_config, NULL);
> > goto done;
> > diff --git a/profiles/audio/a2dp.h b/profiles/audio/a2dp.h
> > index c698bc983..afa02c12d 100644
> > --- a/profiles/audio/a2dp.h
> > +++ b/profiles/audio/a2dp.h
> > @@ -15,7 +15,8 @@ struct a2dp_setup;
> >
> > typedef void (*a2dp_endpoint_select_t) (struct a2dp_setup *setup, void *ret,
> > int size);
> > -typedef void (*a2dp_endpoint_config_t) (struct a2dp_setup *setup, gboolean ret);
> > +typedef void (*a2dp_endpoint_config_t) (struct a2dp_setup *setup,
> > + uint8_t error_code);
> >
> > struct a2dp_endpoint {
> > const char *(*get_name) (struct a2dp_sep *sep, void *user_data);
> > @@ -70,6 +71,8 @@ unsigned int a2dp_select_capabilities(struct avdtp *session,
> > unsigned int a2dp_config(struct avdtp *session, struct a2dp_sep *sep,
> > a2dp_config_cb_t cb, GSList *caps,
> > void *user_data);
> > +uint8_t a2dp_parse_config_error(const char *error_name);
> > +
> > unsigned int a2dp_resume(struct avdtp *session, struct a2dp_sep *sep,
> > a2dp_stream_cb_t cb, void *user_data);
> > unsigned int a2dp_suspend(struct avdtp *session, struct a2dp_sep *sep,
> > diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
> > index 30648251f..ed4e22b26 100644
> > --- a/profiles/audio/avdtp.c
> > +++ b/profiles/audio/avdtp.c
> > @@ -1494,8 +1494,8 @@ static void setconf_cb(struct avdtp *session, struct avdtp_stream *stream,
> > struct conf_rej rej;
> >
> > if (err != NULL) {
> > - rej.error = AVDTP_UNSUPPORTED_CONFIGURATION;
> > - rej.category = err->err.error_code;
> > + rej.error = err->err.error_code;
> > + rej.category = AVDTP_UNSUPPORTED_CONFIGURATION;
> > avdtp_send(session, session->in_cmd.transaction,
> > AVDTP_MSG_TYPE_REJECT, AVDTP_SET_CONFIGURATION,
> > &rej, sizeof(rej));
> > diff --git a/profiles/audio/media.c b/profiles/audio/media.c
> > index 9b3042c18..332f643bb 100644
> > --- a/profiles/audio/media.c
> > +++ b/profiles/audio/media.c
> > @@ -333,7 +333,7 @@ static void endpoint_reply(DBusPendingCall *call, void *user_data)
> > DBusMessage *reply;
> > DBusMessageIter args, props;
> > DBusError err;
> > - gboolean value;
> > + uint8_t error_code;
> > void *ret = NULL;
> > int size = -1;
> >
> > @@ -356,8 +356,12 @@ static void endpoint_reply(DBusPendingCall *call, void *user_data)
> >
> > if (dbus_message_is_method_call(request->msg,
> > MEDIA_ENDPOINT_INTERFACE,
> > - "SetConfiguration"))
> > + "SetConfiguration")) {
> > endpoint_remove_transport(endpoint, request->transport);
> > + error_code = a2dp_parse_config_error(err.name);
> > + ret = &error_code;
> > + size = 1;
> > + }
> >
> > dbus_error_free(&err);
> > goto done;
> > @@ -390,8 +394,8 @@ static void endpoint_reply(DBusPendingCall *call, void *user_data)
> > }
> >
> > size = 1;
> > - value = TRUE;
> > - ret = &value;
> > + error_code = 0;
> > + ret = &error_code;
> >
> > done:
> > dbus_message_unref(reply);
> > @@ -634,9 +638,9 @@ static void config_cb(struct media_endpoint *endpoint, void *ret, int size,
> > void *user_data)
> > {
> > struct a2dp_config_data *data = user_data;
> > - gboolean *ret_value = ret;
> > + uint8_t *ret_value = ret;
> >
> > - data->cb(data->setup, ret_value ? *ret_value : FALSE);
> > + data->cb(data->setup, ret_value ? *ret_value : 1);
> > }
> >
> > static int set_config(struct a2dp_sep *sep, uint8_t *configuration,
> > @@ -1098,7 +1102,7 @@ static void pac_config_cb(struct media_endpoint *endpoint, void *ret, int size,
> > void *user_data)
> > {
> > struct pac_config_data *data = user_data;
> > - gboolean *ret_value = ret;
> > + uint8_t *error_code = ret;
> > struct media_transport *transport;
> >
> > /* If transport was cleared, configuration was cancelled */
> > @@ -1106,7 +1110,7 @@ static void pac_config_cb(struct media_endpoint *endpoint, void *ret, int size,
> > if (!transport)
> > return;
> >
> > - data->cb(data->stream, ret_value ? 0 : -EINVAL);
> > + data->cb(data->stream, error_code == 0 ? 0 : -EINVAL);
> > }
> >
> > static struct media_transport *pac_ucast_config(struct bt_bap_stream *stream,
> > diff --git a/src/error.h b/src/error.h
> > index 47602d39b..8157795c2 100644
> > --- a/src/error.h
> > +++ b/src/error.h
> > @@ -88,3 +88,41 @@ DBusMessage *btd_error_profile_unavailable(DBusMessage *msg);
> > DBusMessage *btd_error_failed(DBusMessage *msg, const char *str);
> > DBusMessage *btd_error_bredr_errno(DBusMessage *msg, int err);
> > DBusMessage *btd_error_le_errno(DBusMessage *msg, int err);
> > +
> > +enum a2dp_error_codes : uint8_t {
> > + A2DP_INVALID_CODEC_TYPE = 0xc1,
> > + A2DP_NOT_SUPPORTED_CODEC_TYPE = 0xc2,
> > + A2DP_INVALID_SAMPLING_FREQUENCY = 0xc3,
> > + A2DP_NOT_SUPPORTED_SAMPLING_FREQUENCY = 0xc4,
> > + A2DP_INVALID_CHANNEL_MODE = 0xc5,
> > + A2DP_NOT_SUPPORTED_CHANNEL_MODE = 0xc6,
> > + A2DP_INVALID_SUBBANDS = 0xc7,
> > + A2DP_NOT_SUPPORTED_SUBBANDS = 0xc8,
> > + A2DP_INVALID_ALLOCATION_METHOD = 0xc9,
> > + A2DP_NOT_SUPPORTED_ALLOCATION_METHOD = 0xca,
> > + A2DP_INVALID_MINIMUM_BITPOOL_VALUE = 0xcb,
> > + A2DP_NOT_SUPPORTED_MINIMUM_BITPOOL_VALUE = 0xcc,
> > + A2DP_INVALID_MAXIMUM_BITPOOL_VALUE = 0xcd,
> > + A2DP_NOT_SUPPORTED_MAXIMUM_BITPOOL_VALUE = 0xce,
> > + A2DP_INVALID_INVALID_LAYER = 0xcf,
> > + A2DP_NOT_SUPPORTED_LAYER = 0xd0,
> > + A2DP_NOT_SUPPORTERD_CRC = 0xd1,
> > + A2DP_NOT_SUPPORTERD_MPF = 0xd2,
> > + A2DP_NOT_SUPPORTERD_VBR = 0xd3,
> > + A2DP_INVALID_BIT_RATE = 0xd4,
> > + A2DP_NOT_SUPPORTED_BIT_RATE = 0xd5,
> > + A2DP_INVALID_OBJECT_TYPE = 0xd6,
> > + A2DP_NOT_SUPPORTED_OBJECT_TYPE = 0xd7,
> > + A2DP_INVALID_CHANNELS = 0xd8,
> > + A2DP_NOT_SUPPORTED_CHANNELS = 0xd9,
> > + A2DP_INVALID_VERSION = 0xda,
> > + A2DP_NOT_SUPPORTED_VERSION = 0xdb,
> > + A2DP_NOT_SUPPORTED_MAXIMUM_SUL = 0xdc,
> > + A2DP_INVALID_BLOCK_LENGTH = 0xdd,
> > + A2DP_INVALID_CP_TYPE = 0xe0,
> > + A2DP_INVALID_CP_FORMAT = 0xe1,
> > + A2DP_INVALID_CODEC_PARAMETER = 0xe2,
> > + A2DP_NOT_SUPPORTED_CODEC_PARAMETER = 0xe3,
> > + A2DP_INVALID_DRC = 0xe4,
> > + A2DP_NOT_SUPPORTED_DRC = 0xe5,
> > +};
>
> Hmm, I guess there should be part of a2dp.h since this error header is
> about D-Bus not A2DP codes.
>
Ok, will move them to a2dp.h
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH BlueZ] audio: Add support for specific error codes for A2DP configuration
2025-09-11 15:12 ` Per Waago (pwaago)
@ 2025-09-11 15:52 ` Luiz Augusto von Dentz
2025-09-11 19:56 ` Per Waago (pwaago)
0 siblings, 1 reply; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2025-09-11 15:52 UTC (permalink / raw)
To: Per Waago (pwaago); +Cc: Pauli Virtanen, linux-bluetooth@vger.kernel.org
Hi Per,
On Thu, Sep 11, 2025 at 11:12 AM Per Waago (pwaago) <pwaago@cisco.com> wrote:
>
> Hi Luiz, thanks for reviewing.
>
> > From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
> > Sent: Thursday, September 11, 2025 16:43
> > To: Per Waago (pwaago); Pauli Virtanen
> > Cc: linux-bluetooth@vger.kernel.org
> > Subject: Re: [PATCH BlueZ] audio: Add support for specific error codes for A2DP configuration
> >
> > Hi Per,
> >
> > On Thu, Sep 11, 2025 at 9:56 AM Per Waagø <pwaago@cisco.com> wrote:
> > >
> > > The A2DP specification defines error codes that shall be used if
> > > the codec capabilities contain improper settings. This change allows
> > > clients to trigger the sending of these specific error codes by
> > > returning the corresponding error messages from
> > > MediaEndpoint1.SetConfiguration.
> > >
> > > This update is fully backwards compatible: clients passing other error
> > > messages will continue to receive the default error code as before. On
> > > older BlueZ versions, these new errors will also result in the default
> > > error code, enabling clients to implement support for the new errors
> > > without breaking compatibility.
> >
> > While I can see the value for debugging I doubt we could do any
> > handling of these errors, so the result would be the same regardless
> > of what error is sent back it is not recoverable.
> >
>
> The main motivation for adding them is to be able to pass the
> mandatory qualification tests, which now checks the errors codes
> returned from SetConfiguration in detail. I don't think they are very
> useful otherwise.
>
> The errors are specified in table 5.5 in the A2DP spec:
> https://www.bluetooth.com/specifications/specs/html/?src=a2dp_v1-4-1_1752513648/A2DP_v1.4.1/out/en/index-en.html#UUID-0ba19ee9-7277-1068-d2dc-b9e638cca568_Table_5.5
>
> I included all of them for completeness. In that table, it is also stated
> which codecs they apply to. Some are SBC-specific, some apply to all codecs or
> other codecs.
Ok this is very annoying if PTS suddenly adds a new test case that
checks error codes that otherwise are only useful for debugging. I'd
say that it probably needs a configuration entry to skip these tests,
btw this seems to be introduced in 1.4.1:
https://www.bluetooth.com/specifications/specs/html/?src=a2dp_v1-4-1_1752513648/A2DP_v1.4.1/out/en/index-en.html#UUID-05a1c924-2070-eb38-c2cc-a9ffa178bdb9
BlueZ SDP record is still 1.4 (a2dp_ver = 0x0104), it seems 1.4.1 is
an errata only change but that introduces new error codes which is
really intrusive to say the least.
> > @Pauli Virtanen Perhaps you can give some feedback regarding these
> > codes, would pipewire be interested in generating specific error
> > codes? Some of them seems to be SBC specific like bitpool.
> >
> > > This change enables passing A2DP/SNK/AVP/* and A2DP/SRC/AVP/*
> > > qualification tests.
> > > ---
> > > doc/org.bluez.MediaEndpoint.rst | 37 ++++++++++++++++
> > > profiles/audio/a2dp.c | 78 ++++++++++++++++++++++++++++++---
> > > profiles/audio/a2dp.h | 5 ++-
> > > profiles/audio/avdtp.c | 4 +-
> > > profiles/audio/media.c | 20 +++++----
> > > src/error.h | 38 ++++++++++++++++
> > > 6 files changed, 165 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/doc/org.bluez.MediaEndpoint.rst b/doc/org.bluez.MediaEndpoint.rst
> > > index 474cc1160..2721d473e 100644
> > > --- a/doc/org.bluez.MediaEndpoint.rst
> > > +++ b/doc/org.bluez.MediaEndpoint.rst
> > > @@ -54,6 +54,43 @@ be configured and the properties must contain the following properties:
> > >
> > > See **org.bluez.MediaTransport(5)** QoS property.
> > >
> > > + Possible errors:
> > > + :org.bluez.Error.A2DP.InvalidCodecType:
> > > + :org.bluez.Error.A2DP.NotSupportedCodecType:
> > > + :org.bluez.Error.A2DP.InvalidSamplingFrequency:
> > > + :org.bluez.Error.A2DP.NotSupportedSamplingFrequency:
> > > + :org.bluez.Error.A2DP.InvalidChannelMode:
> > > + :org.bluez.Error.A2DP.NotSupportedChannelMode:
> > > + :org.bluez.Error.A2DP.InvalidSubbands:
> > > + :org.bluez.Error.A2DP.NotSupportedSubbands:
> > > + :org.bluez.Error.A2DP.InvalidAllocationMethod:
> > > + :org.bluez.Error.A2DP.NotSupportedAllocationMethod:
> > > + :org.bluez.Error.A2DP.InvalidMinimumBitpoolValue:
> > > + :org.bluez.Error.A2DP.NotSupportedMinimumBitpoolValue:
> > > + :org.bluez.Error.A2DP.InvalidMaximumBitpoolValue:
> > > + :org.bluez.Error.A2DP.NotSupportedMaximumBitpoolValue:
> > > + :org.bluez.Error.A2DP.InvalidInvalidLayer:
> > > + :org.bluez.Error.A2DP.NotSupportedLayer:
> > > + :org.bluez.Error.A2DP.NotSupporterdCRC:
> > > + :org.bluez.Error.A2DP.NotSupporterdMPF:
> > > + :org.bluez.Error.A2DP.NotSupporterdVBR:
> > > + :org.bluez.Error.A2DP.InvalidBitRate:
> > > + :org.bluez.Error.A2DP.NotSupportedBitRate:
> > > + :org.bluez.Error.A2DP.InvalidObjectType:
> > > + :org.bluez.Error.A2DP.NotSupportedObjectType:
> > > + :org.bluez.Error.A2DP.InvalidChannels:
> > > + :org.bluez.Error.A2DP.NotSupportedChannels:
> > > + :org.bluez.Error.A2DP.InvalidVersion:
> > > + :org.bluez.Error.A2DP.NotSupportedVersion:
> > > + :org.bluez.Error.A2DP.NotSupportedMaximumSUL:
> > > + :org.bluez.Error.A2DP.InvalidBlockLength:
> > > + :org.bluez.Error.A2DP.InvalidCPType:
> > > + :org.bluez.Error.A2DP.InvalidCPFormat:
> > > + :org.bluez.Error.A2DP.InvalidCodecParameter:
> > > + :org.bluez.Error.A2DP.NotSupportedCodecParameter:
> > > + :org.bluez.Error.A2DP.InvalidDRC:
> > > + :org.bluez.Error.A2DP.NotSupportedDRC:
> >
> > Introducing a error domain for A2DP is probably a good idea, but this
> > only applies to endpoints that are A2DP specific, so this need to be
> > adjusted to e.g.: possible for A2DP or something like that, also I
> > don't know how the client would be able to tell it can return these
> > errors? Or the expectation is that the client can start sending them
> > immediately since the old behavior will convert them to
> > AVDTP_UNSUPPORTED_CONFIGURATION anyway?
> >
>
> That was what I thought. The clients can just start sending these, and
> they will be converted to the correct error code if bluez supports it
> or to AVDTP_UNSUPPORTED_CONFIGURATION otherwise.
>
> > While at it split the commit to have the documentation changes as a
> > separate change.
>
> Will adjust text and split into a separate commit.
>
> >
> > > array{byte} SelectConfiguration(array{byte} capabilities)
> > > `````````````````````````````````````````````````````````
> > >
> > > diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> > > index c0a53eae9..661843a89 100644
> > > --- a/profiles/audio/a2dp.c
> > > +++ b/profiles/audio/a2dp.c
> > > @@ -157,6 +157,73 @@ static GSList *servers = NULL;
> > > static GSList *setups = NULL;
> > > static unsigned int cb_id = 0;
> > >
> > > +struct a2dp_error {
> > > + const char *error_name;
> > > + uint8_t error_code;
> > > +};
> > > +
> > > +#define A2DP_ERROR_PREFIX ERROR_INTERFACE ".A2DP."
> > > +
> > > +static struct a2dp_error config_errors[] = {
> > > + {"InvalidCodecType", A2DP_INVALID_CODEC_TYPE},
> > > + {"NotSupportedCodecType", A2DP_NOT_SUPPORTED_CODEC_TYPE},
> > > + {"InvalidSamplingFrequency", A2DP_INVALID_SAMPLING_FREQUENCY},
> > > + {"NotSupportedSamplingFrequency",
> > > + A2DP_NOT_SUPPORTED_SAMPLING_FREQUENCY},
> > > + {"InvalidChannelMode", A2DP_INVALID_CHANNEL_MODE},
> > > + {"NotSupportedChannelMode", A2DP_NOT_SUPPORTED_CHANNEL_MODE},
> > > + {"InvalidSubbands", A2DP_INVALID_SUBBANDS},
> > > + {"NotSupportedSubbands", A2DP_NOT_SUPPORTED_SUBBANDS},
> > > + {"InvalidAllocationMethod", A2DP_INVALID_ALLOCATION_METHOD},
> > > + {"NotSupportedAllocationMethod", A2DP_NOT_SUPPORTED_ALLOCATION_METHOD},
> > > + {"InvalidMinimumBitpoolValue",
> > > + A2DP_INVALID_MINIMUM_BITPOOL_VALUE},
> > > + {"NotSupportedMinimumBitpoolValue",
> > > + A2DP_NOT_SUPPORTED_MINIMUM_BITPOOL_VALUE},
> > > + {"InvalidMaximumBitpoolValue", A2DP_INVALID_MAXIMUM_BITPOOL_VALUE},
> > > + {"NotSupportedMaximumBitpoolValue",
> > > + A2DP_NOT_SUPPORTED_MAXIMUM_BITPOOL_VALUE},
> > > + {"InvalidInvalidLayer", A2DP_INVALID_INVALID_LAYER},
> > > + {"NotSupportedLayer", A2DP_NOT_SUPPORTED_LAYER},
> > > + {"NotSupporterdCRC", A2DP_NOT_SUPPORTERD_CRC},
> > > + {"NotSupporterdMPF", A2DP_NOT_SUPPORTERD_MPF},
> > > + {"NotSupporterdVBR", A2DP_NOT_SUPPORTERD_VBR},
> > > + {"InvalidBitRate", A2DP_INVALID_BIT_RATE},
> > > + {"NotSupportedBitRate", A2DP_NOT_SUPPORTED_BIT_RATE},
> > > + {"InvalidObjectType", A2DP_INVALID_OBJECT_TYPE},
> > > + {"NotSupportedObjectType", A2DP_NOT_SUPPORTED_OBJECT_TYPE},
> > > + {"InvalidChannels", A2DP_INVALID_CHANNELS},
> > > + {"NotSupportedChannels", A2DP_NOT_SUPPORTED_CHANNELS},
> > > + {"InvalidVersion", A2DP_INVALID_VERSION},
> > > + {"NotSupportedVersion", A2DP_NOT_SUPPORTED_VERSION},
> > > + {"NotSupportedMaximumSUL", A2DP_NOT_SUPPORTED_MAXIMUM_SUL},
> > > + {"InvalidBlockLength", A2DP_INVALID_BLOCK_LENGTH},
> > > + {"InvalidCPType", A2DP_INVALID_CP_TYPE},
> > > + {"InvalidCPFormat", A2DP_INVALID_CP_FORMAT},
> > > + {"InvalidCodecParameter", A2DP_INVALID_CODEC_PARAMETER},
> > > + {"NotSupportedCodecParameter", A2DP_NOT_SUPPORTED_CODEC_PARAMETER},
> > > + {"InvalidDRC", A2DP_INVALID_DRC},
> > > + {"NotSupportedDRC", A2DP_NOT_SUPPORTED_DRC}
> > > +};
> > > +
> > > +uint8_t a2dp_parse_config_error(const char *error_name)
> > > +{
> > > + size_t prefix_length;
> > > + size_t i;
> > > +
> > > + prefix_length = strlen(A2DP_ERROR_PREFIX);
> > > + if (strncmp(A2DP_ERROR_PREFIX, error_name, prefix_length))
> > > + return AVDTP_UNSUPPORTED_CONFIGURATION;
> > > +
> > > + error_name += prefix_length;
> > > + for (i = 0; i < ARRAY_SIZE(config_errors); i++) {
> > > + if (strcmp(config_errors[i].error_name, error_name) == 0)
> > > + return config_errors[i].error_code;
> > > + }
> > > +
> > > + return AVDTP_UNSUPPORTED_CONFIGURATION;
> > > +}
> > > +
> > > static struct a2dp_setup *setup_ref(struct a2dp_setup *setup)
> > > {
> > > setup->ref++;
> > > @@ -688,11 +755,10 @@ done:
> > > return FALSE;
> > > }
> > >
> > > -static void endpoint_setconf_cb(struct a2dp_setup *setup, gboolean ret)
> > > +static void endpoint_setconf_cb(struct a2dp_setup *setup, uint8_t error_code)
> > > {
> > > - if (ret == FALSE)
> > > - setup_error_init(setup, AVDTP_MEDIA_CODEC,
> > > - AVDTP_UNSUPPORTED_CONFIGURATION);
> > > + if (error_code != 0)
> > > + setup_error_init(setup, AVDTP_MEDIA_CODEC, error_code);
> > >
> > > auto_config(setup);
> > > setup_unref(setup);
> > > @@ -865,11 +931,11 @@ static gboolean endpoint_getcap_ind(struct avdtp *session,
> > > return TRUE;
> > > }
> > >
> > > -static void endpoint_open_cb(struct a2dp_setup *setup, gboolean ret)
> > > +static void endpoint_open_cb(struct a2dp_setup *setup, uint8_t error_code)
> > > {
> > > int err = error_to_errno(setup->err);
> > >
> > > - if (ret == FALSE) {
> > > + if (error_code != 0) {
> > > setup->stream = NULL;
> > > finalize_setup_errno(setup, -EPERM, finalize_config, NULL);
> > > goto done;
> > > diff --git a/profiles/audio/a2dp.h b/profiles/audio/a2dp.h
> > > index c698bc983..afa02c12d 100644
> > > --- a/profiles/audio/a2dp.h
> > > +++ b/profiles/audio/a2dp.h
> > > @@ -15,7 +15,8 @@ struct a2dp_setup;
> > >
> > > typedef void (*a2dp_endpoint_select_t) (struct a2dp_setup *setup, void *ret,
> > > int size);
> > > -typedef void (*a2dp_endpoint_config_t) (struct a2dp_setup *setup, gboolean ret);
> > > +typedef void (*a2dp_endpoint_config_t) (struct a2dp_setup *setup,
> > > + uint8_t error_code);
> > >
> > > struct a2dp_endpoint {
> > > const char *(*get_name) (struct a2dp_sep *sep, void *user_data);
> > > @@ -70,6 +71,8 @@ unsigned int a2dp_select_capabilities(struct avdtp *session,
> > > unsigned int a2dp_config(struct avdtp *session, struct a2dp_sep *sep,
> > > a2dp_config_cb_t cb, GSList *caps,
> > > void *user_data);
> > > +uint8_t a2dp_parse_config_error(const char *error_name);
> > > +
> > > unsigned int a2dp_resume(struct avdtp *session, struct a2dp_sep *sep,
> > > a2dp_stream_cb_t cb, void *user_data);
> > > unsigned int a2dp_suspend(struct avdtp *session, struct a2dp_sep *sep,
> > > diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
> > > index 30648251f..ed4e22b26 100644
> > > --- a/profiles/audio/avdtp.c
> > > +++ b/profiles/audio/avdtp.c
> > > @@ -1494,8 +1494,8 @@ static void setconf_cb(struct avdtp *session, struct avdtp_stream *stream,
> > > struct conf_rej rej;
> > >
> > > if (err != NULL) {
> > > - rej.error = AVDTP_UNSUPPORTED_CONFIGURATION;
> > > - rej.category = err->err.error_code;
> > > + rej.error = err->err.error_code;
> > > + rej.category = AVDTP_UNSUPPORTED_CONFIGURATION;
> > > avdtp_send(session, session->in_cmd.transaction,
> > > AVDTP_MSG_TYPE_REJECT, AVDTP_SET_CONFIGURATION,
> > > &rej, sizeof(rej));
> > > diff --git a/profiles/audio/media.c b/profiles/audio/media.c
> > > index 9b3042c18..332f643bb 100644
> > > --- a/profiles/audio/media.c
> > > +++ b/profiles/audio/media.c
> > > @@ -333,7 +333,7 @@ static void endpoint_reply(DBusPendingCall *call, void *user_data)
> > > DBusMessage *reply;
> > > DBusMessageIter args, props;
> > > DBusError err;
> > > - gboolean value;
> > > + uint8_t error_code;
> > > void *ret = NULL;
> > > int size = -1;
> > >
> > > @@ -356,8 +356,12 @@ static void endpoint_reply(DBusPendingCall *call, void *user_data)
> > >
> > > if (dbus_message_is_method_call(request->msg,
> > > MEDIA_ENDPOINT_INTERFACE,
> > > - "SetConfiguration"))
> > > + "SetConfiguration")) {
> > > endpoint_remove_transport(endpoint, request->transport);
> > > + error_code = a2dp_parse_config_error(err.name);
> > > + ret = &error_code;
> > > + size = 1;
> > > + }
> > >
> > > dbus_error_free(&err);
> > > goto done;
> > > @@ -390,8 +394,8 @@ static void endpoint_reply(DBusPendingCall *call, void *user_data)
> > > }
> > >
> > > size = 1;
> > > - value = TRUE;
> > > - ret = &value;
> > > + error_code = 0;
> > > + ret = &error_code;
> > >
> > > done:
> > > dbus_message_unref(reply);
> > > @@ -634,9 +638,9 @@ static void config_cb(struct media_endpoint *endpoint, void *ret, int size,
> > > void *user_data)
> > > {
> > > struct a2dp_config_data *data = user_data;
> > > - gboolean *ret_value = ret;
> > > + uint8_t *ret_value = ret;
> > >
> > > - data->cb(data->setup, ret_value ? *ret_value : FALSE);
> > > + data->cb(data->setup, ret_value ? *ret_value : 1);
> > > }
> > >
> > > static int set_config(struct a2dp_sep *sep, uint8_t *configuration,
> > > @@ -1098,7 +1102,7 @@ static void pac_config_cb(struct media_endpoint *endpoint, void *ret, int size,
> > > void *user_data)
> > > {
> > > struct pac_config_data *data = user_data;
> > > - gboolean *ret_value = ret;
> > > + uint8_t *error_code = ret;
> > > struct media_transport *transport;
> > >
> > > /* If transport was cleared, configuration was cancelled */
> > > @@ -1106,7 +1110,7 @@ static void pac_config_cb(struct media_endpoint *endpoint, void *ret, int size,
> > > if (!transport)
> > > return;
> > >
> > > - data->cb(data->stream, ret_value ? 0 : -EINVAL);
> > > + data->cb(data->stream, error_code == 0 ? 0 : -EINVAL);
> > > }
> > >
> > > static struct media_transport *pac_ucast_config(struct bt_bap_stream *stream,
> > > diff --git a/src/error.h b/src/error.h
> > > index 47602d39b..8157795c2 100644
> > > --- a/src/error.h
> > > +++ b/src/error.h
> > > @@ -88,3 +88,41 @@ DBusMessage *btd_error_profile_unavailable(DBusMessage *msg);
> > > DBusMessage *btd_error_failed(DBusMessage *msg, const char *str);
> > > DBusMessage *btd_error_bredr_errno(DBusMessage *msg, int err);
> > > DBusMessage *btd_error_le_errno(DBusMessage *msg, int err);
> > > +
> > > +enum a2dp_error_codes : uint8_t {
> > > + A2DP_INVALID_CODEC_TYPE = 0xc1,
> > > + A2DP_NOT_SUPPORTED_CODEC_TYPE = 0xc2,
> > > + A2DP_INVALID_SAMPLING_FREQUENCY = 0xc3,
> > > + A2DP_NOT_SUPPORTED_SAMPLING_FREQUENCY = 0xc4,
> > > + A2DP_INVALID_CHANNEL_MODE = 0xc5,
> > > + A2DP_NOT_SUPPORTED_CHANNEL_MODE = 0xc6,
> > > + A2DP_INVALID_SUBBANDS = 0xc7,
> > > + A2DP_NOT_SUPPORTED_SUBBANDS = 0xc8,
> > > + A2DP_INVALID_ALLOCATION_METHOD = 0xc9,
> > > + A2DP_NOT_SUPPORTED_ALLOCATION_METHOD = 0xca,
> > > + A2DP_INVALID_MINIMUM_BITPOOL_VALUE = 0xcb,
> > > + A2DP_NOT_SUPPORTED_MINIMUM_BITPOOL_VALUE = 0xcc,
> > > + A2DP_INVALID_MAXIMUM_BITPOOL_VALUE = 0xcd,
> > > + A2DP_NOT_SUPPORTED_MAXIMUM_BITPOOL_VALUE = 0xce,
> > > + A2DP_INVALID_INVALID_LAYER = 0xcf,
> > > + A2DP_NOT_SUPPORTED_LAYER = 0xd0,
> > > + A2DP_NOT_SUPPORTERD_CRC = 0xd1,
> > > + A2DP_NOT_SUPPORTERD_MPF = 0xd2,
> > > + A2DP_NOT_SUPPORTERD_VBR = 0xd3,
> > > + A2DP_INVALID_BIT_RATE = 0xd4,
> > > + A2DP_NOT_SUPPORTED_BIT_RATE = 0xd5,
> > > + A2DP_INVALID_OBJECT_TYPE = 0xd6,
> > > + A2DP_NOT_SUPPORTED_OBJECT_TYPE = 0xd7,
> > > + A2DP_INVALID_CHANNELS = 0xd8,
> > > + A2DP_NOT_SUPPORTED_CHANNELS = 0xd9,
> > > + A2DP_INVALID_VERSION = 0xda,
> > > + A2DP_NOT_SUPPORTED_VERSION = 0xdb,
> > > + A2DP_NOT_SUPPORTED_MAXIMUM_SUL = 0xdc,
> > > + A2DP_INVALID_BLOCK_LENGTH = 0xdd,
> > > + A2DP_INVALID_CP_TYPE = 0xe0,
> > > + A2DP_INVALID_CP_FORMAT = 0xe1,
> > > + A2DP_INVALID_CODEC_PARAMETER = 0xe2,
> > > + A2DP_NOT_SUPPORTED_CODEC_PARAMETER = 0xe3,
> > > + A2DP_INVALID_DRC = 0xe4,
> > > + A2DP_NOT_SUPPORTED_DRC = 0xe5,
> > > +};
> >
> > Hmm, I guess there should be part of a2dp.h since this error header is
> > about D-Bus not A2DP codes.
> >
>
> Ok, will move them to a2dp.h
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH BlueZ] audio: Add support for specific error codes for A2DP configuration
2025-09-11 14:43 ` [PATCH BlueZ] " Luiz Augusto von Dentz
2025-09-11 15:12 ` Per Waago (pwaago)
@ 2025-09-11 16:25 ` Pauli Virtanen
1 sibling, 0 replies; 8+ messages in thread
From: Pauli Virtanen @ 2025-09-11 16:25 UTC (permalink / raw)
To: Luiz Augusto von Dentz, Per Waagø; +Cc: linux-bluetooth
to, 2025-09-11 kello 10:43 -0400, Luiz Augusto von Dentz kirjoitti:
> Hi Per,
>
> On Thu, Sep 11, 2025 at 9:56 AM Per Waagø <pwaago@cisco.com> wrote:
> >
> > The A2DP specification defines error codes that shall be used if
> > the codec capabilities contain improper settings. This change allows
> > clients to trigger the sending of these specific error codes by
> > returning the corresponding error messages from
> > MediaEndpoint1.SetConfiguration.
> >
> > This update is fully backwards compatible: clients passing other error
> > messages will continue to receive the default error code as before. On
> > older BlueZ versions, these new errors will also result in the default
> > error code, enabling clients to implement support for the new errors
> > without breaking compatibility.
>
> While I can see the value for debugging I doubt we could do any
> handling of these errors, so the result would be the same regardless
> of what error is sent back it is not recoverable.
>
> @Pauli Virtanen Perhaps you can give some feedback regarding these
> codes, would pipewire be interested in generating specific error
> codes? Some of them seems to be SBC specific like bitpool.
Pipewire can start generating specific error codes if required by
specification / PTS, but I think there's probably not much real use for
these codes otherwise since it should be clear from capability bitmasks
what the cause for error is.
> > This change enables passing A2DP/SNK/AVP/* and A2DP/SRC/AVP/*
> > qualification tests.
> > ---
> > doc/org.bluez.MediaEndpoint.rst | 37 ++++++++++++++++
> > profiles/audio/a2dp.c | 78 ++++++++++++++++++++++++++++++---
> > profiles/audio/a2dp.h | 5 ++-
> > profiles/audio/avdtp.c | 4 +-
> > profiles/audio/media.c | 20 +++++----
> > src/error.h | 38 ++++++++++++++++
> > 6 files changed, 165 insertions(+), 17 deletions(-)
> >
> > diff --git a/doc/org.bluez.MediaEndpoint.rst b/doc/org.bluez.MediaEndpoint.rst
> > index 474cc1160..2721d473e 100644
> > --- a/doc/org.bluez.MediaEndpoint.rst
> > +++ b/doc/org.bluez.MediaEndpoint.rst
> > @@ -54,6 +54,43 @@ be configured and the properties must contain the following properties:
> >
> > See **org.bluez.MediaTransport(5)** QoS property.
> >
> > + Possible errors:
> > + :org.bluez.Error.A2DP.InvalidCodecType:
> > + :org.bluez.Error.A2DP.NotSupportedCodecType:
> > + :org.bluez.Error.A2DP.InvalidSamplingFrequency:
> > + :org.bluez.Error.A2DP.NotSupportedSamplingFrequency:
> > + :org.bluez.Error.A2DP.InvalidChannelMode:
> > + :org.bluez.Error.A2DP.NotSupportedChannelMode:
> > + :org.bluez.Error.A2DP.InvalidSubbands:
> > + :org.bluez.Error.A2DP.NotSupportedSubbands:
> > + :org.bluez.Error.A2DP.InvalidAllocationMethod:
> > + :org.bluez.Error.A2DP.NotSupportedAllocationMethod:
> > + :org.bluez.Error.A2DP.InvalidMinimumBitpoolValue:
> > + :org.bluez.Error.A2DP.NotSupportedMinimumBitpoolValue:
> > + :org.bluez.Error.A2DP.InvalidMaximumBitpoolValue:
> > + :org.bluez.Error.A2DP.NotSupportedMaximumBitpoolValue:
> > + :org.bluez.Error.A2DP.InvalidInvalidLayer:
> > + :org.bluez.Error.A2DP.NotSupportedLayer:
> > + :org.bluez.Error.A2DP.NotSupporterdCRC:
> > + :org.bluez.Error.A2DP.NotSupporterdMPF:
> > + :org.bluez.Error.A2DP.NotSupporterdVBR:
> > + :org.bluez.Error.A2DP.InvalidBitRate:
> > + :org.bluez.Error.A2DP.NotSupportedBitRate:
> > + :org.bluez.Error.A2DP.InvalidObjectType:
> > + :org.bluez.Error.A2DP.NotSupportedObjectType:
> > + :org.bluez.Error.A2DP.InvalidChannels:
> > + :org.bluez.Error.A2DP.NotSupportedChannels:
> > + :org.bluez.Error.A2DP.InvalidVersion:
> > + :org.bluez.Error.A2DP.NotSupportedVersion:
> > + :org.bluez.Error.A2DP.NotSupportedMaximumSUL:
> > + :org.bluez.Error.A2DP.InvalidBlockLength:
> > + :org.bluez.Error.A2DP.InvalidCPType:
> > + :org.bluez.Error.A2DP.InvalidCPFormat:
> > + :org.bluez.Error.A2DP.InvalidCodecParameter:
> > + :org.bluez.Error.A2DP.NotSupportedCodecParameter:
> > + :org.bluez.Error.A2DP.InvalidDRC:
> > + :org.bluez.Error.A2DP.NotSupportedDRC:
>
> Introducing a error domain for A2DP is probably a good idea, but this
> only applies to endpoints that are A2DP specific, so this need to be
> adjusted to e.g.: possible for A2DP or something like that, also I
> don't know how the client would be able to tell it can return these
> errors? Or the expectation is that the client can start sending them
> immediately since the old behavior will convert them to
> AVDTP_UNSUPPORTED_CONFIGURATION anyway?
>
> While at it split the commit to have the documentation changes as a
> separate change.
>
> > array{byte} SelectConfiguration(array{byte} capabilities)
> > `````````````````````````````````````````````````````````
> >
> > diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> > index c0a53eae9..661843a89 100644
> > --- a/profiles/audio/a2dp.c
> > +++ b/profiles/audio/a2dp.c
> > @@ -157,6 +157,73 @@ static GSList *servers = NULL;
> > static GSList *setups = NULL;
> > static unsigned int cb_id = 0;
> >
> > +struct a2dp_error {
> > + const char *error_name;
> > + uint8_t error_code;
> > +};
> > +
> > +#define A2DP_ERROR_PREFIX ERROR_INTERFACE ".A2DP."
> > +
> > +static struct a2dp_error config_errors[] = {
> > + {"InvalidCodecType", A2DP_INVALID_CODEC_TYPE},
> > + {"NotSupportedCodecType", A2DP_NOT_SUPPORTED_CODEC_TYPE},
> > + {"InvalidSamplingFrequency", A2DP_INVALID_SAMPLING_FREQUENCY},
> > + {"NotSupportedSamplingFrequency",
> > + A2DP_NOT_SUPPORTED_SAMPLING_FREQUENCY},
> > + {"InvalidChannelMode", A2DP_INVALID_CHANNEL_MODE},
> > + {"NotSupportedChannelMode", A2DP_NOT_SUPPORTED_CHANNEL_MODE},
> > + {"InvalidSubbands", A2DP_INVALID_SUBBANDS},
> > + {"NotSupportedSubbands", A2DP_NOT_SUPPORTED_SUBBANDS},
> > + {"InvalidAllocationMethod", A2DP_INVALID_ALLOCATION_METHOD},
> > + {"NotSupportedAllocationMethod", A2DP_NOT_SUPPORTED_ALLOCATION_METHOD},
> > + {"InvalidMinimumBitpoolValue",
> > + A2DP_INVALID_MINIMUM_BITPOOL_VALUE},
> > + {"NotSupportedMinimumBitpoolValue",
> > + A2DP_NOT_SUPPORTED_MINIMUM_BITPOOL_VALUE},
> > + {"InvalidMaximumBitpoolValue", A2DP_INVALID_MAXIMUM_BITPOOL_VALUE},
> > + {"NotSupportedMaximumBitpoolValue",
> > + A2DP_NOT_SUPPORTED_MAXIMUM_BITPOOL_VALUE},
> > + {"InvalidInvalidLayer", A2DP_INVALID_INVALID_LAYER},
> > + {"NotSupportedLayer", A2DP_NOT_SUPPORTED_LAYER},
> > + {"NotSupporterdCRC", A2DP_NOT_SUPPORTERD_CRC},
> > + {"NotSupporterdMPF", A2DP_NOT_SUPPORTERD_MPF},
> > + {"NotSupporterdVBR", A2DP_NOT_SUPPORTERD_VBR},
> > + {"InvalidBitRate", A2DP_INVALID_BIT_RATE},
> > + {"NotSupportedBitRate", A2DP_NOT_SUPPORTED_BIT_RATE},
> > + {"InvalidObjectType", A2DP_INVALID_OBJECT_TYPE},
> > + {"NotSupportedObjectType", A2DP_NOT_SUPPORTED_OBJECT_TYPE},
> > + {"InvalidChannels", A2DP_INVALID_CHANNELS},
> > + {"NotSupportedChannels", A2DP_NOT_SUPPORTED_CHANNELS},
> > + {"InvalidVersion", A2DP_INVALID_VERSION},
> > + {"NotSupportedVersion", A2DP_NOT_SUPPORTED_VERSION},
> > + {"NotSupportedMaximumSUL", A2DP_NOT_SUPPORTED_MAXIMUM_SUL},
> > + {"InvalidBlockLength", A2DP_INVALID_BLOCK_LENGTH},
> > + {"InvalidCPType", A2DP_INVALID_CP_TYPE},
> > + {"InvalidCPFormat", A2DP_INVALID_CP_FORMAT},
> > + {"InvalidCodecParameter", A2DP_INVALID_CODEC_PARAMETER},
> > + {"NotSupportedCodecParameter", A2DP_NOT_SUPPORTED_CODEC_PARAMETER},
> > + {"InvalidDRC", A2DP_INVALID_DRC},
> > + {"NotSupportedDRC", A2DP_NOT_SUPPORTED_DRC}
> > +};
> > +
> > +uint8_t a2dp_parse_config_error(const char *error_name)
> > +{
> > + size_t prefix_length;
> > + size_t i;
> > +
> > + prefix_length = strlen(A2DP_ERROR_PREFIX);
> > + if (strncmp(A2DP_ERROR_PREFIX, error_name, prefix_length))
> > + return AVDTP_UNSUPPORTED_CONFIGURATION;
> > +
> > + error_name += prefix_length;
> > + for (i = 0; i < ARRAY_SIZE(config_errors); i++) {
> > + if (strcmp(config_errors[i].error_name, error_name) == 0)
> > + return config_errors[i].error_code;
> > + }
> > +
> > + return AVDTP_UNSUPPORTED_CONFIGURATION;
> > +}
> > +
> > static struct a2dp_setup *setup_ref(struct a2dp_setup *setup)
> > {
> > setup->ref++;
> > @@ -688,11 +755,10 @@ done:
> > return FALSE;
> > }
> >
> > -static void endpoint_setconf_cb(struct a2dp_setup *setup, gboolean ret)
> > +static void endpoint_setconf_cb(struct a2dp_setup *setup, uint8_t error_code)
> > {
> > - if (ret == FALSE)
> > - setup_error_init(setup, AVDTP_MEDIA_CODEC,
> > - AVDTP_UNSUPPORTED_CONFIGURATION);
> > + if (error_code != 0)
> > + setup_error_init(setup, AVDTP_MEDIA_CODEC, error_code);
> >
> > auto_config(setup);
> > setup_unref(setup);
> > @@ -865,11 +931,11 @@ static gboolean endpoint_getcap_ind(struct avdtp *session,
> > return TRUE;
> > }
> >
> > -static void endpoint_open_cb(struct a2dp_setup *setup, gboolean ret)
> > +static void endpoint_open_cb(struct a2dp_setup *setup, uint8_t error_code)
> > {
> > int err = error_to_errno(setup->err);
> >
> > - if (ret == FALSE) {
> > + if (error_code != 0) {
> > setup->stream = NULL;
> > finalize_setup_errno(setup, -EPERM, finalize_config, NULL);
> > goto done;
> > diff --git a/profiles/audio/a2dp.h b/profiles/audio/a2dp.h
> > index c698bc983..afa02c12d 100644
> > --- a/profiles/audio/a2dp.h
> > +++ b/profiles/audio/a2dp.h
> > @@ -15,7 +15,8 @@ struct a2dp_setup;
> >
> > typedef void (*a2dp_endpoint_select_t) (struct a2dp_setup *setup, void *ret,
> > int size);
> > -typedef void (*a2dp_endpoint_config_t) (struct a2dp_setup *setup, gboolean ret);
> > +typedef void (*a2dp_endpoint_config_t) (struct a2dp_setup *setup,
> > + uint8_t error_code);
> >
> > struct a2dp_endpoint {
> > const char *(*get_name) (struct a2dp_sep *sep, void *user_data);
> > @@ -70,6 +71,8 @@ unsigned int a2dp_select_capabilities(struct avdtp *session,
> > unsigned int a2dp_config(struct avdtp *session, struct a2dp_sep *sep,
> > a2dp_config_cb_t cb, GSList *caps,
> > void *user_data);
> > +uint8_t a2dp_parse_config_error(const char *error_name);
> > +
> > unsigned int a2dp_resume(struct avdtp *session, struct a2dp_sep *sep,
> > a2dp_stream_cb_t cb, void *user_data);
> > unsigned int a2dp_suspend(struct avdtp *session, struct a2dp_sep *sep,
> > diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
> > index 30648251f..ed4e22b26 100644
> > --- a/profiles/audio/avdtp.c
> > +++ b/profiles/audio/avdtp.c
> > @@ -1494,8 +1494,8 @@ static void setconf_cb(struct avdtp *session, struct avdtp_stream *stream,
> > struct conf_rej rej;
> >
> > if (err != NULL) {
> > - rej.error = AVDTP_UNSUPPORTED_CONFIGURATION;
> > - rej.category = err->err.error_code;
> > + rej.error = err->err.error_code;
> > + rej.category = AVDTP_UNSUPPORTED_CONFIGURATION;
> > avdtp_send(session, session->in_cmd.transaction,
> > AVDTP_MSG_TYPE_REJECT, AVDTP_SET_CONFIGURATION,
> > &rej, sizeof(rej));
> > diff --git a/profiles/audio/media.c b/profiles/audio/media.c
> > index 9b3042c18..332f643bb 100644
> > --- a/profiles/audio/media.c
> > +++ b/profiles/audio/media.c
> > @@ -333,7 +333,7 @@ static void endpoint_reply(DBusPendingCall *call, void *user_data)
> > DBusMessage *reply;
> > DBusMessageIter args, props;
> > DBusError err;
> > - gboolean value;
> > + uint8_t error_code;
> > void *ret = NULL;
> > int size = -1;
> >
> > @@ -356,8 +356,12 @@ static void endpoint_reply(DBusPendingCall *call, void *user_data)
> >
> > if (dbus_message_is_method_call(request->msg,
> > MEDIA_ENDPOINT_INTERFACE,
> > - "SetConfiguration"))
> > + "SetConfiguration")) {
> > endpoint_remove_transport(endpoint, request->transport);
> > + error_code = a2dp_parse_config_error(err.name);
> > + ret = &error_code;
> > + size = 1;
> > + }
> >
> > dbus_error_free(&err);
> > goto done;
> > @@ -390,8 +394,8 @@ static void endpoint_reply(DBusPendingCall *call, void *user_data)
> > }
> >
> > size = 1;
> > - value = TRUE;
> > - ret = &value;
> > + error_code = 0;
> > + ret = &error_code;
> >
> > done:
> > dbus_message_unref(reply);
> > @@ -634,9 +638,9 @@ static void config_cb(struct media_endpoint *endpoint, void *ret, int size,
> > void *user_data)
> > {
> > struct a2dp_config_data *data = user_data;
> > - gboolean *ret_value = ret;
> > + uint8_t *ret_value = ret;
> >
> > - data->cb(data->setup, ret_value ? *ret_value : FALSE);
> > + data->cb(data->setup, ret_value ? *ret_value : 1);
> > }
> >
> > static int set_config(struct a2dp_sep *sep, uint8_t *configuration,
> > @@ -1098,7 +1102,7 @@ static void pac_config_cb(struct media_endpoint *endpoint, void *ret, int size,
> > void *user_data)
> > {
> > struct pac_config_data *data = user_data;
> > - gboolean *ret_value = ret;
> > + uint8_t *error_code = ret;
> > struct media_transport *transport;
> >
> > /* If transport was cleared, configuration was cancelled */
> > @@ -1106,7 +1110,7 @@ static void pac_config_cb(struct media_endpoint *endpoint, void *ret, int size,
> > if (!transport)
> > return;
> >
> > - data->cb(data->stream, ret_value ? 0 : -EINVAL);
> > + data->cb(data->stream, error_code == 0 ? 0 : -EINVAL);
> > }
> >
> > static struct media_transport *pac_ucast_config(struct bt_bap_stream *stream,
> > diff --git a/src/error.h b/src/error.h
> > index 47602d39b..8157795c2 100644
> > --- a/src/error.h
> > +++ b/src/error.h
> > @@ -88,3 +88,41 @@ DBusMessage *btd_error_profile_unavailable(DBusMessage *msg);
> > DBusMessage *btd_error_failed(DBusMessage *msg, const char *str);
> > DBusMessage *btd_error_bredr_errno(DBusMessage *msg, int err);
> > DBusMessage *btd_error_le_errno(DBusMessage *msg, int err);
> > +
> > +enum a2dp_error_codes : uint8_t {
> > + A2DP_INVALID_CODEC_TYPE = 0xc1,
> > + A2DP_NOT_SUPPORTED_CODEC_TYPE = 0xc2,
> > + A2DP_INVALID_SAMPLING_FREQUENCY = 0xc3,
> > + A2DP_NOT_SUPPORTED_SAMPLING_FREQUENCY = 0xc4,
> > + A2DP_INVALID_CHANNEL_MODE = 0xc5,
> > + A2DP_NOT_SUPPORTED_CHANNEL_MODE = 0xc6,
> > + A2DP_INVALID_SUBBANDS = 0xc7,
> > + A2DP_NOT_SUPPORTED_SUBBANDS = 0xc8,
> > + A2DP_INVALID_ALLOCATION_METHOD = 0xc9,
> > + A2DP_NOT_SUPPORTED_ALLOCATION_METHOD = 0xca,
> > + A2DP_INVALID_MINIMUM_BITPOOL_VALUE = 0xcb,
> > + A2DP_NOT_SUPPORTED_MINIMUM_BITPOOL_VALUE = 0xcc,
> > + A2DP_INVALID_MAXIMUM_BITPOOL_VALUE = 0xcd,
> > + A2DP_NOT_SUPPORTED_MAXIMUM_BITPOOL_VALUE = 0xce,
> > + A2DP_INVALID_INVALID_LAYER = 0xcf,
> > + A2DP_NOT_SUPPORTED_LAYER = 0xd0,
> > + A2DP_NOT_SUPPORTERD_CRC = 0xd1,
> > + A2DP_NOT_SUPPORTERD_MPF = 0xd2,
> > + A2DP_NOT_SUPPORTERD_VBR = 0xd3,
> > + A2DP_INVALID_BIT_RATE = 0xd4,
> > + A2DP_NOT_SUPPORTED_BIT_RATE = 0xd5,
> > + A2DP_INVALID_OBJECT_TYPE = 0xd6,
> > + A2DP_NOT_SUPPORTED_OBJECT_TYPE = 0xd7,
> > + A2DP_INVALID_CHANNELS = 0xd8,
> > + A2DP_NOT_SUPPORTED_CHANNELS = 0xd9,
> > + A2DP_INVALID_VERSION = 0xda,
> > + A2DP_NOT_SUPPORTED_VERSION = 0xdb,
> > + A2DP_NOT_SUPPORTED_MAXIMUM_SUL = 0xdc,
> > + A2DP_INVALID_BLOCK_LENGTH = 0xdd,
> > + A2DP_INVALID_CP_TYPE = 0xe0,
> > + A2DP_INVALID_CP_FORMAT = 0xe1,
> > + A2DP_INVALID_CODEC_PARAMETER = 0xe2,
> > + A2DP_NOT_SUPPORTED_CODEC_PARAMETER = 0xe3,
> > + A2DP_INVALID_DRC = 0xe4,
> > + A2DP_NOT_SUPPORTED_DRC = 0xe5,
> > +};
>
> Hmm, I guess there should be part of a2dp.h since this error header is
> about D-Bus not A2DP codes.
>
> > --
> > 2.43.0
> >
> >
>
--
Pauli Virtanen
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH BlueZ] audio: Add support for specific error codes for A2DP configuration
2025-09-11 15:52 ` Luiz Augusto von Dentz
@ 2025-09-11 19:56 ` Per Waago (pwaago)
2025-09-11 20:32 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 8+ messages in thread
From: Per Waago (pwaago) @ 2025-09-11 19:56 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: Pauli Virtanen, linux-bluetooth@vger.kernel.org
> ________________________________________
> From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
> Sent: Thursday, September 11, 2025 17:52
> To: Per Waago (pwaago)
> Cc: Pauli Virtanen; linux-bluetooth@vger.kernel.org
> Subject: Re: [PATCH BlueZ] audio: Add support for specific error codes for A2DP configuration
>
> Hi Per,
>
> On Thu, Sep 11, 2025 at 11:12 AM Per Waago (pwaago) <pwaago@cisco.com> wrote:
> >
> > Hi Luiz, thanks for reviewing.
> >
> > > From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
> > > Sent: Thursday, September 11, 2025 16:43
> > > To: Per Waago (pwaago); Pauli Virtanen
> > > Cc: linux-bluetooth@vger.kernel.org
> > > Subject: Re: [PATCH BlueZ] audio: Add support for specific error codes for A2DP > configuration
> > >
> > > Hi Per,
> > >
> > > On Thu, Sep 11, 2025 at 9:56 AM Per Waagø <pwaago@cisco.com> wrote:
> > > >
> > > > The A2DP specification defines error codes that shall be used if
> > > > the codec capabilities contain improper settings. This change allows
> > > > clients to trigger the sending of these specific error codes by
> > > > returning the corresponding error messages from
> > > > MediaEndpoint1.SetConfiguration.
> > > >
> > > > This update is fully backwards compatible: clients passing other error
> > > > messages will continue to receive the default error code as before. On
> > > > older BlueZ versions, these new errors will also result in the default
> > > > error code, enabling clients to implement support for the new errors
> > > > without breaking compatibility.
> > >
> > > While I can see the value for debugging I doubt we could do any
> > > handling of these errors, so the result would be the same regardless
> > > of what error is sent back it is not recoverable.
> > >
> >
> > The main motivation for adding them is to be able to pass the
> > mandatory qualification tests, which now checks the errors codes
> > returned from SetConfiguration in detail. I don't think they are very
> > useful otherwise.
> >
> > The errors are specified in table 5.5 in the A2DP spec:
> > > https://www.bluetooth.com/specifications/specs/html/?src=a2dp_v1-4-1_1752513648/A2DP_v1.4.1/o> ut/en/index-en.html#UUID-0ba19ee9-7277-1068-d2dc-b9e638cca568_Table_5.5
> >
> > I included all of them for completeness. In that table, it is also stated
> > which codecs they apply to. Some are SBC-specific, some apply to all codecs or
> > other codecs.
>
> Ok this is very annoying if PTS suddenly adds a new test case that
> checks error codes that otherwise are only useful for debugging. I'd
> say that it probably needs a configuration entry to skip these tests,
> btw this seems to be introduced in 1.4.1:
>
> https://www.bluetooth.com/specifications/specs/html/?src=a2dp_v1-4-1_1752513648/A2DP_v1.4.1/o> ut/en/index-en.html#UUID-05a1c924-2070-eb38-c2cc-a9ffa178bdb9
>
> BlueZ SDP record is still 1.4 (a2dp_ver = 0x0104), it seems 1.4.1 is
> an errata only change but that introduces new error codes which is
> really intrusive to say the least.really intrusive to say the least.
I have tried to read the specs in some more detail now. It seems the error
codes themselves have been there all the time. The errata that was incorporated
in 1.4.1 actually eases the requirements a bit, so the spec now says
that these error codes shall be returned if error codes from GAVDP or AVDTP
aren't used. So the way I interpret it, returning AVDTP_UNSUPPORTED_CONFIGURAION
could be ok according to 1.4.1.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH BlueZ] audio: Add support for specific error codes for A2DP configuration
2025-09-11 19:56 ` Per Waago (pwaago)
@ 2025-09-11 20:32 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2025-09-11 20:32 UTC (permalink / raw)
To: Per Waago (pwaago); +Cc: Pauli Virtanen, linux-bluetooth@vger.kernel.org
Hi Per,
On Thu, Sep 11, 2025 at 3:56 PM Per Waago (pwaago) <pwaago@cisco.com> wrote:
>
>
>
>
> > ________________________________________
> > From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
> > Sent: Thursday, September 11, 2025 17:52
> > To: Per Waago (pwaago)
> > Cc: Pauli Virtanen; linux-bluetooth@vger.kernel.org
> > Subject: Re: [PATCH BlueZ] audio: Add support for specific error codes for A2DP configuration
> >
> > Hi Per,
> >
> > On Thu, Sep 11, 2025 at 11:12 AM Per Waago (pwaago) <pwaago@cisco.com> wrote:
> > >
> > > Hi Luiz, thanks for reviewing.
> > >
> > > > From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
> > > > Sent: Thursday, September 11, 2025 16:43
> > > > To: Per Waago (pwaago); Pauli Virtanen
> > > > Cc: linux-bluetooth@vger.kernel.org
> > > > Subject: Re: [PATCH BlueZ] audio: Add support for specific error codes for A2DP > configuration
> > > >
> > > > Hi Per,
> > > >
> > > > On Thu, Sep 11, 2025 at 9:56 AM Per Waagø <pwaago@cisco.com> wrote:
> > > > >
> > > > > The A2DP specification defines error codes that shall be used if
> > > > > the codec capabilities contain improper settings. This change allows
> > > > > clients to trigger the sending of these specific error codes by
> > > > > returning the corresponding error messages from
> > > > > MediaEndpoint1.SetConfiguration.
> > > > >
> > > > > This update is fully backwards compatible: clients passing other error
> > > > > messages will continue to receive the default error code as before. On
> > > > > older BlueZ versions, these new errors will also result in the default
> > > > > error code, enabling clients to implement support for the new errors
> > > > > without breaking compatibility.
> > > >
> > > > While I can see the value for debugging I doubt we could do any
> > > > handling of these errors, so the result would be the same regardless
> > > > of what error is sent back it is not recoverable.
> > > >
> > >
> > > The main motivation for adding them is to be able to pass the
> > > mandatory qualification tests, which now checks the errors codes
> > > returned from SetConfiguration in detail. I don't think they are very
> > > useful otherwise.
> > >
> > > The errors are specified in table 5.5 in the A2DP spec:
> > > > https://www.bluetooth.com/specifications/specs/html/?src=a2dp_v1-4-1_1752513648/A2DP_v1.4.1/o> ut/en/index-en.html#UUID-0ba19ee9-7277-1068-d2dc-b9e638cca568_Table_5.5
> > >
> > > I included all of them for completeness. In that table, it is also stated
> > > which codecs they apply to. Some are SBC-specific, some apply to all codecs or
> > > other codecs.
> >
> > Ok this is very annoying if PTS suddenly adds a new test case that
> > checks error codes that otherwise are only useful for debugging. I'd
> > say that it probably needs a configuration entry to skip these tests,
> > btw this seems to be introduced in 1.4.1:
> >
> > https://www.bluetooth.com/specifications/specs/html/?src=a2dp_v1-4-1_1752513648/A2DP_v1.4.1/o> ut/en/index-en.html#UUID-05a1c924-2070-eb38-c2cc-a9ffa178bdb9
> >
> > BlueZ SDP record is still 1.4 (a2dp_ver = 0x0104), it seems 1.4.1 is
> > an errata only change but that introduces new error codes which is
> > really intrusive to say the least.really intrusive to say the least.
>
> I have tried to read the specs in some more detail now. It seems the error
> codes themselves have been there all the time. The errata that was incorporated
> in 1.4.1 actually eases the requirements a bit, so the spec now says
> that these error codes shall be returned if error codes from GAVDP or AVDTP
> aren't used. So the way I interpret it, returning AVDTP_UNSUPPORTED_CONFIGURAION
> could be ok according to 1.4.1.
Indeed it says 'are not rejected with error codes specified by GAVDTP or AVDTP:
'If the Codec Specific Information Elements include improper settings
and are not rejected with the error codes specified by GAVDP [3] or
AVDTP [4], then an applicable error code from the list in Table 5.5
shall be returned.'
So why it was failing if we returned AVDTP_UNSUPPORTED_CONFIGURATION,
that should still be valid according to the above AVDTP error codes
are still valid.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-09-11 20:32 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-11 13:53 [PATCH BlueZ] audio: Add support for specific error codes for A2DP configuration Per Waagø
2025-09-11 14:08 ` [BlueZ] " bluez.test.bot
2025-09-11 14:43 ` [PATCH BlueZ] " Luiz Augusto von Dentz
2025-09-11 15:12 ` Per Waago (pwaago)
2025-09-11 15:52 ` Luiz Augusto von Dentz
2025-09-11 19:56 ` Per Waago (pwaago)
2025-09-11 20:32 ` Luiz Augusto von Dentz
2025-09-11 16:25 ` Pauli Virtanen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox