| Message ID | 20231206220325.3712902-1-luiz.dentz@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [BlueZ,v1,1/2] bap: Allow setup of multiple stream per endpoint | expand |
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=807629
---Test result---
Test Summary:
CheckPatch PASS 1.43 seconds
GitLint FAIL 0.88 seconds
BuildEll PASS 23.90 seconds
BluezMake PASS 726.49 seconds
MakeCheck PASS 12.22 seconds
MakeDistcheck PASS 156.22 seconds
CheckValgrind PASS 218.64 seconds
CheckSmatch PASS 321.77 seconds
bluezmakeextell PASS 101.23 seconds
IncrementalBuild PASS 1298.14 seconds
ScanBuild WARNING 915.30 seconds
Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[BlueZ,v1,2/2] shared/bap: Make bt_bap_select match the channel map
WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
13: B3 Line contains hard tab characters (\t): " 0000a0201030202010304280001020206000000000a020103020201030428"
14: B3 Line contains hard tab characters (\t): " 0002020206000000000a02010302020103042800"
##############################
Test: ScanBuild - WARNING
Desc: Run Scan Build
Output:
src/shared/bap.c:4767:23: warning: Access to field 'type' results in a dereference of a null pointer (loaded from variable 'lpac')
if (!match.rpac && (lpac->type != BT_BAP_BCAST_SOURCE))
^~~~~~~~~~
1 warning generated.
In file included from tools/mesh-gatt/crypto.c:32:
./src/shared/util.h:228:9: warning: 1st function call argument is an uninitialized value
return be32_to_cpu(get_unaligned((const uint32_t *) ptr));
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./src/shared/util.h:33:26: note: expanded from macro 'be32_to_cpu'
#define be32_to_cpu(val) bswap_32(val)
^~~~~~~~~~~~~
/usr/include/byteswap.h:34:21: note: expanded from macro 'bswap_32'
#define bswap_32(x) __bswap_32 (x)
^~~~~~~~~~~~~~
In file included from tools/mesh-gatt/crypto.c:32:
./src/shared/util.h:238:9: warning: 1st function call argument is an uninitialized value
return be64_to_cpu(get_unaligned((const uint64_t *) ptr));
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./src/shared/util.h:34:26: note: expanded from macro 'be64_to_cpu'
#define be64_to_cpu(val) bswap_64(val)
^~~~~~~~~~~~~
/usr/include/byteswap.h:37:21: note: expanded from macro 'bswap_64'
#define bswap_64(x) __bswap_64 (x)
^~~~~~~~~~~~~~
2 warnings generated.
---
Regards,
Linux Bluetooth
Hi Luiz, ke, 2023-12-06 kello 17:03 -0500, Luiz Augusto von Dentz kirjoitti: > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > bt_bap_pac may actually map to multiple PAC records and each may have a > different channel count that needs to be matched separately, for > instance when trying with EarFun Air Pro: > > < ACL Data TX: Handle 2048 flags 0x00 dlen 85 > ATT: Write Command (0x52) len 80 > Handle: 0x0098 Type: ASE Control Point (0x2bc6) > Data: 010405020206000000000a020103020201030428000602020600000 > 0000a0201030202010304280001020206000000000a020103020201030428 > 0002020206000000000a02010302020103042800 > Opcode: Codec Configuration (0x01) > Number of ASE(s): 4 > ASE: #0 > ASE ID: 0x05 > Target Latency: Balance Latency/Reliability (0x02) > PHY: 0x02 > LE 2M PHY (0x02) > Codec: LC3 (0x06) > Codec Specific Configuration: #0: len 0x02 type 0x01 > Sampling Frequency: 16 Khz (0x03) > Codec Specific Configuration: #1: len 0x02 type 0x02 > Frame Duration: 10 ms (0x01) > Codec Specific Configuration: #2: len 0x03 type 0x04 > Frame Length: 40 (0x0028) > ASE: #1 > ASE ID: 0x06 > Target Latency: Balance Latency/Reliability (0x02) > PHY: 0x02 > LE 2M PHY (0x02) > Codec: LC3 (0x06) > Codec Specific Configuration: #0: len 0x02 type 0x01 > Sampling Frequency: 16 Khz (0x03) > Codec Specific Configuration: #1: len 0x02 type 0x02 > Frame Duration: 10 ms (0x01) > Codec Specific Configuration: #2: len 0x03 type 0x04 > Frame Length: 40 (0x0028) > ASE: #2 > ASE ID: 0x01 > Target Latency: Balance Latency/Reliability (0x02) > PHY: 0x02 > LE 2M PHY (0x02) > Codec: LC3 (0x06) > Codec Specific Configuration: #0: len 0x02 type 0x01 > Sampling Frequency: 16 Khz (0x03) > Codec Specific Configuration: #1: len 0x02 type 0x02 > Frame Duration: 10 ms (0x01) > Codec Specific Configuration: #2: len 0x03 type 0x04 > Frame Length: 40 (0x0028) > ASE: #3 > ASE ID: 0x02 > Target Latency: Balance Latency/Reliability (0x02) > PHY: 0x02 > LE 2M PHY (0x02) > Codec: LC3 (0x06) > Codec Specific Configuration: #0: len 0x02 type 0x01 > Sampling Frequency: 16 Khz (0x03) > Codec Specific Configuration: #1: len 0x02 type 0x02 > Frame Duration: 10 ms (0x01) > Codec Specific Configuration: #2: len 0x03 type 0x04 > Frame Length: 40 (0x0028) > > Fixes: https://github.com/bluez/bluez/issues/612 > --- > profiles/audio/bap.c | 6 +-- > src/shared/bap.c | 87 ++++++++++++++++++++++++++++++++++++++++---- > src/shared/bap.h | 3 +- > src/shared/util.c | 43 ++++++++++++++++++++++ > src/shared/util.h | 6 +++ > 5 files changed, 132 insertions(+), 13 deletions(-) > > diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c > index 965d7efe6561..16c5faee45f9 100644 > --- a/profiles/audio/bap.c > +++ b/profiles/audio/bap.c > @@ -1290,10 +1290,8 @@ static bool pac_found(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac, > } > > /* TODO: Cache LRU? */ > - if (btd_service_is_initiator(service)) { > - if (!bt_bap_select(lpac, rpac, select_cb, ep)) > - ep->data->selecting++; > - } > + if (btd_service_is_initiator(service)) > + bt_bap_select(lpac, rpac, &ep->data->selecting, select_cb, ep); > > return true; > } > diff --git a/src/shared/bap.c b/src/shared/bap.c > index a1495ca84bcc..2450b53232e3 100644 > --- a/src/shared/bap.c > +++ b/src/shared/bap.c > @@ -185,6 +185,7 @@ struct bt_bap_pac { > struct bt_bap_pac_qos qos; > struct iovec *data; > struct iovec *metadata; > + struct queue *chan_map; > struct bt_bap_pac_ops *ops; > void *user_data; > }; > @@ -2417,6 +2418,33 @@ static void *ltv_merge(struct iovec *data, struct iovec *cont) > return iov_append(data, cont->iov_len, cont->iov_base); > } > > +static void bap_pac_foreach_channel(size_t i, uint8_t l, uint8_t t, uint8_t *v, > + void *user_data) > +{ > + struct bt_bap_pac *pac = user_data; > + > + if (!v) > + return; > + > + if (!pac->chan_map) > + pac->chan_map = queue_new(); > + > + printf("PAC %p chan_map 0x%02x\n", pac, *v); > + > + queue_push_tail(pac->chan_map, UINT_TO_PTR(*v)); > +} > + > +static void bap_pac_update_chan_map(struct bt_bap_pac *pac, struct iovec *data) > +{ > + uint8_t type = 0x03; > + > + if (!data) > + return; > + > + util_ltv_foreach(data->iov_base, data->iov_len, &type, > + bap_pac_foreach_channel, pac); > +} Hmm, I though Supported_Audio_Channel_Counts (0x3) is not a channel mapping, but enumerates what number of channels each PAC supports? So e.g. 0x3 = supports 1 or 2 channels, and the PAC could be used to configure either case. IIUC in BAP v1.0.1 Sec. 4.4 the configuration is supposed to work like this: 1. From the bits set in PACS Sink/Source Locations, select which channels we are going to configure for sink and source directions. 2. Decide which channel goes to which ASE. 3. Supported_Audio_Channel_Counts in PACs limit how many channels we can put on a single ASE. 4. The outcome probably should prefer the standard stream configurations defined in BAP. 5. For each ASE Config Codec, set the channel bits in Audio_Channel_Allocation to indicate which channels the ASE got.
Hi Pauli, On Thu, Dec 7, 2023 at 1:00 PM Pauli Virtanen <pav@iki.fi> wrote: > > Hi Luiz, > > ke, 2023-12-06 kello 17:03 -0500, Luiz Augusto von Dentz kirjoitti: > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > > > bt_bap_pac may actually map to multiple PAC records and each may have a > > different channel count that needs to be matched separately, for > > instance when trying with EarFun Air Pro: > > > > < ACL Data TX: Handle 2048 flags 0x00 dlen 85 > > ATT: Write Command (0x52) len 80 > > Handle: 0x0098 Type: ASE Control Point (0x2bc6) > > Data: 010405020206000000000a020103020201030428000602020600000 > > 0000a0201030202010304280001020206000000000a020103020201030428 > > 0002020206000000000a02010302020103042800 > > Opcode: Codec Configuration (0x01) > > Number of ASE(s): 4 > > ASE: #0 > > ASE ID: 0x05 > > Target Latency: Balance Latency/Reliability (0x02) > > PHY: 0x02 > > LE 2M PHY (0x02) > > Codec: LC3 (0x06) > > Codec Specific Configuration: #0: len 0x02 type 0x01 > > Sampling Frequency: 16 Khz (0x03) > > Codec Specific Configuration: #1: len 0x02 type 0x02 > > Frame Duration: 10 ms (0x01) > > Codec Specific Configuration: #2: len 0x03 type 0x04 > > Frame Length: 40 (0x0028) > > ASE: #1 > > ASE ID: 0x06 > > Target Latency: Balance Latency/Reliability (0x02) > > PHY: 0x02 > > LE 2M PHY (0x02) > > Codec: LC3 (0x06) > > Codec Specific Configuration: #0: len 0x02 type 0x01 > > Sampling Frequency: 16 Khz (0x03) > > Codec Specific Configuration: #1: len 0x02 type 0x02 > > Frame Duration: 10 ms (0x01) > > Codec Specific Configuration: #2: len 0x03 type 0x04 > > Frame Length: 40 (0x0028) > > ASE: #2 > > ASE ID: 0x01 > > Target Latency: Balance Latency/Reliability (0x02) > > PHY: 0x02 > > LE 2M PHY (0x02) > > Codec: LC3 (0x06) > > Codec Specific Configuration: #0: len 0x02 type 0x01 > > Sampling Frequency: 16 Khz (0x03) > > Codec Specific Configuration: #1: len 0x02 type 0x02 > > Frame Duration: 10 ms (0x01) > > Codec Specific Configuration: #2: len 0x03 type 0x04 > > Frame Length: 40 (0x0028) > > ASE: #3 > > ASE ID: 0x02 > > Target Latency: Balance Latency/Reliability (0x02) > > PHY: 0x02 > > LE 2M PHY (0x02) > > Codec: LC3 (0x06) > > Codec Specific Configuration: #0: len 0x02 type 0x01 > > Sampling Frequency: 16 Khz (0x03) > > Codec Specific Configuration: #1: len 0x02 type 0x02 > > Frame Duration: 10 ms (0x01) > > Codec Specific Configuration: #2: len 0x03 type 0x04 > > Frame Length: 40 (0x0028) > > > > Fixes: https://github.com/bluez/bluez/issues/612 > > --- > > profiles/audio/bap.c | 6 +-- > > src/shared/bap.c | 87 ++++++++++++++++++++++++++++++++++++++++---- > > src/shared/bap.h | 3 +- > > src/shared/util.c | 43 ++++++++++++++++++++++ > > src/shared/util.h | 6 +++ > > 5 files changed, 132 insertions(+), 13 deletions(-) > > > > diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c > > index 965d7efe6561..16c5faee45f9 100644 > > --- a/profiles/audio/bap.c > > +++ b/profiles/audio/bap.c > > @@ -1290,10 +1290,8 @@ static bool pac_found(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac, > > } > > > > /* TODO: Cache LRU? */ > > - if (btd_service_is_initiator(service)) { > > - if (!bt_bap_select(lpac, rpac, select_cb, ep)) > > - ep->data->selecting++; > > - } > > + if (btd_service_is_initiator(service)) > > + bt_bap_select(lpac, rpac, &ep->data->selecting, select_cb, ep); > > > > return true; > > } > > diff --git a/src/shared/bap.c b/src/shared/bap.c > > index a1495ca84bcc..2450b53232e3 100644 > > --- a/src/shared/bap.c > > +++ b/src/shared/bap.c > > @@ -185,6 +185,7 @@ struct bt_bap_pac { > > struct bt_bap_pac_qos qos; > > struct iovec *data; > > struct iovec *metadata; > > + struct queue *chan_map; > > struct bt_bap_pac_ops *ops; > > void *user_data; > > }; > > @@ -2417,6 +2418,33 @@ static void *ltv_merge(struct iovec *data, struct iovec *cont) > > return iov_append(data, cont->iov_len, cont->iov_base); > > } > > > > +static void bap_pac_foreach_channel(size_t i, uint8_t l, uint8_t t, uint8_t *v, > > + void *user_data) > > +{ > > + struct bt_bap_pac *pac = user_data; > > + > > + if (!v) > > + return; > > + > > + if (!pac->chan_map) > > + pac->chan_map = queue_new(); > > + > > + printf("PAC %p chan_map 0x%02x\n", pac, *v); > > + > > + queue_push_tail(pac->chan_map, UINT_TO_PTR(*v)); > > +} > > + > > +static void bap_pac_update_chan_map(struct bt_bap_pac *pac, struct iovec *data) > > +{ > > + uint8_t type = 0x03; > > + > > + if (!data) > > + return; > > + > > + util_ltv_foreach(data->iov_base, data->iov_len, &type, > > + bap_pac_foreach_channel, pac); > > +} > > Hmm, I though Supported_Audio_Channel_Counts (0x3) is not a channel > mapping, but enumerates what number of channels each PAC supports? > > So e.g. 0x3 = supports 1 or 2 channels, and the PAC could be used to > configure either case. > > > IIUC in BAP v1.0.1 Sec. 4.4 the configuration is supposed to work like > this: > > 1. From the bits set in PACS Sink/Source Locations, select which > channels we are going to configure for sink and source directions. Yep, it is still a wip which I thought it was a good idea to share, so not all details have been sorted out. > 2. Decide which channel goes to which ASE. Actually the ASE and channels don't need to be in a specific order, neither the ASE is location specific. > 3. Supported_Audio_Channel_Counts in PACs limit how many channels we > can put on a single ASE. Yep, that indeed is the case, but there is also the 1:1 relationship with Locations since in that case AC *(i) case where the Number of Channels is set to 0x01 the ChannelAllocation is mandatory in order to differentiate the Left and Right stream for example. > 4. The outcome probably should prefer the standard stream > configurations defined in BAP. Yep, I'm also planning to implement the streaming test cases which includes the AC configuration required, there are quite many though but I hope this cover this well enough to fill where the BAP spec is quite vague in my opinion. > 5. For each ASE Config Codec, set the channel bits in > Audio_Channel_Allocation to indicate which channels the ASE got. > > From the specification I don't quite see how the PACs play role > otherwise in the channel allocation, but maybe I am missing something. > > > I think there's a difficulty in how to split the work between BlueZ and > the sound server here, e.g., if SelectProperties is called many times > how can it set the audio channel allocation correctly. Im playing with the idea of adding another field given to SelectProperties called ChannelAllocation, which the endpoint can choose to use or not, on bluetoothctl I have it detect its presence and automatically add it to the configuration LTV, but I need to check how that gonna play out with the qualification tests to see if that is really the way forward, anyway I don't think solutions using AC *(i) are that great in practice since the intention is to hide the number of devices involved instead of just using something like a coordinate set which are a lot simpler to setup since they actually have only one Location bit set you don't even need to bother with Channel Allocation at all. > > > + > > static void bap_pac_merge(struct bt_bap_pac *pac, struct iovec *data, > > struct iovec *metadata) > > { > > @@ -2426,6 +2454,9 @@ static void bap_pac_merge(struct bt_bap_pac *pac, struct iovec *data, > > else > > pac->data = util_iov_dup(data, 1); > > > > + /* Update channel map */ > > + bap_pac_update_chan_map(pac, data); > > + > > /* Merge metadata into existing record */ > > if (pac->metadata) > > ltv_merge(pac->metadata, metadata); > > @@ -2448,10 +2479,9 @@ static struct bt_bap_pac *bap_pac_new(struct bt_bap_db *bdb, const char *name, > > pac->type = type; > > if (codec) > > pac->codec = *codec; > > - if (data) > > - pac->data = util_iov_dup(data, 1); > > - if (metadata) > > - pac->metadata = util_iov_dup(metadata, 1); > > + > > + bap_pac_merge(pac, data, metadata); > > + > > if (qos) > > pac->qos = *qos; > > > > @@ -2465,6 +2495,7 @@ static void bap_pac_free(void *data) > > free(pac->name); > > util_iov_free(pac->metadata, 1); > > util_iov_free(pac->data, 1); > > + queue_destroy(pac->chan_map, NULL); > > free(pac); > > } > > > > @@ -4505,7 +4536,16 @@ static bool find_ep_pacs(const void *data, const void *user_data) > > if (ep->stream->lpac != match->lpac) > > return false; > > > > - return ep->stream->rpac == match->rpac; > > + if (ep->stream->rpac != match->rpac) > > + return false; > > + > > + switch (ep->state) { > > + case BT_BAP_STREAM_STATE_CONFIG: > > + case BT_BAP_STREAM_STATE_QOS: > > + return true; > > + } > > + > > + return false; > > } > > > > static struct bt_bap_req *bap_req_new(struct bt_bap_stream *stream, > > @@ -4626,16 +4666,47 @@ static bool match_pac(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac, > > } > > > > int bt_bap_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac, > > - bt_bap_pac_select_t func, void *user_data) > > + int *count, bt_bap_pac_select_t func, > > + void *user_data) > > { > > + const struct queue_entry *lchan, *rchan; > > + > > if (!lpac || !rpac || !func) > > return -EINVAL; > > > > if (!lpac->ops || !lpac->ops->select) > > return -EOPNOTSUPP; > > > > - lpac->ops->select(lpac, rpac, &rpac->qos, > > - func, user_data, lpac->user_data); > > + for (lchan = queue_get_entries(lpac->chan_map); lchan; > > + lchan = lchan->next) { > > + uint8_t lmap = PTR_TO_UINT(lchan->data); > > + > > + for (rchan = queue_get_entries(rpac->chan_map); rchan; > > + rchan = rchan->next) { > > + uint8_t rmap = PTR_TO_UINT(rchan->data); > > + > > + printf("lmap 0x%02x rmap 0x%02x\n", lmap, rmap); > > + > > + /* Try matching the channel mapping */ > > + if (lmap & rmap) { > > + lpac->ops->select(lpac, rpac, &rpac->qos, > > + func, user_data, > > + lpac->user_data); > > + if (count) > > + (*count)++; > > + > > + /* Check if there are any channels left */ > > + lmap &= ~(lmap & rmap); > > + if (!lmap) > > + break; > > + > > + /* Check if device require AC*(i) settings */ > > + if (rmap == 0x01) > > + lmap = lmap >> 1; > > + } else > > + break; > > + } > > + } > > > > return 0; > > } > > diff --git a/src/shared/bap.h b/src/shared/bap.h > > index 2c8f9208e6ba..470313e66fc0 100644 > > --- a/src/shared/bap.h > > +++ b/src/shared/bap.h > > @@ -234,7 +234,8 @@ void *bt_bap_pac_get_user_data(struct bt_bap_pac *pac); > > > > /* Stream related functions */ > > int bt_bap_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac, > > - bt_bap_pac_select_t func, void *user_data); > > + int *count, bt_bap_pac_select_t func, > > + void *user_data); > > > > struct bt_bap_stream *bt_bap_stream_new(struct bt_bap *bap, > > struct bt_bap_pac *lpac, > > diff --git a/src/shared/util.c b/src/shared/util.c > > index 34491f4e5a56..c0c2c4a17f12 100644 > > --- a/src/shared/util.c > > +++ b/src/shared/util.c > > @@ -175,6 +175,49 @@ ltv_debugger(const struct util_ltv_debugger *debugger, size_t num, uint8_t type) > > return NULL; > > } > > > > +/* Helper to itertate over LTV entries */ > > +bool util_ltv_foreach(const uint8_t *data, uint8_t len, uint8_t *type, > > + util_ltv_func_t func, void *user_data) > > +{ > > + struct iovec iov; > > + int i; > > + > > + if (!func) > > + return false; > > + > > + iov.iov_base = (void *) data; > > + iov.iov_len = len; > > + > > + for (i = 0; iov.iov_len; i++) { > > + uint8_t l, t, *v; > > + > > + if (!util_iov_pull_u8(&iov, &l)) > > + return false; > > + > > + if (!l) { > > + func(i, l, 0, NULL, user_data); > > + continue; > > + } > > + > > + if (!util_iov_pull_u8(&iov, &t)) > > + return false; > > + > > + l--; > > + > > + if (l) { > > + v = util_iov_pull_mem(&iov, l); > > + if (!v) > > + return false; > > + } else > > + v = NULL; > > + > > + if (!type || *type == t) > > + func(i, l, t, v, user_data); > > + } > > + > > + return true; > > +} > > + > > /* Helper to print debug information of LTV entries */ > > bool util_debug_ltv(const uint8_t *data, uint8_t len, > > const struct util_ltv_debugger *debugger, size_t num, > > diff --git a/src/shared/util.h b/src/shared/util.h > > index 6698d00415de..596663b8519c 100644 > > --- a/src/shared/util.h > > +++ b/src/shared/util.h > > @@ -138,6 +138,12 @@ bool util_debug_ltv(const uint8_t *data, uint8_t len, > > const struct util_ltv_debugger *debugger, size_t num, > > util_debug_func_t function, void *user_data); > > > > +typedef void (*util_ltv_func_t)(size_t i, uint8_t l, uint8_t t, uint8_t *v, > > + void *user_data); > > + > > +bool util_ltv_foreach(const uint8_t *data, uint8_t len, uint8_t *type, > > + util_ltv_func_t func, void *user_data); > > + > > unsigned char util_get_dt(const char *parent, const char *name); > > > > ssize_t util_getrandom(void *buf, size_t buflen, unsigned int flags); > > -- > Pauli Virtanen
Hi, to, 2023-12-07 kello 22:56 -0500, Luiz Augusto von Dentz kirjoitti: > Hi Pauli, > > On Thu, Dec 7, 2023 at 1:00 PM Pauli Virtanen <pav@iki.fi> wrote: > > > > Hi Luiz, > > > > ke, 2023-12-06 kello 17:03 -0500, Luiz Augusto von Dentz kirjoitti: > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > > > > > bt_bap_pac may actually map to multiple PAC records and each may have a > > > different channel count that needs to be matched separately, for > > > instance when trying with EarFun Air Pro: > > > > > > < ACL Data TX: Handle 2048 flags 0x00 dlen 85 > > > ATT: Write Command (0x52) len 80 > > > Handle: 0x0098 Type: ASE Control Point (0x2bc6) > > > Data: 010405020206000000000a020103020201030428000602020600000 > > > 0000a0201030202010304280001020206000000000a020103020201030428 > > > 0002020206000000000a02010302020103042800 > > > Opcode: Codec Configuration (0x01) > > > Number of ASE(s): 4 > > > ASE: #0 > > > ASE ID: 0x05 > > > Target Latency: Balance Latency/Reliability (0x02) > > > PHY: 0x02 > > > LE 2M PHY (0x02) > > > Codec: LC3 (0x06) > > > Codec Specific Configuration: #0: len 0x02 type 0x01 > > > Sampling Frequency: 16 Khz (0x03) > > > Codec Specific Configuration: #1: len 0x02 type 0x02 > > > Frame Duration: 10 ms (0x01) > > > Codec Specific Configuration: #2: len 0x03 type 0x04 > > > Frame Length: 40 (0x0028) > > > ASE: #1 > > > ASE ID: 0x06 > > > Target Latency: Balance Latency/Reliability (0x02) > > > PHY: 0x02 > > > LE 2M PHY (0x02) > > > Codec: LC3 (0x06) > > > Codec Specific Configuration: #0: len 0x02 type 0x01 > > > Sampling Frequency: 16 Khz (0x03) > > > Codec Specific Configuration: #1: len 0x02 type 0x02 > > > Frame Duration: 10 ms (0x01) > > > Codec Specific Configuration: #2: len 0x03 type 0x04 > > > Frame Length: 40 (0x0028) > > > ASE: #2 > > > ASE ID: 0x01 > > > Target Latency: Balance Latency/Reliability (0x02) > > > PHY: 0x02 > > > LE 2M PHY (0x02) > > > Codec: LC3 (0x06) > > > Codec Specific Configuration: #0: len 0x02 type 0x01 > > > Sampling Frequency: 16 Khz (0x03) > > > Codec Specific Configuration: #1: len 0x02 type 0x02 > > > Frame Duration: 10 ms (0x01) > > > Codec Specific Configuration: #2: len 0x03 type 0x04 > > > Frame Length: 40 (0x0028) > > > ASE: #3 > > > ASE ID: 0x02 > > > Target Latency: Balance Latency/Reliability (0x02) > > > PHY: 0x02 > > > LE 2M PHY (0x02) > > > Codec: LC3 (0x06) > > > Codec Specific Configuration: #0: len 0x02 type 0x01 > > > Sampling Frequency: 16 Khz (0x03) > > > Codec Specific Configuration: #1: len 0x02 type 0x02 > > > Frame Duration: 10 ms (0x01) > > > Codec Specific Configuration: #2: len 0x03 type 0x04 > > > Frame Length: 40 (0x0028) > > > > > > Fixes: https://github.com/bluez/bluez/issues/612 > > > --- > > > profiles/audio/bap.c | 6 +-- > > > src/shared/bap.c | 87 ++++++++++++++++++++++++++++++++++++++++---- > > > src/shared/bap.h | 3 +- > > > src/shared/util.c | 43 ++++++++++++++++++++++ > > > src/shared/util.h | 6 +++ > > > 5 files changed, 132 insertions(+), 13 deletions(-) > > > > > > diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c > > > index 965d7efe6561..16c5faee45f9 100644 > > > --- a/profiles/audio/bap.c > > > +++ b/profiles/audio/bap.c > > > @@ -1290,10 +1290,8 @@ static bool pac_found(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac, > > > } > > > > > > /* TODO: Cache LRU? */ > > > - if (btd_service_is_initiator(service)) { > > > - if (!bt_bap_select(lpac, rpac, select_cb, ep)) > > > - ep->data->selecting++; > > > - } > > > + if (btd_service_is_initiator(service)) > > > + bt_bap_select(lpac, rpac, &ep->data->selecting, select_cb, ep); > > > > > > return true; > > > } > > > diff --git a/src/shared/bap.c b/src/shared/bap.c > > > index a1495ca84bcc..2450b53232e3 100644 > > > --- a/src/shared/bap.c > > > +++ b/src/shared/bap.c > > > @@ -185,6 +185,7 @@ struct bt_bap_pac { > > > struct bt_bap_pac_qos qos; > > > struct iovec *data; > > > struct iovec *metadata; > > > + struct queue *chan_map; > > > struct bt_bap_pac_ops *ops; > > > void *user_data; > > > }; > > > @@ -2417,6 +2418,33 @@ static void *ltv_merge(struct iovec *data, struct iovec *cont) > > > return iov_append(data, cont->iov_len, cont->iov_base); > > > } > > > > > > +static void bap_pac_foreach_channel(size_t i, uint8_t l, uint8_t t, uint8_t *v, > > > + void *user_data) > > > +{ > > > + struct bt_bap_pac *pac = user_data; > > > + > > > + if (!v) > > > + return; > > > + > > > + if (!pac->chan_map) > > > + pac->chan_map = queue_new(); > > > + > > > + printf("PAC %p chan_map 0x%02x\n", pac, *v); > > > + > > > + queue_push_tail(pac->chan_map, UINT_TO_PTR(*v)); > > > +} > > > + > > > +static void bap_pac_update_chan_map(struct bt_bap_pac *pac, struct iovec *data) > > > +{ > > > + uint8_t type = 0x03; > > > + > > > + if (!data) > > > + return; > > > + > > > + util_ltv_foreach(data->iov_base, data->iov_len, &type, > > > + bap_pac_foreach_channel, pac); > > > +} > > > > Hmm, I though Supported_Audio_Channel_Counts (0x3) is not a channel > > mapping, but enumerates what number of channels each PAC supports? > > > > So e.g. 0x3 = supports 1 or 2 channels, and the PAC could be used to > > configure either case. > > > > > > IIUC in BAP v1.0.1 Sec. 4.4 the configuration is supposed to work like > > this: > > > > 1. From the bits set in PACS Sink/Source Locations, select which > > channels we are going to configure for sink and source directions. > > Yep, it is still a wip which I thought it was a good idea to share, so > not all details have been sorted out. Right, I missed it's wip. > > 2. Decide which channel goes to which ASE. > > Actually the ASE and channels don't need to be in a specific order, > neither the ASE is location specific. > > > 3. Supported_Audio_Channel_Counts in PACs limit how many channels we > > can put on a single ASE. > > Yep, that indeed is the case, but there is also the 1:1 relationship > with Locations since in that case AC *(i) case where the Number of > Channels is set to 0x01 the ChannelAllocation is mandatory in order to > differentiate the Left and Right stream for example. > > > 4. The outcome probably should prefer the standard stream > > configurations defined in BAP. > > Yep, I'm also planning to implement the streaming test cases which > includes the AC configuration required, there are quite many though > but I hope this cover this well enough to fill where the BAP spec is > quite vague in my opinion. > > > 5. For each ASE Config Codec, set the channel bits in > > Audio_Channel_Allocation to indicate which channels the ASE got. > > > > From the specification I don't quite see how the PACs play role > > otherwise in the channel allocation, but maybe I am missing something. > > > > > > I think there's a difficulty in how to split the work between BlueZ and > > the sound server here, e.g., if SelectProperties is called many times > > how can it set the audio channel allocation correctly. > > Im playing with the idea of adding another field given to > SelectProperties called ChannelAllocation, which the endpoint can > choose to use or not, on bluetoothctl I have it detect its presence > and automatically add it to the configuration LTV, but I need to check > how that gonna play out with the qualification tests to see if that is > really the way forward, anyway I don't think solutions using AC *(i) > are that great in practice since the intention is to hide the number > of devices involved instead of just using something like a coordinate > set which are a lot simpler to setup since they actually have only one > Location bit set you don't even need to bother with Channel Allocation > at all. BlueZ could indeed decide the channel allocation for all ASE beforehand itself, and then do one SelectProperties for each ASE that's going to be configured, and in Locations (or ChannelAllocation) set only those bits that were selected for that ASE. The problem with this is that if there are multiple possible channel selections (eg. device supports many locations, but in BAP standard config we can only select two of them). It probably should be the sound server to decide what to use on per-device basis, probably would need some DBus API addition. The good thing would be that the sound server wouldn't need to think about the ASE configurations. Not clear how the SetConfiguration() API of remote endpoints would work here. Maybe it should take no input parameters at all, and would just restart the SelectProperties dance, to make things simpler? The other alternative could be to punt it all to the sound server, e.g. include NumASE input in SelectProperties(), and have it return configurations and QoS for multiple ASE. > > > > > > + > > > static void bap_pac_merge(struct bt_bap_pac *pac, struct iovec *data, > > > struct iovec *metadata) > > > { > > > @@ -2426,6 +2454,9 @@ static void bap_pac_merge(struct bt_bap_pac *pac, struct iovec *data, > > > else > > > pac->data = util_iov_dup(data, 1); > > > > > > + /* Update channel map */ > > > + bap_pac_update_chan_map(pac, data); > > > + > > > /* Merge metadata into existing record */ > > > if (pac->metadata) > > > ltv_merge(pac->metadata, metadata); > > > @@ -2448,10 +2479,9 @@ static struct bt_bap_pac *bap_pac_new(struct bt_bap_db *bdb, const char *name, > > > pac->type = type; > > > if (codec) > > > pac->codec = *codec; > > > - if (data) > > > - pac->data = util_iov_dup(data, 1); > > > - if (metadata) > > > - pac->metadata = util_iov_dup(metadata, 1); > > > + > > > + bap_pac_merge(pac, data, metadata); > > > + > > > if (qos) > > > pac->qos = *qos; > > > > > > @@ -2465,6 +2495,7 @@ static void bap_pac_free(void *data) > > > free(pac->name); > > > util_iov_free(pac->metadata, 1); > > > util_iov_free(pac->data, 1); > > > + queue_destroy(pac->chan_map, NULL); > > > free(pac); > > > } > > > > > > @@ -4505,7 +4536,16 @@ static bool find_ep_pacs(const void *data, const void *user_data) > > > if (ep->stream->lpac != match->lpac) > > > return false; > > > > > > - return ep->stream->rpac == match->rpac; > > > + if (ep->stream->rpac != match->rpac) > > > + return false; > > > + > > > + switch (ep->state) { > > > + case BT_BAP_STREAM_STATE_CONFIG: > > > + case BT_BAP_STREAM_STATE_QOS: > > > + return true; > > > + } > > > + > > > + return false; > > > } > > > > > > static struct bt_bap_req *bap_req_new(struct bt_bap_stream *stream, > > > @@ -4626,16 +4666,47 @@ static bool match_pac(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac, > > > } > > > > > > int bt_bap_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac, > > > - bt_bap_pac_select_t func, void *user_data) > > > + int *count, bt_bap_pac_select_t func, > > > + void *user_data) > > > { > > > + const struct queue_entry *lchan, *rchan; > > > + > > > if (!lpac || !rpac || !func) > > > return -EINVAL; > > > > > > if (!lpac->ops || !lpac->ops->select) > > > return -EOPNOTSUPP; > > > > > > - lpac->ops->select(lpac, rpac, &rpac->qos, > > > - func, user_data, lpac->user_data); > > > + for (lchan = queue_get_entries(lpac->chan_map); lchan; > > > + lchan = lchan->next) { > > > + uint8_t lmap = PTR_TO_UINT(lchan->data); > > > + > > > + for (rchan = queue_get_entries(rpac->chan_map); rchan; > > > + rchan = rchan->next) { > > > + uint8_t rmap = PTR_TO_UINT(rchan->data); > > > + > > > + printf("lmap 0x%02x rmap 0x%02x\n", lmap, rmap); > > > + > > > + /* Try matching the channel mapping */ > > > + if (lmap & rmap) { > > > + lpac->ops->select(lpac, rpac, &rpac->qos, > > > + func, user_data, > > > + lpac->user_data); > > > + if (count) > > > + (*count)++; > > > + > > > + /* Check if there are any channels left */ > > > + lmap &= ~(lmap & rmap); > > > + if (!lmap) > > > + break; > > > + > > > + /* Check if device require AC*(i) settings */ > > > + if (rmap == 0x01) > > > + lmap = lmap >> 1; > > > + } else > > > + break; > > > + } > > > + } > > > > > > return 0; > > > } > > > diff --git a/src/shared/bap.h b/src/shared/bap.h > > > index 2c8f9208e6ba..470313e66fc0 100644 > > > --- a/src/shared/bap.h > > > +++ b/src/shared/bap.h > > > @@ -234,7 +234,8 @@ void *bt_bap_pac_get_user_data(struct bt_bap_pac *pac); > > > > > > /* Stream related functions */ > > > int bt_bap_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac, > > > - bt_bap_pac_select_t func, void *user_data); > > > + int *count, bt_bap_pac_select_t func, > > > + void *user_data); > > > > > > struct bt_bap_stream *bt_bap_stream_new(struct bt_bap *bap, > > > struct bt_bap_pac *lpac, > > > diff --git a/src/shared/util.c b/src/shared/util.c > > > index 34491f4e5a56..c0c2c4a17f12 100644 > > > --- a/src/shared/util.c > > > +++ b/src/shared/util.c > > > @@ -175,6 +175,49 @@ ltv_debugger(const struct util_ltv_debugger *debugger, size_t num, uint8_t type) > > > return NULL; > > > } > > > > > > +/* Helper to itertate over LTV entries */ > > > +bool util_ltv_foreach(const uint8_t *data, uint8_t len, uint8_t *type, > > > + util_ltv_func_t func, void *user_data) > > > +{ > > > + struct iovec iov; > > > + int i; > > > + > > > + if (!func) > > > + return false; > > > + > > > + iov.iov_base = (void *) data; > > > + iov.iov_len = len; > > > + > > > + for (i = 0; iov.iov_len; i++) { > > > + uint8_t l, t, *v; > > > + > > > + if (!util_iov_pull_u8(&iov, &l)) > > > + return false; > > > + > > > + if (!l) { > > > + func(i, l, 0, NULL, user_data); > > > + continue; > > > + } > > > + > > > + if (!util_iov_pull_u8(&iov, &t)) > > > + return false; > > > + > > > + l--; > > > + > > > + if (l) { > > > + v = util_iov_pull_mem(&iov, l); > > > + if (!v) > > > + return false; > > > + } else > > > + v = NULL; > > > + > > > + if (!type || *type == t) > > > + func(i, l, t, v, user_data); > > > + } > > > + > > > + return true; > > > +} > > > + > > > /* Helper to print debug information of LTV entries */ > > > bool util_debug_ltv(const uint8_t *data, uint8_t len, > > > const struct util_ltv_debugger *debugger, size_t num, > > > diff --git a/src/shared/util.h b/src/shared/util.h > > > index 6698d00415de..596663b8519c 100644 > > > --- a/src/shared/util.h > > > +++ b/src/shared/util.h > > > @@ -138,6 +138,12 @@ bool util_debug_ltv(const uint8_t *data, uint8_t len, > > > const struct util_ltv_debugger *debugger, size_t num, > > > util_debug_func_t function, void *user_data); > > > > > > +typedef void (*util_ltv_func_t)(size_t i, uint8_t l, uint8_t t, uint8_t *v, > > > + void *user_data); > > > + > > > +bool util_ltv_foreach(const uint8_t *data, uint8_t len, uint8_t *type, > > > + util_ltv_func_t func, void *user_data); > > > + > > > unsigned char util_get_dt(const char *parent, const char *name); > > > > > > ssize_t util_getrandom(void *buf, size_t buflen, unsigned int flags); > > > > -- > > Pauli Virtanen > > >
Hi Pauli, On Fri, Dec 8, 2023 at 2:17 PM Pauli Virtanen <pav@iki.fi> wrote: > > Hi, > > to, 2023-12-07 kello 22:56 -0500, Luiz Augusto von Dentz kirjoitti: > > Hi Pauli, > > > > On Thu, Dec 7, 2023 at 1:00 PM Pauli Virtanen <pav@iki.fi> wrote: > > > > > > Hi Luiz, > > > > > > ke, 2023-12-06 kello 17:03 -0500, Luiz Augusto von Dentz kirjoitti: > > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > > > > > > > bt_bap_pac may actually map to multiple PAC records and each may have a > > > > different channel count that needs to be matched separately, for > > > > instance when trying with EarFun Air Pro: > > > > > > > > < ACL Data TX: Handle 2048 flags 0x00 dlen 85 > > > > ATT: Write Command (0x52) len 80 > > > > Handle: 0x0098 Type: ASE Control Point (0x2bc6) > > > > Data: 010405020206000000000a020103020201030428000602020600000 > > > > 0000a0201030202010304280001020206000000000a020103020201030428 > > > > 0002020206000000000a02010302020103042800 > > > > Opcode: Codec Configuration (0x01) > > > > Number of ASE(s): 4 > > > > ASE: #0 > > > > ASE ID: 0x05 > > > > Target Latency: Balance Latency/Reliability (0x02) > > > > PHY: 0x02 > > > > LE 2M PHY (0x02) > > > > Codec: LC3 (0x06) > > > > Codec Specific Configuration: #0: len 0x02 type 0x01 > > > > Sampling Frequency: 16 Khz (0x03) > > > > Codec Specific Configuration: #1: len 0x02 type 0x02 > > > > Frame Duration: 10 ms (0x01) > > > > Codec Specific Configuration: #2: len 0x03 type 0x04 > > > > Frame Length: 40 (0x0028) > > > > ASE: #1 > > > > ASE ID: 0x06 > > > > Target Latency: Balance Latency/Reliability (0x02) > > > > PHY: 0x02 > > > > LE 2M PHY (0x02) > > > > Codec: LC3 (0x06) > > > > Codec Specific Configuration: #0: len 0x02 type 0x01 > > > > Sampling Frequency: 16 Khz (0x03) > > > > Codec Specific Configuration: #1: len 0x02 type 0x02 > > > > Frame Duration: 10 ms (0x01) > > > > Codec Specific Configuration: #2: len 0x03 type 0x04 > > > > Frame Length: 40 (0x0028) > > > > ASE: #2 > > > > ASE ID: 0x01 > > > > Target Latency: Balance Latency/Reliability (0x02) > > > > PHY: 0x02 > > > > LE 2M PHY (0x02) > > > > Codec: LC3 (0x06) > > > > Codec Specific Configuration: #0: len 0x02 type 0x01 > > > > Sampling Frequency: 16 Khz (0x03) > > > > Codec Specific Configuration: #1: len 0x02 type 0x02 > > > > Frame Duration: 10 ms (0x01) > > > > Codec Specific Configuration: #2: len 0x03 type 0x04 > > > > Frame Length: 40 (0x0028) > > > > ASE: #3 > > > > ASE ID: 0x02 > > > > Target Latency: Balance Latency/Reliability (0x02) > > > > PHY: 0x02 > > > > LE 2M PHY (0x02) > > > > Codec: LC3 (0x06) > > > > Codec Specific Configuration: #0: len 0x02 type 0x01 > > > > Sampling Frequency: 16 Khz (0x03) > > > > Codec Specific Configuration: #1: len 0x02 type 0x02 > > > > Frame Duration: 10 ms (0x01) > > > > Codec Specific Configuration: #2: len 0x03 type 0x04 > > > > Frame Length: 40 (0x0028) > > > > > > > > Fixes: https://github.com/bluez/bluez/issues/612 > > > > --- > > > > profiles/audio/bap.c | 6 +-- > > > > src/shared/bap.c | 87 ++++++++++++++++++++++++++++++++++++++++---- > > > > src/shared/bap.h | 3 +- > > > > src/shared/util.c | 43 ++++++++++++++++++++++ > > > > src/shared/util.h | 6 +++ > > > > 5 files changed, 132 insertions(+), 13 deletions(-) > > > > > > > > diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c > > > > index 965d7efe6561..16c5faee45f9 100644 > > > > --- a/profiles/audio/bap.c > > > > +++ b/profiles/audio/bap.c > > > > @@ -1290,10 +1290,8 @@ static bool pac_found(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac, > > > > } > > > > > > > > /* TODO: Cache LRU? */ > > > > - if (btd_service_is_initiator(service)) { > > > > - if (!bt_bap_select(lpac, rpac, select_cb, ep)) > > > > - ep->data->selecting++; > > > > - } > > > > + if (btd_service_is_initiator(service)) > > > > + bt_bap_select(lpac, rpac, &ep->data->selecting, select_cb, ep); > > > > > > > > return true; > > > > } > > > > diff --git a/src/shared/bap.c b/src/shared/bap.c > > > > index a1495ca84bcc..2450b53232e3 100644 > > > > --- a/src/shared/bap.c > > > > +++ b/src/shared/bap.c > > > > @@ -185,6 +185,7 @@ struct bt_bap_pac { > > > > struct bt_bap_pac_qos qos; > > > > struct iovec *data; > > > > struct iovec *metadata; > > > > + struct queue *chan_map; > > > > struct bt_bap_pac_ops *ops; > > > > void *user_data; > > > > }; > > > > @@ -2417,6 +2418,33 @@ static void *ltv_merge(struct iovec *data, struct iovec *cont) > > > > return iov_append(data, cont->iov_len, cont->iov_base); > > > > } > > > > > > > > +static void bap_pac_foreach_channel(size_t i, uint8_t l, uint8_t t, uint8_t *v, > > > > + void *user_data) > > > > +{ > > > > + struct bt_bap_pac *pac = user_data; > > > > + > > > > + if (!v) > > > > + return; > > > > + > > > > + if (!pac->chan_map) > > > > + pac->chan_map = queue_new(); > > > > + > > > > + printf("PAC %p chan_map 0x%02x\n", pac, *v); > > > > + > > > > + queue_push_tail(pac->chan_map, UINT_TO_PTR(*v)); > > > > +} > > > > + > > > > +static void bap_pac_update_chan_map(struct bt_bap_pac *pac, struct iovec *data) > > > > +{ > > > > + uint8_t type = 0x03; > > > > + > > > > + if (!data) > > > > + return; > > > > + > > > > + util_ltv_foreach(data->iov_base, data->iov_len, &type, > > > > + bap_pac_foreach_channel, pac); > > > > +} > > > > > > Hmm, I though Supported_Audio_Channel_Counts (0x3) is not a channel > > > mapping, but enumerates what number of channels each PAC supports? > > > > > > So e.g. 0x3 = supports 1 or 2 channels, and the PAC could be used to > > > configure either case. > > > > > > > > > IIUC in BAP v1.0.1 Sec. 4.4 the configuration is supposed to work like > > > this: > > > > > > 1. From the bits set in PACS Sink/Source Locations, select which > > > channels we are going to configure for sink and source directions. > > > > Yep, it is still a wip which I thought it was a good idea to share, so > > not all details have been sorted out. > > Right, I missed it's wip. > > > > 2. Decide which channel goes to which ASE. > > > > Actually the ASE and channels don't need to be in a specific order, > > neither the ASE is location specific. > > > > > 3. Supported_Audio_Channel_Counts in PACs limit how many channels we > > > can put on a single ASE. > > > > Yep, that indeed is the case, but there is also the 1:1 relationship > > with Locations since in that case AC *(i) case where the Number of > > Channels is set to 0x01 the ChannelAllocation is mandatory in order to > > differentiate the Left and Right stream for example. > > > > > 4. The outcome probably should prefer the standard stream > > > configurations defined in BAP. > > > > Yep, I'm also planning to implement the streaming test cases which > > includes the AC configuration required, there are quite many though > > but I hope this cover this well enough to fill where the BAP spec is > > quite vague in my opinion. > > > > > 5. For each ASE Config Codec, set the channel bits in > > > Audio_Channel_Allocation to indicate which channels the ASE got. > > > > > > From the specification I don't quite see how the PACs play role > > > otherwise in the channel allocation, but maybe I am missing something. > > > > > > > > > I think there's a difficulty in how to split the work between BlueZ and > > > the sound server here, e.g., if SelectProperties is called many times > > > how can it set the audio channel allocation correctly. > > > > Im playing with the idea of adding another field given to > > SelectProperties called ChannelAllocation, which the endpoint can > > choose to use or not, on bluetoothctl I have it detect its presence > > and automatically add it to the configuration LTV, but I need to check > > how that gonna play out with the qualification tests to see if that is > > really the way forward, anyway I don't think solutions using AC *(i) > > are that great in practice since the intention is to hide the number > > of devices involved instead of just using something like a coordinate > > set which are a lot simpler to setup since they actually have only one > > Location bit set you don't even need to bother with Channel Allocation > > at all. > > BlueZ could indeed decide the channel allocation for all ASE beforehand > itself, and then do one SelectProperties for each ASE that's going to > be configured, and in Locations (or ChannelAllocation) set only those > bits that were selected for that ASE. > > The problem with this is that if there are multiple possible channel > selections (eg. device supports many locations, but in BAP standard > config we can only select two of them). It probably should be the sound > server to decide what to use on per-device basis, probably would need > some DBus API addition. Yes, this is valid feedback that perhaps we could introduce a property saying that Channel Allocation shall not be done automatically, or perhaps if we have a way to generalize algorithms to do the matching we could have the endpoint state what is the preferred algorithm it wants to use to allocate channels. > The good thing would be that the sound server wouldn't need to think > about the ASE configurations. > > Not clear how the SetConfiguration() API of remote endpoints would work > here. Maybe it should take no input parameters at all, and would just > restart the SelectProperties dance, to make things simpler? > > > The other alternative could be to punt it all to the sound server, e.g. > include NumASE input in SelectProperties(), and have it return > configurations and QoS for multiple ASE. I'd rather not do that, at least not by default, because that may have an impact on qualification tests based on how the audio subsystem behaves, otherwise we would need to start subjecting the audio server to qualification tests which is not very easy to automate, beside there could be different combinations of BlueZ + audio servers so we would have to duplicate tests, etc. > > > > > > > > > + > > > > static void bap_pac_merge(struct bt_bap_pac *pac, struct iovec *data, > > > > struct iovec *metadata) > > > > { > > > > @@ -2426,6 +2454,9 @@ static void bap_pac_merge(struct bt_bap_pac *pac, struct iovec *data, > > > > else > > > > pac->data = util_iov_dup(data, 1); > > > > > > > > + /* Update channel map */ > > > > + bap_pac_update_chan_map(pac, data); > > > > + > > > > /* Merge metadata into existing record */ > > > > if (pac->metadata) > > > > ltv_merge(pac->metadata, metadata); > > > > @@ -2448,10 +2479,9 @@ static struct bt_bap_pac *bap_pac_new(struct bt_bap_db *bdb, const char *name, > > > > pac->type = type; > > > > if (codec) > > > > pac->codec = *codec; > > > > - if (data) > > > > - pac->data = util_iov_dup(data, 1); > > > > - if (metadata) > > > > - pac->metadata = util_iov_dup(metadata, 1); > > > > + > > > > + bap_pac_merge(pac, data, metadata); > > > > + > > > > if (qos) > > > > pac->qos = *qos; > > > > > > > > @@ -2465,6 +2495,7 @@ static void bap_pac_free(void *data) > > > > free(pac->name); > > > > util_iov_free(pac->metadata, 1); > > > > util_iov_free(pac->data, 1); > > > > + queue_destroy(pac->chan_map, NULL); > > > > free(pac); > > > > } > > > > > > > > @@ -4505,7 +4536,16 @@ static bool find_ep_pacs(const void *data, const void *user_data) > > > > if (ep->stream->lpac != match->lpac) > > > > return false; > > > > > > > > - return ep->stream->rpac == match->rpac; > > > > + if (ep->stream->rpac != match->rpac) > > > > + return false; > > > > + > > > > + switch (ep->state) { > > > > + case BT_BAP_STREAM_STATE_CONFIG: > > > > + case BT_BAP_STREAM_STATE_QOS: > > > > + return true; > > > > + } > > > > + > > > > + return false; > > > > } > > > > > > > > static struct bt_bap_req *bap_req_new(struct bt_bap_stream *stream, > > > > @@ -4626,16 +4666,47 @@ static bool match_pac(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac, > > > > } > > > > > > > > int bt_bap_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac, > > > > - bt_bap_pac_select_t func, void *user_data) > > > > + int *count, bt_bap_pac_select_t func, > > > > + void *user_data) > > > > { > > > > + const struct queue_entry *lchan, *rchan; > > > > + > > > > if (!lpac || !rpac || !func) > > > > return -EINVAL; > > > > > > > > if (!lpac->ops || !lpac->ops->select) > > > > return -EOPNOTSUPP; > > > > > > > > - lpac->ops->select(lpac, rpac, &rpac->qos, > > > > - func, user_data, lpac->user_data); > > > > + for (lchan = queue_get_entries(lpac->chan_map); lchan; > > > > + lchan = lchan->next) { > > > > + uint8_t lmap = PTR_TO_UINT(lchan->data); > > > > + > > > > + for (rchan = queue_get_entries(rpac->chan_map); rchan; > > > > + rchan = rchan->next) { > > > > + uint8_t rmap = PTR_TO_UINT(rchan->data); > > > > + > > > > + printf("lmap 0x%02x rmap 0x%02x\n", lmap, rmap); > > > > + > > > > + /* Try matching the channel mapping */ > > > > + if (lmap & rmap) { > > > > + lpac->ops->select(lpac, rpac, &rpac->qos, > > > > + func, user_data, > > > > + lpac->user_data); > > > > + if (count) > > > > + (*count)++; > > > > + > > > > + /* Check if there are any channels left */ > > > > + lmap &= ~(lmap & rmap); > > > > + if (!lmap) > > > > + break; > > > > + > > > > + /* Check if device require AC*(i) settings */ > > > > + if (rmap == 0x01) > > > > + lmap = lmap >> 1; > > > > + } else > > > > + break; > > > > + } > > > > + } > > > > > > > > return 0; > > > > } > > > > diff --git a/src/shared/bap.h b/src/shared/bap.h > > > > index 2c8f9208e6ba..470313e66fc0 100644 > > > > --- a/src/shared/bap.h > > > > +++ b/src/shared/bap.h > > > > @@ -234,7 +234,8 @@ void *bt_bap_pac_get_user_data(struct bt_bap_pac *pac); > > > > > > > > /* Stream related functions */ > > > > int bt_bap_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac, > > > > - bt_bap_pac_select_t func, void *user_data); > > > > + int *count, bt_bap_pac_select_t func, > > > > + void *user_data); > > > > > > > > struct bt_bap_stream *bt_bap_stream_new(struct bt_bap *bap, > > > > struct bt_bap_pac *lpac, > > > > diff --git a/src/shared/util.c b/src/shared/util.c > > > > index 34491f4e5a56..c0c2c4a17f12 100644 > > > > --- a/src/shared/util.c > > > > +++ b/src/shared/util.c > > > > @@ -175,6 +175,49 @@ ltv_debugger(const struct util_ltv_debugger *debugger, size_t num, uint8_t type) > > > > return NULL; > > > > } > > > > > > > > +/* Helper to itertate over LTV entries */ > > > > +bool util_ltv_foreach(const uint8_t *data, uint8_t len, uint8_t *type, > > > > + util_ltv_func_t func, void *user_data) > > > > +{ > > > > + struct iovec iov; > > > > + int i; > > > > + > > > > + if (!func) > > > > + return false; > > > > + > > > > + iov.iov_base = (void *) data; > > > > + iov.iov_len = len; > > > > + > > > > + for (i = 0; iov.iov_len; i++) { > > > > + uint8_t l, t, *v; > > > > + > > > > + if (!util_iov_pull_u8(&iov, &l)) > > > > + return false; > > > > + > > > > + if (!l) { > > > > + func(i, l, 0, NULL, user_data); > > > > + continue; > > > > + } > > > > + > > > > + if (!util_iov_pull_u8(&iov, &t)) > > > > + return false; > > > > + > > > > + l--; > > > > + > > > > + if (l) { > > > > + v = util_iov_pull_mem(&iov, l); > > > > + if (!v) > > > > + return false; > > > > + } else > > > > + v = NULL; > > > > + > > > > + if (!type || *type == t) > > > > + func(i, l, t, v, user_data); > > > > + } > > > > + > > > > + return true; > > > > +} > > > > + > > > > /* Helper to print debug information of LTV entries */ > > > > bool util_debug_ltv(const uint8_t *data, uint8_t len, > > > > const struct util_ltv_debugger *debugger, size_t num, > > > > diff --git a/src/shared/util.h b/src/shared/util.h > > > > index 6698d00415de..596663b8519c 100644 > > > > --- a/src/shared/util.h > > > > +++ b/src/shared/util.h > > > > @@ -138,6 +138,12 @@ bool util_debug_ltv(const uint8_t *data, uint8_t len, > > > > const struct util_ltv_debugger *debugger, size_t num, > > > > util_debug_func_t function, void *user_data); > > > > > > > > +typedef void (*util_ltv_func_t)(size_t i, uint8_t l, uint8_t t, uint8_t *v, > > > > + void *user_data); > > > > + > > > > +bool util_ltv_foreach(const uint8_t *data, uint8_t len, uint8_t *type, > > > > + util_ltv_func_t func, void *user_data); > > > > + > > > > unsigned char util_get_dt(const char *parent, const char *name); > > > > > > > > ssize_t util_getrandom(void *buf, size_t buflen, unsigned int flags); > > > > > > -- > > > Pauli Virtanen > > > > > > > >
diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c index c279b5b0e133..965d7efe6561 100644 --- a/profiles/audio/bap.c +++ b/profiles/audio/bap.c @@ -62,22 +62,27 @@ #define MEDIA_ENDPOINT_INTERFACE "org.bluez.MediaEndpoint1" #define MEDIA_INTERFACE "org.bluez.Media1" -struct bap_ep { - char *path; - struct bap_data *data; - struct bt_bap_pac *lpac; - struct bt_bap_pac *rpac; +struct bap_setup { + struct bap_ep *ep; struct bt_bap_stream *stream; + struct bt_bap_qos qos; GIOChannel *io; unsigned int io_id; bool recreate; bool cig_active; struct iovec *caps; struct iovec *metadata; - struct bt_bap_qos qos; unsigned int id; - DBusMessage *msg; struct iovec *base; + DBusMessage *msg; +}; + +struct bap_ep { + char *path; + struct bap_data *data; + struct bt_bap_pac *lpac; + struct bt_bap_pac *rpac; + struct queue *setups; }; struct bap_data { @@ -728,84 +733,131 @@ fail: static void qos_cb(struct bt_bap_stream *stream, uint8_t code, uint8_t reason, void *user_data) { - struct bap_ep *ep = user_data; + struct bap_setup *setup = user_data; DBusMessage *reply; DBG("stream %p code 0x%02x reason 0x%02x", stream, code, reason); - ep->id = 0; + setup->id = 0; - if (!ep->msg) + if (!setup->msg) return; if (!code) - reply = dbus_message_new_method_return(ep->msg); + reply = dbus_message_new_method_return(setup->msg); else - reply = btd_error_failed(ep->msg, "Unable to configure"); + reply = btd_error_failed(setup->msg, "Unable to configure"); g_dbus_send_message(btd_get_dbus_connection(), reply); - dbus_message_unref(ep->msg); - ep->msg = NULL; + dbus_message_unref(setup->msg); + setup->msg = NULL; } static void config_cb(struct bt_bap_stream *stream, uint8_t code, uint8_t reason, void *user_data) { - struct bap_ep *ep = user_data; + struct bap_setup *setup = user_data; DBusMessage *reply; DBG("stream %p code 0x%02x reason 0x%02x", stream, code, reason); - ep->id = 0; + setup->id = 0; if (!code) return; - if (!ep->msg) + if (!setup->msg) return; - reply = btd_error_failed(ep->msg, "Unable to configure"); + reply = btd_error_failed(setup->msg, "Unable to configure"); g_dbus_send_message(btd_get_dbus_connection(), reply); - dbus_message_unref(ep->msg); - ep->msg = NULL; + dbus_message_unref(setup->msg); + setup->msg = NULL; } -static void bap_io_close(struct bap_ep *ep) +static void setup_io_close(void *data, void *user_data) { + struct bap_setup *setup = data; int fd; - if (ep->io_id) { - g_source_remove(ep->io_id); - ep->io_id = 0; + if (setup->io_id) { + g_source_remove(setup->io_id); + setup->io_id = 0; } - if (!ep->io) + if (!setup->io) return; - DBG("ep %p", ep); + DBG("setup %p", setup); - fd = g_io_channel_unix_get_fd(ep->io); + fd = g_io_channel_unix_get_fd(setup->io); close(fd); - g_io_channel_unref(ep->io); - ep->io = NULL; - ep->cig_active = false; + g_io_channel_unref(setup->io); + setup->io = NULL; + setup->cig_active = false; + + bt_bap_stream_io_connecting(setup->stream, -1); +} + +static void ep_close(struct bap_ep *ep) +{ + if (!ep) + return; + + queue_foreach(ep->setups, setup_io_close, NULL); +} + +static struct bap_setup *setup_new(struct bap_ep *ep) +{ + struct bap_setup *setup; + + setup = new0(struct bap_setup, 1); + setup->ep = ep; + + if (!ep->setups) + ep->setups = queue_new(); + + queue_push_tail(ep->setups, setup); + + DBG("ep %p setup %p", ep, setup); + + return setup; +} + +static void setup_free(void *data) +{ + struct bap_setup *setup = data; + + DBG("%p", setup); + + if (setup->ep) + queue_remove(setup->ep->setups, setup); + + setup_io_close(setup, NULL); + + util_iov_free(setup->caps, 1); + util_iov_free(setup->metadata, 1); + util_iov_free(setup->base, 1); + + if (bt_bap_stream_get_type(setup->stream) == BT_BAP_STREAM_TYPE_BCAST) + util_iov_free(setup->qos.bcast.bcode, 1); + + free(setup); } static DBusMessage *set_configuration(DBusConnection *conn, DBusMessage *msg, void *data) { struct bap_ep *ep = data; + struct bap_setup *setup; const char *path; DBusMessageIter args, props; - if (ep->msg) - return btd_error_busy(msg); - dbus_message_iter_init(msg, &args); dbus_message_iter_get_basic(&args, &path); @@ -815,59 +867,55 @@ static DBusMessage *set_configuration(DBusConnection *conn, DBusMessage *msg, if (dbus_message_iter_get_arg_type(&props) != DBUS_TYPE_DICT_ENTRY) return btd_error_invalid_args(msg); - /* Disconnect IO if connecting since QoS is going to be reconfigured */ - if (bt_bap_stream_io_is_connecting(ep->stream, NULL)) { - bap_io_close(ep); - bt_bap_stream_io_connecting(ep->stream, -1); - } + /* Disconnect IOs if connecting since QoS is going to be reconfigured */ + ep_close(ep); + + setup = setup_new(ep); if (bt_bap_pac_get_type(ep->lpac) == BT_BAP_BCAST_SOURCE) { /* Mark BIG and BIS to be auto assigned */ - ep->qos.bcast.big = BT_ISO_QOS_BIG_UNSET; - ep->qos.bcast.bis = BT_ISO_QOS_BIS_UNSET; + setup->qos.bcast.big = BT_ISO_QOS_BIG_UNSET; + setup->qos.bcast.bis = BT_ISO_QOS_BIS_UNSET; } else { /* Mark CIG and CIS to be auto assigned */ - ep->qos.ucast.cig_id = BT_ISO_QOS_CIG_UNSET; - ep->qos.ucast.cis_id = BT_ISO_QOS_CIS_UNSET; + setup->qos.ucast.cig_id = BT_ISO_QOS_CIG_UNSET; + setup->qos.ucast.cis_id = BT_ISO_QOS_CIS_UNSET; } - if (parse_configuration(&props, &ep->caps, &ep->metadata, - &ep->base, &ep->qos) < 0) { + if (parse_configuration(&props, &setup->caps, &setup->metadata, + &setup->base, &setup->qos) < 0) { DBG("Unable to parse configuration"); + setup_free(setup); return btd_error_invalid_args(msg); } - /* TODO: Check if stream capabilities match add support for Latency - * and PHY. - */ - if (!ep->stream) - ep->stream = bt_bap_stream_new(ep->data->bap, ep->lpac, - ep->rpac, &ep->qos, ep->caps); + setup->stream = bt_bap_stream_new(ep->data->bap, ep->lpac, ep->rpac, + &setup->qos, setup->caps); - ep->id = bt_bap_stream_config(ep->stream, &ep->qos, ep->caps, - config_cb, ep); - if (!ep->id) { + setup->id = bt_bap_stream_config(setup->stream, &setup->qos, + setup->caps, config_cb, ep); + if (!setup->id) { DBG("Unable to config stream"); - free(ep->caps); - ep->caps = NULL; + setup_free(setup); return btd_error_invalid_args(msg); } - bt_bap_stream_set_user_data(ep->stream, ep->path); + bt_bap_stream_set_user_data(setup->stream, ep->path); - if (ep->metadata && ep->metadata->iov_len) - bt_bap_stream_metadata(ep->stream, ep->metadata, NULL, NULL); + if (setup->metadata && setup->metadata->iov_len) + bt_bap_stream_metadata(setup->stream, setup->metadata, NULL, + NULL); - switch (bt_bap_stream_get_type(ep->stream)) { + switch (bt_bap_stream_get_type(setup->stream)) { case BT_BAP_STREAM_TYPE_UCAST: - ep->msg = dbus_message_ref(msg); + setup->msg = dbus_message_ref(msg); break; case BT_BAP_STREAM_TYPE_BCAST: /* No message sent over the air for broadcast */ if (bt_bap_pac_get_type(ep->lpac) == BT_BAP_BCAST_SINK) - ep->msg = dbus_message_ref(msg); + setup->msg = dbus_message_ref(msg); else - ep->id = 0; + setup->id = 0; return g_dbus_create_reply(msg, DBUS_TYPE_INVALID); } @@ -901,20 +949,14 @@ static void update_bcast_qos(struct bt_iso_qos *qos, sizeof(qos->bcast.bcode)); } -static bool match_ep_type(const void *data, const void *user_data) -{ - const struct bap_ep *ep = data; - - return (bt_bap_pac_get_type(ep->lpac) == PTR_TO_INT(user_data)); -} - static void iso_bcast_confirm_cb(GIOChannel *io, GError *err, void *user_data) { - struct bap_data *data = user_data; + struct bap_setup *setup = user_data; + struct bap_ep *ep = setup->ep; + struct bap_data *data = ep->data; struct bt_iso_qos qos; struct bt_iso_base base; char address[18]; - struct bap_ep *ep; int fd; struct iovec *base_io; uint32_t presDelay; @@ -938,32 +980,28 @@ static void iso_bcast_confirm_cb(GIOChannel *io, GError *err, void *user_data) DBG("BCAST ISO: sync with %s (BIG 0x%02x BIS 0x%02x)", address, qos.bcast.big, qos.bcast.bis); - ep = queue_find(data->bcast, match_ep_type, - INT_TO_PTR(BT_BAP_BCAST_SINK)); - if (!ep) - return; - - update_bcast_qos(&qos, &ep->qos); + update_bcast_qos(&qos, &setup->qos); base_io = new0(struct iovec, 1); util_iov_memcpy(base_io, base.base, base.base_len); parse_base(base_io->iov_base, base_io->iov_len, bap_debug, &presDelay, &numSubgroups, &numBis, - &codec, &ep->caps, &ep->metadata); + &codec, &setup->caps, &setup->metadata); /* Update pac with BASE information */ - bt_bap_update_bcast_source(ep->rpac, &codec, ep->caps, ep->metadata); - ep->id = bt_bap_stream_config(ep->stream, &ep->qos, - ep->caps, NULL, NULL); + bt_bap_update_bcast_source(ep->rpac, &codec, setup->caps, + setup->metadata); + setup->id = bt_bap_stream_config(setup->stream, &setup->qos, + setup->caps, NULL, NULL); data->listen_io = io; - bt_bap_stream_set_user_data(ep->stream, ep->path); + bt_bap_stream_set_user_data(setup->stream, ep->path); fd = g_io_channel_unix_get_fd(io); - if (bt_bap_stream_set_io(ep->stream, fd)) { - bt_bap_stream_enable(ep->stream, true, NULL, NULL, NULL); + if (bt_bap_stream_set_io(setup->stream, fd)) { + bt_bap_stream_enable(setup->stream, true, NULL, NULL, NULL); g_io_channel_set_close_on_unref(io, FALSE); return; } @@ -1008,16 +1046,10 @@ static const GDBusMethodTable ep_methods[] = { static void ep_free(void *data) { struct bap_ep *ep = data; + struct queue *setups = ep->setups; - if (ep->id) - bt_bap_stream_cancel(ep->stream, ep->id); - - bap_io_close(ep); - - util_iov_free(ep->caps, 1); - util_iov_free(ep->metadata, 1); - if (bt_bap_stream_get_type(ep->stream) == BT_BAP_STREAM_TYPE_BCAST) - util_iov_free(ep->qos.bcast.bcode, 1); + ep->setups = NULL; + queue_destroy(setups, setup_free); free(ep->path); free(ep); } @@ -1077,12 +1109,10 @@ static struct bap_ep *ep_register_bcast(struct bap_data *data, case BT_BAP_BCAST_SOURCE: err = asprintf(&ep->path, "%s/pac_%s%d", adapter_get_path(adapter), suffix, i); - ep->base = new0(struct iovec, 1); break; case BT_BAP_BCAST_SINK: err = asprintf(&ep->path, "%s/pac_%s%d", device_get_path(device), suffix, i); - ep->base = new0(struct iovec, 1); break; } @@ -1181,33 +1211,38 @@ static struct bap_ep *ep_register(struct btd_service *service, return ep; } -static void bap_config(void *data, void *user_data) +static void setup_config(void *data, void *user_data) { - struct bap_ep *ep = data; + struct bap_setup *setup = data; + struct bap_ep *ep = setup->ep; - DBG("ep %p caps %p metadata %p", ep, ep->caps, ep->metadata); - - if (!ep->caps) - return; + DBG("setup %p caps %p metadata %p", setup, setup->caps, + setup->metadata); /* TODO: Check if stream capabilities match add support for Latency * and PHY. */ - if (!ep->stream) - ep->stream = bt_bap_stream_new(ep->data->bap, ep->lpac, - ep->rpac, &ep->qos, ep->caps); + if (!setup->stream) + setup->stream = bt_bap_stream_new(ep->data->bap, ep->lpac, + ep->rpac, &setup->qos, + setup->caps); - ep->id = bt_bap_stream_config(ep->stream, &ep->qos, ep->caps, - config_cb, ep); - if (!ep->id) { + setup->id = bt_bap_stream_config(setup->stream, &setup->qos, + setup->caps, config_cb, setup); + if (!setup->id) { DBG("Unable to config stream"); - util_iov_free(ep->caps, 1); - ep->caps = NULL; - util_iov_free(ep->metadata, 1); - ep->metadata = NULL; + setup_free(setup); + return; } - bt_bap_stream_set_user_data(ep->stream, ep->path); + bt_bap_stream_set_user_data(setup->stream, ep->path); +} + +static void bap_config(void *data, void *user_data) +{ + struct bap_ep *ep = data; + + queue_foreach(ep->setups, setup_config, NULL); } static void select_cb(struct bt_bap_pac *pac, int err, struct iovec *caps, @@ -1215,6 +1250,7 @@ static void select_cb(struct bt_bap_pac *pac, int err, struct iovec *caps, void *user_data) { struct bap_ep *ep = user_data; + struct bap_setup *setup; if (err) { error("err %d", err); @@ -1222,15 +1258,10 @@ static void select_cb(struct bt_bap_pac *pac, int err, struct iovec *caps, goto done; } - util_iov_free(ep->caps, 1); - ep->caps = util_iov_dup(caps, 1); - - if (metadata && metadata->iov_base && metadata->iov_len) { - ep->metadata = util_iov_dup(metadata, 1); - bt_bap_stream_metadata(ep->stream, ep->metadata, NULL, NULL); - } - - ep->qos = *qos; + setup = setup_new(ep); + setup->caps = util_iov_dup(caps, 1); + setup->metadata = util_iov_dup(metadata, 1); + setup->qos = *qos; DBG("selecting %d", ep->data->selecting); ep->data->selecting--; @@ -1293,28 +1324,41 @@ static void bap_ready(struct bt_bap *bap, void *user_data) bt_bap_foreach_pac(bap, BT_BAP_SINK, pac_found, service); } -static bool match_ep_by_stream(const void *data, const void *user_data) +static bool match_setup_stream(const void *data, const void *user_data) +{ + const struct bap_setup *setup = data; + const struct bt_bap_stream *stream = user_data; + + return setup->stream == stream; +} + +static bool match_ep_stream(const void *data, const void *user_data) { const struct bap_ep *ep = data; const struct bt_bap_stream *stream = user_data; - return ep->stream == stream; + return queue_find(ep->setups, match_setup_stream, stream); } -static struct bap_ep *bap_find_ep_by_stream(struct bap_data *data, +static struct bap_setup *bap_find_setup_by_stream(struct bap_data *data, struct bt_bap_stream *stream) { struct bap_ep *ep; switch (bt_bap_stream_get_type(stream)) { case BT_BAP_STREAM_TYPE_UCAST: - ep = queue_find(data->snks, match_ep_by_stream, stream); + ep = queue_find(data->snks, match_ep_stream, stream); if (ep) - return ep; + return queue_find(ep->setups, match_setup_stream, + stream); - return queue_find(data->srcs, match_ep_by_stream, stream); + ep = queue_find(data->srcs, match_ep_stream, stream); + if (ep) + return queue_find(ep->setups, match_setup_stream, + stream); + break; case BT_BAP_STREAM_TYPE_BCAST: - return queue_find(data->bcast, match_ep_by_stream, stream); + return queue_find(data->bcast, match_ep_stream, stream); } return NULL; @@ -1435,8 +1479,9 @@ drop: g_io_channel_shutdown(io, TRUE, NULL); } -static void bap_accept_io(struct bap_ep *ep, struct bt_bap_stream *stream, - int fd, int defer) +static void setup_accept_io(struct bap_setup *setup, + struct bt_bap_stream *stream, + int fd, int defer) { char c; struct pollfd pfd; @@ -1472,7 +1517,7 @@ static void bap_accept_io(struct bap_ep *ep, struct bt_bap_stream *stream, } } - ep->cig_active = true; + setup->cig_active = true; return; @@ -1485,12 +1530,20 @@ struct cig_busy_data { uint8_t cig; }; +static bool match_cig_active(const void *data, const void *match_data) +{ + const struct bap_setup *setup = data; + const struct cig_busy_data *info = match_data; + + return (setup->qos.ucast.cig_id == info->cig) && setup->cig_active; +} + static bool cig_busy_ep(const void *data, const void *match_data) { const struct bap_ep *ep = data; const struct cig_busy_data *info = match_data; - return (ep->qos.ucast.cig_id == info->cig) && ep->cig_active; + return queue_find(ep->setups, match_cig_active, info); } static bool cig_busy_session(const void *data, const void *match_data) @@ -1518,32 +1571,40 @@ static bool is_cig_busy(struct bap_data *data, uint8_t cig) return queue_find(sessions, cig_busy_session, &info); } -static void bap_create_io(struct bap_data *data, struct bap_ep *ep, +static void setup_create_io(struct bap_data *data, struct bap_setup *setup, struct bt_bap_stream *stream, int defer); -static gboolean bap_io_recreate(void *user_data) +static gboolean setup_io_recreate(void *user_data) { - struct bap_ep *ep = user_data; + struct bap_setup *setup = user_data; - DBG("ep %p", ep); + DBG("%p", setup); - ep->io_id = 0; + setup->io_id = 0; - bap_create_io(ep->data, ep, ep->stream, true); + setup_create_io(setup->ep->data, setup, setup->stream, true); return FALSE; } -static void recreate_cig_ep(void *data, void *match_data) +static void setup_recreate(void *data, void *match_data) { - struct bap_ep *ep = (struct bap_ep *)data; + struct bap_setup *setup = data; struct cig_busy_data *info = match_data; - if (ep->qos.ucast.cig_id != info->cig || !ep->recreate || ep->io_id) + if (setup->qos.ucast.cig_id != info->cig || !setup->recreate || + setup->io_id) return; - ep->recreate = false; - ep->io_id = g_idle_add(bap_io_recreate, ep); + setup->recreate = false; + setup->io_id = g_idle_add(setup_io_recreate, setup); +} + +static void recreate_cig_ep(void *data, void *match_data) +{ + struct bap_ep *ep = data; + + queue_foreach(ep->setups, setup_recreate, match_data); } static void recreate_cig_session(void *data, void *match_data) @@ -1558,38 +1619,39 @@ static void recreate_cig_session(void *data, void *match_data) queue_foreach(session->srcs, recreate_cig_ep, match_data); } -static void recreate_cig(struct bap_ep *ep) +static void recreate_cig(struct bap_setup *setup) { - struct bap_data *data = ep->data; + struct bap_data *data = setup->ep->data; struct cig_busy_data info; info.adapter = device_get_adapter(data->device); - info.cig = ep->qos.ucast.cig_id; + info.cig = setup->qos.ucast.cig_id; - DBG("adapter %p ep %p recreate CIG %d", info.adapter, ep, info.cig); + DBG("adapter %p setup %p recreate CIG %d", info.adapter, setup, + info.cig); - if (ep->qos.ucast.cig_id == BT_ISO_QOS_CIG_UNSET) { - recreate_cig_ep(ep, &info); + if (setup->qos.ucast.cig_id == BT_ISO_QOS_CIG_UNSET) { + recreate_cig_ep(setup->ep, &info); return; } queue_foreach(sessions, recreate_cig_session, &info); } -static gboolean bap_io_disconnected(GIOChannel *io, GIOCondition cond, +static gboolean setup_io_disconnected(GIOChannel *io, GIOCondition cond, gpointer user_data) { - struct bap_ep *ep = user_data; + struct bap_setup *setup = user_data; - DBG("ep %p recreate %s", ep, ep->recreate ? "true" : "false"); + DBG("%p recreate %s", setup, setup->recreate ? "true" : "false"); - ep->io_id = 0; + setup->io_id = 0; - bap_io_close(ep); + setup_io_close(setup, NULL); /* Check if connecting recreate IO */ - if (!is_cig_busy(ep->data, ep->qos.ucast.cig_id)) - recreate_cig(ep); + if (!is_cig_busy(setup->ep->data, setup->qos.ucast.cig_id)) + recreate_cig(setup); return FALSE; } @@ -1597,25 +1659,25 @@ static gboolean bap_io_disconnected(GIOChannel *io, GIOCondition cond, static void bap_connect_bcast_io_cb(GIOChannel *chan, GError *err, gpointer user_data) { - struct bap_ep *ep = user_data; + struct bap_setup *setup = user_data; - if (!ep->stream) + if (!setup->stream) return; - iso_connect_bcast_cb(chan, err, ep->stream); + iso_connect_bcast_cb(chan, err, setup->stream); } static void bap_connect_io_cb(GIOChannel *chan, GError *err, gpointer user_data) { - struct bap_ep *ep = user_data; + struct bap_setup *setup = user_data; - if (!ep->stream) + if (!setup->stream) return; - iso_connect_cb(chan, err, ep->stream); + iso_connect_cb(chan, err, setup->stream); } -static void bap_connect_io(struct bap_data *data, struct bap_ep *ep, +static void setup_connect_io(struct bap_data *data, struct bap_setup *setup, struct bt_bap_stream *stream, struct bt_iso_qos *qos, int defer) { @@ -1626,39 +1688,40 @@ static void bap_connect_io(struct bap_data *data, struct bap_ep *ep, /* If IO already set skip creating it again */ if (bt_bap_stream_get_io(stream)) { - DBG("ep %p stream %p has existing io", ep, stream); + DBG("setup %p stream %p has existing io", setup, stream); return; } if (bt_bap_stream_io_is_connecting(stream, &fd)) { - bap_accept_io(ep, stream, fd, defer); + setup_accept_io(setup, stream, fd, defer); return; } /* If IO channel still up or CIG is busy, wait for it to be * disconnected and then recreate. */ - if (ep->io || is_cig_busy(data, ep->qos.ucast.cig_id)) { - DBG("ep %p stream %p defer %s wait recreate", ep, stream, + if (setup->io || is_cig_busy(data, setup->qos.ucast.cig_id)) { + DBG("setup %p stream %p defer %s wait recreate", setup, stream, defer ? "true" : "false"); - ep->recreate = true; + setup->recreate = true; return; } - if (ep->io_id) { - g_source_remove(ep->io_id); - ep->io_id = 0; + if (setup->io_id) { + g_source_remove(setup->io_id); + setup->io_id = 0; } - DBG("ep %p stream %p defer %s", ep, stream, defer ? "true" : "false"); + DBG("setup %p stream %p defer %s", setup, stream, + defer ? "true" : "false"); - io = bt_io_connect(bap_connect_io_cb, ep, NULL, &err, + io = bt_io_connect(bap_connect_io_cb, setup, NULL, &err, BT_IO_OPT_SOURCE_BDADDR, btd_adapter_get_address(adapter), BT_IO_OPT_DEST_BDADDR, - device_get_address(ep->data->device), + device_get_address(data->device), BT_IO_OPT_DEST_TYPE, - device_get_le_address_type(ep->data->device), + device_get_le_address_type(data->device), BT_IO_OPT_MODE, BT_IO_MODE_ISO, BT_IO_OPT_QOS, qos, BT_IO_OPT_DEFER_TIMEOUT, defer, @@ -1669,18 +1732,19 @@ static void bap_connect_io(struct bap_data *data, struct bap_ep *ep, return; } - ep->io_id = g_io_add_watch(io, G_IO_HUP | G_IO_ERR | G_IO_NVAL, - bap_io_disconnected, ep); + setup->io_id = g_io_add_watch(io, G_IO_HUP | G_IO_ERR | G_IO_NVAL, + setup_io_disconnected, setup); - ep->io = io; - ep->cig_active = !defer; + setup->io = io; + setup->cig_active = !defer; bt_bap_stream_io_connecting(stream, g_io_channel_unix_get_fd(io)); } -static void bap_connect_io_broadcast(struct bap_data *data, struct bap_ep *ep, - struct bt_bap_stream *stream, - struct bt_iso_qos *qos) +static void setup_connect_io_broadcast(struct bap_data *data, + struct bap_setup *setup, + struct bt_bap_stream *stream, + struct bt_iso_qos *qos) { struct btd_adapter *adapter = data->user_data; GIOChannel *io = NULL; @@ -1695,18 +1759,19 @@ static void bap_connect_io_broadcast(struct bap_data *data, struct bap_ep *ep, if (bt_bap_stream_get_io(stream)) return; - if (ep->io_id) { - g_source_remove(ep->io_id); - ep->io_id = 0; + if (setup->io_id) { + g_source_remove(setup->io_id); + setup->io_id = 0; } - base.base_len = ep->base->iov_len; + base.base_len = setup->base->iov_len; memset(base.base, 0, 248); - memcpy(base.base, ep->base->iov_base, ep->base->iov_len); - DBG("ep %p stream %p ", ep, stream); + memcpy(base.base, setup->base->iov_base, setup->base->iov_len); ba2str(btd_adapter_get_address(adapter), addr); - io = bt_io_connect(bap_connect_bcast_io_cb, ep, NULL, &err, + DBG("setup %p stream %p", setup, stream); + + io = bt_io_connect(bap_connect_bcast_io_cb, setup, NULL, &err, BT_IO_OPT_SOURCE_BDADDR, btd_adapter_get_address(adapter), BT_IO_OPT_DEST_BDADDR, @@ -1725,15 +1790,15 @@ static void bap_connect_io_broadcast(struct bap_data *data, struct bap_ep *ep, return; } - ep->io_id = g_io_add_watch(io, G_IO_HUP | G_IO_ERR | G_IO_NVAL, - bap_io_disconnected, ep); + setup->io_id = g_io_add_watch(io, G_IO_HUP | G_IO_ERR | G_IO_NVAL, + setup_io_disconnected, setup); - ep->io = io; + setup->io = io; bt_bap_stream_io_connecting(stream, g_io_channel_unix_get_fd(io)); } -static void bap_listen_io(struct bap_data *data, struct bt_bap_stream *stream, +static void setup_listen_io(struct bap_data *data, struct bt_bap_stream *stream, struct bt_iso_qos *qos) { struct btd_adapter *adapter = device_get_adapter(data->device); @@ -1765,8 +1830,10 @@ static void bap_listen_io(struct bap_data *data, struct bt_bap_stream *stream, data->listen_io = io; } -static void bap_listen_io_broadcast(struct bap_data *data, struct bap_ep *ep, - struct bt_bap_stream *stream, struct bt_iso_qos *qos) +static void setup_listen_io_broadcast(struct bap_data *data, + struct bap_setup *setup, + struct bt_bap_stream *stream, + struct bt_iso_qos *qos) { GIOChannel *io; GError *err = NULL; @@ -1784,9 +1851,9 @@ static void bap_listen_io_broadcast(struct bap_data *data, struct bap_ep *ep, if (bt_bap_stream_get_io(stream) || data->listen_io) return; - io = bt_io_listen(NULL, iso_pa_sync_confirm_cb, ep->data, NULL, &err, + io = bt_io_listen(NULL, iso_pa_sync_confirm_cb, setup, NULL, &err, BT_IO_OPT_SOURCE_BDADDR, - btd_adapter_get_address(ep->data->adapter), + btd_adapter_get_address(data->adapter), BT_IO_OPT_DEST_BDADDR, device_get_address(data->device), BT_IO_OPT_DEST_TYPE, @@ -1800,12 +1867,14 @@ static void bap_listen_io_broadcast(struct bap_data *data, struct bap_ep *ep, error("%s", err->message); g_error_free(err); } - ep->io = io; - ep->data->listen_io = io; + setup->io = io; + data->listen_io = io; } -static void bap_create_ucast_io(struct bap_data *data, struct bap_ep *ep, - struct bt_bap_stream *stream, int defer) +static void setup_create_ucast_io(struct bap_data *data, + struct bap_setup *setup, + struct bt_bap_stream *stream, + int defer) { struct bt_bap_qos *qos[2] = {}; struct bt_iso_qos iso_qos; @@ -1825,14 +1894,15 @@ static void bap_create_ucast_io(struct bap_data *data, struct bap_ep *ep, bap_iso_qos(qos[0], &iso_qos.ucast.in); bap_iso_qos(qos[1], &iso_qos.ucast.out); - if (ep) - bap_connect_io(data, ep, stream, &iso_qos, defer); + if (setup) + setup_connect_io(data, setup, stream, &iso_qos, defer); else - bap_listen_io(data, stream, &iso_qos); + setup_listen_io(data, stream, &iso_qos); } -static void bap_create_bcast_io(struct bap_data *data, struct bap_ep *ep, - struct bt_bap_stream *stream, int defer) +static void setup_create_bcast_io(struct bap_data *data, + struct bap_setup *setup, + struct bt_bap_stream *stream, int defer) { struct bt_iso_qos iso_qos; @@ -1841,33 +1911,35 @@ static void bap_create_bcast_io(struct bap_data *data, struct bap_ep *ep, if (!defer) goto done; - iso_qos.bcast.big = ep->qos.bcast.big; - iso_qos.bcast.bis = ep->qos.bcast.bis; - iso_qos.bcast.sync_factor = ep->qos.bcast.sync_factor; - iso_qos.bcast.packing = ep->qos.bcast.packing; - iso_qos.bcast.framing = ep->qos.bcast.framing; - iso_qos.bcast.encryption = ep->qos.bcast.encryption; - if (ep->qos.bcast.bcode) - memcpy(iso_qos.bcast.bcode, ep->qos.bcast.bcode->iov_base, 16); - iso_qos.bcast.options = ep->qos.bcast.options; - iso_qos.bcast.skip = ep->qos.bcast.skip; - iso_qos.bcast.sync_timeout = ep->qos.bcast.sync_timeout; - iso_qos.bcast.sync_cte_type = ep->qos.bcast.sync_cte_type; - iso_qos.bcast.mse = ep->qos.bcast.mse; - iso_qos.bcast.timeout = ep->qos.bcast.timeout; - memcpy(&iso_qos.bcast.out, &ep->qos.bcast.io_qos, + iso_qos.bcast.big = setup->qos.bcast.big; + iso_qos.bcast.bis = setup->qos.bcast.bis; + iso_qos.bcast.sync_factor = setup->qos.bcast.sync_factor; + iso_qos.bcast.packing = setup->qos.bcast.packing; + iso_qos.bcast.framing = setup->qos.bcast.framing; + iso_qos.bcast.encryption = setup->qos.bcast.encryption; + if (setup->qos.bcast.bcode) + memcpy(iso_qos.bcast.bcode, setup->qos.bcast.bcode->iov_base, + 16); + iso_qos.bcast.options = setup->qos.bcast.options; + iso_qos.bcast.skip = setup->qos.bcast.skip; + iso_qos.bcast.sync_timeout = setup->qos.bcast.sync_timeout; + iso_qos.bcast.sync_cte_type = setup->qos.bcast.sync_cte_type; + iso_qos.bcast.mse = setup->qos.bcast.mse; + iso_qos.bcast.timeout = setup->qos.bcast.timeout; + memcpy(&iso_qos.bcast.out, &setup->qos.bcast.io_qos, sizeof(struct bt_iso_io_qos)); done: - if (bt_bap_pac_get_type(ep->lpac) == BT_BAP_BCAST_SOURCE) - bap_connect_io_broadcast(data, ep, stream, &iso_qos); + if (bt_bap_pac_get_type(setup->ep->lpac) == BT_BAP_BCAST_SOURCE) + setup_connect_io_broadcast(data, setup, stream, &iso_qos); else - bap_listen_io_broadcast(data, ep, stream, &iso_qos); + setup_listen_io_broadcast(data, setup, stream, &iso_qos); } -static void bap_create_io(struct bap_data *data, struct bap_ep *ep, +static void setup_create_io(struct bap_data *data, struct bap_setup *setup, struct bt_bap_stream *stream, int defer) { - DBG("ep %p stream %p defer %s", ep, stream, defer ? "true" : "false"); + DBG("setup %p stream %p defer %s", setup, stream, + defer ? "true" : "false"); if (!data->streams) data->streams = queue_new(); @@ -1877,10 +1949,10 @@ static void bap_create_io(struct bap_data *data, struct bap_ep *ep, switch (bt_bap_stream_get_type(stream)) { case BT_BAP_STREAM_TYPE_UCAST: - bap_create_ucast_io(data, ep, stream, defer); + setup_create_ucast_io(data, setup, stream, defer); break; case BT_BAP_STREAM_TYPE_BCAST: - bap_create_bcast_io(data, ep, stream, defer); + setup_create_bcast_io(data, setup, stream, defer); break; } } @@ -1889,7 +1961,7 @@ static void bap_state(struct bt_bap_stream *stream, uint8_t old_state, uint8_t new_state, void *user_data) { struct bap_data *data = user_data; - struct bap_ep *ep; + struct bap_setup *setup; DBG("stream %p: %s(%u) -> %s(%u)", stream, bt_bap_stream_statestr(old_state), old_state, @@ -1902,21 +1974,20 @@ static void bap_state(struct bt_bap_stream *stream, uint8_t old_state, if (new_state == old_state && new_state != BT_BAP_STREAM_STATE_CONFIG) return; - ep = bap_find_ep_by_stream(data, stream); + setup = bap_find_setup_by_stream(data, stream); switch (new_state) { case BT_BAP_STREAM_STATE_IDLE: /* Release stream if idle */ - if (ep) { - bap_io_close(ep); - ep->stream = NULL; - } else + if (setup) + setup_free(setup); + else queue_remove(data->streams, stream); break; case BT_BAP_STREAM_STATE_CONFIG: - if (ep && !ep->id) { - bap_create_io(data, ep, stream, true); - if (!ep->io) { + if (setup && !setup->id) { + setup_create_io(data, setup, stream, true); + if (!setup->io) { error("Unable to create io"); bt_bap_stream_release(stream, NULL, NULL); return; @@ -1925,9 +1996,10 @@ static void bap_state(struct bt_bap_stream *stream, uint8_t old_state, if (bt_bap_stream_get_type(stream) == BT_BAP_STREAM_TYPE_UCAST) { /* Wait QoS response to respond */ - ep->id = bt_bap_stream_qos(stream, &ep->qos, - qos_cb, ep); - if (!ep->id) { + setup->id = bt_bap_stream_qos(stream, + &setup->qos, + qos_cb, setup); + if (!setup->id) { error("Failed to Configure QoS"); bt_bap_stream_release(stream, NULL, NULL); @@ -1938,12 +2010,12 @@ static void bap_state(struct bt_bap_stream *stream, uint8_t old_state, case BT_BAP_STREAM_STATE_QOS: if (bt_bap_stream_get_type(stream) == BT_BAP_STREAM_TYPE_UCAST) { - bap_create_io(data, ep, stream, true); + setup_create_io(data, setup, stream, true); } break; case BT_BAP_STREAM_STATE_ENABLING: - if (ep) - bap_create_io(data, ep, stream, false); + if (setup) + setup_create_io(data, setup, stream, false); break; case BT_BAP_STREAM_STATE_STREAMING: break; @@ -2115,66 +2187,69 @@ static void bap_connecting(struct bt_bap_stream *stream, bool state, int fd, void *user_data) { struct bap_data *data = user_data; - struct bap_ep *ep; + struct bap_setup *setup; + struct bt_bap_qos *qos; GIOChannel *io; if (!state) return; - ep = bap_find_ep_by_stream(data, stream); - if (!ep) + setup = bap_find_setup_by_stream(data, stream); + if (!setup) return; - ep->recreate = false; + setup->recreate = false; + qos = &setup->qos; - if (!ep->io) { + if (!setup->io) { io = g_io_channel_unix_new(fd); - ep->io_id = g_io_add_watch(io, G_IO_HUP | G_IO_ERR | G_IO_NVAL, - bap_io_disconnected, ep); - ep->io = io; + setup->io_id = g_io_add_watch(io, + G_IO_HUP | G_IO_ERR | G_IO_NVAL, + setup_io_disconnected, setup); + setup->io = io; } else - io = ep->io; + io = setup->io; g_io_channel_set_close_on_unref(io, FALSE); - switch (bt_bap_stream_get_type(ep->stream)) { + switch (bt_bap_stream_get_type(setup->stream)) { case BT_BAP_STREAM_TYPE_UCAST: /* Attempt to get CIG/CIS if they have not been set */ - if (ep->qos.ucast.cig_id == BT_ISO_QOS_CIG_UNSET || - ep->qos.ucast.cis_id == BT_ISO_QOS_CIS_UNSET) { - struct bt_iso_qos qos; + if (qos->ucast.cig_id == BT_ISO_QOS_CIG_UNSET || + qos->ucast.cis_id == BT_ISO_QOS_CIS_UNSET) { + struct bt_iso_qos iso_qos; - if (!io_get_qos(io, &qos)) { + if (!io_get_qos(io, &iso_qos)) { g_io_channel_unref(io); return; } - ep->qos.ucast.cig_id = qos.ucast.cig; - ep->qos.ucast.cis_id = qos.ucast.cis; + qos->ucast.cig_id = iso_qos.ucast.cig; + qos->ucast.cis_id = iso_qos.ucast.cis; } DBG("stream %p fd %d: CIG 0x%02x CIS 0x%02x", stream, fd, - ep->qos.ucast.cig_id, ep->qos.ucast.cis_id); + qos->ucast.cig_id, qos->ucast.cis_id); break; case BT_BAP_STREAM_TYPE_BCAST: /* Attempt to get BIG/BIS if they have not been set */ - if (ep->qos.bcast.big == BT_ISO_QOS_BIG_UNSET || - ep->qos.bcast.bis == BT_ISO_QOS_BIS_UNSET) { - struct bt_iso_qos qos; + if (setup->qos.bcast.big == BT_ISO_QOS_BIG_UNSET || + setup->qos.bcast.bis == BT_ISO_QOS_BIS_UNSET) { + struct bt_iso_qos iso_qos; - if (!io_get_qos(io, &qos)) { + if (!io_get_qos(io, &iso_qos)) { g_io_channel_unref(io); return; } - ep->qos.bcast.big = qos.bcast.big; - ep->qos.bcast.bis = qos.bcast.bis; - bt_bap_stream_config(ep->stream, &ep->qos, - ep->caps, NULL, NULL); + qos->bcast.big = iso_qos.bcast.big; + qos->bcast.bis = iso_qos.bcast.bis; + bt_bap_stream_config(setup->stream, qos, setup->caps, + NULL, NULL); } DBG("stream %p fd %d: BIG 0x%02x BIS 0x%02x", stream, fd, - ep->qos.bcast.big, ep->qos.bcast.bis); + qos->bcast.big, qos->bcast.bis); } }
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> Remote endpoints actually represents PAC records of the same codec and their capabilities are merged together thus is should be possible to create multiple streams depending on the AC configuration. --- profiles/audio/bap.c | 613 ++++++++++++++++++++++++------------------- 1 file changed, 344 insertions(+), 269 deletions(-)