[PATCH BlueZ 0/4] BAP broadcast fixes

[PATCH BlueZ 0/4] BAP broadcast fixes

[Michal Dzik]

Those are some fixes I did when working with BAP broadcast receiver.

Michal Dzik (4):
  transport: allow setting Metadata property on broadcast
  shared/bap: run callback after setting the broadcast metadata
  audio/bap: fill BIG and BIS for receiver
  audio: update QoS dbus property after it is set

 profiles/audio/bap.c       | 5 ++++-
 profiles/audio/transport.c | 9 +++++++--
 src/shared/bap.c           | 3 +++
 3 files changed, 14 insertions(+), 3 deletions(-)

-- 
2.43.0

[PATCH BlueZ 1/4] transport: allow setting Metadata property on broadcast

[Michal Dzik]

Also fixes a problem where 64-bit iov_len was incorrectly initialized with
32-bit value in dbus_message_iter_get_fixed_array().
---
 profiles/audio/transport.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index d9feef768..81a63c365 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -1336,6 +1336,7 @@ static void set_metadata(const GDBusPropertyTable *property,
 	struct bap_transport *bap = transport->data;
 	DBusMessageIter array;
 	struct iovec iov;
+	int len;
 	int ret;
 
 	if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_ARRAY) {
@@ -1347,7 +1348,8 @@ static void set_metadata(const GDBusPropertyTable *property,
 
 	dbus_message_iter_recurse(iter, &array);
 	dbus_message_iter_get_fixed_array(&array, &iov.iov_base,
-					(int *)&iov.iov_len);
+					&len);
+	iov.iov_len = len;
 
 	ret = bt_bap_stream_metadata(bap->stream, &iov, bap_metadata_complete,
 				     UINT_TO_PTR(id));
@@ -1614,7 +1616,7 @@ static const GDBusPropertyTable transport_bap_bc_properties[] = {
 	{ "QoS", "a{sv}", get_bcast_qos, set_bcast_qos, qos_bcast_exists },
 	{ "Endpoint", "o", get_endpoint, NULL, endpoint_exists },
 	{ "Location", "u", get_location },
-	{ "Metadata", "ay", get_metadata },
+	{ "Metadata", "ay", get_metadata, set_metadata },
 	{ "Links", "ao", get_links, set_links, NULL },
 	{ }
 };
-- 
2.43.0

RE: BAP broadcast fixes

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2691 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=1082418

---Test result---

Test Summary:
CheckPatch                    FAIL      1.53 seconds
GitLint                       PASS      1.08 seconds
BuildEll                      PASS      20.08 seconds
BluezMake                     PASS      644.28 seconds
MakeCheck                     PASS      18.64 seconds
MakeDistcheck                 PASS      243.92 seconds
CheckValgrind                 PASS      289.61 seconds
CheckSmatch                   WARNING   345.58 seconds
bluezmakeextell               PASS      180.77 seconds
IncrementalBuild              PASS      664.51 seconds
ScanBuild                     PASS      1005.91 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[BlueZ,3/4] audio/bap: fill BIG and BIS for receiver
WARNING:TYPO_SPELLING: 'explicitely' may be misspelled - perhaps 'explicitly'?
#104: 
explicitely in BASE, but we can use subgroup index with success.
^^^^^^^^^^^

WARNING:LONG_LINE: line length of 81 exceeds 80 columns
#138: FILE: profiles/audio/bap.c:1556:
+	create_stream_for_bis(data, lpac, sid, bis, sgrp, qos, caps, meta, path);

/github/workspace/src/patch/14528085.patch total: 0 errors, 2 warnings, 23 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/patch/14528085.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
src/shared/bap.c:312:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:312:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:312:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structures


https://github.com/bluez/bluez/pull/2051

---
Regards,
Linux Bluetooth

[PATCH BlueZ 2/4] shared/bap: run callback after setting the broadcast metadata

[Michal Dzik]

Without this callback, dbus property setting will stall.
---
 src/shared/bap.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/shared/bap.c b/src/shared/bap.c
index 8da626fe3..78ba22259 100644
--- a/src/shared/bap.c
+++ b/src/shared/bap.c
@@ -2526,6 +2526,9 @@ static unsigned int bap_bcast_metadata(struct bt_bap_stream *stream,
 	util_iov_free(stream->meta, 1);
 	stream->meta = util_iov_dup(data, 1);
 
+	if (func)
+		func(stream, BT_ASCS_RSP_SUCCESS, 0x00, user_data);
+
 	return 1;
 }
 
-- 
2.43.0

[PATCH BlueZ 3/4] audio/bap: fill BIG and BIS for receiver

[Michal Dzik]

This is for some reason a missing feature.
We receive QoS structure from the kernel via BT_ISO_QOS, but BIG and BIS
are not set there. In fact it is even impossible to pass them that way,
as it is only one structure per remote device. That's why we have to parse
BASE data to extract BIS and set it to stream->qos. BIG is not present
explicitely in BASE, but we can use subgroup index with success.

As a result of this commit, MediaTransport1 object has now a valid BIG
and BIS set in QoS property.
---
 profiles/audio/bap.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c
index 375026440..9c4428b1c 100644
--- a/profiles/audio/bap.c
+++ b/profiles/audio/bap.c
@@ -1502,6 +1502,7 @@ static void iso_bcast_confirm_cb(GIOChannel *io, GError *err, void *user_data)
 
 static void create_stream_for_bis(struct bap_data *bap_data,
 				struct bt_bap_pac *lpac, uint8_t sid,
+				uint8_t bis, uint8_t sgrp,
 				struct bt_bap_qos *qos, struct iovec *caps,
 				struct iovec *meta, char *path)
 {
@@ -1512,6 +1513,8 @@ static void create_stream_for_bis(struct bap_data *bap_data,
 
 	/* Create an internal copy for bcode */
 	setup->qos.bcast.bcode = util_iov_dup(qos->bcast.bcode, 1);
+	setup->qos.bcast.big = sgrp;
+	setup->qos.bcast.bis = bis;
 
 	setup->data = bap_data;
 
@@ -1550,7 +1553,7 @@ static void bis_handler(uint8_t sid, uint8_t bis, uint8_t sgrp,
 			sid, bis) < 0)
 		return;
 
-	create_stream_for_bis(data, lpac, sid, qos, caps, meta, path);
+	create_stream_for_bis(data, lpac, sid, bis, sgrp, qos, caps, meta, path);
 }
 
 static gboolean big_info_report_cb(GIOChannel *io, GIOCondition cond,
-- 
2.43.0

[PATCH BlueZ 4/4] audio: update QoS dbus property after it is set

[Michal Dzik]

Dbus set and get on QoS property use different data:
- get uses bap->qos
- set uses bap->stream->qos
This works, because bap->qos is a copy of bap->stream->qos. The only
issue is when we set this property and then try to get it. We see old, not
updated copy. Simply calling bap_update_bcast_qos() fixes the problem.
---
 profiles/audio/transport.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index 81a63c365..5c2a2777e 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -1556,6 +1556,8 @@ static void bcast_qos_set(void *user_data, int err)
 					"Failed to set Broadcast Code");
 }
 
