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

What version of webp-imageio is used? #67

Closed
joostoudeman opened this issue Feb 27, 2023 · 17 comments
Closed

What version of webp-imageio is used? #67

joostoudeman opened this issue Feb 27, 2023 · 17 comments

Comments

@joostoudeman
Copy link

Hey,

I've got the need to support the sharp_yuv property of the wep_imageio.
So far no other fork on github seems to have this, so I did write my own solution for it at https://github.com/joostoudeman/webp-imageio. However I don't want to add another fork to the maven central, and spread out the options, and the project I originally forked from is no longer maintaining.

So I would like to send a pull request to your project, but I don't know if the precompiled libraries you use are up to date enough. Can you tell me what version it is?

@mateuszkwiecinski
Copy link
Member

Hey @joostoudeman 👋 Sorry I somehow missed your comment :/
If you still haven't published your changes anywhere then sure, I'll be willing to help :)

To clarify on what this fork is: I failed to build the binaries myself, so this repository serves as a sort of wrapper for binaries https://github.com/gotson/webp-imageio has built (including the ARM one from gotson#1 I needed for https://github.com/usefulness/easylauncher-gradle-plugin).

Answering your question directly: I don't know which version of libwebp the binaries were compiled with, but they should match the ones published on gotson@4b29c51

@joostoudeman
Copy link
Author

Hey,

That's fine.
I did manage to build the binaries, I had to since the ones from gotson were too old.
the stuff I did can be found back in gotson#2, maybe it might help you out to be able to build it?

I could of course try to make a pull request on your repo, including rebuilds. Would that work?

@joostoudeman
Copy link
Author

joostoudeman commented Jun 16, 2023

So, I just made a pull request to get the sharp_yuv support in there.
Also included an update to 1.3.0, and added some logic to crossbuild several binaries.
#81

Of course you can always pick the changes you would like to have, and drop those you don't want.

@mateuszkwiecinski
Copy link
Member

Thanks! I have little to no knowledge on building native binaries/bindings :/
So far, seeing the libwebp project exposes also building via Gradle and I gave it a go on this brach: https://github.com/usefulness/webp-imageio/blob/updates/.github/workflows/testing.yml#L41 but it seemed to build executable only for a single architecture and in general Gradle isn't really the preferred way of building webp binaries :/

I'm willing to merge your PR in the full scope (and I truly appreciate the effort 🙇), but can you also help me understand:

  1. how to build all the binaries (.so, .dll and .dylib) locally? What are the prerequisites etc? Ideally a step-by-step solution would be great, so I could move it to the CI so it could be built automatically.
  2. Seeing the arm for MacOS binary is added manually, do you happen to know what's missing to include it in the dockcross setup?
  3. I tried at the very beginning, but I'm having trouble understanding how dockcross manages to build binaries for all architectures. Can you point my at any resources you find useful to get familar with what happens in your PR? I'm willing to read anything that explains cmake or dockcross.

@joostoudeman
Copy link
Author

Building the binaries is something I think is convenient to have in the project, hence the 'libwebp-jni' folder I added.
The approach is indeed to build it locally to update binaries.

