Subject: [PATCH BlueZ 0/2] Update BAP Broadcast Source state machine

Update BAP Broadcast Source state machine states to use BAP define states
for source Idle, Config, Streaming, and an intermediary state enabling.

Updated test-bap to.

output from broadcast-source.bt

[bluetooth]# endpoint.register 00001852-0000-1000-8000-00805f9b34fb 0x06
[bluetooth]# 1;39m[/local/endpoint/ep0] Auto Accept (yes/no): y
a/local/endpoint/ep0] Auto Accept (yes/no): Max Transports (auto/value
3/local/endpoint/ep0] Max Transports (auto/value): Locations
4/local/endpoint/ep0] Locations: Supported Context (value)
[/local/endpoint/ep0] Supported Context (value): Capabilities:
[bluetooth]# 03 01 ff 00 02 02 03 02 03 03 05 04 1a 00 f0 00
[bluetooth]# Metadata:
[bluetooth]# Capabilities:
[bluetooth]# 03 01 ff 00 02 02 03 02 03 03 05 04 1a 00 f0 00
[bluetooth]# Metadata:
[bluetooth]# [CHG] Controller C0:07:E8:8B:69:F2 Pairable: yes
[bluetooth]# [CHG] Controller 00:AA:01:00:00:03 Pairable: yes
[bluetooth]# [NEW] Endpoint /org/bluez/hci3/pac_bcast0
[bluetooth]# Endpoint /local/endpoint/ep0 registered
endpoint.config /org/bluez/hci3/pac_bcast0 /local/endpoint/ep0 16_2_1
[bluetooth]# 1;39m[/local/endpoint/ep0] BIG (auto/value): 1
3/local/endpoint/ep0] BIG (auto/value): Enter channel location (value/no
0x03 0x02 0x04 0x000] Enter channel location (value/no): Metadata
[/local/endpoint/ep0] Enter Metadata (value/no):
Configuration.#0: len 0x02 type 0x01
[bluetooth]# Configuration.Sampling Frequency: 16 Khz (0x03)
[bluetooth]# Configuration.#1: len 0x02 type 0x02
[bluetooth]# Configuration.Frame Duration: 10 ms (0x01)
[bluetooth]# Configuration.#2: len 0x03 type 0x04
[bluetooth]# Configuration.Frame Length: 40 (0x0028)
[bluetooth]# Configuration.#3: len 0x05 type 0x03
[bluetooth]# Configuration.Location: 0x00000003
[bluetooth]# Configuration.Location: Front Left (0x00000001)
[bluetooth]# Configuration.Location: Front Right (0x00000002)
[bluetooth]# Metadata.#0: len 0x03 type 0x02
[bluetooth]# Metadata.Context: 0x0004
[bluetooth]# Metadata.Context Media (0x0004)
[bluetooth]# BIG 0x01
[bluetooth]# BIS 0x00
[bluetooth]# BCode:
[bluetooth]# 01 02 68 05 53 f1 41 5a a2 65 bb af c6 ea 03 b8
[bluetooth]# Framing 0x00
[bluetooth]# PresentationDelay 40000
[bluetooth]# Interval 10000
[bluetooth]# PHY 0x02
[bluetooth]# SDU 80
[bluetooth]# Retransmissions 2
[bluetooth]# Latency 10
[bluetooth]# [NEW] Endpoint /org/bluez/hci2/pac_bcast0
[bluetooth]# Endpoint /local/endpoint/ep0 registered
[bluetooth]# transport.acquire /org/bluez/hci3/pac_bcast0/fd0
[bluetooth]# Transport /org/bluez/hci3/pac_bcast0/fd0 not found
[bluetooth]# AdvertisementMonitor path registered
[bluetooth]# [NEW] Transport /org/bluez/hci3/pac_bcast0/fd0
[bluetooth]# Endpoint: SetConfiguration
[bluetooth]# Transport /org/bluez/hci3/pac_bcast0/fd0
[bluetooth]# Properties.Device: /org/bluez/hci3
[bluetooth]# Auto Accepting...
[bluetooth]# Endpoint /local/endpoint/ep0 configured

Silviu Florian Barbulescu (2):
shared/bap: Update BAP Broadcast Source state machine states
bap: Update BAP Broadcast Source state machine states

profiles/audio/bap.c | 13 ++++++++-----
src/shared/bap.c | 39 ++++++++++++++++++++-------------------
unit/test-bap.c | 4 ++--
3 files changed, 30 insertions(+), 26 deletions(-)


base-commit: 62d38d6c9ef4017ff6a84af0a9cd9242a7f6a211
--
2.40.1



