Skip to content

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

Closed
beaudet opened this issue May 16, 2017 · 125 comments
Closed

off by one problem when generating pyramidal tiff #659

beaudet opened this issue May 16, 2017 · 125 comments

Comments

@beaudet
Copy link

beaudet commented May 16, 2017

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
image

and same resolution from vips generated pyramidal tiff (note flatness on right side and bottom) image
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.

@beaudet
Copy link
Author

beaudet commented May 16, 2017

Here's the configure info:

  • build options
    native win32: no
    native OS X: no
    open files in binary mode: no
    enable debug: no
    build deprecated components: yes
    build docs with gtkdoc: no
    gobject introspection: yes
    build vips7 Python binding: no
    install vips8 Python overrides: no
    (requires pygobject-3.12.0 or later)
    build radiance support: yes
    build analyze support: yes
    build PPM support: yes

  • optional dependencies
    use fftw3 for FFT: no
    Magick package:
    file import with libMagick: no
    accelerate loops with orc: no
    (requires orc-0.4.11 or later)
    ICC profile support with lcms: no
    file import with OpenEXR: no
    file import with OpenSlide: no
    (requires openslide-3.3.0 or later)
    file import with matio: no
    PDF import with poppler-glib: no
    (requires poppler-glib 0.16.0 or later)
    SVG import with librsvg-2.0: no
    (requires librsvg-2.0 2.34.0 or later)
    zlib: yes
    file import with cfitsio: no
    file import/export with libwebp: no
    (requires libwebp-0.1.3 or later)
    text rendering with pangoft2: no
    file import/export with libpng: no
    (requires libpng-1.2.9 or later)
    file import/export with libtiff: yes (pkg-config libtiff-4)
    file import/export with giflib: no
    file import/export with libjpeg: no
    image pyramid export: no
    (requires libgsf-1 1.14.26 or later)
    use libexif to load/save JPEG metadata: no

Vips-8.0.typelib will install to /usr/local/lib/girepository-1.0, but your
system repository seems to be /usr/lib64/girepository-1.0.
You may need to add this directory to your typelib path, for example:

    export GI_TYPELIB_PATH="/usr/local/lib/girepository-1.0"

@beaudet
Copy link
Author

beaudet commented May 17, 2017

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 )
for( x = 0; x < target->width; x++ ) {
TYPE *tp = (TYPE *) p;
TYPE *tp1 = (TYPE *) (p + ls); \ <-- I believe that only averaging with the the pixel one scanline below results in cropping the image at the bottom
TYPE *tq = (TYPE ) q;

for( z = 0; z < nb; z++ ) {
int tot = tp[z] + tp[z + nb] +
tp1[z] + tp1[z + nb];

tq[z] = tot >> 2;
}

/
Move on two pels in input. \ <---- this discards data and results in chopping off the last column of image data rather than averaging it - this has the effect of cropping the image on the right side
*/
p += ps << 1;
q += ps;
}

@jcupitt
Copy link
Member

jcupitt commented May 18, 2017

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 rint(), which rounds up and down alternately, and so should preserve size. Or maybe calculate each layer size from the original image size, rather than clipping repeatedly.

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 SHRINK_TYPE_INT, it's not discarding pixels, is it? Each pixel in a layer is the average of the corresponding four pixels in the layer above.

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 tot >> 2 should probably be (tot + 1) >> 2.

@beaudet
Copy link
Author

beaudet commented May 18, 2017

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.

@beaudet
Copy link
Author

beaudet commented May 18, 2017

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.

@jcupitt
Copy link
Member

jcupitt commented May 18, 2017

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 i once rather than 1000 times. Compilers will do some optimisations like this automatically, but when loops get complex they often fail. gcc will also automatically vectorise version 2, which is nice. It might vectorise v1, but I'd be a little surprised.

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 +1 missing from before the shift, but it will not displace the image.

@jcupitt
Copy link
Member

jcupitt commented May 18, 2017

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.

@beaudet
Copy link
Author

beaudet commented May 18, 2017

In resolution 1 (50% of original size), ImageMagick and vips are pixel-aligned and there seems to be no stretching or removal of any columns. These jpeg levels might not be the same so don't try to compare for quality here - I'm just looking at the pixel alignment.

Vips generated and zoomed in 500% at right edge of resolution 1
image

ImageMagick generated and zoomed in at 500% at right edge of resolution 1
image

At level 2, the differences start to emerge - if you open these into two tabs, you can see that the entire image is being stretched to the right and down in the vips version and the last column appears to be "discarded".

vips at level 2:
image

image magick at level 2:
image

@beaudet
Copy link
Author

beaudet commented May 18, 2017

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

@beaudet
Copy link
Author

beaudet commented May 18, 2017

Thanks John.

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 +1 missing from before the shift, but it will not displace the image.

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
---|--- => Q0
P2 | P3|

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.

Vips with a source that's 11008 pixels instead of 11038
image

IM with a source that's 11038 pixels
image

@jcupitt
Copy link
Member

jcupitt commented May 18, 2017

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.

@jcupitt
Copy link
Member

jcupitt commented May 18, 2017

Oh, you're right, it should be +2, I'll change it.

jcupitt referenced this issue May 18, 2017
we were not rounding int averages to nearest, thanks beaudet

see https://github.com/jcupitt/libvips/issues/659
@beaudet
Copy link
Author

beaudet commented May 18, 2017

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.

@jcupitt
Copy link
Member

jcupitt commented May 20, 2017

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:

$ vips tiffsave A15324_XBD.tif x.tif --pyramid --tile
$ vips copy x.tif[page=6] x6.png

x6

@beaudet
Copy link
Author

beaudet commented May 22, 2017

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.

@jcupitt
Copy link
Member

jcupitt commented May 22, 2017

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.

@jcupitt
Copy link
Member

jcupitt commented Jun 7, 2017

Hello again, did you get a chance to look at this? Should I change it to round-to-nearest?

@beaudet
Copy link
Author

beaudet commented Jun 7, 2017

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.

@beaudet
Copy link
Author

beaudet commented Jun 14, 2017

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

ImageMagick
image

@jcupitt
Copy link
Member

jcupitt commented Jun 14, 2017 via email

@beaudet
Copy link
Author

beaudet commented Jun 14, 2017

yup, it appears that IM is significantly reducing the vibrancy of the last column - not entirely clear why that is, but might have to do with the re-sampling algorithm it uses by default. See below for examples with -quality 100 jpeg compression. You'll have to load these individually and zoom back a bit to see the right most and bottom-most columns / rows of pixels which was 1 px wide in the source. Disregard the wider band on the left and top - I wasn't testing that portion and it's wider than 1 pix in source image.

imagemagick conversion from original 4003x4003 source file:
image

imagemagick conversion / extraction of the tile from vips generated ptif
image

through IIP image server using PNG output
image

through IIP image server using JPG output
image

@beaudet
Copy link
Author

beaudet commented Jun 14, 2017

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?

@jcupitt
Copy link
Member

jcupitt commented Jun 15, 2017

I mean I'll update that branch, hang on.

@beaudet
Copy link
Author

beaudet commented Jun 15, 2017

Acknowledged

@beaudet
Copy link
Author

beaudet commented Jun 15, 2017

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.

image

@beaudet
Copy link
Author

beaudet commented Jun 15, 2017

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.

@jcupitt
Copy link
Member

jcupitt commented Jan 22, 2018

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 --round-up flag makes it round up instead, so it always generates the extra column in the output for odd input.

@beaudet
Copy link
Author

beaudet commented Jan 22, 2018

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.

@beaudet
Copy link
Author

beaudet commented Jul 10, 2018

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.

@beaudet
Copy link
Author

beaudet commented Jul 10, 2018

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.

@beaudet
Copy link
Author

beaudet commented Jul 11, 2018

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

@jcupitt
Copy link
Member

jcupitt commented Jul 12, 2018

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.

@beaudet
Copy link
Author

beaudet commented Jul 12, 2018

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

@jcupitt
Copy link
Member

jcupitt commented Jul 12, 2018

No, no one's made a Perl binding, as far as I know.

@beaudet
Copy link
Author

beaudet commented Jul 13, 2018

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:

#!/bin/bash

function handle_error() {
  echo "Script exited with status ${2} at line ${1}"
}

trap 'handle_error ${LINENO} $?' ERR

# cause the script to fail if any commands exist with non-zero status
set -e

if [[ -z $1 || -z $2 || -z $3 ]]; then
    echo "Usage:make_pyramidal_tiff.bash <tmpdir> <full path to image source> <full path to output target>";
    exit 1;
fi
if [[ ! -d $1 || ! -f $2 ]]; then
    echo "Either the tmpdir or input file do not exist.";
    exit 1;
fi

if [ -f $3 ]; then
    echo "The output file already exists! Aborting.";
    exit 1;
fi

touch $3;

if [ ! -f $3 ]; then
    echo "Could not create the output file.";
fi

\rm $3

tmpdir=$1
input=$2
outfile=$3
pid=$$;
tmpprefix=${tmpdir}/stripped_tiff_${pid}
outprefix=${tmpdir}/icc_tiff_${pid}

# cleanup temp files that might already exist
\rm $tmpprefix* || true
\rm $outprefix* || true

W=$(vipsheader -f width $input)
H=$(vipsheader -f height $input)
c=0;
while [ 1 ]; do
    # echo ${c}, ${W}, ${H}
    if (( c > 0 )); then
        d=$((c - 1))
        # since we already have a stripped and color converted tiff that
        # is twice the resolution as the one are about to create, use that
        # one instead of the original when resizing which will save quite
        # a bit of processing time
        vipsthumbnail ${outprefix}_$d.tif --size $Wx$H -o ${outprefix}_$c.tif[compression=none]
    else
        # create a native resolution uncompressed tiff with all metadata stripped out to save space
        vipsthumbnail ${input} --size $Wx$H -o ${tmpprefix}_$c.tif[compression=none,strip] --eprofile /usr/local/nga/etc/sRGBProfile.icc
        # then re-embed the ICC profile since we want to have an ICC in the file pyramid
        vipsthumbnail ${tmpprefix}_$c.tif --size $Wx$H -o ${outprefix}_$c.tif[compression=none,profile=/usr/local/nga/etc/sRGBProfile.icc]
    fi

    # reduce height and width by half and repeat process until small enough
    W=$(( W / 2));
    H=$(( H / 2));
    c=$(( c + 1));
    if (( W < 1 || H < 1 || (( W < 129 && H < 129 )) )); then
        break;
    fi
done

# once we have all sizes created, then we use tiffcp to perform the pyramid
# assembly and jpeg compression at 90 quality

# note: tiffcp defaults to ycbcr photometric interpretation and presumably therefore
# chroma subsampling so by default it produces JPEGs about 3x smaller with ycbcr
# vs. 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 ${outprefix}_*.tif -a ${outfile}

# cleanup temp files but leave the outputput file in place
\rm $tmpprefix*
\rm $outprefix*

# sanity check
WF=$(vipsheader -f width $outfile)
HF=$(vipsheader -f height $outfile)

echo "Pyramid width: ${WF}"
echo "Pyramid height: ${HF}"

# final check
exit $(( WF == W && HF == H ))

@jcupitt
Copy link
Member

jcupitt commented Jul 14, 2018

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.

@beaudet
Copy link
Author

beaudet commented Jul 16, 2018

two problems I ran into:

  1. there's a bug in the script $Wx$H needs to be changed to ${W}x${H}
  2. vipsthumbnail doesn't honor pixel-perfect resize requests - presumably it's deciding to round up or down based on the original aspect ratio
icc_tiff_18403_0.tif: 15918x10823 uchar, 3 bands, srgb, tiffload

# vipsthumbnail icc_tiff_18403_0.tif --size 7959x5411 -o icc_tiff_18403_1.tif[compression=none]

# vipsheader icc_tiff_18403_1.tif
icc_tiff_18403_1.tif: 7958x5411 uchar, 3 bands, srgb, tiffload

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.

@jcupitt
Copy link
Member

jcupitt commented Jul 17, 2018

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
= 0.5
vertical scale factor = 5411 / 10823
= 0.49995

So it picks the vertical scale. Then the new horizontal size is:

new horizontal size = 15918 * 5411 / 10823
= 7958.2646216391

Then round to nearest gives 7958.

You can force it to break the aspect ratio with a "!" in the size specification:

vipsthumbnail icc_tiff_18403_0.tif --size 7959x5411! -o icc_tiff_18403_1.tif[compression=none]

It might be better in your case.

@beaudet
Copy link
Author

beaudet commented Jul 17, 2018

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.

@jcupitt
Copy link
Member

jcupitt commented Jul 17, 2018

You're right, there was no note about ! in the docs, I added something. Thanks!

@beaudet
Copy link
Author

beaudet commented Jul 17, 2018

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.

#!/bin/bash

function handle_error() {
  echo "Script exited with status ${2} at line ${1}"
}

trap 'handle_error ${LINENO} $?' ERR

# cause the script to fail if any commands exist with non-zero status
set -e

if [[ -z $1 || -z $2 || -z $3 ]]; then
    echo "Usage:tiff_to_pyramid.bash <tmpdir> <full path to image source> <full path to output target>";
    exit 1;
fi
if [[ ! -d $1 || ! -f $2 ]]; then
    echo "Either the tmpdir or input file do not exist.";
    exit 1;
fi

if [ -f $3 ]; then
    echo "The output file already exists! Aborting.";
    exit 1;
fi

touch $3;

if [ ! -f $3 ]; then
    echo "Could not create the output file.";
fi

\rm $3

tmpdir=$1
input=$2
outfile=$3
pid=$$;
tmpprefix=${tmpdir}/stripped_srgb_${pid}
outprefix=${tmpdir}/srgb_${pid}

# cleanup temp files that might already exist
\rm $tmpprefix* || true
\rm $outprefix* || true

