mbox series

[BlueZ,v2,0/3] Broadcast source reconfiguration support

Message ID 20240313170300.40974-1-silviu.barbulescu@nxp.com (mailing list archive)
Headers show
Series Broadcast source reconfiguration support | expand

Message

Silviu Florian Barbulescu March 13, 2024, 5:02 p.m. UTC 
This patch adds support for broadcast source to reconfigure a BIS.
endpoint.config command has a new prompt for broadcast source:
" This is a BIS Reconfiguration? (yes/no)"
Yes or y if a BIS reconfiguration is required
No or n if it is a new BIS configuration
"The BIS index is assigned in the order of the configuration
starting with 1"
"BIS Index (value):" a value representing the BIS index if this is a
BIS reconfiguration.
endpoint.register 00001852-0000-1000-8000-00805f9b34fb 0x06
endpoint.config /org/bluez/hci0/pac_bcast0 /local/endpoint/ep0 48_4_1
BIS Index for reconfiguration? (value(1-31)/no): 0
[/local/endpoint/ep0] BIG (auto/value): 0
[/local/endpoint/ep0] Enter channel location (value/no): 1
[/local/endpoint/ep0] Enter Metadata (value/no): n
endpoint.config /org/bluez/hci0/pac_bcast0 /local/endpoint/ep0 48_4_1
[/local/endpoint/ep0] BIS Index for reconfiguration? (value(1-31)/no): 1
[/local/endpoint/ep0] BIG (auto/value): 0
[/local/endpoint/ep0] Enter channel location (value/no): 2
[/local/endpoint/ep0] Enter Metadata (value/no): n
transport.show /org/bluez/hci0/pac_bcast0/fd0 to view the update
Transport /org/bluez/hci0/pac_bcast0/fd0

Configuration.Location: 0x00000002
Configuration.Location: Front Right (0x00000002)


Silviu Florian Barbulescu (3):
  player: Add reconfiguration prompt for broadcast source
  transport: Add support to update the transport configuration
  bap: Broadcast source reconfiguration support added

 client/player.c            | 33 ++++++++++++++++-
 profiles/audio/bap.c       | 76 ++++++++++++++++++++++++++++++++++++++
 profiles/audio/transport.c | 27 +++++++++++++-
 profiles/audio/transport.h |  1 +
 src/shared/bap.c           | 11 +++++-
 5 files changed, 144 insertions(+), 4 deletions(-)


base-commit: c9dddb642663b8bafe8998fef6132a2bb36d609d

Comments

Luiz Augusto von Dentz March 13, 2024, 8:04 p.m. UTC | #1 
Hi Silviu,

On Wed, Mar 13, 2024 at 1:03 PM Silviu Florian Barbulescu
<silviu.barbulescu@nxp.com> wrote:
>
> This patch adds support for broadcast source to reconfigure a BIS.
> endpoint.config command has a new prompt for broadcast source:
> " This is a BIS Reconfiguration? (yes/no)"
> Yes or y if a BIS reconfiguration is required
> No or n if it is a new BIS configuration
> "The BIS index is assigned in the order of the configuration
> starting with 1"
> "BIS Index (value):" a value representing the BIS index if this is a
> BIS reconfiguration.
> endpoint.register 00001852-0000-1000-8000-00805f9b34fb 0x06
> endpoint.config /org/bluez/hci0/pac_bcast0 /local/endpoint/ep0 48_4_1
> BIS Index for reconfiguration? (value(1-31)/no): 0
> [/local/endpoint/ep0] BIG (auto/value): 0
> [/local/endpoint/ep0] Enter channel location (value/no): 1
> [/local/endpoint/ep0] Enter Metadata (value/no): n
> endpoint.config /org/bluez/hci0/pac_bcast0 /local/endpoint/ep0 48_4_1
> [/local/endpoint/ep0] BIS Index for reconfiguration? (value(1-31)/no): 1
> [/local/endpoint/ep0] BIG (auto/value): 0
> [/local/endpoint/ep0] Enter channel location (value/no): 2
> [/local/endpoint/ep0] Enter Metadata (value/no): n
> transport.show /org/bluez/hci0/pac_bcast0/fd0 to view the update
> Transport /org/bluez/hci0/pac_bcast0/fd0
>
> Configuration.Location: 0x00000002
> Configuration.Location: Front Right (0x00000002)
>
>
> Silviu Florian Barbulescu (3):
>   player: Add reconfiguration prompt for broadcast source
>   transport: Add support to update the transport configuration
>   bap: Broadcast source reconfiguration support added
>
>  client/player.c            | 33 ++++++++++++++++-
>  profiles/audio/bap.c       | 76 ++++++++++++++++++++++++++++++++++++++
>  profiles/audio/transport.c | 27 +++++++++++++-
>  profiles/audio/transport.h |  1 +
>  src/shared/bap.c           | 11 +++++-
>  5 files changed, 144 insertions(+), 4 deletions(-)
>
>
> base-commit: c9dddb642663b8bafe8998fef6132a2bb36d609d
> --
> 2.39.2

Something's not quite right, although it seems it does update the
configuration at D-Bus level:

transport.show /org/bluez/hci0/pac_bcast0/fd0
Transport /org/bluez/hci0/pac_bcast0/fd0
    UUID: 00001852-0000-1000-8000-00805f9b34fb
    Codec: 0x06 (6)
    Configuration.#0: len 0x02 type 0x01
    Configuration.Sampling Frequency: 48 Khz (0x08)
    Configuration.#1: len 0x02 type 0x02
    Configuration.Frame Duration: 10 ms (0x01)
    Configuration.#2: len 0x03 type 0x04
    Configuration.Frame Length: 120 (0x0078)
    Configuration.#3: len 0x05 type 0x03
    Configuration.Location: 0x00000001
    Configuration.Location: Front Left (0x00000001)
    Device: /org/bluez/hci0
    State: active
    Endpoint: /org/bluez/hci0/pac_bcast0
    QoS.BIG: 0x00 (0)
    QoS.BIS: 0x01 (1)
    QoS.SyncFactor: 0x01 (1)
    QoS.Packing: 0x00 (0)
    QoS.Framing: 0x00 (0)
    QoS.BCode:
  01 02 68 05 53 f1 41 5a a2 65 bb af c6 ea 03 b8  ..h.S.AZ.e......
    QoS.Options: 0x00 (0)
    QoS.Skip: 0x0000 (0)
    QoS.SyncTimeout: 0x07d0 (2000)
    QoS.SyncType: 0x00 (0)
    QoS.MSE: 0x00 (0)
    QoS.Timeout: 0x07d0 (2000)
    QoS.Interval: 0x00002710 (10000)
    QoS.Latency: 0x0014 (20)
    QoS.SDU: 0x0078 (120)
    QoS.PHY: 0x02 (2)
    QoS.Retransmissions: 0x04 (4)
    Location: 0x00000000 (0)

It doesn't do that at HCI level so it still uses the old configuration
for some reason:

< HCI Command: LE Set Periodic Advertising Data (0x08|0x003f) plen 41
                         #2839 [hci2] 16:00:16.948281
        Handle: 1
        Operation: Complete ext advertising data (0x03)
        Data length: 0x26
        Service Data: Basic Audio Announcement (0x1851)
          Presetation Delay: 40000
          Number of Subgroups: 1
            Subgroup #0:
            Number of BIS(s): 1
            Codec: LC3 (0x06)
            Codec Specific Configuration: #0: len 0x02 type 0x01
            Codec Specific Configuration: Sampling Frequency: 16 Khz (0x03)
            Codec Specific Configuration: #1: len 0x02 type 0x02
            Codec Specific Configuration: Frame Duration: 10 ms (0x01)
            Codec Specific Configuration: #2: len 0x03 type 0x04
            Codec Specific Configuration: Frame Length: 40 (0x0028)
            Codec Specific Configuration: #3: len 0x05 type 0x03
            Codec Specific Configuration: Location: 0x00000003
            Codec Specific Configuration: Location: Front Left (0x00000001)
            Codec Specific Configuration: Location: Front Right (0x00000002)
            Metadata: #0: len 0x03 type 0x02
            Metadata: Context: 0x0004
            Metadata: Context    Media (0x0004)
              BIS #0:
              Index: 1

Perhaps this is a kernel bug though, but it needs to be fixed anyway.
Luiz Augusto von Dentz March 13, 2024, 8:49 p.m. UTC | #2 
Hi Silviu,

On Wed, Mar 13, 2024 at 4:04 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Silviu,
>
> On Wed, Mar 13, 2024 at 1:03 PM Silviu Florian Barbulescu
> <silviu.barbulescu@nxp.com> wrote:
> >
> > This patch adds support for broadcast source to reconfigure a BIS.
> > endpoint.config command has a new prompt for broadcast source:
> > " This is a BIS Reconfiguration? (yes/no)"
> > Yes or y if a BIS reconfiguration is required
> > No or n if it is a new BIS configuration
> > "The BIS index is assigned in the order of the configuration
> > starting with 1"
> > "BIS Index (value):" a value representing the BIS index if this is a
> > BIS reconfiguration.
> > endpoint.register 00001852-0000-1000-8000-00805f9b34fb 0x06
> > endpoint.config /org/bluez/hci0/pac_bcast0 /local/endpoint/ep0 48_4_1
> > BIS Index for reconfiguration? (value(1-31)/no): 0
> > [/local/endpoint/ep0] BIG (auto/value): 0
> > [/local/endpoint/ep0] Enter channel location (value/no): 1
> > [/local/endpoint/ep0] Enter Metadata (value/no): n
> > endpoint.config /org/bluez/hci0/pac_bcast0 /local/endpoint/ep0 48_4_1
> > [/local/endpoint/ep0] BIS Index for reconfiguration? (value(1-31)/no): 1
> > [/local/endpoint/ep0] BIG (auto/value): 0
> > [/local/endpoint/ep0] Enter channel location (value/no): 2
> > [/local/endpoint/ep0] Enter Metadata (value/no): n
> > transport.show /org/bluez/hci0/pac_bcast0/fd0 to view the update
> > Transport /org/bluez/hci0/pac_bcast0/fd0
> >
> > Configuration.Location: 0x00000002
> > Configuration.Location: Front Right (0x00000002)
> >
> >
> > Silviu Florian Barbulescu (3):
> >   player: Add reconfiguration prompt for broadcast source
> >   transport: Add support to update the transport configuration
> >   bap: Broadcast source reconfiguration support added
> >
> >  client/player.c            | 33 ++++++++++++++++-
> >  profiles/audio/bap.c       | 76 ++++++++++++++++++++++++++++++++++++++
> >  profiles/audio/transport.c | 27 +++++++++++++-
> >  profiles/audio/transport.h |  1 +
> >  src/shared/bap.c           | 11 +++++-
> >  5 files changed, 144 insertions(+), 4 deletions(-)
> >
> >
> > base-commit: c9dddb642663b8bafe8998fef6132a2bb36d609d
> > --
> > 2.39.2
>
> Something's not quite right, although it seems it does update the
> configuration at D-Bus level:
>
> transport.show /org/bluez/hci0/pac_bcast0/fd0
> Transport /org/bluez/hci0/pac_bcast0/fd0
>     UUID: 00001852-0000-1000-8000-00805f9b34fb
>     Codec: 0x06 (6)
>     Configuration.#0: len 0x02 type 0x01
>     Configuration.Sampling Frequency: 48 Khz (0x08)
>     Configuration.#1: len 0x02 type 0x02
>     Configuration.Frame Duration: 10 ms (0x01)
>     Configuration.#2: len 0x03 type 0x04
>     Configuration.Frame Length: 120 (0x0078)
>     Configuration.#3: len 0x05 type 0x03
>     Configuration.Location: 0x00000001
>     Configuration.Location: Front Left (0x00000001)
>     Device: /org/bluez/hci0
>     State: active
>     Endpoint: /org/bluez/hci0/pac_bcast0
>     QoS.BIG: 0x00 (0)
>     QoS.BIS: 0x01 (1)
>     QoS.SyncFactor: 0x01 (1)
>     QoS.Packing: 0x00 (0)
>     QoS.Framing: 0x00 (0)
>     QoS.BCode:
>   01 02 68 05 53 f1 41 5a a2 65 bb af c6 ea 03 b8  ..h.S.AZ.e......
>     QoS.Options: 0x00 (0)
>     QoS.Skip: 0x0000 (0)
>     QoS.SyncTimeout: 0x07d0 (2000)
>     QoS.SyncType: 0x00 (0)
>     QoS.MSE: 0x00 (0)
>     QoS.Timeout: 0x07d0 (2000)
>     QoS.Interval: 0x00002710 (10000)
>     QoS.Latency: 0x0014 (20)
>     QoS.SDU: 0x0078 (120)
>     QoS.PHY: 0x02 (2)
>     QoS.Retransmissions: 0x04 (4)
>     Location: 0x00000000 (0)
>
> It doesn't do that at HCI level so it still uses the old configuration
> for some reason:
>
> < HCI Command: LE Set Periodic Advertising Data (0x08|0x003f) plen 41
>                          #2839 [hci2] 16:00:16.948281
>         Handle: 1
>         Operation: Complete ext advertising data (0x03)
>         Data length: 0x26
>         Service Data: Basic Audio Announcement (0x1851)
>           Presetation Delay: 40000
>           Number of Subgroups: 1
>             Subgroup #0:
>             Number of BIS(s): 1
>             Codec: LC3 (0x06)
>             Codec Specific Configuration: #0: len 0x02 type 0x01
>             Codec Specific Configuration: Sampling Frequency: 16 Khz (0x03)
>             Codec Specific Configuration: #1: len 0x02 type 0x02
>             Codec Specific Configuration: Frame Duration: 10 ms (0x01)
>             Codec Specific Configuration: #2: len 0x03 type 0x04
>             Codec Specific Configuration: Frame Length: 40 (0x0028)
>             Codec Specific Configuration: #3: len 0x05 type 0x03
>             Codec Specific Configuration: Location: 0x00000003
>             Codec Specific Configuration: Location: Front Left (0x00000001)
>             Codec Specific Configuration: Location: Front Right (0x00000002)
>             Metadata: #0: len 0x03 type 0x02
>             Metadata: Context: 0x0004
>             Metadata: Context    Media (0x0004)
>               BIS #0:
>               Index: 1
>
> Perhaps this is a kernel bug though, but it needs to be fixed anyway.

Looks like the setup->base is not being updated, in fact I don't know
why we stop the base like that instead of just doing:

https://gist.github.com/Vudentz/ab241cd1a373f5d0f7182355f23227c7

Anyway, the fact that the configuration over HCI doesn't update tells
me that perhaps we need some extra steps to validate these changes,
having to test everything you send to the list is not really practical
so I will start requiring evidence that things really work end-to-end,
also it doesn't seem you had made changes to the existing scripts at
client/scripts since you had changed endpoint.config to require
entering the bis index it broke the likes of
client/scripts/broadcast-source.bt.

>
> --
> Luiz Augusto von Dentz