Skip to content

LE Audio: Broadcast source subgroup support #49946

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
Merged
merged 1 commit into from
Nov 24, 2022
Merged

LE Audio: Broadcast source subgroup support #49946

merged 1 commit into from
Nov 24, 2022

Conversation

@Thalley
Copy link
Contributor

@Thalley Thalley commented Sep 5, 2022 •
edited
Loading

Updates the BAP broadcast API to support setting multiple subgroups as well as BIS specific codec configuration data.

Depends on #49872
fixes #47243
fixes #47242

@Thalley Thalley changed the title LE AUdio: Broadcast source subgroup support LE Audio: Broadcast source subgroup support Sep 5, 2022
@Thalley Thalley force-pushed the broadcast_source_subgroup branch 3 times, most recently from da6a9b4 to 32f3383 Compare September 6, 2022 12:58
@Thalley Thalley self-assigned this Sep 6, 2022
@Thalley Thalley force-pushed the broadcast_source_subgroup branch from 32f3383 to cb040be Compare September 8, 2022 14:02
@Thalley Thalley mentioned this pull request Sep 8, 2022
Merged
@Thalley Thalley force-pushed the broadcast_source_subgroup branch 2 times, most recently from 85962a8 to 207f7fd Compare September 9, 2022 11:55
@Thalley Thalley force-pushed the broadcast_source_subgroup branch from 207f7fd to 3fa06a5 Compare September 26, 2022 15:07
@Thalley Thalley force-pushed the broadcast_source_subgroup branch from 3fa06a5 to babb733 Compare October 14, 2022 08:02
@Thalley Thalley marked this pull request as ready for review October 14, 2022 08:02
@Thalley Thalley requested review from cvinayak and kruithofa October 14, 2022 08:02
@Thalley Thalley force-pushed the broadcast_source_subgroup branch from babb733 to b3174fe Compare October 24, 2022 09:50
@Thalley Thalley mentioned this pull request Oct 24, 2022
Closed
@Thalley Thalley force-pushed the broadcast_source_subgroup branch from b3174fe to 6d1cc9c Compare November 7, 2022 09:54
@Thalley
Copy link
Contributor Author

Thalley commented Nov 7, 2022

Rebased to solve merge conflicts

@Thalley Thalley force-pushed the broadcast_source_subgroup branch 2 times, most recently from e7f8652 to 17ea5f2 Compare November 9, 2022 13:54
@Thalley
Copy link
Contributor Author

Thalley commented Nov 9, 2022

Rebased to solve merge conflicts

MariuszSkamra
MariuszSkamra previously approved these changes Nov 14, 2022
View reviewed changes
kruithofa
kruithofa suggested changes Nov 15, 2022
View reviewed changes
subsys/bluetooth/audio/broadcast_source.c Outdated
Copy link

@kruithofa kruithofa Nov 14, 2022

Choose a reason for hiding this comment

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

SInce this is an internal function, shouldn't the bt_audio prefix be dropped?
If so, there are a few more

@Thalley Thalley force-pushed the broadcast_source_subgroup branch from 17ea5f2 to 058fb17 Compare November 15, 2022 08:48
MariuszSkamra
MariuszSkamra previously approved these changes Nov 16, 2022
View reviewed changes
larsgk
larsgk reviewed Nov 17, 2022
View reviewed changes
include/zephyr/bluetooth/audio/audio.h Outdated
Copy link
Contributor

@larsgk larsgk Nov 17, 2022

Choose a reason for hiding this comment

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

It's not entirely clear to me, but is it here it would be possible to set e.g. language metadata as well?

Copy link
Contributor Author

@Thalley Thalley Nov 17, 2022

Choose a reason for hiding this comment

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

The metadata parameters are supplied in the codec parameter on the next line, along with the codec configuration parameters

@larsgk
Copy link
Contributor

larsgk commented Nov 17, 2022

LGTM. Maybe it would be nice to have some different metadata on the subgroups (e.g. lang = english/spanish as the spec example) + a broadcast_audio_sink sample to handle it (e.g. select the one with spanish). Can be another PR.

