-
-
Notifications
You must be signed in to change notification settings - Fork 699
off by one problem when generating pyramidal tiff #659
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
Comments
Here's the configure info:
Vips-8.0.typelib will install to /usr/local/lib/girepository-1.0, but your
|
This seems to be a problem with the way vips is sampling from the next higher resolution. Since it literally discards every other column of pixels and then averages the pixel with the pixel from one scanline below, this is causing the image to shift to both the right and down. The effect is compounded from one resolution to the next since sampling works in succession rather than from the original full size source. I think the general nearest neighbor style approach would be fine with tweaks to how the resampling is performed but as written, it's wrong. I'm referring to this definition in libvips/iofuncs/region.c: #define SHRINK_TYPE_INT( TYPE ) |
Hi @beaudet, I think my instinct would be to blame the strict round down tiffsave is doing when it makes the layer below: https://github.com/jcupitt/libvips/blob/master/libvips/foreign/vips2tiff.c#L393 On average you'll lose 0.5 pixels per layer off the right and bottom, so in 10 layers 5 pixels will go. Would that be enough to explain your missing pixels? I think the simplest solution might be to use The advantage of the strict round-down is that it avoids having to handle half pixels at the edges. Some code would need to be added to cope with this. The deepzoom writer has something that could be nicked. https://github.com/jcupitt/libvips/blob/master/libvips/foreign/dzsave.c#L1311 I'll see if I can copy something like that over. On https://github.com/jcupitt/libvips/blob/master/libvips/iofuncs/region.c#L1170 That's a standard x2 box downsize, I don't think there's a problem there. The |
John, thanks for your quick response. The algorithm seems perfectly fine when applied once. When I look at all the resolutions, no stretching is apparent at 50% of the original (at least that I can detect), but the effect begins to show up at 25% and gets worse from there so it's likely to be associated with the operation being repeated many times. I think you're right that it's a rounding issue and probably one associated with the averaging of pixel values. I think using the true floating point center of the source vs target to weight the pixels would work but that might be a little slower and would probably no longer be a pure implementation of the box downsize filter. I missed the section in the code that's actually averaging pixels along the X dimension but will take another look. |
Also, on an unrelated topic, do you find that performing pointer addition rather than using indexes into an array performs better and if so, by approximately how much? Similar question regarding the choice to use a define rather than casts in the SHRINK algorithm. Just curious. |
The pointer addition is an optimisation called strength reduction. You could write: for (i = 0; i < 1000; i++)
for (j = 0; j < 1000; j++)
sum += array[i + j]; or the equivalent: for (i = 0; i < 1000; i++) {
restrict int *p = array + i;
for (j = 0; j < 1000; j++)
sum += p[j];
} Now you add The defines serve the same purpose: help the compiler vectorise by giving it the simplest code path you can. There's no translation or stretching, just truncation along the right and bottom edge of the image. The averaging will tend to make the image slightly darker, since there's a |
Oh heh, of course it won't vectorise the inner shrink loop, it's too small. Anyway, you need to instantiate many version of the code to handle the different pixel types, and a macro is the simplest way to do that. |
Here's a link to the 300+MB original bits in case you want to try to replicate the issue: https://media.nga.gov/public/objects/4/1/5/8/1/tests/divs_original/A15324_XBD.tif |
Thanks John.
I see that int tot = tp[z] + tp[z + nb] + tp1[z] + tp1[z + nb]; is taking the pixel to the right as you say. The current algorithm sets target pixel Q0 = average of source pixels P0+P1+P2+P3 P0 | P1 That seems perfectly correct to me (with the addition of the tot + 1 [or should it be +2 since you have four pixels?) fix). Like you already said, I think the issue is the size of the canvas that is calculated from one step to the other. It alternates from even to odd numbers and loses a half pixel whenever dividing an odd resolution by half. For example, if the canvas starts as 11038 for resolution 0, then resolution 1 is 5519, but resolution 2 becomes 2759 which, when filling in the last pixel at pixel index 2758 computes with 2*2758 = 5516 whereas it really should be taking it from 5517 at that point. This is what's leaving out the last column of data. So, it's losing 0.5 pixels every other layer like you already said. I think there are going to be problems with the existing algorithm unless it's changed to a weighted algorithm based on floating point distance from the pixel center of the source pixel to the floating point center of the target pixel. It seems that even if edges are treated specially, you're still going to have to drop a column of pixels somewhere in the image which is going to shift the rest of the image after that point right? And the image will still be unevenly scaled somewhere. Maybe an initial downscaling operation could be performed on resolution 0 or on resolution 1 so that it becomes a multiple of 2^(R-1) where R is the number of resolutions? That way, the existing resampling algorithm will work perfectly for successive tile sizes all the way down without losing any pixels. For example, I've re-sampled the original file to be 11008 pixels instead of 11038. 11008 is evenly divisible by 2^(7-1) all the way to 172 which was the originally computed smallest tile size. It works perfectly with the existing algorithms in place and matches the imagemagick generated ptif in terms of scale and pixel placement. |
The dzsave saver handles this by expanding odd-sized layers. I'm planning to try this in the tiff writer. I'll make a branch. |
Oh, you're right, it should be +2, I'll change it. |
we were not rounding int averages to nearest, thanks beaudet see https://github.com/jcupitt/libvips/issues/659
Sounds good John, and thanks! We're all looking forward to finally having an end-to-end solution that can produce high quality derivatives on-the-fly with IIP Image server so we no longer have to create and track web-specific derivatives in our DAM and associated systems. This vips fix is the last piece of that puzzle. |
Here's a branch that rounds up rather than down: https://github.com/jcupitt/libvips/tree/tiff2vips-round-layers-up What do you think? You might find layers can be too large now, perhaps round to nearest would be better. Here's what I get for:
|
I'm a little concerned about the effect that copying the rightmost column will have on the end result and on successive layers, but I'll try with a bunch of different resolutions and see what happens. |
The copying happens in the layer above, so all it's doing is changing the rightmost column from a 2x2 average to a 1x2 average. |
Hello again, did you get a chance to look at this? Should I change it to round-to-nearest? |
I was assigned three short term critical projects the day I was last going to look at this. I'm nearing completion of those and hope to take a look this week or next. |
It's looking much much better now. Maybe done? I think you're probably there, but I have a question. If you pull the two images below into a browser and flash between them, you'll see that the VIPS generated image is actually now slightly smaller than the imagemagick generated image which was created using the same geometry as the computed tile size. One could argue that's actually more correct - I'm not sure what vips would be filling into the last column to pad the space otherwise - it's certainly not manufacturing pixel data and I see actual data in that last column so presumably there's a bug in IM w.r.t. image resizing too :) I'll give this version a pirouette and hopefully transition to using it to recompute all of our 80k images soon. |
Would you mind testing round to nearest as well? I'll change it tomorrow.
…On Wed, 14 Jun 2017 at 19:51, Dave Beaudet ***@***.***> wrote:
It's looking much much better now. Maybe done? I think you're probably
there, but I have a question. If you pull the two images below into a
browser and flash between them, you'll see that the VIPS generated image is
actually now slightly smaller than the imagemagick generated image which
was created using the same geometry as the computed tile size. One could
argue that's actually more correct - I'm not sure what vips would be
filling into the last column to pad the space otherwise - it's certainly
not manufacturing pixel data and I see actual data in that last column so
presumably there's a bug in IM w.r.t. image resizing too :)
I'll give this version a pirouette and hopefully transition to using it to
recompute all of our 80k images soon.
VIPS
[image: image]
<https://user-images.githubusercontent.com/77699/27148841-bdf82cc4-510f-11e7-9e73-2e0fed7640d9.png>
ImageMagick
[image: image]
<https://user-images.githubusercontent.com/77699/27148835-b723bf26-510f-11e7-8bc9-f3366789ca8f.png>
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<https://github.com/jcupitt/libvips/issues/659#issuecomment-308524684>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAjc6-TLhXIYeQoGdWA8ThxOZxiZopp6ks5sECuZgaJpZM4NdBYF>
.
|
Sure John, no problem, but how would I go about testing round to nearest? Is there a branch or command line switch for round to nearest? |
I mean I'll update that branch, hang on. |
Acknowledged |
Hmmm... there might be a problem with this build where some images are getting mixed up. I'll determine whether this is due to vips or IIP although I've never seen this happen with IIP. It's not consistent. This is the first of roughly 50 images that I regenerated last night where this has happened. And it's occurring only in the inner resolutions for this particular image. |
This seems to be an IIP protocol related bug - the IIIF viewers render fine with the same IIP server in use. I'll report it to Ruven. |
It doesn't really discard pixels on the input (the 1001 pixel image), it just doesn't generate the 501-st pixel in the output -- that is, it rounds the canvas size down by default. This makes a difference for lanczos3, which uses a 12x12 stencil rather than 2x2. The |
I recall seeing some minor problems even with -fancy + round-up, but it might be time to do some more tests with it including testing of the client side to ascertain support for round-up. I know the IIP stack has been fixed to support this at least. |
John, I'm getting back to your suggestion of using tiffcp to combine the resized images into a pyramidal tiff. Are you suggesting creating a series of standard TIFFS with JPEG compression and then just combining those into one file to create the pyramidal tiff? That seems to work ok, but requires the -a parameter to append to the result file, otherwise it gets overwritten with each source image. However, regarding the pyramidal nature of the tiff, should I be looking at the -t option as well to set the tile size? Also, it seems that tiffcp compresses again regardless of whether the source file is already a tiff with jpeg compression, so it might be faster to create uncompressed files and then use tiffcp to do the compression. I'll have to run some tests to compare the results. |
I might have answered my own question: seems -t is probably required as tiffinfo reports Tile Width and Tile Length when using the -t option. I'll have to research whether the embedded color profiles are retained as well. If so, this will probably work well for me in which case I'll return here and document the exact commands I end up using. |
It's working quite well now using a mix of custom pixel calculations along with vipsthumbnail for doing the resizing and color conversion and tiffcp to assemble the pyramid and to apply the JPEG compression as a final step. If you have a chance to review the approach which is contained in the script below, I'd appreciate it. Thanks for the suggestion John. Glad to finally be getting around to doing this. I've compared the ImageMagick RGB jpeg images with the tiffcp ycbcr images and the differences in quality are nearly imperceptible at full resolution - you really have to explode the image to notice. The resize operations and color profile conversion are all spot-on. #!/bin/bash
# convert a raw TIFF with arbitrary color profile to an uncompressed, color
# normalized sRGB in resolutions reducing by 2x with each step
# set W = full image width, H = full image height
# set c = 0;
# set input to input file name of raw tiff
input=139886.tif
W=11038;
H=11038;
c=0;
while [ 1 ]; do
echo ${c}, ${W}, ${H}
# it would be nice if there was a way to strip everything BUT the final
# color profile to avoid having to re-inject it, but at least that's a
# very fast operation regardless
vipsthumbnail ${input} --size $Wx$H -o tmp_$c.tif[compression=none,strip] --eprofile /usr/local/nga/etc/sRGBProfile.icc
vipsthumbnail tmp_$c.tif --size $Wx$H -o output_$c.tif[compression=none,profile=/usr/local/nga/etc/sRGBProfile.icc]
W=$(( W / 2 ));
H=$(( H / 2 ));
c=$(( c + 1 ));
if (( W < 1 || H < 1 || (( W < 65 && H < 65 )) )); then
break;
fi
done
# once we have all sizes created, then we use tiffcp to perform the pyramid
# assembly and jpeg compression
#
# tiffcp defaults to ycbcr photometric interpretation and presumably therefore
# chroma subsampling so by default it produces JPEGs about 3x smaller than
# when photometric interpretation is set to rgb
# e.g. -c jpeg:r:90 vs. -c jpeg:90
tiffcp -c jpeg:90 -t -w 256 -l 256 output_*.tif -a ${input}_tiffcp_90.ptif |
Yes, that looks reasonable. thank you for uploading your script, David! I thought of a few things: You are shrinking down to 64x64, but your final tiles are 256x256. Should those be the same? You are removing the profile, but then reinjecting an sRGB profile. Wouldn't no profile default to sRGB on most platforms? Perhaps you don't need to reinject. You could make your script a little more automatic with: input=$1
W=$(vipsheader -f width $input)
H=$(vipsheader -f height $input) And it'll work for any input file type, not just tiff. |
Thanks for taking a look John. I'd have to dig into the internals of IIP again regarding the tile size vs. resolutions but maybe you know since you wrote the first version of it. Off the top of my head I think it would still try to fetch the smallest region / resolution it can to render the requested pixels. That is, if a 50x50 was requested, it would just grab the entire 64x64 image from the smallest resolution tiff page rather than asking for one or more tiles from a larger resolution. There's a balance when selecting the optimum tile size to use since a tile size that's too small results in too many tile load operations vs. too many pixels from a smaller number of tiles. In prior testing 256 seemed close to the sweet spot. My sense is that unless we get a ton of requests for thumbnail size images, using a minimum resolution of 256 would probably be fine - although the overhead of including even smaller resolutions is negligible. My intention (which seems to be working) is to convert from the original profile to sRGB and then, yes, to reinject a 3144 byte sRGB profile back into each page of the pyramidal tiff. In testing performed several years ago, NOT injecting an ICC profile into the image caused Safari to render colors differently. It's possible that's been resolved by now, but I'd prefer to be explicit with the color profile even though it carries some overhead. I've also modified IIP to detect and re-inject this profile into the PNG (another enhancement) and the JPEG images it produces. Thanks for the tip on the vipsheader - that's great!) I will probably also try to use vipsthumbnail on each subsequent tiff page using the source image at the next higher resolution since there's no discernible quality improvement in using > 2x the target size as a source. That should further speed up the generation of the pyramids. I'll also provide some benchmarks of IM vs. this vips + tiffcp approach once I've managed to consume this approach in a Perl script that handles the generation. By the way, are there Perl bindings for VIPS? I googled but didn't find any. Thanks again, Dave |
No, no one's made a Perl binding, as far as I know. |
Maybe one of these days, I'll write a Perl binding, but for now, the command line approach seems to be working very well. I've revised the script which is pasted below. Benchmarking IM against vips+tiffcp using our largest image (26700 x 26700 pixels) shows that the vips + tiffcp approach is six times faster (40 vs. 250 seconds) than ImageMagick, uses less than 30% of the memory (4GB vs. 14GB - and that's tiffcp - vips uses < 1GB even for the large image), and doesn't even saturate the processor cores like IM does in all cases (maybe tiffcp could be even faster?). Furthermore, this approach produces a file that's nearly 3x smaller than with I.M. due to chroma subsampling support for jpeg compressed tiff files. I'm getting similar results against a smaller image (11038x11038): 9 sec vs. 41 sec, < 1GB RAM vs. 3.2 GB, barely a blip on the CPU vs. CPU saturation, and a file that's only 35% the size of the one produced with ImageMagick. Here's the improved script:
|
Thanks for the update! I think I would shrink until the image fits within one tile, so < 257, not < 129, as you have. It probably doesn't make much difference though. |
two problems I ran into:
I requested 7959x5411 but vipsthumbnail returned 7958x5411. I assume if I switch to using the standard vips command line, I can work around this. Time to try that out. |
It doesn't try to calculate the horizontal and vertical scale factors together. Instead, it does one then the other and picks the smaller. In your example, the calculation is: horizontal scale factor = 7959 / 15918 So it picks the vertical scale. Then the new horizontal size is: new horizontal size = 15918 * 5411 / 10823 Then round to nearest gives 7958. You can force it to break the aspect ratio with a
It might be better in your case. |
Thanks John. Yeah, I discovered the "!" syntax after looking at the code and noticing VIPS_FORCE_RESIZE - a google search didn't really turn up any documentation about the syntax though. |
oops, this was missing, thanks beaudet see https://github.com/jcupitt/libvips/issues/659
You're right, there was no note about |
Sure thing. I ended up using a combination of vips icc_transform and vips tiffsave to create the initial color transformed image with an injected ICC profile, then vipsthumbnail with the WxH! syntax for everything else. The updated script is below and is running on a test of about 1200 images. It will be interesting to see how much faster this runs than the ImageMagick version since I'm able to ramp up the parallel processes to 8 from 2 due to the much lower CPU and memory constraints.
|
I re-ran a test of 1147 images ( 200 GB of source images with average resolution of 7000 x 7200 ) including downloading the original assets from Amazon S3 just prior to processing each one and the entire job completed without any errors in 85 minutes. That's quite a lot faster than with ImageMagick. I was also able to run 24 simultaneous vips processes which seemed to do a reasonably good job of maximizing the 8 CPU cores without coming close to exhausting the server's 48GB of RAM). I can almost certainly cut the RAM by 50% and repeat without any problems. Finally, I created a web page listing all of the 1147 images via their IIIF URLs (uses the NGA PROD fork of IIP behind the scenes) and they all rendered quickly and perfectly. |
I have followed this thread with interest and I am converting @beaudet 's script into Python. Ideally I would like to use only pyvips. Basically what I am planning to do is creating a number of images with decreasing size using an arbitrary kernel (say Lanczos3), then assembling them in a multi-page TIFF file. However, I noticed the
I notice no artifacts. I tried with a 4096x4096 image and a 3001x3001 image derived from the former. See screenshot from the 3001 image opened in Gimp, showing the stacked pyramid images: Am I supposed to see similar issues as @beaudet 's round image? |
Hello @scossu, Yes, the tiff saver has a very fancy pyramid builder built in. It's able to write all the layers of the pyramid in parallel in a single pass through the image, so it's fast and only uses a little memory. It has a couple of issues though:
There's a branch up there ^^ which adds lanczos, though it was never merged. I think if you make a perfect circle that exactly touches the image edges and then pyramid it, you may notice slight flattening of the right and bottom edges in some layers. Thinking about this again, simply adding one pixel of white along the bottom and right edge of the input would be enough to prevent that. For example:
To create |
We should close this -- let's start a new issue if there's still a problem here. |
Thanks @jcupitt. A couple of last post-closing related questions:
Is there a parameter to specify the down-sampling algorithm in the python function? (I don't see enums for existing algorithms in the Python API and I'm not sure what mean, mode and median correspond to.) Also, for full customization to replicate @beaudet 's specific tweaks, I though I could combine resize and insert, but it seems like |
It's like any optional argument. Try:
So in py you can write: image = pyvips.Image.new_from_file("input/filename")
image.write_to_file("output/filename.tif", region_shrink="mode") or image.tiffsave("somefilename", region_shrink="median") Mean, mode and median are how it shrinks each 2x2 block of pixels. For
Mean is just Docs here: https://libvips.github.io/libvips/API/current/VipsForeignSave.html#vips-tiffsave To implement the pyramiding yourself (I would only do this is you really MUST have lanczos or sub-pixel perfect alignment of edges), you'll need to make a set of separate TIFF images and combine them with tiffcp. libvips does support writing many-page TIFFs, but all the pages have to be the same size, so it won't work for pyramids. |
Ah. It was the |
For anyone looking for the latest tiff_to_pyramid.bash script, you can find it here: https://github.com/NationalGalleryOfArt/iipsrv/tree/master/imagescripts @scossu has also turned this script into Python code |
Original issue noted here: ruven/iipsrv#98 but copied below since this is really a vips issue, not an IIP issue.
I have confirmed that there is an ongoing resize bug in vips that is reproducible on the latest up-to-date RHEL6 and RHEL7 systems using either vips 7.x or the latest 8.4 version of vips. See the differences between the images below which were taken from the pyramidal tiff. The first was created using imagemagick and the second with vips 8.4.5 on RHEL7. Aside from being slower, we had found a few years back that IM creates intermittent corrupted ptifs that IIP can't open. I think the only option is to find the bug in vips or the library it's using and fix it - or switch to IM if the PTIF generation has been fixed since we last looked at it about 4 or 5 years ago. Any advice appreciated.
One of the resolutions from an ImageMagick generated pyramidal tiff image

and same resolution from vips generated pyramidal tiff (note flatness on right side and bottom) image

command used to generate the ptif:
vips im_vips2tiff ./source_tiff_file.tif vips_regen_8.4.5_rhel7.ptif:jpeg:90,tile:256x256,pyramid
info about the original TIFF source: TIFF 11038x11038 11038x11038+0+0 8-bit DirectClass 365.5MB
command used to generate the output above
convert vips_regen_8.4.5_rhel7.ptif -quality 100 vips_regen_8.4.5_rhel7.jpg
This produces one jpg for each of the 6 or 7 resolutions that vips generates in the ptif. All of the exported jpegs from a ptif generated by vips exhibit this problem whereas none of the exported jpgs generated with imagemagick do.
The text was updated successfully, but these errors were encountered: