[pulseaudio-discuss] [PATCH v5 0/7] New API for Bluetooth A2DP codecs

Wed Jan 23 17:52:51 UTC 2019

On Wednesday 23 January 2019 19:16:16 Luiz Augusto von Dentz wrote:
> Hi Pali,
> On Sat, Jan 19, 2019 at 7:19 PM Pali Rohár <pali.rohar at gmail.com> wrote:
> >
> > I think that this patch series is now ready for review and testing. All
> > needed parts are finished.
> >
> > The only problem is with asynchronous profile switching as function
> > pa_card_set_profile() does not support it. All details are in patch
> > "[PATCH v5 5/7] bluetooth: Modular API for A2DP codecs".
> >
> > So can you look at this patch series and review it?
> 
> I tested this today, it seems to work fine with sbc but with
> faststream there seems to be some problem somewhere as it does seems
> to consume too much cpu time. Regarding set_profile that is something
> I think we will need to address otherwise the UI is pretty much
> useless to switch as it doesn't seems to react well when -EAGAIN is
> returned, we could perhaps block on SetConfiguration since we do block
> on Acquire anyway, or you have already tried that?

Yes, I tried it. Pulseaudio blocks communication and therefore it also
does not receive following DBus requests from bluez and so
SetConfiguration timeout.

In fact -EAGAIN is just dropped in pulseaudio bluetooth module and
replaced by -1. I just wanted to "highlight" that part of code, so I
used -EAGAIN, but it has same meaning as any other non-zero value.

I was thinking about it and we could block this on another layer.
pa_card_set_profile() is called when application request to change
profile via PA socket. So extend pa_card_set_profile() API to pass
callback function which would be called after profile is changed
asynchronously. And just block user request until callback is called.

Maybe some other people who understand pulseaudio better can help? How
to implement asynchronous profile switching?

> Another thing I noticed, we should start taking into accound the
> algorithm delay of each codec instead of adding a fixed delay of 25
> ms, in fact 25 ms seems wrong even for SBC since it has about 5 ms of
> delay not 25 ms.

Yes, this is something I wanted to start addressing. aptX has fixed
delay of 90 samples. Do you have some pointer there can be that delay
configured? Or where is that fixed delay hardcoded?

> I will do a proper review of each patch tomorrow, but for a user point
> of view I thing these details matter the most.
> 
> > On Saturday 19 January 2019 18:11:49 Pali Rohár wrote:
> > > This is 5th version of my patch series for modular A2DP codec API and
> > > aptX support. The only change for v5 is support for switching codecs.
> > >
> > > This patch series provides new modular API for Bluetooth A2DP codecs,
> > > clean up module-bluez5-device and bluez5-util to be codec independent
> > > and convert SBC codec into this new API for A2DP codecs. Also it adds
> > > support for aptX, aptX HD and FastStream A2DP codecs.
> > >
> > > New codec API is designed in way, that for adding new codec is not
> > > needed to touch bluez5-util nor module-bluez5-device files. Whole
> > > codec registration is done in a2dp-codec-util.c file, without need
> > > to update any header file.
> > >
> > > Some A2DP codecs are bidirectional and support backchannel for
> > > microphone voice. This new A2DP codec API fully supports this feature
> > > and module-bluez5-device was extended to support microphone voice source
> > > for codecs which declares such support. FastStream is such codec.
> > >
> > > For every A2DP codec is created card profile. When using bluez patches
> > > from https://marc.info/?l=linux-bluetooth&m=154696260401673&w=2 then
> > > only those profiles codec profiles are created which are supported
> > > by remote headset/endpoint.
> > >
> > > With this new modular API it should be easy to add other codec support
> > > like MP3, AAC or LDAC.
> > >
> > > API is designed also for ability to register "more variants" or one
> > > A2DP codec. E.g. SBC codec forced to high quality, SBC codec forced
> > > to low quality, SBC codec with automatic quality.
> > >
> > > Once we agree on final version of codec API, I can prepare patches
> > > also for choosing above SBC codec quality.
> > >
> > > I tested playback with SBC, aptX and FastStream codecs and it is
> > > working fine. Also microphone voice in FastStream is working and has
> > > slightly better quality as in HSP profile.
> > >
> > > Changes in v5:
> > > * Added support for switching between A2DP codec via profiles.
> > >   This requires bluez patches from above link.
> > >
> > > Changes in v4:
> > > * Added support for aptX HD codec
> > > * Changed codec API for reading block size and reducing encoder bitrate
> > > * Added support for microphone backchannel into module-bluez5-device
> > > * Added support for microphone backchannel into FastStream codec
> > > * Fixed parsing FastStream A2DP config buffer
> > > * Fixed validation for config buffers
> > > * Fixed calculating block size for aptX codec
> > > * Fixed resetting SBC and FastStream codecs
> > >
> > > Changes in v3:
> > > * Fixed problems pointed by Tanu
> > >
> > > Pali Rohár (7):
> > >   switch-on-port-available: Fix null pointer dereference
> > >   bluetooth: policy: Remove BlueZ 4 related code
> > >   bluetooth: policy: Reflect a2dp profile names
> > >   bluetooth: Update a2dp-codecs.h from upstream bluez project
> > >   bluetooth: Modular API for A2DP codecs
> > >   bluetooth: Add A2DP aptX and aptX HD codecs support
> > >   bluetooth: Add A2DP FastStream codec support
> > >
> > >  configure.ac                                    |  36 +
> > >  src/Makefile.am                                 |  20 +-
> > >  src/modules/bluetooth/a2dp-codec-api.h          |  80 +++
> > >  src/modules/bluetooth/a2dp-codec-aptx.c         | 547 +++++++++++++++
> > >  src/modules/bluetooth/a2dp-codec-faststream.c   | 433 ++++++++++++
> > >  src/modules/bluetooth/a2dp-codec-sbc.c          | 638 +++++++++++++++++
> > >  src/modules/bluetooth/a2dp-codec-util.c         |  66 ++
> > >  src/modules/bluetooth/a2dp-codec-util.h         |  34 +
> > >  src/modules/bluetooth/a2dp-codecs.h             | 354 +++++++++-
> > >  src/modules/bluetooth/bluez5-util.c             | 768 +++++++++++++-------
> > >  src/modules/bluetooth/bluez5-util.h             |  40 +-
> > >  src/modules/bluetooth/module-bluetooth-policy.c |  19 +-
> > >  src/modules/bluetooth/module-bluez5-device.c    | 884 +++++++++++-------------
> > >  src/modules/module-switch-on-port-available.c   |   2 +-
> > >  14 files changed, 3181 insertions(+), 740 deletions(-)
> > >  create mode 100644 src/modules/bluetooth/a2dp-codec-api.h
> > >  create mode 100644 src/modules/bluetooth/a2dp-codec-aptx.c
> > >  create mode 100644 src/modules/bluetooth/a2dp-codec-faststream.c
> > >  create mode 100644 src/modules/bluetooth/a2dp-codec-sbc.c
> > >  create mode 100644 src/modules/bluetooth/a2dp-codec-util.c
> > >  create mode 100644 src/modules/bluetooth/a2dp-codec-util.h
> >
> > --
> > Pali Rohár
> > pali.rohar at gmail.com
> 
> 
> 

-- 
Pali Rohár
pali.rohar at gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/pulseaudio-discuss/attachments/20190123/632ac877/attachment.sig>