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

[Pali Rohár]

On Thursday 24 January 2019 11:13:35 Luiz Augusto von Dentz wrote:
> Hi Pali,
> 
> On Wed, Jan 23, 2019 at 7:52 PM Pali Rohár <pali.rohar at gmail.com> wrote:
> >
> > 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.
> 
> Right there are D-Bus messages when switching the endpoint, though
> what we could perhaps do is create the sink suspended like how it is
> done in case of oFono, or at least that is how I remembered it.
> 
> > 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?
> 
> It is hardcoded in FIXED_LATENCY_PLAYBACK_A2DP which is added like this:
> 
>     pa_sink_set_fixed_latency_within_thread(u->sink,
> 
> (pa_bluetooth_profile_is_a2dp(u->profile) ?
> 
> FIXED_LATENCY_PLAYBACK_A2DP : FIXED_LATENCY_PLAYBACK_SCO) +
> 
> pa_bytes_to_usec(u->write_block_size, &u->encoder_sample_spec));

Thanks!

> Perhaps that was used to compensate the transmission latency, but that
> would be different for SCO and A2DP, but I tried with just 5 ms and it
> seems to work pretty well:

Is is possible to retrieve (from kernel? bluez?) transmission latency?

> I: [lt-pulseaudio] protocol-native.c: Final latency 100.00 ms = 25.01
> ms + 2*24.99 ms + 25.01 ms
> 
> As oppose to something like 140 ms which the original code produces.

I think that adding a new function for codec API which calculates codec
latency is the correct way how to deal with it.

-- 
Pali Rohár
pali.rohar at gmail.com