mbox series

[BlueZ,v4,0/5] Broadcast source reconfiguration support

Message ID 20240318163853.68598-1-silviu.barbulescu@nxp.com
Headers show
Series Broadcast source reconfiguration support | expand

Message

Silviu Florian Barbulescu March 18, 2024, 4:38 p.m. UTC
This patch adds support for broadcast source to reconfigure a BIS.
endpoint.config command has a new prompt for broadcast source:
BIS Index for reconfiguration? (value(1-31)/no):
Values n or 0 represent that no reconfiguration is required
Values between (1-31) specify which BIS to be reconfigured
example form client/scripts/broadcast-source.bt
endpoint.register 00001852-0000-1000-8000-00805f9b34fb 0x06
endpoint.config /org/bluez/hci0/pac_bcast0 /local/endpoint/ep0 16_2_1
BIS Index for reconfiguration? (value(1-31)/no): 0
[/local/endpoint/ep0] BIG (auto/value): 1
[/local/endpoint/ep0] Enter channel location (value/no): 1
[/local/endpoint/ep0] Enter Metadata (value/no): 0x03 0x02 0x04 0x00
transport.acquire /org/bluez/hci0/pac_bcast0/fd0
HCI Command: LE Set Periodic Ad.. (0x08|0x003f) plen 41
Handle: 1
Operation: Complete ext advertising data (0x03)
Data length: 0x26
Service Data: Basic Audio Announcement (0x1851)
Presetation Delay: 40000
Number of Subgroups: 1
Subgroup #0:
Number of BIS(s): 1
Codec: LC3 (0x06)
Codec Specific Configuration: #0: len 0x02 type 0x01
Codec Specific Configuration: Sampling Frequency: 16 Khz (0x03)
Codec Specific Configuration: #1: len 0x02 type 0x02
Codec Specific Configuration: Frame Duration: 10 ms (0x01)
Codec Specific Configuration: #2: len 0x03 type 0x04
Codec Specific Configuration: Frame Length: 40 (0x0028)
Codec Specific Configuration: #3: len 0x05 type 0x03
Codec Specific Configuration: Location: 0x00000001
Codec Specific Configuration: Location: Front Left (0x00000001)
Metadata: #0: len 0x03 type 0x02
Metadata: Context: 0x0004
Metadata: Context Media (0x0004)
BIS #0:
Index: 1
transport.release /org/bluez/hci0/pac_bcast0/fd0
endpoint.config /org/bluez/hci0/pac_bcast0 /local/endpoint/ep0 16_2_1
[/local/endpoint/ep0] BIS Index for reconfiguration? (value(1-31)/no): 1
[/local/endpoint/ep0] BIG (auto/value): 1
[/local/endpoint/ep0] Enter channel location (value/no): 2
[/local/endpoint/ep0] Enter Metadata (value/no): 0x03 0x02 0x04 0x01
transport.acquire /org/bluez/hci0/pac_bcast0/fd0
HCI Command: LE Set Periodic Ad.. (0x08|0x003f) plen 41 #47 [hci0]
Handle: 1
Operation: Complete ext advertising data (0x03)
Data length: 0x26
Service Data: Basic Audio Announcement (0x1851)
Presetation Delay: 40000
Number of Subgroups: 1
Subgroup #0:
Number of BIS(s): 1
Codec: LC3 (0x06)
Codec Specific Configuration: #0: len 0x02 type 0x01
Codec Specific Configuration: Sampling Frequency: 16 Khz (0x03)
Codec Specific Configuration: #1: len 0x02 type 0x02
Codec Specific Configuration: Frame Duration: 10 ms (0x01)
Codec Specific Configuration: #2: len 0x03 type 0x04
Codec Specific Configuration: Frame Length: 40 (0x0028)
Codec Specific Configuration: #3: len 0x05 type 0x03
Codec Specific Configuration: Location: 0x00000002
Codec Specific Configuration: Location: Front Right (0x00000002)
Metadata: #0: len 0x03 type 0x02
Metadata: Context: 0x0104
Metadata: Context Media (0x0004)
Metadata: Context Notifications (0x0100)
BIS #0:
Index: 1

Silviu Florian Barbulescu (5):
  player: Add reconfiguration prompt for broadcast source
  transport: Add support to update the transport configuration
  bap: Broadcast source reconfiguration support added
  player.c: Remove bt_shell_noninteractive_quit on  acquire,release
    commands
  client: update broadcast source script to support the BIS
    reconfiguration

 client/player.c                    | 35 ++++++++++++--
 client/scripts/broadcast-source.bt | 12 ++++-
 profiles/audio/bap.c               | 76 ++++++++++++++++++++++++++++++
 profiles/audio/transport.c         | 27 ++++++++++-
 profiles/audio/transport.h         |  1 +
 src/shared/bap.c                   | 11 ++++-
 6 files changed, 154 insertions(+), 8 deletions(-)


base-commit: 84628e5d109cbec0bbd515c12c4b5224380784fe

Comments

Luiz Augusto von Dentz March 18, 2024, 5:38 p.m. UTC | #1
Hi Silviu,

On Mon, Mar 18, 2024 at 4:39 PM Silviu Florian Barbulescu
<silviu.barbulescu@nxp.com> wrote:
>
> If a BIS is reconfigured, the metadata and codec capabilities are updated.
> Also, the BASE is updated to reflect the update.
>
> ---
>  profiles/audio/bap.c       | 76 ++++++++++++++++++++++++++++++++++++++
>  profiles/audio/transport.c |  6 ++-
>  src/shared/bap.c           | 11 +++++-
>  3 files changed, 91 insertions(+), 2 deletions(-)
>
> diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c
> index 964ba9c21..e508e03ba 100644
> --- a/profiles/audio/bap.c
> +++ b/profiles/audio/bap.c
> @@ -580,6 +580,11 @@ static int setup_parse_bcast_qos(struct bap_setup *setup, const char *key,
>                         return -EINVAL;
>
>                 dbus_message_iter_get_basic(iter, &qos->bcast.big);
> +       } else if (!strcasecmp(key, "BIS")) {
> +               if (var != DBUS_TYPE_BYTE)
> +                       return -EINVAL;
> +
> +               dbus_message_iter_get_basic(iter, &qos->bcast.bis);
>         } else if (!strcasecmp(key, "Options")) {
>                 if (var != DBUS_TYPE_BYTE)
>                         return -EINVAL;
> @@ -884,6 +889,53 @@ static void setup_free(void *data)
>         free(setup);
>  }
>
> +static void iterate_setups(struct bap_setup *setup)
> +{
> +       const struct queue_entry *entry;
> +       struct bap_setup *ent_setup;
> +       uint8_t bis_cnt = 1;
> +
> +       for (entry = queue_get_entries(setup->ep->setups);
> +                               entry; entry = entry->next) {
> +               ent_setup = entry->data;
> +
> +               if (setup->qos.bcast.big != ent_setup->qos.bcast.big)
> +                       continue;
> +
> +               util_iov_free(ent_setup->base, 1);
> +               ent_setup->base = NULL;

I do wonder why we need to store the base as part of the setup? Can't
we just generate it later when it is actually about to be configured
into the socket like I did?

> +               if (setup->qos.bcast.bis == bis_cnt) {
> +                       bt_bap_stream_config(ent_setup->stream, &setup->qos,
> +                                               setup->caps, NULL, NULL);
> +                       bt_bap_stream_metadata(ent_setup->stream,
> +                                       setup->metadata, NULL, NULL);
> +               }
> +
> +               bis_cnt++;
> +       }
> +}
> +
> +static bool verify_state(struct bap_setup *setup)
> +{
> +       const struct queue_entry *entry;
> +       struct bap_setup *ent_setup;
> +
> +       for (entry = queue_get_entries(setup->ep->setups);
> +                               entry; entry = entry->next) {
> +               ent_setup = entry->data;
> +
> +               if (setup->qos.bcast.big != ent_setup->qos.bcast.big)
> +                       continue;
> +
> +               if (bt_bap_stream_get_state(ent_setup->stream) ==
> +                               BT_BAP_STREAM_STATE_STREAMING)
> +                       return false;
> +       }
> +
> +       return true;
> +}
> +
>  static DBusMessage *set_configuration(DBusConnection *conn, DBusMessage *msg,
>                                                                 void *data)
>  {
> @@ -925,6 +977,30 @@ static DBusMessage *set_configuration(DBusConnection *conn, DBusMessage *msg,
>                 util_iov_free(setup->metadata, 1);
>                 setup->metadata = util_iov_dup(
>                                 bt_bap_pac_get_metadata(ep->rpac), 1);
> +       } else if (bt_bap_pac_get_type(ep->lpac) == BT_BAP_BCAST_SOURCE) {
> +               if (setup->qos.bcast.bis != BT_ISO_QOS_BIS_UNSET) {
> +                       if ((setup->qos.bcast.bis > queue_length(ep->setups)) ||
> +                                       (setup->qos.bcast.bis == 0)) {
> +                               setup_free(setup);
> +                               return btd_error_invalid_args(msg);
> +                       }
> +
> +                       /* Verify that no BIS in the BIG is in streaming state
> +                        */
> +                       if (!verify_state(setup)) {
> +                               setup_free(setup);
> +                               return btd_error_not_permitted(msg,
> +                                               "Broadcast Audio Stream state is invalid");
> +                       }

I was thinking we could delay the BASE setup until later when the code
actually needs it then the code for stream_get_base can actually
detect what are the streams with the same BIG and generate the BASE
just once.

> +                       /* Find and update the BIS specified in
> +                        * set_configuration command
> +                        */
> +                       iterate_setups(setup);
> +
> +                       setup_free(setup);
> +                       return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
> +               }
>         }
>
>         setup->stream = bt_bap_stream_new(ep->data->bap, ep->lpac, ep->rpac,
> diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
> index 122c3339e..a060f8c61 100644
> --- a/profiles/audio/transport.c
> +++ b/profiles/audio/transport.c
> @@ -1643,8 +1643,12 @@ static void bap_state_changed(struct bt_bap_stream *stream, uint8_t old_state,
>                 bap_update_links(transport);
>                 if (!media_endpoint_is_broadcast(transport->endpoint))
>                         bap_update_qos(transport);
> -               else if (bt_bap_stream_io_dir(stream) != BT_BAP_BCAST_SOURCE)
> +               else if (bt_bap_stream_io_dir(stream) != BT_BAP_BCAST_SOURCE) {
>                         bap_update_bcast_qos(transport);
> +                       if (old_state == BT_BAP_STREAM_STATE_QOS)
> +                               bap_update_bcast_config(transport);
> +               }
> +
>                 transport_update_playing(transport, FALSE);
>                 return;
>         case BT_BAP_STREAM_STATE_DISABLING:
> diff --git a/src/shared/bap.c b/src/shared/bap.c
> index fd99cbbca..603d6d646 100644
> --- a/src/shared/bap.c
> +++ b/src/shared/bap.c
> @@ -1701,7 +1701,16 @@ 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)
>  {
> -       stream->qos = *qos;
> +       if (qos) {
> +               stream->qos = *qos;
> +               stream->qos.bcast.bcode = util_iov_dup(qos->bcast.bcode, 1);
> +       }
> +
> +       if (data) {
> +               util_iov_free(stream->cc, 1);
> +               stream->cc = util_iov_dup(data, 1);
> +       }
> +
>         stream->lpac->ops->config(stream, stream->cc, &stream->qos,
>                         ep_config_cb, stream->lpac->user_data);
>
> --
> 2.39.2

These changes to shared/bap shall be submitted in a separate patch
with a proper description.