search

honzasp / rust-turbojpeg

Rust bindings for TurboJPEG library
MIT License
16 stars 12 forks source link

Fix build on windows #8

Closed rschoon closed 2 years ago

rschoon commented 2 years ago

This fixes two problems that seem to prevent the library from building on windows.

The first problem is the static version of library is ending up being called libturbojpeg-static.lib instead of just libturbojpeg.lib.

The second problem is that the tjMCUWidth and tjMCUHeight symbols are not found. This appears to be due to them being declared as static within the turbojpeg header, but in that case I'm not sure why they would ever be linkable. To work around it I added aliases that do get included/exported via an additional C file (so it should work independent of how bindgen is run).

I verified it works by running cargo test --features image successfully for both Linux and windows-msvc. I was not able to verify windows-gnu works (cmake is picking up clang and failing for some reason, probably a local problem). I was also not able to get it working while regenerating bindgen because I seem to get different types for the enums. I also only attempted to make it work on windows with a static build of libturbojpeg.

honzasp commented 2 years ago

Hi Robin, thank you for the PR :) However, rather than introducing extra.c, I think it would be simpler to just implement the same functionality with a match. I created a branch winbuild-fix in this repo that replaces your last commit, do you think you could test whether it works for you? :) If yes, I will merge it and release a new version.

honzasp commented 2 years ago

Feel free to take the commit caf08b into your branch, especially if you find that it needs some modifications. If I just took your commit and merged it separately, Github would not understand that your PR was in fact merged :)

rschoon commented 2 years ago

I don't know the significance of those constants and if they even could ever change from version to version, but that should work, and it's definitely simpler. I'll try with it.

rschoon commented 2 years ago

Ok, looks good and works well for me. I reset my branch to yours, so you can merge whenever if you'd like.

honzasp commented 2 years ago

Awesome, thank you very much for your contribution :) I've released a new patch version of both crates.

honzasp commented 2 years ago

The constants describe the size of the MCU block (minimal coded unit), depending on the type of chroma subsampling. This is important for some lossless transforms, and the values are fixed constants, so it is OK to hard-code them. (In the same way as they are hard-coded in the turbojpeg.h header file.)