-
Notifications
You must be signed in to change notification settings - Fork 89
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
Redo FFI bindings using rust-bindgen and move them into a unique portaudio-sys
crate along with the build script.
#137
Comments
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 Our options are:
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 I gave a quick look at 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 |
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)? |
Yeah i agree this sounds like the best/simplest option 👍
…On Fri, 26 May 2017 19:22 joeschman ***@***.***> 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
<#137 (comment)>,
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 #128 because I'm facing the same issue I think. Wondering whether this discussion was concluded. |
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 usingrust-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, whileportaudio-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.
The text was updated successfully, but these errors were encountered: