Allowing disabling of pixel limit #316
Conversation
|
Hello, thanks for adding this feature. I'll add comments inline. Are you able to update the API docs to mention the acceptance of a boolean value? I'm happy for any always-positive parameters to change from signed to unsigned integer if that helps too. |
| @@ -805,6 +805,19 @@ describe('Input/output', function() { | |||
| }); | |||
| }); | |||
|
|
|||
| it('Disabling limit works', function(done) { | |||
| sharp(fixtures.inputJpg).metadata(function(err, metadata) { | |||
There was a problem hiding this comment.
Is the use of metadata required here?
|
@lovell should be able to handle the noted things and docs, would you be opposed to adding a really large image to the test suite to test these things better? |
Great idea. As long as the compressed file size is not "really large", this would also provide a good example of libvip's low memory usage. A single-colour, non-interlaced PNG (with row filtering disabled?) should compress the best. Decompression memory usage relates to image width rather than height, so perhaps try something like 2000px wide and whatever-you-need px high. |
|
lovell: I have a use case where an analyst needs the ability to specify an area of an image that needs further "review"... in this case, the images are what I would consider "massive", 2-3Gb tiffs that i convert to JPGs, normally in the range of 40k-50k pixels wide/tall. What intrigues me about your sharp library, besides the performance metrics, is the ability to read once...crop multiple times unlike GM or others where it is necessary to re-read the image each time an action is required. Simple "tiling" doesn't work here because I need to apply proprietary "filters" to the image where edge sampling is important. Obviously these images are larger that the existing 268million pix limit. Is this limit a 'NIX system limitation? |
|
just an update my holidays were a little hectic but I do plan on getting back to wrapping this up in January |
|
Great to hear... Hope you had at least a little down to time to unwind/relax/recoup?! Regards, Scott Sent from my iPhone
|
|
@kentongray No worries, thanks for the update. Let me know if you need any guidance. |
|
The master branch is now using the vips8 C++ bindings so you'll notice a conflict or two when rebasing. |
|
This would be a great feature. Is anyone currently working on @lovell comments? How is the current permitted value calculated for I get a exception telling me the permitted range but assume it varies depending on the machine because I noticed others using higher values than I (current: 268402689). Is this correct? |
|
@webbrandon Please go ahead and work on this if you're willing and able. |
|
Hey guys sorry for dropping off the planet on this one, but nothing will make progress like needed a feature (can't wait to use the padding). OK so I made the following changes.
The only problem I potentially see is the large image test takes a bit (16267ms) on my machine. Not sure if that is acceptable or not. |
|
i'm guessing the failing test is due to the large image... let me know your thoughts on how to proceed blarg really was trying to wrap this up today |
|
Loving the cyan, thanks for the updates @kentongray. It looks like the new test is timing out on Windows CI. Are you able to double the Please can you squash to a single commit (I've recently enabled PR squashing as the default behaviour but this PR was created before that, so you'll have to do it manually). More importantly, please add your good self as a fully-fledged contributor. |
kentongray
force-pushed
the
master
branch
3 times, most recently
from
0695fe1
to
ed5b393
Compare
Update docs Added a giant image for testing Adding myself to contributors Added tests to verify giant image can be opened Extend test-win time limit (because of large images)
|
unfortunately still failed it seems 😭 |
|
No worries, I'll look after the failing test (either add more time or skip on Windows). Thanks again Kenton for all your work on this feature - this will be included in v0.14.1. |
|
@kentongray your test is now <50s on Windows CI with 71fb839 |
|
yay thanks @lovell |
|
no problem @kentongray . thank you very much for your contribution. |
As discussed in #250 allowing boolean values for limitInputPixels this partially removes the limit. Unfortunately there are other limits based upon the int32 for width/height that I stumbled on shortly after so I'd like to take a stab at that but this gets you significantly larger images. Let me know your thoughts