search

geosolutions-it / imageio-ext

Additional plugins and extension for the standard Java ImageIO library
Other
139 stars 80 forks source link

imageio-ext-turbojpeg depends on an obsolete version of libjpeg-turbo (v1.2.1) #305

Open miceg opened 1 month ago

miceg commented 1 month ago

Originally reported in: https://github.com/kartoza/docker-geoserver/issues/653

When using the GeoServer libjpeg-turbo-plugin with libjpeg-turbo v3.0.3 (the current version) to render a layer as JPEG over WMS, it fails with:

16 May 14:36:59 WARN   [geoserver.servlets] - OutputStream could not be aborted in time. An error has occurred and could not be sent to the user.
16 May 14:36:59 ERROR  [geoserver.ows] - 
java.lang.UnsatisfiedLinkError: 'int org.libjpegturbo.turbojpeg.TJCompressor.compress(byte[], int, int, int, int, byte[], int, int, int)'
    at org.libjpegturbo.turbojpeg.TJCompressor.compress(Native Method)
    at org.libjpegturbo.turbojpeg.TJCompressor.compress(TJCompressor.java:146)
    at org.libjpegturbo.turbojpeg.TJCompressor.compress(TJCompressor.java:164)
    at it.geosolutions.imageio.plugins.turbojpeg.TurboJpegImageWriter.write(TurboJpegImageWriter.java:201)
    at org.geoserver.map.turbojpeg.TurboJpegImageWorker.writeTurboJPEG(TurboJpegImageWorker.java:126)

There are similar issues when rendering a raster layer compressed with JPEG.

The root cause of the issue is the it.geosolutions.imageio-ext.imageio-ext-turbojpeg package.

Detail

it.geosolutions.imageio-ext.imageio-ext-turbojpeg (as used in the GeoServer TurboJPEG plugin) currently depends on libjpegturbo "v1.2.1.5" Java wrappers.

Despite the package being org.libjpegturbo.turbojpeg-wrapper v1.2.1.5, it appears that GeoSolutions IT publishes its own fork of the library with the same class names, and it appears to be forked from a pre-1.0 version of libjpeg-turbo from 2013 – not v1.2.1 (released in 2015) EDIT: GitHub's timestamps are wrong, v1.2.1 was actually released in 2012; so this is not a pre-1.0 version.

Upstream libjpeg-turbo still maintains their own libraries, but don't appear to publish them in Maven.

It doesn't look like this repository includes any of the JNI C code (turbojpeg-jni.c), so this fork depends on upstream's libturbojpeg.so.

Around September 2022, upstream removed a bunch of methods from the Java and JNI wrappers, breaking ABI compatibility. These landed in libjpeg-turbo v2.1.90/3.0 beta1 (released in February 2023), and are now in stable v3.0.x releases (from July 2023) – so probably been broken since at least then... but it could have been anything in the last 11 years.

What I think needs to be done here

In any case, expecting someone to use an 11+ year old pre-1.0 version of libjpeg-turbo is totally unreasonable. 😄

aaime commented 1 month ago

The direction you propose is more than reasonable. We don't "expect" anyone to use the turbo-jpeg plugin, but would gladly accept a pull request. The existing code is free, development time is not.

NyakudyaA commented 1 month ago

If the expectation is that no one should use the plugin I guess the documentation also needs to be updated to reflect such i.e https://docs.geoserver.org/main/en/user/extensions/libjpeg-turbo/index.html

aaime commented 1 month ago

No, the expectation is that anyone is free to do what they want. We have no complaints from customers about the 11 years old library, so it stays that way. If anyone steps up to upgrade the library, or is willing to fund such activity, then it will happen. Complaints won't achieve anything.

In the GeoServer wiki we have this useful guide on how to get things done, which is factual and applies to all upstream GeoServer projects managed by the same community of developers (GeoTools, GeoWebCache, jai-ext, imageio-ext): read warmly recommended.

miceg commented 1 month ago

Understood re: development effort.

For what it's worth, I don't think the original author intended or anticipated this outcome. I think they were just trying to implement a feature, and took the path of least resistance which then unintentionally introduced this dependency.

As for my comment about "expecting" someone to run an old version of libjpeg-turbo:

It's lucky that it did not happen sooner. 😅

Assuming that there are no issues with using v2.1.x (< v2.1.90) ABIs, at least v2.1.x still gets maintenance support, and many Linux distros haven't migrated to v3.0.x yet, so users of that GeoServer plugin can still apply (security) updates. As for how long that will last, time will tell, but it will certainly not last forever.

That all said, many Linux distros have switched from libjpeg to libjpeg-turbo, and it is now an official reference implementation. libjpeg-turbo can be built to be ABI and API compatible with various versions of libjpeg, so it may not be necessary to use the TurboJPEG-specific Java APIs to take advantage of libjpeg-turbo's performance improvements.

It might not be worth the effort to "fix" this, and it may be a signal to retire the plugin.

I'll have to see what I can do about employer funding / support for that work (to potentially test performance assertions, make a formal proposal and then ship it), as I'm evaluating GeoServer among other choices for a couple of different projects.

miceg commented 1 month ago

I've posted an issue on GeoServer's issue tracker, asking for opinions on the different options: https://osgeo-org.atlassian.net/browse/GEOS-11417

Once I have a better idea of what people want to do and what the bar is, I can figure out what to ask for from my employer.

aaime commented 1 month ago

I guess you're not realizing that the same group of people work on the entire stack of projects. I'll answer you there. (surprise surprise!)

miceg commented 1 month ago

I understand it to be a similar group of people, and I saw your involvement on GeoServer PRs for that plugin. 😄

I don't know the scopes of all of the various issue trackers, and who "owns" what. I apologise if this came across as hostile.

My impression was that as the root cause of the issue is in this repository owned by GeoSolutions IT, that it wasn't GeoServer's problem to solve – even if GeoServer is the only one to see the impact – which is why I posted the initial issue here.

I want to find the most efficient way to solve the issue, which at that point requires a choice about code in GeoServer's repositories... and so I felt it belonged in that issue tracker. I don't expect the wider group to be across what is happening in every one of their library dependencies. 😄