jack2 icon indicating copy to clipboard operation
jack2 copied to clipboard

ALSA backend: Multi ALSA device support

Open twischer opened this issue 5 years ago • 12 comments

This PR depends currently on https://github.com/jackaudio/jack2/pull/463

This PR contains the following features for the ALSA audio backend

  • Support multiple devices without using ALSA multi plug-in. Compared to ALSA multi plugin it reduces the CPU load because snd_pcm_avail_update() only needs to be called once for each device per period. alsa_in/out applications use additional audio buffers and therefore increase the audio latency. audio devices attached with the ALSA multi device feature have the same latency as using the original backend implementation. This extension is also required to be able to open and close audio devices dynamically at JACK runtime (see underneath).
  • Rework error handling including retry for read and write
  • Support open close feature (With this feature it is possible to close or stop ALSA devices independently if they are not required but the JACK daemon can run continuously and all JACK client connects stay active. For clients it looks only like a short Xrun. See new -c and -x option)

ALSA multi plugin versus this new ALSA multi device feature In case of ALSA multi plugin poll() might return when not all slave devices of ALSA multi plugin have frames available because the poll() event of each devices will be forwarded to JACK. Therefore we have to check with snd_pcm_avail_update() This means a high probability that snd_pcm_avail_update() has to be called for the same devices at the same period multiple times.

With the multi devices support we can repeat poll() till all devices are done (after this poll() the data will be available for all devices.)

twischer avatar Jun 30 '20 12:06 twischer

You have all the qnx stuff mixed in, so it is impossible to review.

Did you try jackadapter? even though it uses an extra buffer like alsa_in/out, it is an internal client and thus does not have the issue of IPC context switches.

falkTX avatar Jun 30 '20 22:06 falkTX

@falkTX with regards to "multi-device" support in alsa backend, are you open to it on concept level? I could find the time to separate the QNX stuff away in case there was a chance it would be accepted...

amiartus avatar Jul 02 '20 12:07 amiartus

It is hard to say. Again, did you try jackadapter? how does it compare to this?

falkTX avatar Jul 02 '20 13:07 falkTX

Hello @falkTX,

Did you try jackadapter? even though it uses an extra buffer like alsa_in/out, it is an internal client and thus does not have the issue of IPC context switches.

Our main focus is low latency between all devices. Therefore we used a solution which avoids this additional buffering. Due to our hard audio latency requirements, we are not able to spend an additional audio buffer here. One example use case is Bluetooth Hands free call. Therefore the audio data has to be pre and post processed by echo cancellation noise reduction algorithms and forwarded from mic to Bluetooth and from Bluetooth to speaker (4 audio devices).

twischer avatar Jul 02 '20 19:07 twischer

quickly looking at the changes, they are just too intrusive. they also change the parameter types for number of input and output channels, breaking DBus compatibility. and introduce new arguments, which would need to be documented

did you made some measurements to compare the latency between audioadapter and your approach? I am very interested to see such numbers just wondering how worth all of this is..

falkTX avatar Jul 02 '20 19:07 falkTX

I have done some testing on my not-so-ideal HW, maybe someone could pick up on better HW later...

I was able to open the additional audio device with audioadapter but was getting errors like

../linux/alsa/JackAlsaAdapter.h:377, reading samples : Invalid argument(-22)
JackRingBuffer::Write : consumer too slow, skip frames = 96

Interestingly enough for the same devices "alsa multi-dev" solution worked without xruns

here are my commands for audioadapter:

jackd -R -d alsa -C hw:1,0 -P hw:1,0 -i 2 -o 2 -r 48000 -p 96 -n 2 &
jack_load audioadapter -i "-i 0 -o 2 -r 48000 -p 96 -n 2 -C null -d hw:0,3"

and for comparison alsa "multi-dev" command:

jackd -R -d alsa -C hw:1,0 -P "hw:1,0 hw:0,3" -i 2 -o "2 2" -r 48000 -p 96 -n 2

this will print jack audioadapter help when jackd is running

jack_load audioadapter -i "-h"

I'm not fully aware of capabilities of the audioadapter, but as far as I remember the multidev solution was running with 6 or more devices at the same time with period sizes of 2ms on both linux and QNX, so it is working quite well and is tested, it would be great to find a way to fit it into upstream. As far as parameters being changes and breaking DBUS - it could be solved by adding more parameters and leaving the existing as is.

amiartus avatar Jul 02 '20 20:07 amiartus

I did some tests of my own with audio-adapter, alsa_in/out and even zita-a2j/j2a. not impressed at all with the audio-adapter, I get same results as you most of the time. zita one (when running as internal client) works mostly okay, but takes very long to recover when anything at all goes wrong. alsa_in/out ends up working the best, though it has some issues (does not handle device disconnect for one)

all of those have the issue of an extra latency cycle. so I am very curious to see how the multi-dev setup would work instead. from what I can see, it is "yes please!" on that front.

The change of arguments is a blocker though, as mentioned before. Does this PR accept multiple capture and playback devices separately? (in your example CLI args, you only use "-P")

falkTX avatar Oct 11 '20 20:10 falkTX

Does this PR accept multiple capture and playback devices separately?

additional capture and playback devices can be listed space separated e.g. ... -C "hw:1,0 hw:0,0" -P "hw:1,0 hw:0,3" ...

Currently -P and -C can only specified once. But we could also think about modifying it to support multiple -P and -C options.

twischer avatar Oct 13 '20 19:10 twischer

The change of arguments is a blocker though, as mentioned before.

@falkTX I'm keen to have this merged eventually and I can find the time to adapt the arguments to whatever the acceptable requirements are. Could you let me know what would be acceptable change in the parameters?

As Timo has commented -C and -P work as string list of devices separated by a space, however the original behavior of having only one device in -C or -P is still preserved, so it should not break usage in scripts that were using the parameters before this change.

amiartus avatar May 18 '21 14:05 amiartus

Could you let me know what would be acceptable change in the parameters?

I can no longer remember why I was so against this use of "spaces". It does look ugly, but it is also functional I guess.

But anyway if this is going in, it really needs to be in smaller chunks and not one super big PR.

falkTX avatar May 20 '21 10:05 falkTX

@twischer I think we could separate the first 10 or so commits into QNX support / bugfix PR

I could have a look at the other half and separate it to couple of PRs

do you still would like to work on this to have it merged?

amiartus avatar May 24 '21 08:05 amiartus

@miartad

I think we could separate the first 10 or so commits into QNX support

Yes removing QNX support from this PR is fine for me because QNX is not in scope of community. (See https://github.com/jackaudio/jack2/pull/463#issuecomment-645436523)

twischer avatar May 25 '21 17:05 twischer