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

Missing snd_pcm_sw_params_set_timestamp_type #6

Closed
mitchmindtree opened this issue Apr 27, 2020 · 6 comments
Closed

Missing snd_pcm_sw_params_set_timestamp_type #6

mitchmindtree opened this issue Apr 27, 2020 · 6 comments

Comments

@mitchmindtree
Copy link

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
Copy link
Owner

diwic commented Apr 27, 2020

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:

  • Bindgen replicates the entire of libc into the bindings, causing conflicts between libc and alsa (e g, now pollfd is defined in both crates).
  • Bindgen creates data for the current architecture, i e, a header generated on an amd64 and one generated on armhf will not look the same. There is a flag --sizet-is-usize that fixes one of those issues but I'm a bit scared that something else will fall through.
  • 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).

@mitchmindtree
Copy link
Author

mitchmindtree commented Apr 27, 2020

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 👍

@diwic
Copy link
Owner

diwic commented Apr 27, 2020

@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
Copy link
Owner

diwic commented Apr 27, 2020

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

@diwic diwic closed this as completed in 30b1210 Apr 27, 2020
@diwic
Copy link
Owner

diwic commented Apr 27, 2020

@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
Copy link
Author

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

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 😅 I'll discuss this further at RustAudio/cpal#397.

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

No branches or pull requests

2 participants