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

Resizing then extracting Webp images past 16,383 now fails from sharp 0.30.x #3262

Closed
blacha opened this issue Jun 15, 2022 · 8 comments · Fixed by #3267
Closed

Resizing then extracting Webp images past 16,383 now fails from sharp 0.30.x #3262

blacha opened this issue Jun 15, 2022 · 8 comments · Fixed by #3267

Comments

@blacha
Copy link
Contributor

blacha commented Jun 15, 2022

Possible bug

Is this a possible bug in a feature of sharp, unrelated to installation?

  • [ x ] Running npm install sharp completes without error.
  • [ x ] Running node -e "require('sharp')" completes without error.

Are you using the latest version of sharp?

  • [ x ] I am using the latest version of sharp as reported by npm view sharp dist-tags.latest.

What are the steps to reproduce?

Example https://github.com/blacha/sharp-resize

What is the expected behaviour?

In sharp 0.29.x I can resize a webp image to basically any size and then extract a region from the enlarged image without any issue. In sharp 0.30.x this now fails with Error: webp: bad image dimensions

await sharp(buffer).resize(32768, 32768)
  .extract({ top: 16384, left: 16384, width: 1024, height: 1024 })
  .webp().toBuffer();

I see some work was done in libvips recently to improve image resizing especially with SVGs, is there a easy way to go back to the old resizing method?

Please provide a minimal, standalone code sample, without other dependencies, that demonstrates this problem

Example https://github.com/blacha/sharp-resize

Please provide sample image(s) that help explain this problem

Sample image in repo.

@blacha blacha added the triage label Jun 15, 2022
@lovell
Copy link
Owner

lovell commented Jun 15, 2022

sharp v0.30.x now takes greater advantage of libwebp's scale-on-load feature, however it doesn't appear to allow upscaling beyond the maximum dimensions for WebP. What we're seeing is an "encoding" error when decoding, which is maybe a bug in libwebp, or at least a (lack of) feature.

https://developers.google.com/speed/webp/docs/api#advanced_decoding_api

I guess we could avoid any upscaling via libwebp (we can downscale still, as that provides a good speedup) by checking the value of scale approximately here:

sharp/src/pipeline.cc

Lines 224 to 232 in b10d8f8

if (baton->input->buffer != nullptr) {
// Reload WebP buffer
VipsBlob *blob = vips_blob_new(nullptr, baton->input->buffer, baton->input->bufferLength);
image = VImage::webpload_buffer(blob, option);
vips_area_unref(reinterpret_cast<VipsArea*>(blob));
} else {
// Reload WebP file
image = VImage::webpload(const_cast<char*>(baton->input->file.data()), option);
}

Happy to accept a PR, if you're able.

@lovell lovell added bug and removed triage labels Jun 15, 2022
@blacha
Copy link
Contributor Author

blacha commented Jun 15, 2022

Would you just disable this logic if scale > 1.0? Its been a while since I've done any C++ so quite rusty in that space, lets see if I can get a dev env working!

Also side note this seems to greatly affect the performance of extracting after scale too.

Doing a resize(size) then extracting a 256x256 region in 0.29.x is pretty similar times for all scales

resize:0.29.x:1024: 11.435ms
resize:0.29.x:2048: 9.845ms
resize:0.29.x:4096: 9.83ms
resize:0.29.x:8192: 10.089ms

where as 0.30.x has pretty big slow downs the bigger the scale is!

resize:0.30.x:1024: 20.907ms
resize:0.30.x:2048: 38.729ms
resize:0.30.x:4096: 124.823ms
resize:0.30.x:8192: 359.103ms

Maybe I'm approaching this is a weird way, basically my use case is to take many files scale them up slightly and extract regions out of them and join them back together.

The current logic uses a resize -> extract. so to work around this issue I was thinking of doing a extract -> resize but this leads to pretty big artefacts around the edges,

Extract -> Resize
extract-resize
Resize -> Extract
resize-extract

These test cases are super extreme and generally the edges are no where near as bad but it does lead to some very weird looking images.

const scale = 8;
// Resize Extract
sharp(buffer).resize(256 * scale, 256 * scale).extract({ top: 128 * scale, left: 128 * scale, width: 256, height: 256 }).webp().toBuffer(),

// Extract -> Resize
sharp(buffer).extract({ top: 128, left: 128, width: 256 / scale, height: 256 / scale  }).resize(256, 256).webp().toBuffer()

@lovell
Copy link
Owner

lovell commented Jun 17, 2022

Would you just disable this logic if scale > 1.0?

Yes, this sounds like the right approach, especially as the upsampling performance of libwebp is much worse than libvips.

Did you see that sharp supports tile-based output to generate deep zoom image pyramids? I'm not sure it quite meets your needs, but the lowest level of tiles might be close to what you're doing. https://sharp.pixelplumbing.com/api-output#tile

@blacha
Copy link
Contributor Author

blacha commented Jun 17, 2022

Would you just disable this logic if scale > 1.0?

Yes, this sounds like the right approach, especially as the upsampling performance of libwebp is much worse than libvips.

Ok ill have a go at that when i get some spare time!

Did you see that sharp supports tile-based output to generate deep zoom image pyramids? I'm not sure it quite meets your needs, but the lowest level of tiles might be close to what you're doing. https://sharp.pixelplumbing.com/api-output#tile

This is basically what we are doing right now, not sure how detail you want 😁

We (LINZ) are a govt agency are distributing TBs of imagery to end users (https://basemaps.linz.govt.nz) we create cloud optimised geotiffs which are basically geo referenced pyramids with GDAL, then use sharp/libvips to join our various imagery sets to together base don user requirements, eg highest resolution imagery vs most recent imagery.

All the source code is open here https://github.com/linz/basemaps

@jcupitt
Copy link
Contributor

jcupitt commented Jun 19, 2022

we create cloud optimised geotiffs which are basically geo referenced pyramids with GDAL

If you're only chopping huge images into tile pyramids, then sharp should be a lot quicker and lighter than gdal. You'll need to use gdal if you are applying a projection, of course.

@blacha
Copy link
Contributor Author

blacha commented Jun 19, 2022

we create cloud optimised geotiffs which are basically geo referenced pyramids with GDAL

If you're only chopping huge images into tile pyramids, then sharp should be a lot quicker and lighter than gdal. You'll need to use gdal if you are applying a projection, of course.

Everything is georeferenced, all the tile pyramids also have to align to a TileMatrixSet, We use both a local New Zealand projection (EPSG:2193) and a global projection (EPSG:3857) so GDAL does a ton of heavy lifting for the pyramid creation.

@lovell lovell reopened this Jun 20, 2022
@lovell lovell added this to the v0.30.7 milestone Jun 20, 2022
@lovell
Copy link
Owner

lovell commented Jun 22, 2022

v0.30.7 now available, thank you @blacha for both reporting and fixing this.

@blacha
Copy link
Contributor Author

blacha commented Jun 22, 2022

Awesome thanks for all the hard work @lovell ! We have deployed the new version and it fixes our issue 🎉

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

Successfully merging a pull request may close this issue.

3 participants