Subject: [PATCH BlueZ 1/2] shared/bap: Update BAP Broadcast Source state machine

Update BAP Broadcast Source state machine states to use BAP define states
for source Idle, Config, Streaming, and an intermediary state enabling.

Updated test-bap too.

---
src/shared/bap.c | 39 ++++++++++++++++++++-------------------
unit/test-bap.c | 4 ++--
2 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/src/shared/bap.c b/src/shared/bap.c
index 6572ef1d1..639149520 100644
--- a/src/shared/bap.c
+++ b/src/shared/bap.c
@@ -1361,14 +1361,6 @@ static void ep_config_cb(struct bt_bap_stream *stream, int err)
if (err)
return;

- if (bt_bap_stream_get_type(stream) == BT_BAP_STREAM_TYPE_BCAST) {
- if (bt_bap_stream_io_dir(stream) == BT_BAP_BCAST_SINK)
- stream_set_state(stream, BT_BAP_STREAM_STATE_QOS);
- else if (bt_bap_stream_io_dir(stream) == BT_BAP_BCAST_SOURCE)
- stream_set_state(stream, BT_BAP_STREAM_STATE_CONFIG);
- return;
- }
-
stream_set_state(stream, BT_BAP_STREAM_STATE_CONFIG);
}

@@ -1759,6 +1751,15 @@ static unsigned int bap_stream_metadata(struct bt_bap_stream *stream,
return req->id;
}

+static unsigned int bap_bcast_qos(struct bt_bap_stream *stream,
+ struct bt_bap_qos *data,
+ bt_bap_stream_func_t func,
+ void *user_data)
+{
+ stream->qos = *data;
+ return 1;
+}
+
static unsigned int bap_bcast_config(struct bt_bap_stream *stream,
struct bt_bap_qos *qos, struct iovec *data,
bt_bap_stream_func_t func, void *user_data)
@@ -2071,7 +2072,7 @@ static unsigned int bap_bcast_get_state(struct bt_bap_stream *stream)
return stream->state;
}

-static unsigned int bap_bcast_enable(struct bt_bap_stream *stream,
+static unsigned int bap_bcast_sink_enable(struct bt_bap_stream *stream,
bool enable_links, struct iovec *data,
bt_bap_stream_func_t func,
void *user_data)
@@ -2081,22 +2082,21 @@ static unsigned int bap_bcast_enable(struct bt_bap_stream *stream,
return 1;
}

-static unsigned int bap_bcast_start(struct bt_bap_stream *stream,
+static unsigned int bap_bcast_src_enable(struct bt_bap_stream *stream,
+ bool enable_links, struct iovec *data,
bt_bap_stream_func_t func,
void *user_data)
{
- stream_set_state(stream, BT_BAP_STREAM_STATE_STREAMING);
+ stream_set_state(stream, BT_BAP_STREAM_STATE_ENABLING);

return 1;
}

-static unsigned int bap_bcast_sink_disable(struct bt_bap_stream *stream,
- bool disable_links,
+static unsigned int bap_bcast_start(struct bt_bap_stream *stream,
bt_bap_stream_func_t func,
void *user_data)
{
- bap_stream_io_detach(stream);
- stream_set_state(stream, BT_BAP_STREAM_STATE_CONFIG);
+ stream_set_state(stream, BT_BAP_STREAM_STATE_STREAMING);

return 1;
}
@@ -2106,7 +2106,8 @@ static unsigned int bap_bcast_disable(struct bt_bap_stream *stream,
bt_bap_stream_func_t func,
void *user_data)
{
- stream_set_state(stream, BT_BAP_STREAM_STATE_DISABLING);
+ bap_stream_io_detach(stream);
+ stream_set_state(stream, BT_BAP_STREAM_STATE_CONFIG);

return 1;
}
@@ -2205,14 +2206,14 @@ static const struct bt_bap_stream_ops stream_ops[] = {
bap_ucast_release, bap_ucast_detach),
STREAM_OPS(BT_BAP_BCAST_SINK, bap_bcast_set_state,
bap_bcast_get_state,
- bap_bcast_config, NULL, bap_bcast_enable,
- bap_bcast_start, bap_bcast_sink_disable, NULL,
+ bap_bcast_config, NULL, bap_bcast_sink_enable,
+ bap_bcast_start, bap_bcast_disable, NULL,
bap_bcast_metadata, bap_bcast_sink_get_dir,
bap_bcast_get_location,
bap_bcast_release, bap_bcast_sink_detach),
STREAM_OPS(BT_BAP_BCAST_SOURCE, bap_bcast_set_state,
bap_bcast_get_state,
- bap_bcast_config, NULL, bap_bcast_enable,
+ bap_bcast_config, bap_bcast_qos, bap_bcast_src_enable,
bap_bcast_start, bap_bcast_disable, NULL,
bap_bcast_metadata, bap_bcast_src_get_dir,
bap_bcast_get_location,
diff --git a/unit/test-bap.c b/unit/test-bap.c
index 46ee0e4e5..10f9e348c 100644
--- a/unit/test-bap.c
+++ b/unit/test-bap.c
@@ -547,10 +547,10 @@ static void bsrc_state(struct bt_bap_stream *stream, uint8_t old_state,
struct test_data *data = user_data;

switch (new_state) {
- case BT_BAP_STREAM_STATE_QOS:
+ case BT_BAP_STREAM_STATE_CONFIG:
bt_bap_stream_enable(stream, true, NULL, NULL, NULL);
break;
- case BT_BAP_STREAM_STATE_CONFIG:
+ case BT_BAP_STREAM_STATE_ENABLING:
data->base = bt_bap_stream_get_base(stream);

g_assert(data->base);
--
2.40.1


Subject: [PATCH BlueZ 2/2] bap: Update BAP Broadcast Source state machine states

Update BAP Broadcast Source state machine states

---
profiles/audio/bap.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c
index 06dc4e254..65684cac4 100644
--- a/profiles/audio/bap.c
+++ b/profiles/audio/bap.c
@@ -2474,7 +2474,7 @@ static uint8_t get_streams_nb_by_state(struct bap_setup *setup)
*/
return 1;
else if (bt_bap_stream_get_state(ent_setup->stream) !=
- BT_BAP_STREAM_STATE_CONFIG)
+ BT_BAP_STREAM_STATE_ENABLING)
/* Not all streams form a BIG have received transport
* acquire, so wait for the other streams to.
*/
@@ -2516,8 +2516,12 @@ static void bap_state_bcast_src(struct bt_bap_stream *stream, uint8_t old_state,
queue_remove(data->streams, stream);
break;
case BT_BAP_STREAM_STATE_CONFIG:
- if (!setup || setup->id)
- break;
+ // TO DO Reconfiguration
+ break;
+ /* On transport acquire use BT_BAP_STREAM_STATE_ENABLING
+ * as a substate of BT_BAP_STREAM_STATE_CONFIG
+ */
+ case BT_BAP_STREAM_STATE_ENABLING:
/* If the stream attached to a broadcast
* source endpoint generate the base.
*/
@@ -2855,8 +2859,7 @@ static void bap_connecting_bcast(struct bt_bap_stream *stream, bool state,

setup->qos.bcast.big = iso_qos.bcast.big;
setup->qos.bcast.bis = iso_qos.bcast.bis;
- bt_bap_stream_config(setup->stream, &setup->qos, setup->caps,
- NULL, NULL);
+ bt_bap_stream_qos(setup->stream, &setup->qos, NULL, NULL);
}

DBG("stream %p fd %d: BIG 0x%02x BIS 0x%02x", stream, fd,
--
2.40.1


2024-05-20 16:40:53

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BlueZ 1/2] shared/bap: Update BAP Broadcast Source state machine

Hi Silviu,

On Mon, May 20, 2024 at 12:28 PM Silviu Florian Barbulescu
<[email protected]> wrote:
>
> Update BAP Broadcast Source state machine states to use BAP define states
> for source Idle, Config, Streaming, and an intermediary state enabling.

Not really following, what is an intermediary state? Or do you mean
internal state? And more importantly why would we need an internal
state like that?

> Updated test-bap too.
>
> ---
> src/shared/bap.c | 39 ++++++++++++++++++++-------------------
> unit/test-bap.c | 4 ++--
> 2 files changed, 22 insertions(+), 21 deletions(-)
>
> diff --git a/src/shared/bap.c b/src/shared/bap.c
> index 6572ef1d1..639149520 100644
> --- a/src/shared/bap.c
> +++ b/src/shared/bap.c
> @@ -1361,14 +1361,6 @@ static void ep_config_cb(struct bt_bap_stream *stream, int err)
> if (err)
> return;
>
> - if (bt_bap_stream_get_type(stream) == BT_BAP_STREAM_TYPE_BCAST) {
> - if (bt_bap_stream_io_dir(stream) == BT_BAP_BCAST_SINK)
> - stream_set_state(stream, BT_BAP_STREAM_STATE_QOS);
> - else if (bt_bap_stream_io_dir(stream) == BT_BAP_BCAST_SOURCE)
> - stream_set_state(stream, BT_BAP_STREAM_STATE_CONFIG);
> - return;
> - }
> -
> stream_set_state(stream, BT_BAP_STREAM_STATE_CONFIG);
> }
>
> @@ -1759,6 +1751,15 @@ static unsigned int bap_stream_metadata(struct bt_bap_stream *stream,
> return req->id;
> }
>
> +static unsigned int bap_bcast_qos(struct bt_bap_stream *stream,
> + struct bt_bap_qos *data,
> + bt_bap_stream_func_t func,
> + void *user_data)
> +{
> + stream->qos = *data;
> + return 1;
> +}
> +
> static unsigned int bap_bcast_config(struct bt_bap_stream *stream,
> struct bt_bap_qos *qos, struct iovec *data,
> bt_bap_stream_func_t func, void *user_data)
> @@ -2071,7 +2072,7 @@ static unsigned int bap_bcast_get_state(struct bt_bap_stream *stream)
> return stream->state;
> }
>
> -static unsigned int bap_bcast_enable(struct bt_bap_stream *stream,
> +static unsigned int bap_bcast_sink_enable(struct bt_bap_stream *stream,
> bool enable_links, struct iovec *data,
> bt_bap_stream_func_t func,
> void *user_data)
> @@ -2081,22 +2082,21 @@ static unsigned int bap_bcast_enable(struct bt_bap_stream *stream,
> return 1;
> }
>
> -static unsigned int bap_bcast_start(struct bt_bap_stream *stream,
> +static unsigned int bap_bcast_src_enable(struct bt_bap_stream *stream,
> + bool enable_links, struct iovec *data,
> bt_bap_stream_func_t func,
> void *user_data)
> {
> - stream_set_state(stream, BT_BAP_STREAM_STATE_STREAMING);
> + stream_set_state(stream, BT_BAP_STREAM_STATE_ENABLING);
>
> return 1;
> }
>
> -static unsigned int bap_bcast_sink_disable(struct bt_bap_stream *stream,
> - bool disable_links,
> +static unsigned int bap_bcast_start(struct bt_bap_stream *stream,
> bt_bap_stream_func_t func,
> void *user_data)
> {
> - bap_stream_io_detach(stream);
> - stream_set_state(stream, BT_BAP_STREAM_STATE_CONFIG);
> + stream_set_state(stream, BT_BAP_STREAM_STATE_STREAMING);
>
> return 1;
> }
> @@ -2106,7 +2106,8 @@ static unsigned int bap_bcast_disable(struct bt_bap_stream *stream,
> bt_bap_stream_func_t func,
> void *user_data)
> {
> - stream_set_state(stream, BT_BAP_STREAM_STATE_DISABLING);
> + bap_stream_io_detach(stream);
> + stream_set_state(stream, BT_BAP_STREAM_STATE_CONFIG);
>
> return 1;
> }
> @@ -2205,14 +2206,14 @@ static const struct bt_bap_stream_ops stream_ops[] = {
> bap_ucast_release, bap_ucast_detach),
> STREAM_OPS(BT_BAP_BCAST_SINK, bap_bcast_set_state,
> bap_bcast_get_state,
> - bap_bcast_config, NULL, bap_bcast_enable,
> - bap_bcast_start, bap_bcast_sink_disable, NULL,
> + bap_bcast_config, NULL, bap_bcast_sink_enable,
> + bap_bcast_start, bap_bcast_disable, NULL,
> bap_bcast_metadata, bap_bcast_sink_get_dir,
> bap_bcast_get_location,
> bap_bcast_release, bap_bcast_sink_detach),
> STREAM_OPS(BT_BAP_BCAST_SOURCE, bap_bcast_set_state,
> bap_bcast_get_state,
> - bap_bcast_config, NULL, bap_bcast_enable,
> + bap_bcast_config, bap_bcast_qos, bap_bcast_src_enable,
> bap_bcast_start, bap_bcast_disable, NULL,
> bap_bcast_metadata, bap_bcast_src_get_dir,
> bap_bcast_get_location,
> diff --git a/unit/test-bap.c b/unit/test-bap.c
> index 46ee0e4e5..10f9e348c 100644
> --- a/unit/test-bap.c
> +++ b/unit/test-bap.c
> @@ -547,10 +547,10 @@ static void bsrc_state(struct bt_bap_stream *stream, uint8_t old_state,
> struct test_data *data = user_data;
>
> switch (new_state) {
> - case BT_BAP_STREAM_STATE_QOS:
> + case BT_BAP_STREAM_STATE_CONFIG:
> bt_bap_stream_enable(stream, true, NULL, NULL, NULL);
> break;
> - case BT_BAP_STREAM_STATE_CONFIG:
> + case BT_BAP_STREAM_STATE_ENABLING:
> data->base = bt_bap_stream_get_base(stream);
>
> g_assert(data->base);
> --
> 2.40.1
>


--
Luiz Augusto von Dentz

2024-05-20 18:31:30

by bluez.test.bot

[permalink] [raw]
Subject: RE: Update BAP Broadcast Source state machine

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=854453

---Test result---

Test Summary:
CheckPatch PASS 0.97 seconds
GitLint PASS 0.67 seconds
BuildEll PASS 24.44 seconds
BluezMake PASS 1665.06 seconds
MakeCheck PASS 13.33 seconds
MakeDistcheck PASS 177.64 seconds
CheckValgrind PASS 249.99 seconds
CheckSmatch WARNING 353.75 seconds
bluezmakeextell PASS 119.52 seconds
IncrementalBuild PASS 3024.77 seconds
ScanBuild PASS 1002.02 seconds

Details
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
src/shared/bap.c:286: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:286: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:286:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structures


---
Regards,
Linux Bluetooth

Subject: Re: [PATCH BlueZ 1/2] shared/bap: Update BAP Broadcast Source state machine

Hi Luiz,

> -----Original Message-----
> From: Luiz Augusto von Dentz <[email protected]>
> Sent: Monday, May 20, 2024 7:40 PM
> To: Silviu Florian Barbulescu <[email protected]>
> Cc: [email protected]; Mihai-Octavian Urzica <mihai-
> [email protected]>; Vlad Pruteanu <[email protected]>; Andrei
> Istodorescu <[email protected]>; Iulia Tanasescu
> <[email protected]>
> Subject: [EXT] Re: [PATCH BlueZ 1/2] shared/bap: Update BAP Broadcast Source
> state machine
>
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report this
> email' button
>
>
> Hi Silviu,
>
> On Mon, May 20, 2024 at 12:28 PM Silviu Florian Barbulescu
> <[email protected]> wrote:
> >
> > Update BAP Broadcast Source state machine states to use BAP define
> > states for source Idle, Config, Streaming, and an intermediary state enabling.
>
> Not really following, what is an intermediary state? Or do you mean internal
> state? And more importantly why would we need an internal state like that?
>

Hi Luiz we use the ENABLING state to know when a transport linked to a stream
has been acquired by a process and in the case of a BIG with one BIS stream goes
in the ENABLING state waiting for the response from the kernel that the BIG has
been created so it can go to the streaming state.
For the case of a BIG with multiple BISes, the BIG is created when all BISes
are acquired. So we use the ENABLING state to verify that all transports
attached to that streams form BIG have been acquired so we can create the BIG.
If we don’t use this internal state we can use a flag in the stream structure
to be informed that the transport attached to the stream is in the acquiring
procedure, but this would not look good, as we will have a flag intercalated
with the stream state machine.

> > Updated test-bap too.
> >
> > ---
> > src/shared/bap.c | 39 ++++++++++++++++++++-------------------
> > unit/test-bap.c | 4 ++--
> > 2 files changed, 22 insertions(+), 21 deletions(-)
> >
> > diff --git a/src/shared/bap.c b/src/shared/bap.c index
> > 6572ef1d1..639149520 100644
> > --- a/src/shared/bap.c
> > +++ b/src/shared/bap.c
> > @@ -1361,14 +1361,6 @@ static void ep_config_cb(struct bt_bap_stream
> *stream, int err)
> > if (err)
> > return;
> >
> > - if (bt_bap_stream_get_type(stream) == BT_BAP_STREAM_TYPE_BCAST)
> {
> > - if (bt_bap_stream_io_dir(stream) == BT_BAP_BCAST_SINK)
> > - stream_set_state(stream, BT_BAP_STREAM_STATE_QOS);
> > - else if (bt_bap_stream_io_dir(stream) == BT_BAP_BCAST_SOURCE)
> > - stream_set_state(stream, BT_BAP_STREAM_STATE_CONFIG);
> > - return;
> > - }
> > -
> > stream_set_state(stream, BT_BAP_STREAM_STATE_CONFIG); }
> >
> > @@ -1759,6 +1751,15 @@ static unsigned int bap_stream_metadata(struct
> bt_bap_stream *stream,
> > return req->id;
> > }
> >
> > +static unsigned int bap_bcast_qos(struct bt_bap_stream *stream,
> > + struct bt_bap_qos *data,
> > + bt_bap_stream_func_t func,
> > + void *user_data) {
> > + stream->qos = *data;
> > + return 1;
> > +}
> > +
> > static unsigned int bap_bcast_config(struct bt_bap_stream *stream,
> > struct bt_bap_qos *qos, struct iovec *data,
> > bt_bap_stream_func_t func, void
> > *user_data) @@ -2071,7 +2072,7 @@ static unsigned int
> bap_bcast_get_state(struct bt_bap_stream *stream)
> > return stream->state;
> > }
> >
> > -static unsigned int bap_bcast_enable(struct bt_bap_stream *stream,
> > +static unsigned int bap_bcast_sink_enable(struct bt_bap_stream
> > +*stream,
> > bool enable_links, struct iovec *data,
> > bt_bap_stream_func_t func,
> > void *user_data) @@ -2081,22
> > +2082,21 @@ static unsigned int bap_bcast_enable(struct bt_bap_stream
> *stream,
> > return 1;
> > }
> >
> > -static unsigned int bap_bcast_start(struct bt_bap_stream *stream,
> > +static unsigned int bap_bcast_src_enable(struct bt_bap_stream *stream,
> > + bool enable_links, struct
> > +iovec *data,
> > bt_bap_stream_func_t func,
> > void *user_data) {
> > - stream_set_state(stream, BT_BAP_STREAM_STATE_STREAMING);
> > + stream_set_state(stream, BT_BAP_STREAM_STATE_ENABLING);
> >
> > return 1;
> > }
> >
> > -static unsigned int bap_bcast_sink_disable(struct bt_bap_stream *stream,
> > - bool disable_links,
> > +static unsigned int bap_bcast_start(struct bt_bap_stream *stream,
> > bt_bap_stream_func_t func,
> > void *user_data)
> > {
> > - bap_stream_io_detach(stream);
> > - stream_set_state(stream, BT_BAP_STREAM_STATE_CONFIG);
> > + stream_set_state(stream, BT_BAP_STREAM_STATE_STREAMING);
> >
> > return 1;
> > }
> > @@ -2106,7 +2106,8 @@ static unsigned int bap_bcast_disable(struct
> bt_bap_stream *stream,
> > bt_bap_stream_func_t func,
> > void *user_data)
> > {
> > - stream_set_state(stream, BT_BAP_STREAM_STATE_DISABLING);
> > + bap_stream_io_detach(stream);
> > + stream_set_state(stream, BT_BAP_STREAM_STATE_CONFIG);
> >
> > return 1;
> > }
> > @@ -2205,14 +2206,14 @@ static const struct bt_bap_stream_ops
> stream_ops[] = {
> > bap_ucast_release, bap_ucast_detach),
> > STREAM_OPS(BT_BAP_BCAST_SINK, bap_bcast_set_state,
> > bap_bcast_get_state,
> > - bap_bcast_config, NULL, bap_bcast_enable,
> > - bap_bcast_start, bap_bcast_sink_disable, NULL,
> > + bap_bcast_config, NULL, bap_bcast_sink_enable,
> > + bap_bcast_start, bap_bcast_disable, NULL,
> > bap_bcast_metadata, bap_bcast_sink_get_dir,
> > bap_bcast_get_location,
> > bap_bcast_release, bap_bcast_sink_detach),
> > STREAM_OPS(BT_BAP_BCAST_SOURCE, bap_bcast_set_state,
> > bap_bcast_get_state,
> > - bap_bcast_config, NULL, bap_bcast_enable,
> > + bap_bcast_config, bap_bcast_qos, bap_bcast_src_enable,
> > bap_bcast_start, bap_bcast_disable, NULL,
> > bap_bcast_metadata, bap_bcast_src_get_dir,
> > bap_bcast_get_location,
> > diff --git a/unit/test-bap.c b/unit/test-bap.c
> > index 46ee0e4e5..10f9e348c 100644
> > --- a/unit/test-bap.c
> > +++ b/unit/test-bap.c
> > @@ -547,10 +547,10 @@ static void bsrc_state(struct bt_bap_stream
> *stream, uint8_t old_state,
> > struct test_data *data = user_data;
> >
> > switch (new_state) {
> > - case BT_BAP_STREAM_STATE_QOS:
> > + case BT_BAP_STREAM_STATE_CONFIG:
> > bt_bap_stream_enable(stream, true, NULL, NULL, NULL);
> > break;
> > - case BT_BAP_STREAM_STATE_CONFIG:
> > + case BT_BAP_STREAM_STATE_ENABLING:
> > data->base = bt_bap_stream_get_base(stream);
> >
> > g_assert(data->base);
> > --
> > 2.40.1
> >
>
>
> --
> Luiz Augusto von Dentz


2024-05-23 14:16:57

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BlueZ 1/2] shared/bap: Update BAP Broadcast Source state machine

Hi Silviu,

On Thu, May 23, 2024 at 6:46 AM Silviu Florian Barbulescu
<[email protected]> wrote:
>
> Hi Luiz,
>
> > -----Original Message-----
> > From: Luiz Augusto von Dentz <[email protected]>
> > Sent: Monday, May 20, 2024 7:40 PM
> > To: Silviu Florian Barbulescu <[email protected]>
> > Cc: [email protected]; Mihai-Octavian Urzica <mihai-
> > [email protected]>; Vlad Pruteanu <[email protected]>; Andrei
> > Istodorescu <[email protected]>; Iulia Tanasescu
> > <[email protected]>
> > Subject: [EXT] Re: [PATCH BlueZ 1/2] shared/bap: Update BAP Broadcast Source
> > state machine
> >
> > Caution: This is an external email. Please take care when clicking links or
> > opening attachments. When in doubt, report the message using the 'Report this
> > email' button
> >
> >
> > Hi Silviu,
> >
> > On Mon, May 20, 2024 at 12:28 PM Silviu Florian Barbulescu
> > <[email protected]> wrote:
> > >
> > > Update BAP Broadcast Source state machine states to use BAP define
> > > states for source Idle, Config, Streaming, and an intermediary state enabling.
> >
> > Not really following, what is an intermediary state? Or do you mean internal
> > state? And more importantly why would we need an internal state like that?
> >
>
> Hi Luiz we use the ENABLING state to know when a transport linked to a stream
> has been acquired by a process and in the case of a BIG with one BIS stream goes
> in the ENABLING state waiting for the response from the kernel that the BIG has
> been created so it can go to the streaming state.
> For the case of a BIG with multiple BISes, the BIG is created when all BISes
> are acquired. So we use the ENABLING state to verify that all transports
> attached to that streams form BIG have been acquired so we can create the BIG.
> If we don’t use this internal state we can use a flag in the stream structure
> to be informed that the transport attached to the stream is in the acquiring
> procedure, but this would not look good, as we will have a flag intercalated
> with the stream state machine.

Fair enough, please add this information as a comment so in the future
we know why we are using enabling for broadcast.

> > > Updated test-bap too.
> > >
> > > ---
> > > src/shared/bap.c | 39 ++++++++++++++++++++-------------------
> > > unit/test-bap.c | 4 ++--
> > > 2 files changed, 22 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/src/shared/bap.c b/src/shared/bap.c index
> > > 6572ef1d1..639149520 100644
> > > --- a/src/shared/bap.c
> > > +++ b/src/shared/bap.c
> > > @@ -1361,14 +1361,6 @@ static void ep_config_cb(struct bt_bap_stream
> > *stream, int err)
> > > if (err)
> > > return;
> > >
> > > - if (bt_bap_stream_get_type(stream) == BT_BAP_STREAM_TYPE_BCAST)
> > {
> > > - if (bt_bap_stream_io_dir(stream) == BT_BAP_BCAST_SINK)
> > > - stream_set_state(stream, BT_BAP_STREAM_STATE_QOS);
> > > - else if (bt_bap_stream_io_dir(stream) == BT_BAP_BCAST_SOURCE)
> > > - stream_set_state(stream, BT_BAP_STREAM_STATE_CONFIG);
> > > - return;
> > > - }
> > > -
> > > stream_set_state(stream, BT_BAP_STREAM_STATE_CONFIG); }
> > >
> > > @@ -1759,6 +1751,15 @@ static unsigned int bap_stream_metadata(struct
> > bt_bap_stream *stream,
> > > return req->id;
> > > }
> > >
> > > +static unsigned int bap_bcast_qos(struct bt_bap_stream *stream,
> > > + struct bt_bap_qos *data,
> > > + bt_bap_stream_func_t func,
> > > + void *user_data) {
> > > + stream->qos = *data;
> > > + return 1;
> > > +}
> > > +
> > > static unsigned int bap_bcast_config(struct bt_bap_stream *stream,
> > > struct bt_bap_qos *qos, struct iovec *data,
> > > bt_bap_stream_func_t func, void
> > > *user_data) @@ -2071,7 +2072,7 @@ static unsigned int
> > bap_bcast_get_state(struct bt_bap_stream *stream)
> > > return stream->state;
> > > }
> > >
> > > -static unsigned int bap_bcast_enable(struct bt_bap_stream *stream,
> > > +static unsigned int bap_bcast_sink_enable(struct bt_bap_stream
> > > +*stream,
> > > bool enable_links, struct iovec *data,
> > > bt_bap_stream_func_t func,
> > > void *user_data) @@ -2081,22
> > > +2082,21 @@ static unsigned int bap_bcast_enable(struct bt_bap_stream
> > *stream,
> > > return 1;
> > > }
> > >
> > > -static unsigned int bap_bcast_start(struct bt_bap_stream *stream,
> > > +static unsigned int bap_bcast_src_enable(struct bt_bap_stream *stream,
> > > + bool enable_links, struct
> > > +iovec *data,
> > > bt_bap_stream_func_t func,
> > > void *user_data) {
> > > - stream_set_state(stream, BT_BAP_STREAM_STATE_STREAMING);
> > > + stream_set_state(stream, BT_BAP_STREAM_STATE_ENABLING);
> > >
> > > return 1;
> > > }
> > >
> > > -static unsigned int bap_bcast_sink_disable(struct bt_bap_stream *stream,
> > > - bool disable_links,
> > > +static unsigned int bap_bcast_start(struct bt_bap_stream *stream,
> > > bt_bap_stream_func_t func,
> > > void *user_data)
> > > {
> > > - bap_stream_io_detach(stream);
> > > - stream_set_state(stream, BT_BAP_STREAM_STATE_CONFIG);
> > > + stream_set_state(stream, BT_BAP_STREAM_STATE_STREAMING);
> > >
> > > return 1;
> > > }
> > > @@ -2106,7 +2106,8 @@ static unsigned int bap_bcast_disable(struct
> > bt_bap_stream *stream,
> > > bt_bap_stream_func_t func,
> > > void *user_data)
> > > {
> > > - stream_set_state(stream, BT_BAP_STREAM_STATE_DISABLING);
> > > + bap_stream_io_detach(stream);
> > > + stream_set_state(stream, BT_BAP_STREAM_STATE_CONFIG);
> > >
> > > return 1;
> > > }
> > > @@ -2205,14 +2206,14 @@ static const struct bt_bap_stream_ops
> > stream_ops[] = {
> > > bap_ucast_release, bap_ucast_detach),
> > > STREAM_OPS(BT_BAP_BCAST_SINK, bap_bcast_set_state,
> > > bap_bcast_get_state,
> > > - bap_bcast_config, NULL, bap_bcast_enable,
> > > - bap_bcast_start, bap_bcast_sink_disable, NULL,
> > > + bap_bcast_config, NULL, bap_bcast_sink_enable,
> > > + bap_bcast_start, bap_bcast_disable, NULL,
> > > bap_bcast_metadata, bap_bcast_sink_get_dir,
> > > bap_bcast_get_location,
> > > bap_bcast_release, bap_bcast_sink_detach),
> > > STREAM_OPS(BT_BAP_BCAST_SOURCE, bap_bcast_set_state,
> > > bap_bcast_get_state,
> > > - bap_bcast_config, NULL, bap_bcast_enable,
> > > + bap_bcast_config, bap_bcast_qos, bap_bcast_src_enable,
> > > bap_bcast_start, bap_bcast_disable, NULL,
> > > bap_bcast_metadata, bap_bcast_src_get_dir,
> > > bap_bcast_get_location,
> > > diff --git a/unit/test-bap.c b/unit/test-bap.c
> > > index 46ee0e4e5..10f9e348c 100644
> > > --- a/unit/test-bap.c
> > > +++ b/unit/test-bap.c
> > > @@ -547,10 +547,10 @@ static void bsrc_state(struct bt_bap_stream
> > *stream, uint8_t old_state,
> > > struct test_data *data = user_data;
> > >
> > > switch (new_state) {
> > > - case BT_BAP_STREAM_STATE_QOS:
> > > + case BT_BAP_STREAM_STATE_CONFIG:
> > > bt_bap_stream_enable(stream, true, NULL, NULL, NULL);
> > > break;
> > > - case BT_BAP_STREAM_STATE_CONFIG:
> > > + case BT_BAP_STREAM_STATE_ENABLING:
> > > data->base = bt_bap_stream_get_base(stream);
> > >
> > > g_assert(data->base);
> > > --
> > > 2.40.1
> > >
> >
> >
> > --
> > Luiz Augusto von Dentz
>


--
Luiz Augusto von Dentz