diwic / alsa-sys

Rust raw FFI bindings for ALSA
MIT License
15 stars 11 forks source link

Missing `snd_pcm_sw_params_set_timestamp_type` #6

Closed mitchmindtree closed 4 years ago

mitchmindtree commented 4 years ago

Hi @diwic!

I'm currently implementing RustAudio/cpal#397 for ALSA and noticed that the aforementioned function is missing both from alsa and alsa-sys. Here's the docs for the function:

https://www.alsa-project.org/alsa-doc/alsa-lib/group___p_c_m___s_w___params.html#ga912bad749f6317000eede607bb0bc935

This is required in order to allow CPAL to request timestamps of type SND_PCM_TSTAMP_TYPE_MONOTONIC_RAW.

Perhaps we can acquire this missing function by regenerating the bindings? I began work on regenerating the bindings a few months back at the CPAL repo not realising alsa-sys was moved here. During that process, I documented the command used to generate the bindings here:

https://github.com/RustAudio/cpal/pull/375/files#diff-fada08261a16e31b4cb0bdcc88a4662f

At least this way we can keep deps low (by not relying on bindgen at build time and avoiding problems like RustAudio/cpal#383) but still provide an easy way to regenerate bindings as required.

What are your thoughts?

diwic commented 4 years ago

I'm afraid this is not as easy as it sounds. I tried recreating the bindings using the command you provided, and there are quite a few problems:

mitchmindtree commented 4 years ago

Bindgen replicates the entire of libc into the bindings, causing conflicts between libc and alsa (e g, now pollfd is defined in both crates).

Ahh yes I remember running into this now. It should be possible to blacklist certain items from being generated, but I'm unsure how much control is provided over this in bindgen and how much work is involved to do this.

Also, there is quite a few things to change in alsa-rs (250 errors, but a lot of them looked like the same in different places, so probably fixable by find-and-replace).

We might be able to avoid some of these by removing the rustified enum flag - the results may match more closely to the existing generated bindings without it, though I'm uncertain.

Edit: I guess worst case, for now we can always just add in the individual functions we're missing by hand :+1:

diwic commented 4 years ago

@mitchmindtree Still, given how it looks, it must have been generated using bindgen at some point. Maybe I can find some command line switches that gets rid of most of the alsa-rs errors, as well as most of the libc stuff it generates as well. The whitelisting switches look hopeful.

This probably won't be finished today so feel free to submit PR's that add the functions and types manually as an interim solution.

diwic commented 4 years ago

Okay, already down to 41 errors, this is looking promising 🙂

diwic commented 4 years ago

@mitchmindtree So actually this wasn't as hard as it looked like! I've pushed commits to both alsa-sys and alsa-rs now, hopefully implementing what you're looking for. Would you like to take the code for a spin before I release new versions of them?

mitchmindtree commented 4 years ago

Thanks @diwic! It looks like I can compile alsa and alsa-sys master with cpal and call the set_tstamp_type method successfully :+1:

That said, it's hard to tell if the new method is working or not as I can't manage to produce a valid timestamp on my system :sweat_smile: I'll discuss this further at rustaudio/cpal#397.