Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non usb8 buffers #3

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

aartaka
Copy link
Contributor

@aartaka aartaka commented Mar 31, 2022

This enables support for non-unsigned-byte-8 buffers (single, double, unsigned-byte-16 etc) natively for buffers of pcm-stream. The cost for it is a nasty cffi::unsure-parsed-type hack, which is licely to work for more than a decade from now, though.

The reason for the change was that I wanted to play some float-based data with ALSA and realized I need to cast it to usb8, which is not exacly easy in Lisp. Thus, this pretty changeset that allows setting buffers to any valid ALSA-friendly type, including floats.

I've tested it, and it seems to work, although the output is somewhat slow.

@varjagg, does this change make sense?

@aartaka
Copy link
Contributor Author

aartaka commented Mar 31, 2022

Oh wait, it plays twice as slow when there are two channels, as compared to one channel. Did I break something? :D

@varjagg
Copy link
Owner

varjagg commented Apr 8, 2022

Hello,
Thanks for the PR. Just from looking at this am not sure about runtime impact of it. This library is used a lot on constrained embedded systems and it took a lot to ensure the perf is acceptable on exotics like LW on ARM32. Also don't quite have my heart on using unexported code from CFFI.

@aartaka aartaka force-pushed the non-usb8-buffers branch from b2e8ee6 to 83fa716 Compare April 9, 2022 19:23
@aartaka
Copy link
Contributor Author

aartaka commented Apr 9, 2022

Thanks for the PR. Just from looking at this am not sure about runtime impact of it. This library is used a lot on constrained embedded systems and it took a lot to ensure the perf is acceptable on exotics like LW on ARM32.

This part of the context I was unaware of. I've just pushed a fix (see 83fa716) that uses shareable vectors for the (unsigned-byte 8) buffers. Should cover most performance-sensitive cases. After all, everything was cased to usb8 before :)

What is not exactly cast-able are floats. And my use case (cl-pure-data) relies on float buffers. Before this PR, float buffers were practically impossible to use in also-alsa.

So, given 83fa716 with the performance optimization, and 18032b7 with type convenience, also-alsa becomes easier to use with a good performance when necessary :)

Also don't quite have my heart on using unexported code from CFFI.

Neither do I, but there's no good way to construct foreign arrays out of Lisp data without wither working with raw pointers or «cheating» CFFI like in 18032b7. Given that foreign types are the core of CFFI, there shoudn't be much change in the API, I hope :)

@aartaka
Copy link
Contributor Author

aartaka commented Apr 20, 2022

@varjagg, any more feedback on the current iteration of the code? :)

@varjagg
Copy link
Owner

varjagg commented Apr 21, 2022

Hi, I'll have a look, eventually test on existing projects if it breaks anything and will get back - been a bit busy lately.

@aartaka
Copy link
Contributor Author

aartaka commented Apr 21, 2022

Thank you!

@varjagg
Copy link
Owner

varjagg commented Apr 22, 2022

It breaks for other formats, such as signed 16 bit.

@aartaka
Copy link
Contributor Author

aartaka commented Jun 19, 2022

@varjagg, I may have been unclear: it does work for signed 16-bit, but it requires writing the code without all this bit twiddling that used to be there before, which I consider a virtue. See 9897619 for the rewamped README example showcasing how easy it now is interfacing with also-alsa on your (signed-byte 16) example :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants