wxWidgets icon indicating copy to clipboard operation
wxWidgets copied to clipboard

Add support for WebP image file format via libwebp.

Open hoehermann opened this issue 2 years ago • 7 comments

This is a pull request for #23489. Please refer to the conceptual discussion there. Discuss suggestions regarding this particular pull-request here.

Update: I consider this ready for merging.

hoehermann avatar Apr 16 '24 13:04 hoehermann

Because this requires a library that is not built by wxWidgets itself, I'm assuming that anyone wanting to use this functionality will not be able to use any of the pre-built binaries? Or are you envisioning that all the pre-built binaries should have webp support turned on by default? If the pre-built binaries aren't going to have webp support, then it should be documented that the user must build wxWidgets from source to get this functionality.

I'm not familiar with the Chromium repository system, but I do note that the issue you refered to ( https://chromium-review.googlesource.com/c/webm/libwebp/+/4868215 ) states that the change was merged on 9/19/2023 -- so wouldn't the fix be in the current 1.4 release (released 4/12/2024)?

Randalphwa avatar Apr 27 '24 14:04 Randalphwa

I answered to Randalphwa's question at https://github.com/wxWidgets/wxWidgets/issues/23489#issuecomment-2080917306 to keep the discussion in one place.

hoehermann avatar May 03 '24 19:05 hoehermann

@vadz Is this okay or do I need to improve something?

hoehermann avatar Jun 01 '24 01:06 hoehermann

Sorry, I've completely missed that it was ready, thanks for the ping. I'll try to look at it but, due personal circumstances, it might take some time.

Any reviews/tests by others would be very appreciated!

vadz avatar Jun 08 '24 14:06 vadz

Thanks a lot once again for your work, it's great to have support for WebP and globally it looks pretty good.

Thank you. ☺

But an even more important consideration is to ensure that this code is actually getting tested, i.e. we need to modify the CI jobs to install libwebp-dev etc, to at least test it under Unix. If we can test it under Windows it would be great too, of course, but I guess this might be more complicated — but let me know if you see a way to do it.

Does the CI on Windows already employ vcpkg by any chance? I think just executed vcpkg.exe install --triplet x64-windows-static libwebp, CMake picked it up automagically and all was fine.

Sorry again for the delay!

No problem, however…

I would like to simplify the code creating WebPDemux as explained, if possible, please let me know if you can do it or if I should do it myself.

…right now, things are rather stressful on my end. I can do some more work here, but not before November 2024, I am afraid.

hoehermann avatar Sep 22 '24 20:09 hoehermann

Does the CI on Windows already employ vcpkg by any chance?

I'm afraid it doesn't. It could be a good idea adding a build installing the external libraries using vcpkg, but for now I'd settle for just installing the WebP libraries from the distribution repositories under Linux — this should be trivial and, while it wouldn't test Windows at all, still much better than nothing.

I would like to simplify the code creating WebPDemux as explained, if possible, please let me know if you can do it or if I should do it myself.

…right now, things are rather stressful on my end.

Sorry to hear this!

I can do some more work here, but not before November 2024, I am afraid.

There is no particular urgency with this, so if you think you'll have time to return to this later, I'd rather leave it to you. But I'd like to release 3.3.0 before the end of the year (even if I'm far from sure that we'll manage to do it), and it would be nice to have WebP support in it.

vadz avatar Sep 22 '24 21:09 vadz

I'm afraid it doesn't. It could be a good idea adding a build installing the external libraries using vcpkg, but for now I'd settle for just installing the WebP libraries from the distribution repositories under Linux — this should be trivial and, while it wouldn't test Windows at all, still much better than nothing.

It might work in the msys2/mingw64 build, when adding the mingw-w64-x86_64-libwebp package to the install section. When I check the contents of this package it seems to install the required webp cmake files.

MaartenBent avatar Sep 24 '24 08:09 MaartenBent