Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

LE Audio: Codec meta lang update #72584

Merged
merged 2 commits into from
Jun 12, 2024
Merged

LE Audio: Codec meta lang update #72584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
85b1f1e
Bluetooth: Audio: Rename stream_lang to lang
Thalley May 10, 2024
04a56f0
Bluetooth: Audio: Change lang to 3-byte value from uint32_t
Thalley May 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions doc/connectivity/bluetooth/api/audio/shell/bap.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ Commands
[pref_ctx <context>]
[stream_ctx <context>]
[program_info <program info>]
[stream_lang <ISO 639-3 lang>]
[lang <ISO 639-3 lang>]
[ccid_list <ccids>]
[parental_rating <rating>]
[program_info_uri <URI>]
Expand Down Expand Up @@ -336,7 +336,7 @@ any stream previously configured.
[pref_ctx <context>]
[stream_ctx <context>]
[program_info <program info>]
[stream_lang <ISO 639-3 lang>]
[lang <ISO 639-3 lang>]
[ccid_list <ccids>]
[parental_rating <rating>]
[program_info_uri <URI>]
Expand Down Expand Up @@ -415,7 +415,7 @@ assigned numbers values.
00000000: 08 00 |.. |
QoS: interval 10000 framing 0x00 phy 0x02 sdu 80 rtn 2 latency 10 pd 40000

uart:~$ bap preset sink 32_2_1 config freq 10 meta stream_lang "eng" stream_ctx 4
uart:~$ bap preset sink 32_2_1 config freq 10 meta lang "eng" stream_ctx 4
32_2_1
codec cfg id 0x06 cid 0x0000 vid 0x0000 count 16
data #0: type 0x01 value_len 1
Expand Down
20 changes: 20 additions & 0 deletions doc/releases/migration-guide-3.7.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,26 @@ Bluetooth Audio
the :c:func:`bt_bap_stream_connect` shall now be called before :c:func:`bt_bap_stream_start`.
(:github:`73032`)

* Renamed ``stream_lang`` to just ``lang`` to better fit with the assigned numbers document.
aescolar marked this conversation as resolved.
Show resolved Hide resolved
This affects the ``BT_AUDIO_METADATA_TYPE_LANG`` macro and the following functions:

* :c:func:`bt_audio_codec_cap_meta_set_lang`
* :c:func:`bt_audio_codec_cap_meta_get_lang`
* :c:func:`bt_audio_codec_cfg_meta_set_lang`
* :c:func:`bt_audio_codec_cfg_meta_get_lang`

(:github:`72584`)

* Changed ``lang`` from ``uint32_t`` to ``uint8_t [3]``. This modifies the following functions:

* :c:func:`bt_audio_codec_cap_meta_set_lang`
* :c:func:`bt_audio_codec_cap_meta_get_lang`
* :c:func:`bt_audio_codec_cfg_meta_set_lang`
* :c:func:`bt_audio_codec_cfg_meta_get_lang`

The result of this is that string values such as ``"eng"`` and ``"deu"`` can now be used to set
new values, and to prevent unnecessary copies of data when getting the values. (:github:`72584`)

* All occurrences of ``set_sirk`` have been changed to just ``sirk`` as the ``s`` in ``sirk`` stands
for set. (:github:`73413`)

Expand Down
48 changes: 28 additions & 20 deletions include/zephyr/bluetooth/audio/audio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ extern "C" {

#define BT_AUDIO_BROADCAST_CODE_SIZE 16

/** Size of the stream language value, e.g. "eng" */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe a ref to ISO 639-3 (3 ascii chars)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to add a reference to ISO 639-3 in all places where this is used?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not strictly needed - but a ref would make it very clear why it is always 3

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case we should refer to the assigned numbers document, which is the document that, in this context, define it as 3 octets :)

I don't disagree that references are nice, but if we want to add references to each defined value here, we should do it across the board, which we can do in a follow up PR

#define BT_AUDIO_LANG_SIZE 3

/**
* @brief Codec capability types
*
Expand Down Expand Up @@ -373,11 +376,12 @@ enum bt_audio_metadata_type {
/** UTF-8 encoded title or summary of stream content */
BT_AUDIO_METADATA_TYPE_PROGRAM_INFO = 0x03,

/** @brief Stream language
/** @brief Language
*
* 3 octet lower case language code defined by ISO 639-3
* Possible values can be found at https://iso639-3.sil.org/code_tables/639/data
*/
BT_AUDIO_METADATA_TYPE_STREAM_LANG = 0x04,
BT_AUDIO_METADATA_TYPE_LANG = 0x04,

/** Array of 8-bit CCID values */
BT_AUDIO_METADATA_TYPE_CCID_LIST = 0x05,
Expand Down Expand Up @@ -1243,31 +1247,33 @@ int bt_audio_codec_cfg_meta_get_program_info(const struct bt_audio_codec_cfg *co
int bt_audio_codec_cfg_meta_set_program_info(struct bt_audio_codec_cfg *codec_cfg,
const uint8_t *program_info, size_t program_info_len);

/** @brief Extract stream language
/** @brief Extract language
*
* See @ref BT_AUDIO_METADATA_TYPE_STREAM_LANG for more information about this value.
* See @ref BT_AUDIO_METADATA_TYPE_LANG for more information about this value.
*
* @param codec_cfg The codec data to search in.
* @param[in] codec_cfg The codec data to search in.
* @param[out] lang Pointer to the language bytes (of length BT_AUDIO_LANG_SIZE)
*
* @retval The stream language if positive or 0
* @retval The language if positive or 0
* @retval -EINVAL if arguments are invalid
* @retval -ENODATA if not found
* @retval -EBADMSG if found value has invalid size
*/
int bt_audio_codec_cfg_meta_get_stream_lang(const struct bt_audio_codec_cfg *codec_cfg);
int bt_audio_codec_cfg_meta_get_lang(const struct bt_audio_codec_cfg *codec_cfg,
const uint8_t **lang);

/**
* @brief Set the stream language of a codec configuration metadata.
* @brief Set the language of a codec configuration metadata.
*
* @param codec_cfg The codec configuration to set data for.
* @param stream_lang The 24-bit stream language to set.
* @param lang The 24-bit language to set.
*
* @retval The data_len of @p codec_cfg on success
* @retval -EINVAL if arguments are invalid
* @retval -ENOMEM if the new value could not set or added due to memory
*/
int bt_audio_codec_cfg_meta_set_stream_lang(struct bt_audio_codec_cfg *codec_cfg,
uint32_t stream_lang);
int bt_audio_codec_cfg_meta_set_lang(struct bt_audio_codec_cfg *codec_cfg,
const uint8_t lang[BT_AUDIO_LANG_SIZE]);

/** @brief Extract CCID list
*
Expand Down Expand Up @@ -1766,31 +1772,33 @@ int bt_audio_codec_cap_meta_get_program_info(const struct bt_audio_codec_cap *co
int bt_audio_codec_cap_meta_set_program_info(struct bt_audio_codec_cap *codec_cap,
const uint8_t *program_info, size_t program_info_len);

/** @brief Extract stream language
/** @brief Extract language
*
* See @ref BT_AUDIO_METADATA_TYPE_STREAM_LANG for more information about this value.
* See @ref BT_AUDIO_METADATA_TYPE_LANG for more information about this value.
*
* @param codec_cap The codec data to search in.
* @param[in] codec_cap The codec data to search in.
* @param[out] lang Pointer to the language bytes (of length BT_AUDIO_LANG_SIZE)
*
* @retval The stream language if positive or 0
* @retval 0 On success
* @retval -EINVAL if arguments are invalid
* @retval -ENODATA if not found
* @retval -EBADMSG if found value has invalid size
*/
int bt_audio_codec_cap_meta_get_stream_lang(const struct bt_audio_codec_cap *codec_cap);
int bt_audio_codec_cap_meta_get_lang(const struct bt_audio_codec_cap *codec_cap,
const uint8_t **lang);

/**
* @brief Set the stream language of a codec capability metadata.
* @brief Set the language of a codec capability metadata.
*
* @param codec_cap The codec capability to set data for.
* @param stream_lang The 24-bit stream language to set.
* @param lang The 24-bit language to set.
*
* @retval The data_len of @p codec_cap on success
* @retval -EINVAL if arguments are invalid
* @retval -ENOMEM if the new value could not set or added due to memory
*/
int bt_audio_codec_cap_meta_set_stream_lang(struct bt_audio_codec_cap *codec_cap,
uint32_t stream_lang);
int bt_audio_codec_cap_meta_set_lang(struct bt_audio_codec_cap *codec_cap,
const uint8_t lang[BT_AUDIO_LANG_SIZE]);

/** @brief Extract CCID list
*
Expand Down
4 changes: 2 additions & 2 deletions subsys/bluetooth/audio/ascs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2097,8 +2097,8 @@ static bool ascs_parse_metadata(struct bt_data *data, void *user_data)

break;
}
case BT_AUDIO_METADATA_TYPE_STREAM_LANG:
if (data_len != 3) {
case BT_AUDIO_METADATA_TYPE_LANG:
if (data_len != BT_AUDIO_LANG_SIZE) {
*result->rsp = BT_BAP_ASCS_RSP(BT_BAP_ASCS_RSP_CODE_METADATA_INVALID,
data_type);
result->err = -EBADMSG;
Expand Down
57 changes: 31 additions & 26 deletions subsys/bluetooth/audio/codec.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -796,7 +796,7 @@ static int codec_meta_set_program_info(uint8_t meta[], size_t meta_len, size_t m
program_info, program_info_len);
}

static int codec_meta_get_stream_lang(const uint8_t meta[], size_t meta_len)
static int codec_meta_get_lang(const uint8_t meta[], size_t meta_len, const uint8_t **lang)
{
const uint8_t *data;
int ret;
Expand All @@ -806,37 +806,40 @@ static int codec_meta_get_stream_lang(const uint8_t meta[], size_t meta_len)
return -EINVAL;
}

ret = codec_meta_get_val(meta, meta_len, BT_AUDIO_METADATA_TYPE_STREAM_LANG, &data);
CHECKIF(lang == NULL) {
LOG_DBG("lang is NULL");
return -EINVAL;
}

ret = codec_meta_get_val(meta, meta_len, BT_AUDIO_METADATA_TYPE_LANG, &data);
if (data == NULL) {
return -ENODATA;
}

if (ret != 3) { /* Stream language is 3 octets */
if (ret != BT_AUDIO_LANG_SIZE) {
return -EBADMSG;
}

return sys_get_le24(data);
*lang = data;

return 0;
}