alexsven
alexsven reviewed Nov 18, 2022
View reviewed changes
subsys/bluetooth/audio/broadcast_source.c Outdated
Copy link
Contributor

@alexsven alexsven Nov 18, 2022

Choose a reason for hiding this comment

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

This is returning an uninitialized err

Copy link
Contributor Author

@Thalley Thalley Nov 18, 2022

Choose a reason for hiding this comment

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

Fixed

@Thalley Thalley force-pushed the broadcast_source_subgroup branch from 058fb17 to 9becb58 Compare November 18, 2022 14:26
@Thalley Thalley requested review from MariuszSkamra, alexsven and larsgk and removed request for larsgk November 18, 2022 14:26
@Thalley
Bluetooth: Audio: Broadcast source subgroup and BIS codec support
c9153ee 
Updates the broadcast source API to create subgroups and
to set BIS specific codec configuration

Signed-off-by: Emil Gydesen <[email protected]>
@Thalley Thalley force-pushed the broadcast_source_subgroup branch from 9becb58 to c9153ee Compare November 18, 2022 14:36
larsgk
larsgk approved these changes Nov 21, 2022
View reviewed changes
Copy link
Contributor

@larsgk larsgk left a comment

Choose a reason for hiding this comment

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

LGTM. In a separate PR, we should consider having some info in the docs besides the sample code on how to do broadcast sources/sinks to avoid confusion when folks want to do real world implementations.

samples/bluetooth/broadcast_audio_source/src/main.c
struct bt_audio_broadcast_source_subgroup_param
subgroup_param[CONFIG_BT_AUDIO_BROADCAST_SRC_SUBGROUP_COUNT];
struct bt_audio_broadcast_source_create_param create_param;
const size_t streams_per_subgroup = ARRAY_SIZE(stream_params) / ARRAY_SIZE(subgroup_param);
Copy link
Contributor

@larsgk larsgk Nov 21, 2022

Choose a reason for hiding this comment

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

This works when the stream count is the same per group. It can stay as is or maybe add a comment or check for clarification for devs picking up the sample.

Copy link
Contributor Author

@Thalley Thalley Nov 21, 2022

Choose a reason for hiding this comment

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

This was added to properly test multiple subsgroups with multiple streams in it.

I think when we work towards #52347 this will be modified again, and this comment will be resolved by that

@carlescufi carlescufi merged commit 40e3930 into zephyrproject-rtos:main Nov 24, 2022
@Thalley Thalley deleted the broadcast_source_subgroup branch November 24, 2022 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@larsgk larsgk larsgk approved these changes

@MariuszSkamra MariuszSkamra MariuszSkamra approved these changes

@jhedberg jhedberg Awaiting requested review from jhedberg

@Vudentz Vudentz Awaiting requested review from Vudentz

@wopu-ot wopu-ot Awaiting requested review from wopu-ot

@sjanc sjanc Awaiting requested review from sjanc

@alwa-nordic alwa-nordic Awaiting requested review from alwa-nordic

@cvinayak cvinayak Awaiting requested review from cvinayak

@asbjornsabo asbjornsabo Awaiting requested review from asbjornsabo

@Casper-Bonde-Bose Casper-Bonde-Bose Awaiting requested review from Casper-Bonde-Bose

@hermabe hermabe Awaiting requested review from hermabe

@jori-nordic jori-nordic Awaiting requested review from jori-nordic

+2 more reviewers

@alexsven alexsven alexsven approved these changes

@kruithofa kruithofa kruithofa approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@Thalley Thalley

Labels

area: Bluetooth Audio area: Bluetooth

Projects

Bluetooth LE Audio
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

LE Audio: Add support for stream specific codec configurations for broadcast source LE Audio: Add subgroup support for broadcast source

7 participants

@Thalley @larsgk @MariuszSkamra @alexsven @kruithofa @carlescufi @zephyrbot