rust-portaudio
rust-portaudio copied to clipboard
Redo FFI bindings using rust-bindgen and move them into a unique `portaudio-sys` crate along with the build script.
Many users have been experiencing issues with the bindings, particularly with the Linux ALSA backend. As the ffi.rs module was written by hand quite a while ago, there's a chance that some error might have been introduced due to human error. By generating the bindings using rust-bindgen we can have higher assurance that the struct layouts are correct and that we aren't missing bindings to any parts of the API.
By moving the bindings into their own crate portaudio-sys we can isolate FFI related issues and hopefully achieve slightly faster compilation. We could also move the build script into this crate in order to keep rust-portaudio clean and focused on the rust-esque abstractions, while portaudio-sys takes care of building the C portaudio lib and FFI. All bindings to platform-specific extensions could be done in this crate also.
#128, #136 and #133 may be related.
I guess this can be marked as resolved now due to merging #152 .
Almost, I just realised we won't be able to publish the changes without first publishing the portaudio-sys crate. I believe that in order to publish using cargo, all dependencies must also be on crates.io. I'd publish it as portaudio-sys, however it already exists.
Our options are:
- Publish our generated bindings under a different name like
rust-portaudio-sys. - See if we can use the existing
portaudio-syscrate. If they too used rust-bindgen, then I imagine their bindings may almost be identical.
I think option 2 would be nice to avoid more fragmentation and share the maintenance burden, however they don't have a linked repository on crates.io so I'm unsure how easy it would be to collaborate or add patches if we need them.
For reference, the bindgen command I used is here and everything after /* automatically generated by rust-bindgen */ is untouched.
I gave a quick look at portaudio-sys, I’d say it is less like the default output. Might be because of different parameters, a couple of manual edits, or they used an older version of bindgen. The biggest difference is the use of libc.
I don’t remember why I constified some enums exactly, because of the way error codes are handled maybe.
To answer a previous message, libc is still used by Buffer in stream.rs which needs malloc and free.
I just found the other repo. @mitchmindtree I definitely agree that sharing the burden and preventing fragmentation is reasonable but i can't estimate whether it's a lot of work to integrate the other bindings. I think it might turn out to as much afford as #152. @cdghibaudo what do you think?
Just thought about it again: Isn't it maybe better to just make our portaudio-sys an internal module (not a seperate crate)? Basically I think the bindgen generated part shouldn't be edited at all, to make sure that no ffi-related bugs occur. Since the ffi shouldn't change noone would have to maintain the ffi. Furthermore if we would do edit the ffi for some reason we would not have to watch out for compability with portaudio-rs.
Yeah i agree this sounds like the best/simplest option :+1:
On Fri, 26 May 2017 19:22 joeschman [email protected] wrote:
Just thought about it again. Isn't it maybe better to just make our portaudio-sys an internal module (not a seperate crate). Basically I think the bindgen generated part shouldn't be edited at all, to make sure that no ffi-related bugs occur. Since the ffi shouldn't change noone would have to maintain the ffi. Furthermore if we would do edit the ffi for some reason we would not have to watch out for compability with portaudio-rs.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/RustAudio/rust-portaudio/issues/137#issuecomment-304234884, or mute the thread https://github.com/notifications/unsubscribe-auth/AEX_bWB6_GvFKWO6MfCrz_AFrPjBBb09ks5r9pnDgaJpZM4Ii-wm .
I agree, it isn’t much of a burden.
I came here following from https://github.com/RustAudio/rust-portaudio/issues/128 because I'm facing the same issue I think. Wondering whether this discussion was concluded.