jackaudio / jack1

jack1 codebase
Other
250 stars 71 forks source link

Force 16-byte buffer alignment for SIMD #83

Open 0EVSG opened 5 years ago

0EVSG commented 5 years ago

Hello,

I stumbled upon this when I investigated jackd consistently crashing in some configurations of OSS on FreeBSD. In summary, the buffer size is reset according to the number of samples requested by the driver backend. If this number is not a multiple of 4, jackd will misalign its buffers for SSE SIMD instructions and die with SIGBUS when processing them. The proposed fix introduces some padding to align the buffers, in case this happens.

Please note that even though these odd buffer sizes seem to be unusual, the problem is not necessarily tied to the OSS backend.

More details can be found in the commit message and in the original bug report: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=234574 The proposed patch is the same as currently applied in the FreeBSD ports tree.

Thanks for your consideration

Florian Walpen


Commit message: With build option USE_DYNSIMD, SSE SIMD instructions are enabled that require 16-byte aligned buffer data. Multiple buffers are allocated in one contiguous block of memory, the buffer size is determined by the number of 4-byte float samples (nframes) per buffer. If this number is not set to a multiple of 4, the offset of subsequent buffers will be misaligned and jackd crashes with SIGBUS.

An example of this actually happening would be using 24 bit sample size in the OSS backend, where the page sized system buffers may result in an odd number of samples per buffer. See https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=234574 for a detailed bug report.

This change effectively adds some padding by rounding up the allocated buffer size to a multiple of 16 bytes, given that both:

a) SIMD instructions are enabled in the build. b) The requested buffer size is not already a multiple of 16 bytes.

All other cases should be unaffected. The existing buffer handling, copy and mix code seems to be well prepared for padded buffers, with the number of samples (nframes) and buffer offsets kept separately.

0EVSG commented 5 years ago

Hi @falkTX and @DomT4, sorry to bother you, is anyone of you still maintaining this repo? If not, do you know who I could contact to get some feedback on this matter? Thanks a lot!

falkTX commented 5 years ago

I am maintaining this repo. Currently busy with other things, I am giving more time for stuff to accumulate on JACK to then do all at once.

Your patch makes sense. The same issue happens on JACK2, which I have experienced it myself.

0EVSG commented 5 years ago

Ok, good to know - no hurry from my side either.