+static void bap_update_bcast_qos(const struct media_transport *transport);
+
 static void set_bcast_qos(const GDBusPropertyTable *property,
 			DBusMessageIter *dict, GDBusPendingPropertySet id,
 			void *data)
@@ -1604,6 +1606,7 @@ static void set_bcast_qos(const GDBusPropertyTable *property,
 	}
 
 	bt_bap_stream_qos(bap->stream, bap_qos, NULL, NULL);
+	bap_update_bcast_qos(transport);
 	g_dbus_pending_property_success(id);
 }
 
-- 
2.43.0

[PATCH BlueZ v2 1/4] transport: allow setting Metadata property on broadcast

[Michal Dzik]

Also fixes a problem where 64-bit iov_len was incorrectly initialized with
32-bit value in dbus_message_iter_get_fixed_array().
---
 profiles/audio/transport.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index d9feef768..81a63c365 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -1336,6 +1336,7 @@ static void set_metadata(const GDBusPropertyTable *property,
 	struct bap_transport *bap = transport->data;
 	DBusMessageIter array;
 	struct iovec iov;
+	int len;
 	int ret;
 
 	if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_ARRAY) {
@@ -1347,7 +1348,8 @@ static void set_metadata(const GDBusPropertyTable *property,
 
 	dbus_message_iter_recurse(iter, &array);
 	dbus_message_iter_get_fixed_array(&array, &iov.iov_base,
-					(int *)&iov.iov_len);
+					&len);
+	iov.iov_len = len;
 
 	ret = bt_bap_stream_metadata(bap->stream, &iov, bap_metadata_complete,
 				     UINT_TO_PTR(id));
@@ -1614,7 +1616,7 @@ static const GDBusPropertyTable transport_bap_bc_properties[] = {
 	{ "QoS", "a{sv}", get_bcast_qos, set_bcast_qos, qos_bcast_exists },
 	{ "Endpoint", "o", get_endpoint, NULL, endpoint_exists },
 	{ "Location", "u", get_location },
-	{ "Metadata", "ay", get_metadata },
+	{ "Metadata", "ay", get_metadata, set_metadata },
 	{ "Links", "ao", get_links, set_links, NULL },
 	{ }
 };
-- 
2.43.0

RE: BAP broadcast fixes

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1600 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=1082491

---Test result---

Test Summary:
CheckPatch                    PASS      1.23 seconds
GitLint                       PASS      0.86 seconds
BuildEll                      PASS      17.39 seconds
BluezMake                     PASS      598.33 seconds
MakeCheck                     PASS      18.56 seconds
MakeDistcheck                 PASS      216.71 seconds
CheckValgrind                 PASS      272.46 seconds
CheckSmatch                   WARNING   305.10 seconds
bluezmakeextell               PASS      163.38 seconds
IncrementalBuild              PASS      621.83 seconds
ScanBuild                     PASS      901.92 seconds

Details
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
src/shared/bap.c:312:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:312:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:312:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structures


https://github.com/bluez/bluez/pull/2052

---
Regards,
Linux Bluetooth