# first, run an icc_transform to convert the original to sRGB and strip all metadata from the file
vips icc_transform $input ${tmpprefix}.tif[compression=none,strip] /usr/local/nga/etc/sRGBProfile.icc
# now, embed the ICC profile since it is stripped out with the strip metadata directive
vips tiffsave ${tmpprefix}.tif ${outprefix}_0.tif --compression none --profile /usr/local/nga/etc/sRGBProfile.icc

# read the width and height of the transformed file
W=$(vipsheader -f width ${outprefix}_0.tif)
H=$(vipsheader -f height ${outprefix}_0.tif)
c=0;
while [ 1 ]; do
    W=$(( W / 2 ));
    H=$(( H / 2 ));
    #echo ${c} ${W} ${H}

    # since we already have a stripped and color transformed tiff that
    # is twice the resolution as the one are about to create, use that
    # one instead of the original when resizing which will save quite
    # a bit of processing time
    vipsthumbnail ${outprefix}_$c.tif --size ${W}x${H}\! -o ${outprefix}_$(( c + 1 )).tif[compression=none]

    # reduce height and width by half and repeat process until small enough
    if (( W < 1 || H < 1 || (( W < 129 && H < 129 )) )); then
        break;
    fi
    c=$(( c + 1 ));
done

# once we have all sizes created, then we use tiffcp to perform the pyramid
# assembly and jpeg compression at 90 quality

# note: tiffcp defaults to ycbcr photometric interpretation and presumably therefore
# chroma subsampling so by default it produces JPEGs about 3x smaller with ycbcr
# vs. 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 ${outprefix}_*.tif -a ${outfile}

# cleanup temp files but leave the outputput file in place
\rm $tmpprefix*
\rm $outprefix*

# sanity check
WF=$(vipsheader -f width $outfile)
HF=$(vipsheader -f height $outfile)

echo "Pyramid width: ${WF}"
echo "Pyramid height: ${HF}"

# final check
exit $(( WF == W && HF == H ))

@beaudet
Copy link
Author

beaudet commented Jul 23, 2018

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.

@scossu
Copy link

scossu commented Feb 24, 2019

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 pyramid flag of tiffsave that automates the derivative generation. So I created a test square image with a diamond whose corners touch the edges of the image and used this script on it:

im1 = pyvips.Image.new_from_file('/home/scossu/tmp/test_diamond_3001.tif') 
im1.tiffsave(
    '/home/scossu/tmp/pyramid_diamond_3001_pyvips.tif',
    tile=True, tile_width=256, tiile_height=256, pyramid=True
)

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:

2019-02-23_845x652

Am I supposed to see similar issues as @beaudet 's round image?

@jcupitt
Copy link
Member

jcupitt commented Feb 24, 2019

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:

  1. Because of the way it works, it can only do a exact x2 shrink for each layer. @beaudet needed to vary both the shrink factor and the aspect ratio between layers to get his circular images to fit exactly.
  2. It does support different types of downsampling, but only 2x2 mean, mode and median. Lanczos is very slightly sharper, though it doesn't feel like a huge difference to me. Perhaps I need better glasses.

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:

W=$(vipsheader -f width $input)
H=$(vipsheader -f height $input)
vips gravity A15.tif my-pyr.tif[pyramid,tile,compression=jpeg,Q=80] north-west $((W+1)) $((H+1)) --extend white

To create my-pyr.tif.

@jcupitt
Copy link
Member

jcupitt commented Feb 24, 2019

We should close this -- let's start a new issue if there's still a problem here.

@jcupitt jcupitt closed this as completed Feb 24, 2019
@scossu
Copy link

scossu commented Feb 24, 2019

Thanks @jcupitt. A couple of last post-closing related questions:

  1. It does support different types of downsampling, but only 2x2 mean, mode and median. Lanczos is very slightly sharper, though it doesn't feel like a huge difference to me. Perhaps I need better glasses.

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 insert flattens the image. Is there a straightforward way to insert an image as a new layer?

@jcupitt
Copy link
Member

jcupitt commented Feb 24, 2019

It's like any optional argument. Try:

$ vips tiffsave
save image to tiff file
usage:
   tiffsave in filename [--option-name option-value ...]
where:
   in           - Image to save, input VipsImage
   filename     - Filename to save to, input gchararray
optional arguments:
... etc.
   region-shrink - Method to shrink regions, input VipsRegionShrink
			default: mean
			allowed: mean, median, mode

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

a b 
c d

Mean is just (a + b + c + d) / 4, median is [a, b, c, d].sort()[1], mode is the most frequent value. Median and mode are useful for shrinking images where pixel values are not lightness, but something like label or index.

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.

@scossu
Copy link

scossu commented Feb 24, 2019

Ah. It was the shrink_region parameter that I did not quite grasp. It looks like the default value is good enough for now, anyways. Thanks again.

@beaudet
Copy link
Author

beaudet commented Mar 15, 2019

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

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

No branches or pull requests

4 participants