static int codec_meta_set_stream_lang(uint8_t meta[], size_t meta_len, size_t meta_size,
uint32_t stream_lang)
static int codec_meta_set_lang(uint8_t meta[], size_t meta_len, size_t meta_size,
const uint8_t lang[BT_AUDIO_LANG_SIZE])
{
uint8_t stream_lang_le[3];

CHECKIF(meta == NULL) {
LOG_DBG("meta is NULL");
return -EINVAL;
}

if ((stream_lang & 0xFFFFFFU) != stream_lang) {
LOG_DBG("Invalid stream_lang value: %d", stream_lang);
CHECKIF(lang == NULL) {
LOG_DBG("lang is NULL");
return -EINVAL;
}

sys_put_le24(stream_lang, stream_lang_le);

return codec_meta_set_val(meta, meta_len, meta_size, BT_AUDIO_METADATA_TYPE_STREAM_LANG,
stream_lang_le, sizeof(stream_lang_le));
return codec_meta_set_val(meta, meta_len, meta_size, BT_AUDIO_METADATA_TYPE_LANG, lang,
BT_AUDIO_LANG_SIZE);
}

static int codec_meta_get_ccid_list(const uint8_t meta[], size_t meta_len,
Expand Down Expand Up @@ -1248,23 +1251,24 @@ int bt_audio_codec_cfg_meta_set_program_info(struct bt_audio_codec_cfg *codec_cf
return ret;
}

int bt_audio_codec_cfg_meta_get_stream_lang(const struct bt_audio_codec_cfg *codec_cfg)
int bt_audio_codec_cfg_meta_get_lang(const struct bt_audio_codec_cfg *codec_cfg,
const uint8_t **lang)
{
CHECKIF(codec_cfg == NULL) {
LOG_DBG("codec_cfg is NULL");
return -EINVAL;
}

return codec_meta_get_stream_lang(codec_cfg->meta, codec_cfg->meta_len);
return codec_meta_get_lang(codec_cfg->meta, codec_cfg->meta_len, lang);
}

int bt_audio_codec_cfg_meta_set_stream_lang(struct bt_audio_codec_cfg *codec_cfg,
uint32_t stream_lang)
int bt_audio_codec_cfg_meta_set_lang(struct bt_audio_codec_cfg *codec_cfg,
const uint8_t lang[BT_AUDIO_LANG_SIZE])
{
int ret;

ret = codec_meta_set_stream_lang(codec_cfg->meta, codec_cfg->meta_len,
ARRAY_SIZE(codec_cfg->meta), stream_lang);
ret = codec_meta_set_lang(codec_cfg->meta, codec_cfg->meta_len, ARRAY_SIZE(codec_cfg->meta),
lang);
if (ret >= 0) {
codec_cfg->meta_len = ret;
}
Expand Down Expand Up @@ -1575,23 +1579,24 @@ int bt_audio_codec_cap_meta_set_program_info(struct bt_audio_codec_cap *codec_ca
return ret;
}

int bt_audio_codec_cap_meta_get_stream_lang(const struct bt_audio_codec_cap *codec_cap)
int bt_audio_codec_cap_meta_get_lang(const struct bt_audio_codec_cap *codec_cap,
const uint8_t **lang)
{
CHECKIF(codec_cap == NULL) {
LOG_DBG("codec_cap is NULL");
return -EINVAL;
}

return codec_meta_get_stream_lang(codec_cap->meta, codec_cap->meta_len);
return codec_meta_get_lang(codec_cap->meta, codec_cap->meta_len, lang);
}

int bt_audio_codec_cap_meta_set_stream_lang(struct bt_audio_codec_cap *codec_cap,
uint32_t stream_lang)
int bt_audio_codec_cap_meta_set_lang(struct bt_audio_codec_cap *codec_cap,
const uint8_t lang[BT_AUDIO_LANG_SIZE])
{
int ret;

ret = codec_meta_set_stream_lang(codec_cap->meta, codec_cap->meta_len,
ARRAY_SIZE(codec_cap->meta), stream_lang);
ret = codec_meta_set_lang(codec_cap->meta, codec_cap->meta_len, ARRAY_SIZE(codec_cap->meta),
lang);
if (ret >= 0) {
codec_cap->meta_len = ret;
}
Expand Down
18 changes: 7 additions & 11 deletions subsys/bluetooth/audio/shell/audio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,14 +362,10 @@ static inline void print_codec_meta_program_info(const struct shell *sh, size_t
}

static inline void print_codec_meta_language(const struct shell *sh, size_t indent,
uint32_t stream_lang)
const uint8_t lang[BT_AUDIO_LANG_SIZE])
{
uint8_t lang_array[3];

sys_put_be24(stream_lang, lang_array);

shell_print(sh, "%*sLanguage: %c%c%c", indent, "", (char)lang_array[0], (char)lang_array[1],
(char)lang_array[2]);
shell_print(sh, "%*sLanguage: %c%c%c", indent, "", (char)lang[0], (char)lang[1],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could %.*s (with 3, lang) be used instead of %c%c%c ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that work with non-terminated strings?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should, afaik - as it then clearly states that N chars are used

(char)lang[2]);
}

static inline void print_codec_meta_ccid_list(const struct shell *sh, size_t indent,
Expand Down Expand Up @@ -721,9 +717,9 @@ static inline void print_codec_cap(const struct shell *sh, size_t indent,
print_codec_meta_program_info(sh, indent, data, (uint8_t)ret);
}

ret = bt_audio_codec_cap_meta_get_stream_lang(codec_cap);
ret = bt_audio_codec_cap_meta_get_lang(codec_cap, &data);
if (ret >= 0) {
print_codec_meta_language(sh, indent, (uint32_t)ret);
print_codec_meta_language(sh, indent, data);
}

ret = bt_audio_codec_cap_meta_get_ccid_list(codec_cap, &data);
Expand Down Expand Up @@ -957,9 +953,9 @@ static inline void print_codec_cfg(const struct shell *sh, size_t indent,
print_codec_meta_program_info(sh, indent, data, (uint8_t)ret);
}

ret = bt_audio_codec_cfg_meta_get_stream_lang(codec_cfg);
ret = bt_audio_codec_cfg_meta_get_lang(codec_cfg, &data);
if (ret >= 0) {
print_codec_meta_language(sh, indent, (uint32_t)ret);
print_codec_meta_language(sh, indent, data);
}

ret = bt_audio_codec_cfg_meta_get_ccid_list(codec_cfg, &data);
Expand Down
15 changes: 6 additions & 9 deletions subsys/bluetooth/audio/shell/bap.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1957,7 +1957,7 @@ static ssize_t parse_config_meta_args(const struct shell *sh, size_t argn, size_

return -1;
}
} else if (strcmp(arg, "stream_lang") == 0) {
} else if (strcmp(arg, "lang") == 0) {
if (++argn == argc) {
shell_help(sh);

Expand All @@ -1966,18 +1966,15 @@ static ssize_t parse_config_meta_args(const struct shell *sh, size_t argn, size_

arg = argv[argn];

if (strlen(arg) != 3) {
shell_error(sh, "Failed to parse stream lang from %s", arg);
if (strlen(arg) != BT_AUDIO_LANG_SIZE) {
shell_error(sh, "Failed to parse lang from %s", arg);

return -1;
}

val = sys_get_le24(arg);

err = bt_audio_codec_cfg_meta_set_stream_lang(codec_cfg, (uint32_t)val);
err = bt_audio_codec_cfg_meta_set_lang(codec_cfg, arg);
if (err < 0) {
shell_error(sh, "Failed to set stream lang with value %lu: %d", val,
err);
shell_error(sh, "Failed to set lang with value %s: %d", arg, err);

return -1;
}
Expand Down Expand Up @@ -3933,7 +3930,7 @@ static int cmd_print_ase_info(const struct shell *sh, size_t argc, char *argv[])

#define HELP_CFG_META \
"\n[meta" HELP_SEP "[pref_ctx <context>]" HELP_SEP "[stream_ctx <context>]" HELP_SEP \
"[program_info <program info>]" HELP_SEP "[stream_lang <ISO 639-3 lang>]" HELP_SEP \
"[program_info <program info>]" HELP_SEP "[lang <ISO 639-3 lang>]" HELP_SEP \
"[ccid_list <ccids>]" HELP_SEP "[parental_rating <rating>]" HELP_SEP \
"[program_info_uri <URI>]" HELP_SEP "[audio_active_state <state>]" HELP_SEP \
"[bcast_flag]" HELP_SEP "[extended <meta>]" HELP_SEP "[vendor <meta>]]"
Expand Down
Loading
Loading