search

garkimasera / vlc-rs

Rust bindings for libVLC media framework.
MIT License
66 stars 18 forks source link

Use bindgen #8

Open garkimasera opened 4 years ago

garkimasera commented 4 years ago

issue: #6

alexandre-janniaux commented 4 years ago

Hi, overall this works really fine for me.

Few points:

garkimasera commented 4 years ago

Thank you for reviewing!

it might make sense to use pkg-config in the build.rs to find VLC.

It will be better, thanks.

should libvlc version be features or different versions ? features allow simpler breaking change signalling but is confusing for the user (especially in the documentation) and harder to maintain.

Is it the same question in #6 ? I have not yet determined.

it might make sense to use things like bitfield_enum or rustified_enum with bindgen to use a better enum field for some variants, and have more readable wrapper code, or maybe through the ParseCallbacks in a second step ? It can also be improved through prepend_enum_name.

I missed the enum generator option of bindgen. I will try it.

should we enable enable_function_attribute_detection ?

I can not find differences in generated bindings.rs between the option enabled and disabled. I think remaining default is better for now.

alexandre-janniaux commented 4 years ago

Hi,

Is it the same question in #6 ? I have not yet determined.

Actually yes :)

garkimasera commented 4 years ago
garkimasera commented 4 years ago

I mean that vlc-rs 0.4 does not include functions added since libvlc 3.0.0. There is no problem using vlc-rs 0.4 with the new libvlc.

mfkl commented 4 years ago

Are you still planning to work on this @garkimasera?

garkimasera commented 4 years ago

This PR works without fixes. But I have no plan for detail API design because I have no longer used libvlc in my project. I can merge this PR if bindgen version is needed.

mfkl commented 4 years ago

Ok, are you still planning to work on vlc-rs in the future? I think a rust binding for the libvlc ecosystem would be good.

garkimasera commented 4 years ago

I cannot review PRs that depend on the latest libvlc API specification. So, transferring or forking this repository will be the best choice for me.

alexandre-janniaux commented 4 years ago

I cannot review PRs that depend on the latest libvlc API specification. So, transferring or forking this repository will be the best choice for me.

Transferring to code.videolan.org/videolan and the ownership of vlc-rs crate to Mfkl is probably a good idea then! Do you have private projects depending on the current state, and constraints that you would like to be accounted?

mfkl commented 4 years ago

@garkimasera You can mail me at me@martinfinkel.com.

garkimasera commented 4 years ago

OK, I'll invite new owner to vlc-rs and libvlc-sys.

Do you have private projects depending on the current state, and constraints that you would like to be accounted?

There are not any constraints.