In libwebp-jni, there's a git submodule pointing to the libwebp of google, checking out a new tag is how to prepare to build a newer version.

  1. Building is done using docker, or in my case podman, so you need to have that installed. 'compile-all.sh' will kick off all the builds. it uses dockcross to build the linux and windows binaries. I suppose the CI can just run the same commands as compile-all.sh does. The Mac x86_64 is built using a crossbuild docker image once made by gotson, it's based on multiarch crossbuild. I might want to use the original instead...
  2. I don't know of a way to reliably crossbuild the OSX arm binary yet, so that one was built on a mac with arm chip, using the similar command as used for the other Mac: ./compile.sh Mac aarch64 /workdir/multiarch-darwin.cmake
    There is a whole discussion about it here: Support for Apple M1 arm64/aarch64 chips multiarch/crossbuild#51
  3. Did you find the github page for dockcross? (https://github.com/dockcross/dockcross) That's actually the best resource I can find. cmake is actually just the command to compile the binaries.

I also see the branch fails some checks. I also can't seem to find out why it doesn't build. Do you know?

@mateuszkwiecinski
Copy link
Member

mateuszkwiecinski commented Jun 16, 2023

Ok, surprisingly I was able to build binaries locally - installing podman was enough, that is helpful, thanks! 🚀

I tried to read the dockcross Readme, but since nothing worked for me locally and their Readme assumed I have some basic knowledge alread it felt like I have to first understand Docker and Cmake. As a reasult, given my available time, I shifted towards hosting just the binaries in this repo. Now that things work for me I should be able to grow some knowledge around it. Thanks again 🙏

@joostoudeman
Copy link
Author

Indeed docker knowledge will help a lot, as well as cmake.
Happy I can contribute to this library. It is my preferred solution opposed to yet another new fork.

@mateuszkwiecinski
Copy link
Member

Thanks. I'll plan to do some work this weekend to:

  1. Attempt to add a test confirming the YUV sharp support, just to make sure the recent changes actually work
  2. Add a few multi-platform smoke tests. Since this fork gained some traction, and with your recent changes other projects than my own may find it useful, I'll try to add more coverage to ensure it works in the most common simple setups.

I'll try to publish a release in 2-3 days from now, and hopefully I'll be finally able to answer question from this issue's title 😛

@mateuszkwiecinski
Copy link
Member

Okay, so my attempt to do 1. from above can be found here and here

Maybe I don't understand something, but I expected to see different images produced after toggling the useSharpYUV flag, but regardless off useSharpYUV state produced images are blurry :/
It looks like all the bindings work (so the library as an ImageIO wrapper works), but I'm unable to pass proper arguments to libwebp natives? 🤔

Would you be able to help me to fix a failing test here:

@Disabled("useSharpYUV doesn't make any difference")
?

@mateuszkwiecinski
Copy link
Member

I think I've got it 🎉 (Or at least I have a passing test now 😅)
It turned out the logic to call the native functions has changed: 301adf1 (#88) - that's what I concluded reading examples - link.

As a result basically re-did the most of the native layer, because it turned out the existing bindings (dating back the original luciad project, 3 forks ago 😅) were out-of-sync with that Java code expected 🤷 I managed to expose almost all options available in the libwebp project, except few enums(?) which I wasn't sure how to map using JNI.

Things look promising so far 🤞

@joostoudeman
Copy link
Author

You've been busy.
I was actually just thinking I introduced a bit of a problem when it comes to old operating systems.
This is something I locally solved by using older images for cross compiling.

I'll see if I can puzzle out how I fixed that, and get a MR for that. The one example I know is that the compilation will not work on RedHat 8 machines (yes, some people still use that one...)

@mateuszkwiecinski
Copy link
Member

I was actually just thinking I introduced a bit of a problem when it comes to old operating systems.

oh :/ Do you mean when building binaries (running the ./compile script)? Or when using the binaries via ImageIO?

@joostoudeman
Copy link
Author

The use via ImageIO.
It has something to do with the glibc library. When building with a newer glibc it doesn't work with the lower version. It's a classic problem when it comes to crosscompiling.

@mateuszkwiecinski
Copy link
Member

I can finally answer the question from the title:

What version of webp-imageio is used?

this ImageIO wrapper ships with binaries built from v1.3.0 of libwebp 🎉

I hope I didn't break anything since your last PR 🤞 All the changes, seem to work for me locally, and I hope you'll be able to leverage them as well. 0.3.0 should shortly be available on mavenCentral().
I'll close the issue for now, but in case any other challenges appear - your contributions are always welcome! 🙇

@kaiwidmann
Copy link

In September 2023 a critical security issue was found in libwebp, see https://snyk.io/de/blog/critical-webp-0-day-cve-2023-4863/
Is someone able to update the webp native libraries to v1.4.0? I'd like to help, but as of yet I'm not successful in building the libs myself.

@mateuszkwiecinski
Copy link
Member

Is someone able to update the webp native libraries to v1.4.0?

You can find binaries built from 1.4.0 tag in #195

@kaiwidmann
Copy link

Thanks so much, I didn't realize they were already in here. This seems to be the most active fork of webp-imageio, so I'll use this one instead of the original.
Thanks for your great work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants