diff mbox series

[v2,3/3] bap:Update transport acquire/release flow for bcast src

Message ID 20231002153352.3163-4-iulia.tanasescu@nxp.com (mailing list archive)
State New, archived
Headers show
Series Update transport acquire/release flow BAP bcast source | 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

Iulia Tanasescu Oct. 2, 2023, 3:33 p.m. UTC 
From: Silviu Florian Barbulescu <silviu.barbulescu@nxp.com>

Update transport acquire/release flow for BAP bcast source

---
 profiles/audio/bap.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

Comments

Luiz Augusto von Dentz Oct. 2, 2023, 9:30 p.m. UTC | #1 
Hi Iulia,

On Mon, Oct 2, 2023 at 8:34 AM Iulia Tanasescu <iulia.tanasescu@nxp.com> wrote:
>
> From: Silviu Florian Barbulescu <silviu.barbulescu@nxp.com>
>
> Update transport acquire/release flow for BAP bcast source
>
> ---
>  profiles/audio/bap.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c
> index 48a1a4f86..9a46b16ab 100644
> --- a/profiles/audio/bap.c
> +++ b/profiles/audio/bap.c
> @@ -1328,6 +1328,10 @@ static void iso_connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
>         fd = g_io_channel_unix_get_fd(chan);
>
>         if (bt_bap_stream_set_io(stream, fd)) {
> +               if (bt_bap_stream_get_type(stream) ==
> +                                               BT_BAP_STREAM_TYPE_BCAST) {
> +                       bt_bap_stream_start(stream, NULL, NULL);
> +               }

Perhaps it would be a better idea to have a dedicated callback for
broadcast in this case, so we don't mix the flows with unicast, since
the handling is quite different. In fact I was going to suggest we
change the broadcast code to live on bcaa driver rather than bap, so
we can move the handing on adapter_probe/adapter_remove there instead
of messing up with bap.

>                 g_io_channel_set_close_on_unref(chan, FALSE);
>                 return;
>         }
> @@ -1887,18 +1891,16 @@ static void bap_state(struct bt_bap_stream *stream, uint8_t old_state,
>                 }
>                 break;
>         case BT_BAP_STREAM_STATE_QOS:
> -               bap_create_io(data, ep, stream, true);
> +               if (bt_bap_stream_get_type(stream) ==
> +                                       BT_BAP_STREAM_TYPE_UCAST) {
> +                       bap_create_io(data, ep, stream, true);
> +               }
>                 break;
>         case BT_BAP_STREAM_STATE_ENABLING:
>                 if (ep)
>                         bap_create_io(data, ep, stream, false);
>                 break;
>         case BT_BAP_STREAM_STATE_STREAMING:
> -               if (bt_bap_stream_get_type(stream) ==
> -                               BT_BAP_STREAM_TYPE_BCAST) {
> -                       if (ep)
> -                               bap_create_io(data, ep, stream, false);
> -               }
>                 break;
>         }
>  }
> @@ -2116,6 +2118,8 @@ static void bap_connecting(struct bt_bap_stream *stream, bool state, int fd,
>
>                         ep->qos.bcast.big = qos.bcast.big;
>                         ep->qos.bcast.bis = qos.bcast.bis;
> +                       bt_bap_stream_config(ep->stream, &ep->qos,
> +                                       ep->caps, NULL, NULL);
>                 }
>
>                 DBG("stream %p fd %d: BIG 0x%02x BIS 0x%02x", stream, fd,
> --
> 2.39.2
>
Silviu Florian Barbulescu Oct. 4, 2023, 3:33 p.m. UTC | #2 
Hi Luiz,

>Hi Iulia,
>
>On Mon, Oct 2, 2023 at 8:34 AM Iulia Tanasescu <iulia.tanasescu@nxp.com> wrote:
>>
>> From: Silviu Florian Barbulescu <silviu.barbulescu@nxp.com>
>>
>> Update transport acquire/release flow for BAP bcast source
>>
>> ---
>>  profiles/audio/bap.c | 16 ++++++++++------
>>  1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c index 
>> 48a1a4f86..9a46b16ab 100644
>> --- a/profiles/audio/bap.c
>> +++ b/profiles/audio/bap.c
>> @@ -1328,6 +1328,10 @@ static void iso_connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
>>         fd = g_io_channel_unix_get_fd(chan);
>>
>>         if (bt_bap_stream_set_io(stream, fd)) {
>> +               if (bt_bap_stream_get_type(stream) ==
>> +                                               BT_BAP_STREAM_TYPE_BCAST) {
>> +                       bt_bap_stream_start(stream, NULL, NULL);
>> +               }
>
>Perhaps it would be a better idea to have a dedicated callback for broadcast in this case, so we don't mix the flows with unicast, since the handling is quite different. In fact I was going to suggest we change the broadcast code to live on bcaa driver rather than bap, so we can move the handing on adapter_probe/adapter_remove there instead of messing up with bap.
You suggest moving the bap_adapter_probe and bap_adapter_remove from bap to bcaa and creating new bap_state and bap_connecting functions for broadcast correct? Not moving all the broadcast code out of BAP(shared\bap.c,shared\bap.h) and creating other files for it.
We will create another patch for this suggestion.
>
>>                 g_io_channel_set_close_on_unref(chan, FALSE);
>>                 return;
>>         }
>> @@ -1887,18 +1891,16 @@ static void bap_state(struct bt_bap_stream *stream, uint8_t old_state,
>>                 }
>>                 break;
>>         case BT_BAP_STREAM_STATE_QOS:
>> -               bap_create_io(data, ep, stream, true);
>> +               if (bt_bap_stream_get_type(stream) ==
>> +                                       BT_BAP_STREAM_TYPE_UCAST) {
>> +                       bap_create_io(data, ep, stream, true);
>> +               }
>>                 break;
>>         case BT_BAP_STREAM_STATE_ENABLING:
>>                 if (ep)
>>                         bap_create_io(data, ep, stream, false);
>>                 break;
>>         case BT_BAP_STREAM_STATE_STREAMING:
>> -               if (bt_bap_stream_get_type(stream) ==
>> -                               BT_BAP_STREAM_TYPE_BCAST) {
>> -                       if (ep)
>> -                               bap_create_io(data, ep, stream, false);
>> -               }
>>                 break;
>>         }
>>  }
>> @@ -2116,6 +2118,8 @@ static void bap_connecting(struct bt_bap_stream 
>> *stream, bool state, int fd,
>>
>>                         ep->qos.bcast.big = qos.bcast.big;
>>                         ep->qos.bcast.bis = qos.bcast.bis;
>> +                       bt_bap_stream_config(ep->stream, &ep->qos,
>> +                                       ep->caps, NULL, NULL);
>>                 }
>>
>>                 DBG("stream %p fd %d: BIG 0x%02x BIS 0x%02x", stream, 
>> fd,
>> --
>> 2.39.2
>>
>
>
>--
>Luiz Augusto von Dentz

Have a nice day, Silviu
diff mbox series

Patch

diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c
index 48a1a4f86..9a46b16ab 100644
--- a/profiles/audio/bap.c
+++ b/profiles/audio/bap.c
@@ -1328,6 +1328,10 @@  static void iso_connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
 	fd = g_io_channel_unix_get_fd(chan);
 
 	if (bt_bap_stream_set_io(stream, fd)) {
+		if (bt_bap_stream_get_type(stream) ==
+						BT_BAP_STREAM_TYPE_BCAST) {
+			bt_bap_stream_start(stream, NULL, NULL);
+		}
 		g_io_channel_set_close_on_unref(chan, FALSE);
 		return;
 	}
@@ -1887,18 +1891,16 @@  static void bap_state(struct bt_bap_stream *stream, uint8_t old_state,
 		}
 		break;
 	case BT_BAP_STREAM_STATE_QOS:
-		bap_create_io(data, ep, stream, true);
+		if (bt_bap_stream_get_type(stream) ==
+					BT_BAP_STREAM_TYPE_UCAST) {
+			bap_create_io(data, ep, stream, true);
+		}
 		break;
 	case BT_BAP_STREAM_STATE_ENABLING:
 		if (ep)
 			bap_create_io(data, ep, stream, false);
 		break;
 	case BT_BAP_STREAM_STATE_STREAMING:
-		if (bt_bap_stream_get_type(stream) ==
-				BT_BAP_STREAM_TYPE_BCAST) {
-			if (ep)
-				bap_create_io(data, ep, stream, false);
-		}
 		break;
 	}
 }
@@ -2116,6 +2118,8 @@  static void bap_connecting(struct bt_bap_stream *stream, bool state, int fd,
 
 			ep->qos.bcast.big = qos.bcast.big;
 			ep->qos.bcast.bis = qos.bcast.bis;
+			bt_bap_stream_config(ep->stream, &ep->qos,
+					ep->caps, NULL, NULL);
 		}
 
 		DBG("stream %p fd %d: BIG 0x%02x BIS 0x%02x", stream, fd,