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

Separate OpenCV C bindings into sys crate and use bindgen on them #115

Open
vadixidav opened this issue Mar 5, 2019 · 10 comments
Open

Separate OpenCV C bindings into sys crate and use bindgen on them #115

vadixidav opened this issue Mar 5, 2019 · 10 comments

Comments

@vadixidav
Copy link

It is not necessary to write the C bindings again in Rust after they have been written in C. An example of this is in the crate flann-sys where the C bindings are wrappers around C++ bindings and bindgen is used on those C bindings. Separating these generated bindings into their own sys crate will also be helpful for allowing others to interact with them directly if desired.

@Pzixel
Copy link
Collaborator

Pzixel commented Mar 6, 2019

Extract sys crate is a long needed feature, but project is in somewhat stalled state. Original author doesn't commit for a year, I guess, and I got dissapointed by manually writing wrappers. There should be some kind of codegen (bindgen won't help here), but there is none. I created an issue but it seems that nobody in opencv is interested in it. See opencv/opencv#11666

@vadixidav
Copy link
Author

Well, if you will continue maintaining the repository, I can continue to write more bindings for things I am interested in. It would be good to join with the photogrammetry community in Rust (see discord issue) and maybe we can get more done collectively. I would also recommend publishing to crates.io to get more attention.

If you would like, we can discuss better ways to potentially do binding generation.

@vadixidav
Copy link
Author

Would you be fine if I manually separated the C bindings into a cv-sys crate? It shouldn't take too long and will ease the process of writing new bindings.

@Pzixel
Copy link
Collaborator

Pzixel commented Mar 6, 2019

Ideally there should be 3 crates:

  1. sys-crate with wrappers
  2. rust crate with ideomatic wrappers
  3. crate with instruction how to properly build opencv (current .ci/.windows folders content).

But there still issues with original repo creator licensing/ownership...

Of course you're free to write whenever binding you want, I'l merge them while they conform the overall repo code, but I'm sure manually writing solves nothing. I wrote my considerations in linked issue. I just warn you to not waste your time at manually writing something that must be automatised. Of course having some bindings today is better than having them generated in some future, but only if your app is blocked with repo issues (as was mine when I wrote my image detecting bot).

@Pzixel
Copy link
Collaborator

Pzixel commented Mar 6, 2019

P.S. I'm not the best seller as you can see cause I probably hardly demotivated you to write anything, but that's true: writing binding manually is a dead end.

@vadixidav
Copy link
Author

vadixidav commented Mar 6, 2019

We cannot generate Rust bindings from C++ in an automated fashion right now. The best we might get is some assisted C++ to C bindings. We can investigate that if you are interested. It would take me about a minute per method/function to create an appropriate C binding based on the docs if the Rust side was automatically generated from C. I get your concern, but I am convinced that I can easily add any API that I need from the OpenCV C++ docs to this crate every time I need it. It will only take me a minute or two to write the C binding, a few minutes to write the Rust binding and document/test it, and a minute to open a PR on here.

@Pzixel
Copy link
Collaborator

Pzixel commented Mar 6, 2019

Well, it's up to you and your time. I think you're too optimistic about the ease of writing bindings.

@vadixidav
Copy link
Author

In that case I'll separate everything out into a sys crate after work today.

@vadixidav
Copy link
Author

I am actually finding some success in generating C++ bindings. See my PR at #119 for more details. I decided to go beyond just putting the bindings in their own crate and I am attempting to use wrapper C++ bindings instead. After that, it wont be too difficult to simply remove the wrapper entirely. That will require careful whitelisting of types for the build script, but it is entirely possible to expose the C++ bindings correctly directly from OpenCV. It even handles name mangling:

extern "C" {
    #[link_name = "\u{1}?hmm_drop@cvsys@@YAXPEAU?$Ptr@VOCRHMMDecoder@text@cv@@@cv@@@Z"]
    pub fn cvsys_hmm_drop(ocr: *mut u8);
}

The PR is still only limited to putting this in a sys crate. I will make a follow-up effort to remove the wrapper entirely soon thereafter.

@vadixidav
Copy link
Author

This will be closed after #117 is merged.

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