Hi Iulia,
On Mon, Jan 29, 2024 at 10:29 AM Iulia Tanasescu
<iulia.tanasescu@xxxxxxx> wrote:
>
> This updates the input prompts for broadcast endpoint register and
> config.
>
> To register a broadcast endpoint, the user will be asked to enter
> the supported stream locations and context types.
>
> At broadcast source endpoint config, the user will provide stream
> config options: The BIG that the new stream will be part of, the
> stream Channel Allocation, and the metadata of the subgroup to
> include the stream. These options will be used to configure the
> BASE and the BIG.
Please add some samples if the changes affect user input, since this
does look like it will have some impact on when BIG/BIS is requested,
also make sure this works out with the likes of -e command line
option, since that automatically registers endpoints.
> ---
> client/player.c | 186 ++++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 149 insertions(+), 37 deletions(-)
>
> diff --git a/client/player.c b/client/player.c
> index 623519209..d9a4bce87 100644
> --- a/client/player.c
> +++ b/client/player.c
> @@ -4,7 +4,7 @@
> * BlueZ - Bluetooth protocol stack for Linux
> *
> * Copyright (C) 2020 Intel Corporation. All rights reserved.
> - * Copyright 2023 NXP
> + * Copyright 2023-2024 NXP
> *
> *
> */
> @@ -3259,12 +3259,8 @@ static void endpoint_iso_group(const char *input, void *user_data)
> ep->iso_group = value;
> }
>
> - if (!ep->broadcast)
> - bt_shell_prompt_input(ep->path, "CIS (auto/value):",
> - endpoint_iso_stream, ep);
> - else
> - bt_shell_prompt_input(ep->path, "BIS (auto/value):",
> - endpoint_iso_stream, ep);
> + bt_shell_prompt_input(ep->path, "CIS (auto/value):",
> + endpoint_iso_stream, ep);
> }
>
> static void endpoint_context(const char *input, void *user_data)
> @@ -3282,12 +3278,8 @@ static void endpoint_context(const char *input, void *user_data)
>
> ep->context = value;
>
> - if (ep->broadcast)
> - bt_shell_prompt_input(ep->path, "BIG (auto/value):",
> - endpoint_iso_group, ep);
> - else
> - bt_shell_prompt_input(ep->path, "CIG (auto/value):",
> - endpoint_iso_group, ep);
> + bt_shell_prompt_input(ep->path, "CIG (auto/value):",
> + endpoint_iso_group, ep);
> }
>
> static void endpoint_supported_context(const char *input, void *user_data)
> @@ -3305,6 +3297,11 @@ static void endpoint_supported_context(const char *input, void *user_data)
>
> ep->supported_context = value;
>
> + if (ep->broadcast) {
> + endpoint_register(ep);
> + return;
> + }
> +
> bt_shell_prompt_input(ep->path, "Context (value):", endpoint_context,
> ep);
> }
> @@ -3354,13 +3351,6 @@ static void endpoint_auto_accept(const char *input, void *user_data)
> {
> struct endpoint *ep = user_data;
>
> - if (!strcmp(ep->uuid, BCAA_SERVICE_UUID) ||
> - !strcmp(ep->uuid, BAA_SERVICE_UUID)) {
> - ep->broadcast = true;
> - } else {
> - ep->broadcast = false;
> - }
> -
> if (!strcasecmp(input, "y") || !strcasecmp(input, "yes")) {
> ep->auto_accept = true;
> bt_shell_prompt_input(ep->path, "Max Transports (auto/value):",
> @@ -3478,6 +3468,13 @@ static void cmd_register_endpoint(int argc, char *argv[])
> g_list_length(local_endpoints));
> local_endpoints = g_list_append(local_endpoints, ep);
>
> + if (!strcmp(ep->uuid, BCAA_SERVICE_UUID) ||
> + !strcmp(ep->uuid, BAA_SERVICE_UUID)) {
> + ep->broadcast = true;
> + } else {
> + ep->broadcast = false;
> + }
> +
> if (strrchr(argv[2], ':')) {
> ep->codec = 0xff;
> parse_vendor_codec(argv[2], &ep->cid, &ep->vid);
> @@ -3626,6 +3623,134 @@ static void endpoint_config(const char *input, void *user_data)
>
> static struct endpoint *endpoint_new(const struct capabilities *cap);
>
> +static void endpoint_set_metadata_cfg(const char *input, void *user_data)
> +{
> + struct endpoint_config *cfg = user_data;
> +
> + if (!strcasecmp(input, "n") || !strcasecmp(input, "no"))
> + goto done;
> +
> + if (!cfg->meta)
> + cfg->meta = g_new0(struct iovec, 1);
> +
> + cfg->meta->iov_base = str2bytearray((char *) input,
> + &cfg->meta->iov_len);
> + if (!cfg->meta->iov_base) {
> + free(cfg->meta);
> + cfg->meta = NULL;
> + }
> +
> +done:
> + endpoint_set_config(cfg);
> +}
> +
> +static void config_endpoint_channel_location(const char *input, void *user_data)
> +{
> + struct endpoint_config *cfg = user_data;
> + char *endptr = NULL;
> + uint32_t location;
> +
> + if (!strcasecmp(input, "n") || !strcasecmp(input, "no"))
> + goto add_meta;
> +
> + location = strtol(input, &endptr, 0);
> +
> + if (!endptr || *endptr != '\0') {
> + bt_shell_printf("Invalid argument: %s\n", input);
> + return bt_shell_noninteractive_quit(EXIT_FAILURE);
> + }
> +
> + /* Add Channel Allocation LTV in capabilities */
> + {
> + uint8_t ltv[6] = { 0x05, LC3_CONFIG_CHAN_ALLOC };
> +
> + location = cpu_to_le32(location);
> + memcpy(<v[2], &location, sizeof(location));
> + iov_append(&cfg->caps, ltv, sizeof(ltv));
> + }
Not a big fan of creating scopes like above in the middle of the
function, how about we have a dedicated function for setting these?
> +add_meta:
> + /* Add metadata */
> + bt_shell_prompt_input(cfg->ep->path, "Enter Metadata (value/no):",
> + endpoint_set_metadata_cfg, cfg);
> +}
> +
> +static void ltv_find(size_t i, uint8_t l, uint8_t t, uint8_t *v,
> + void *user_data)
> +{
> + bool *found = user_data;
> +
> + *found = true;
> +}
> +
> +static void config_endpoint_iso_group(const char *input, void *user_data)
> +{
> + struct endpoint_config *cfg = user_data;
> + char *endptr = NULL;
> + int value;
> + bool found = false;
> +
> + value = strtol(input, &endptr, 0);
> +
> + if (!endptr || *endptr != '\0' || value > UINT8_MAX) {
> + bt_shell_printf("Invalid argument: %s\n", input);
> + return bt_shell_noninteractive_quit(EXIT_FAILURE);
> + }
> +
> + cfg->ep->iso_group = value;
> +
> + /* Check if Channel Allocation is present in caps */
> + {
> + uint8_t type = LC3_CONFIG_CHAN_ALLOC;
> +
> + util_ltv_foreach(cfg->caps->iov_base,
> + cfg->caps->iov_len, &type,
> + ltv_find, &found);
> + }
Ditto, just have the type defined in the beginning of the function.
> +
> + /* Add Channel Allocation if it is not present in caps */
> + if (!found) {
> + bt_shell_prompt_input(cfg->ep->path,
> + "Enter channel location (value/no):",
> + config_endpoint_channel_location, cfg);
> + } else {
> + /* Add metadata */
> + bt_shell_prompt_input(cfg->ep->path,
> + "Enter Metadata (value/no):",
> + endpoint_set_metadata_cfg, cfg);
> + }
> +}
> +
> +static void endpoint_set_config_bcast(struct endpoint_config *cfg)
> +{
> + cfg->ep->bcode = g_new0(struct iovec, 1);
> + iov_append(&cfg->ep->bcode, bcast_code,
> + sizeof(bcast_code));
> +
> + /* Add periodic advertisement parameters */
> + cfg->sync_factor = BCAST_SYNC_FACTOR;
> + cfg->options = BCAST_OPTIONS;
> + cfg->skip = BCAST_SKIP;
> + cfg->sync_timeout = BCAST_SYNC_TIMEOUT;
> + cfg->sync_cte_type = BCAST_SYNC_CTE_TYPE;
> +
> + /* Add BIG create sync parameters */
> + cfg->mse = BCAST_MSE;
> + cfg->timeout = BCAST_TIMEOUT;
> +
> + if ((strcmp(cfg->ep->uuid, BAA_SERVICE_UUID) == 0)) {
> + /* A broadcast sink endpoint config does not need
> + * user input.
> + */
> + endpoint_set_config(cfg);
> + return;
> + }
> +
> + bt_shell_prompt_input(cfg->ep->path,
> + "BIG (value):",
> + config_endpoint_iso_group, cfg);
> +}
> +
> static void cmd_config_endpoint(int argc, char *argv[])
> {
> struct endpoint_config *cfg;
> @@ -3662,24 +3787,11 @@ static void cmd_config_endpoint(int argc, char *argv[])
> /* Set QoS parameters */
> cfg->qos = preset->qos;
>
> - if (cfg->ep->broadcast) {
> - cfg->ep->bcode = g_new0(struct iovec, 1);
> - iov_append(&cfg->ep->bcode, bcast_code,
> - sizeof(bcast_code));
> -
> - /* Add periodic advertisement parameters */
> - cfg->sync_factor = BCAST_SYNC_FACTOR;
> - cfg->options = BCAST_OPTIONS;
> - cfg->skip = BCAST_SKIP;
> - cfg->sync_timeout = BCAST_SYNC_TIMEOUT;
> - cfg->sync_cte_type = BCAST_SYNC_CTE_TYPE;
> - /* Add BIG create sync parameters */
> - cfg->mse = BCAST_MSE;
> - cfg->timeout = BCAST_TIMEOUT;
> -
> - endpoint_set_config(cfg);
> - } else
> + if (cfg->ep->broadcast)
> + endpoint_set_config_bcast(cfg);
> + else
> endpoint_set_config(cfg);
> +
> return;
> }
>
> --
> 2.39.2
>
--
Luiz Augusto von Dentz
