Skip to content

Bluetooth: audio: Handle lack of channel allocation structure #50249

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

Closed
wants to merge 1 commit into from
Closed

Bluetooth: audio: Handle lack of channel allocation structure #50249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions include/zephyr/bluetooth/audio/audio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,11 @@ struct bt_codec_data {
* These values are defined by the Generic Audio Assigned Numbers, bluetooth.com
*/
enum bt_audio_location {
/*
* As Audio Location is not mandatory, for handling lack of this field
* in codec configuration we introduce special opcode with unspecified location.
*/
BT_AUDIO_LOCATION_UNSPECIFIED = 0,
BT_AUDIO_LOCATION_FRONT_LEFT = BIT(0),
BT_AUDIO_LOCATION_FRONT_RIGHT = BIT(1),
BT_AUDIO_LOCATION_FRONT_CENTER = BIT(2),
Expand Down
7 changes: 5 additions & 2 deletions subsys/bluetooth/audio/codec.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,16 @@ int bt_codec_cfg_get_chan_allocation_val(const struct bt_codec *codec,

*chan_allocation = 0;
if (bt_codec_get_val(codec, BT_CODEC_CONFIG_LC3_CHAN_ALLOC, &element)) {

*chan_allocation = sys_le32_to_cpu(*((uint32_t *)&element->data.data[0]));

return BT_AUDIO_CODEC_PARSE_ERR_SUCCESS;
}

return BT_AUDIO_CODEC_PARSE_ERR_TYPE_NOT_FOUND;
/* BAP v1.0.1 4.3.2 Codec_Specific_Configuration LTV requirements
* The absence of the Audio_Channel_Allocation LTV structure
* shall be interpreted as a single channel with no specified Audio Location.
*/
return BT_AUDIO_LOCATION_UNSPECIFIED;
Copy link
Contributor

@MariuszSkamra MariuszSkamra Sep 15, 2022

Choose a reason for hiding this comment

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

I would put here sentence taken from the spec:

BAP v1.0.1 4.3.2 Codec_Specific_Configuration LTV requirements
The absence of the Audio_Channel_Allocation LTV structure
shall be interpreted as a single channel with no specified Audio Location.

Copy link
Contributor Author

@szymon-czapracki szymon-czapracki Sep 15, 2022

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@Thalley Thalley Sep 25, 2022

Choose a reason for hiding this comment

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

What is the reasoning here to return 0 (BT_AUDIO_LOCATION_UNSPECIFIED, which is btw also an bt_audio_location and not an int) here, instead of an error code?

Seems odd to return error code in all other instances, except this one (and even more weirdly to return 0, i.e. success, in that case).

If we just want to return 0/sucess, instead of an error, I suggest that we just return 0 directly, instead of putting BT_AUDIO_LOCATION_UNSPECIFIED into bt_audio_location and use that here.

}

int bt_codec_cfg_get_octets_per_frame(const struct bt_codec *codec)
Expand Down