diff mbox series

[v3,4/5] profiles/audio/transport.c: Add support multiple BIS

Message ID 20231102155827.4219-5-silviu.barbulescu@nxp.com (mailing list archive)
State Superseded
Headers show
Series Add support for bcast multiple BISes | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/CheckPatch success CheckPatch PASS
tedd_an/GitLint success Gitlint PASS
tedd_an/IncrementalBuild success Incremental Build PASS

Commit Message

Silviu Florian Barbulescu Nov. 2, 2023, 3:58 p.m. UTC 
Add support for bcast multiple BISes

---
 profiles/audio/transport.c | 59 ++++++++++++++++++++++++++------------
 1 file changed, 41 insertions(+), 18 deletions(-)

Comments

Luiz Augusto von Dentz Nov. 14, 2023, 11:53 p.m. UTC | #1 
Hi Silviu,

On Thu, Nov 2, 2023 at 11:58 AM Silviu Florian Barbulescu
<silviu.barbulescu@nxp.com> wrote:
>
> Add support for bcast multiple BISes
>
> ---
>  profiles/audio/transport.c | 59 ++++++++++++++++++++++++++------------
>  1 file changed, 41 insertions(+), 18 deletions(-)
>
> diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
> index 23ea267f6..eaafd8a35 100644
> --- a/profiles/audio/transport.c
> +++ b/profiles/audio/transport.c
> @@ -163,7 +163,9 @@ find_transport_by_bap_stream(const struct bt_bap_stream *stream)
>                 struct bap_transport *bap;
>
>                 if (strcasecmp(uuid, PAC_SINK_UUID) &&
> -                               strcasecmp(uuid, PAC_SOURCE_UUID))
> +                               strcasecmp(uuid, PAC_SOURCE_UUID) &&
> +                               strcasecmp(uuid, BCAA_SERVICE_UUID) &&
> +                               strcasecmp(uuid, BAA_SERVICE_UUID))
>                         continue;
>
>                 bap = transport->data;
> @@ -312,9 +314,11 @@ static void media_transport_remove_owner(struct media_transport *transport)
>                 media_request_reply(owner->pending, EIO);
>
>         transport->owner = NULL;
> -       if (bap->linked)
> -               queue_foreach(bt_bap_stream_io_get_links(bap->stream),
> -                               linked_transport_remove_owner, owner);
> +       if (bt_bap_stream_get_type(bap->stream) ==
> +                                       BT_BAP_STREAM_TYPE_UCAST)
> +               if (bap->linked)
> +                       queue_foreach(bt_bap_stream_io_get_links(bap->stream),
> +                                       linked_transport_remove_owner, owner);

I wonder it it wouldn't be better to have something like
bt_bap_stream_foreach_link which would take care of detecting if there
are any links, in fact this whole thing about checking the stream type
logic shall probably be keep internal to shared/bap.c, we could
perhaps create a bt_bap_stream_ops which would register callbacks
based on the stream type that way we only need to check the stream
type at creation.

>         if (owner->watch)
>                 g_dbus_remove_watch(btd_get_dbus_connection(), owner->watch);
> @@ -496,9 +500,11 @@ static void media_transport_set_owner(struct media_transport *transport,
>         DBG("Transport %s Owner %s", transport->path, owner->name);
>         transport->owner = owner;
>
> -       if (bap->linked)
> -               queue_foreach(bt_bap_stream_io_get_links(bap->stream),
> -                               linked_transport_set_owner, owner);
> +       if (bt_bap_stream_get_type(bap->stream) ==
> +                                       BT_BAP_STREAM_TYPE_UCAST)
> +               if (bap->linked)
> +                       queue_foreach(bt_bap_stream_io_get_links(bap->stream),
> +                                       linked_transport_set_owner, owner);

Ditto.

>         owner->transport = transport;
>         owner->watch = g_dbus_add_disconnect_watch(btd_get_dbus_connection(),
> @@ -641,6 +647,7 @@ static DBusMessage *release(DBusConnection *conn, DBusMessage *msg,
>         const char *sender;
>         struct media_request *req;
>         guint id;
> +       const char *uuid;
>
>         sender = dbus_message_get_sender(msg);
>
> @@ -669,9 +676,12 @@ static DBusMessage *release(DBusConnection *conn, DBusMessage *msg,
>         req = media_request_create(msg, id);
>         media_owner_add(owner, req);
>
> -       if (bt_bap_stream_get_type(bap->stream) ==
> -                       BT_BAP_STREAM_TYPE_BCAST) {
> -               bap_disable_complete(bap->stream, 0x00, 0x00, owner);
> +       uuid = media_endpoint_get_uuid(transport->endpoint);
> +       if (!strcasecmp(uuid, BCAA_SERVICE_UUID) ||
> +                               !strcasecmp(uuid, BAA_SERVICE_UUID)) {
> +               if (bt_bap_stream_get_type(bap->stream) ==
> +                                               BT_BAP_STREAM_TYPE_BCAST)
> +                       bap_disable_complete(bap->stream, 0x00, 0x00, owner);
>         }
>
>         return NULL;
> @@ -686,7 +696,11 @@ static gboolean get_device(const GDBusPropertyTable *property,
>         if (transport->device)
>                 path = device_get_path(transport->device);
>         else
> -               path = adapter_get_path(transport->adapter);
> +               /*
> +                *Use remote endpoint path as fake device path
> +                *for broadcast source transport
> +                */
> +               path = transport->remote_endpoint;
>
>         dbus_message_iter_append_basic(iter, DBUS_TYPE_OBJECT_PATH, &path);
>
> @@ -1272,7 +1286,9 @@ static bool match_link_transport(const void *data, const void *user_data)
>         if (!transport)
>                 return false;
>
> -       bap_update_links(transport);
> +       if (bt_bap_stream_get_type((struct bt_bap_stream *)stream) ==
> +                                               BT_BAP_STREAM_TYPE_UCAST)
> +               bap_update_links(transport);
>
>         return true;
>  }
> @@ -1378,7 +1394,9 @@ static guint resume_bap(struct media_transport *transport,
>         if (bap->resume_id)
>                 return 0;
>
> -       bap_update_links(transport);
> +       if (bt_bap_stream_get_type(bap->stream) ==
> +                                       BT_BAP_STREAM_TYPE_UCAST)
> +               bap_update_links(transport);

It is also a bad practice to put test like the above upfront instead
of just embbed it inside bap_update_links otherwise we just keep
duplicating the same check over and over.

>
>         switch (bt_bap_stream_get_state(bap->stream)) {
>         case BT_BAP_STREAM_STATE_ENABLING:
> @@ -1416,7 +1434,9 @@ static guint suspend_bap(struct media_transport *transport,
>         else
>                 transport_set_state(transport, TRANSPORT_STATE_IDLE);
>
> -       bap_update_links(transport);
> +       if (bt_bap_stream_get_type(bap->stream) ==
> +                                       BT_BAP_STREAM_TYPE_UCAST)
> +               bap_update_links(transport);

Ditto.

>         return bt_bap_stream_disable(bap->stream, bap->linked, func, owner);
>  }
> @@ -1491,9 +1511,10 @@ static void bap_state_changed(struct bt_bap_stream *stream, uint8_t old_state,
>                 /* If a request is pending wait it to complete */
>                 if (owner && owner->pending)
>                         return;
> -               bap_update_links(transport);
> -               if (!media_endpoint_is_broadcast(transport->endpoint))
> +               if (!media_endpoint_is_broadcast(transport->endpoint)) {
> +                       bap_update_links(transport);
>                         bap_update_qos(transport);
> +               }
>                 else if (bt_bap_stream_io_dir(stream) != BT_BAP_BCAST_SOURCE)
>                         bap_update_bcast_qos(transport);
>                 transport_update_playing(transport, FALSE);
> @@ -1510,7 +1531,7 @@ static void bap_state_changed(struct bt_bap_stream *stream, uint8_t old_state,
>                         bap_update_bcast_qos(transport);
>                 break;
>         case BT_BAP_STREAM_STATE_RELEASING:
> -               if (bt_bap_stream_io_dir(stream) == BT_BAP_BCAST_SINK)
> +               if (!bt_bap_stream_io_dir(stream))
>                         return;
>                 break;
>         }
> @@ -1555,7 +1576,9 @@ static void bap_connecting(struct bt_bap_stream *stream, bool state, int fd,
>         if (bap->stream != stream)
>                 return;
>
> -       bap_update_links(transport);
> +       if (bt_bap_stream_get_type(bap->stream) ==
> +                                       BT_BAP_STREAM_TYPE_UCAST)
> +               bap_update_links(transport);

Ditto.

>  }
>
>  static void free_bap(void *data)
> --
> 2.39.2
>
diff mbox series

Patch

diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index 23ea267f6..eaafd8a35 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -163,7 +163,9 @@  find_transport_by_bap_stream(const struct bt_bap_stream *stream)
 		struct bap_transport *bap;
 
 		if (strcasecmp(uuid, PAC_SINK_UUID) &&
-				strcasecmp(uuid, PAC_SOURCE_UUID))
+				strcasecmp(uuid, PAC_SOURCE_UUID) &&
+				strcasecmp(uuid, BCAA_SERVICE_UUID) &&
+				strcasecmp(uuid, BAA_SERVICE_UUID))
 			continue;
 
 		bap = transport->data;
@@ -312,9 +314,11 @@  static void media_transport_remove_owner(struct media_transport *transport)
 		media_request_reply(owner->pending, EIO);
 
 	transport->owner = NULL;
-	if (bap->linked)
-		queue_foreach(bt_bap_stream_io_get_links(bap->stream),
-				linked_transport_remove_owner, owner);
+	if (bt_bap_stream_get_type(bap->stream) ==
+					BT_BAP_STREAM_TYPE_UCAST)
+		if (bap->linked)
+			queue_foreach(bt_bap_stream_io_get_links(bap->stream),
+					linked_transport_remove_owner, owner);
 
 	if (owner->watch)
 		g_dbus_remove_watch(btd_get_dbus_connection(), owner->watch);
@@ -496,9 +500,11 @@  static void media_transport_set_owner(struct media_transport *transport,
 	DBG("Transport %s Owner %s", transport->path, owner->name);
 	transport->owner = owner;
 
-	if (bap->linked)
-		queue_foreach(bt_bap_stream_io_get_links(bap->stream),
-				linked_transport_set_owner, owner);
+	if (bt_bap_stream_get_type(bap->stream) ==
+					BT_BAP_STREAM_TYPE_UCAST)
+		if (bap->linked)
+			queue_foreach(bt_bap_stream_io_get_links(bap->stream),
+					linked_transport_set_owner, owner);
 
 	owner->transport = transport;
 	owner->watch = g_dbus_add_disconnect_watch(btd_get_dbus_connection(),
@@ -641,6 +647,7 @@  static DBusMessage *release(DBusConnection *conn, DBusMessage *msg,
 	const char *sender;
 	struct media_request *req;
 	guint id;
+	const char *uuid;
 
 	sender = dbus_message_get_sender(msg);
 
@@ -669,9 +676,12 @@  static DBusMessage *release(DBusConnection *conn, DBusMessage *msg,
 	req = media_request_create(msg, id);
 	media_owner_add(owner, req);
 
-	if (bt_bap_stream_get_type(bap->stream) ==
-			BT_BAP_STREAM_TYPE_BCAST) {
-		bap_disable_complete(bap->stream, 0x00, 0x00, owner);
+	uuid = media_endpoint_get_uuid(transport->endpoint);
+	if (!strcasecmp(uuid, BCAA_SERVICE_UUID) ||
+				!strcasecmp(uuid, BAA_SERVICE_UUID)) {
+		if (bt_bap_stream_get_type(bap->stream) ==
+						BT_BAP_STREAM_TYPE_BCAST)
+			bap_disable_complete(bap->stream, 0x00, 0x00, owner);
 	}
 
 	return NULL;
@@ -686,7 +696,11 @@  static gboolean get_device(const GDBusPropertyTable *property,
 	if (transport->device)
 		path = device_get_path(transport->device);
 	else
-		path = adapter_get_path(transport->adapter);
+		/*
+		 *Use remote endpoint path as fake device path
+		 *for broadcast source transport
+		 */
+		path = transport->remote_endpoint;
 
 	dbus_message_iter_append_basic(iter, DBUS_TYPE_OBJECT_PATH, &path);
 
@@ -1272,7 +1286,9 @@  static bool match_link_transport(const void *data, const void *user_data)
 	if (!transport)
 		return false;
 
-	bap_update_links(transport);
+	if (bt_bap_stream_get_type((struct bt_bap_stream *)stream) ==
+						BT_BAP_STREAM_TYPE_UCAST)
+		bap_update_links(transport);
 
 	return true;
 }
@@ -1378,7 +1394,9 @@  static guint resume_bap(struct media_transport *transport,
 	if (bap->resume_id)
 		return 0;
 
-	bap_update_links(transport);
+	if (bt_bap_stream_get_type(bap->stream) ==
+					BT_BAP_STREAM_TYPE_UCAST)
+		bap_update_links(transport);
 
 	switch (bt_bap_stream_get_state(bap->stream)) {
 	case BT_BAP_STREAM_STATE_ENABLING:
@@ -1416,7 +1434,9 @@  static guint suspend_bap(struct media_transport *transport,
 	else
 		transport_set_state(transport, TRANSPORT_STATE_IDLE);
 
-	bap_update_links(transport);
+	if (bt_bap_stream_get_type(bap->stream) ==
+					BT_BAP_STREAM_TYPE_UCAST)
+		bap_update_links(transport);
 
 	return bt_bap_stream_disable(bap->stream, bap->linked, func, owner);
 }
@@ -1491,9 +1511,10 @@  static void bap_state_changed(struct bt_bap_stream *stream, uint8_t old_state,
 		/* If a request is pending wait it to complete */
 		if (owner && owner->pending)
 			return;
-		bap_update_links(transport);
-		if (!media_endpoint_is_broadcast(transport->endpoint))
+		if (!media_endpoint_is_broadcast(transport->endpoint)) {
+			bap_update_links(transport);
 			bap_update_qos(transport);
+		}
 		else if (bt_bap_stream_io_dir(stream) != BT_BAP_BCAST_SOURCE)
 			bap_update_bcast_qos(transport);
 		transport_update_playing(transport, FALSE);
@@ -1510,7 +1531,7 @@  static void bap_state_changed(struct bt_bap_stream *stream, uint8_t old_state,
 			bap_update_bcast_qos(transport);
 		break;
 	case BT_BAP_STREAM_STATE_RELEASING:
-		if (bt_bap_stream_io_dir(stream) == BT_BAP_BCAST_SINK)
+		if (!bt_bap_stream_io_dir(stream))
 			return;
 		break;
 	}
@@ -1555,7 +1576,9 @@  static void bap_connecting(struct bt_bap_stream *stream, bool state, int fd,
 	if (bap->stream != stream)
 		return;
 
-	bap_update_links(transport);
+	if (bt_bap_stream_get_type(bap->stream) ==
+					BT_BAP_STREAM_TYPE_UCAST)
+		bap_update_links(transport);
 }
 
 static void free_bap(void *data)