Skip to content

vips_get_tile_size missing #33

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
kleisauke opened this issue Feb 12, 2017 · 12 comments
Closed

vips_get_tile_size missing #33

kleisauke opened this issue Feb 12, 2017 · 12 comments

Comments

@kleisauke
Copy link
Member

To reduce memory usage and improve performance we did some tests with sequential mode read, from our tests in looks promising. However we're missing 1 function that is in libvips and not in the PHP binding:

General question:
Sequential read mode does not work with rotate, sharpen, gaussblur and our trim function. If we have to deal with these operations we force the access mode to VIPS_ACCESS_RANDOM, do you recommend this? I was not sure if a tilecache fixes this.

@jcupitt
Copy link
Member

jcupitt commented Feb 15, 2017 via email

@kleisauke
Copy link
Member Author

There's no hurry, have a great holiday!

@jcupitt
Copy link
Member

jcupitt commented Feb 19, 2017

Hi again, rotate needs full random access, but sharpen/blur/crop should work with sequential. For example:

#!/usr/bin/env php
<?php

require __DIR__ . '/vendor/autoload.php';

use Jcupitt\Vips;

$image = Vips\Image::newFromFile($argv[1], ['access' => 'sequential']);
$image = $image->gaussblur(4);
$image->writeToFile($argv[2]);

There are two sequential modes. The regular seq mode (as above) keeps a buffer of a few 100 lines behind the processing point so that operations can refer backwards. This is how blur and sharpen are able to work.

There's also sequential-unbuffered, which keeps no history at all. This will only work for operations which do absolutely no referring back, such as crop / left-right flip, invert etc.

Why do you need get_tile_size()? I think just setting the access mode should be enough.

@kleisauke
Copy link
Member Author

I've made a sample script which occasionally fails with sequential mode read + resize + blur. See: https://gist.github.com/kleisauke/32b8b19263ab32d89ed44d4022afef89
(I removed the premultiply steps because the test image has no transparency)

For the test image: https://rbx.weserv.nl/lichtenstein.jpg

This scripts fails sometimes with this warning:

2017/02/19 12:28:14 [error] 16900#16900: *121711 FastCGI sent in stderr: "PHP message: PHP Fatal error:  Uncaught Jcupitt\Vips\Exception: VipsJpeg: out of order read at line 1376
 in /var/www/t0.nl/vendor/jcupitt/vips/src/Image.php:635
Stack trace:
#0 /var/www/t0.nl/vendor/jcupitt/vips/src/Image.php(756): Jcupitt\Vips\Image::errorVips()
#1 /var/www/t0.nl/public_html/test.php(125): Jcupitt\Vips\Image->writeToBuffer('.jpg')
#2 {main}
  thrown in /var/www/t0.nl/vendor/jcupitt/vips/src/Image.php on line 635" while reading response header from upstream, client: 2001:610:600:850f:f901:92b5:6e3b:734b, server: t0.nl, request: "GET /test.php HTTP/1.1", upstream: "fastcgi://unix:/var/run/php-fpm/php-fpm.sock:", host: "t0.nl"
2017/02/19 12:28:15 [error] 16900#16900: *121711 FastCGI sent in stderr: "PHP message: PHP Fatal error:  Uncaught Jcupitt\Vips\Exception: VipsJpeg: out of order read at line 1528
 in /var/www/t0.nl/vendor/jcupitt/vips/src/Image.php:635
Stack trace:
#0 /var/www/t0.nl/vendor/jcupitt/vips/src/Image.php(756): Jcupitt\Vips\Image::errorVips()
#1 /var/www/t0.nl/public_html/test.php(125): Jcupitt\Vips\Image->writeToBuffer('.jpg')
#2 {main}
  thrown in /var/www/t0.nl/vendor/jcupitt/vips/src/Image.php on line 635" while reading response header from upstream, client: 2001:610:600:850f:f901:92b5:6e3b:734b, server: t0.nl, request: "GET /test.php HTTP/1.1", upstream: "fastcgi://unix:/var/run/php-fpm/php-fpm.sock:", host: "t0.nl"

For a live demo: http://t0.nl/test.php (just hit F5 and you'll see occasionally HTTP 500 errors)

I think I will need a tile cache at line 85-86.

@jcupitt
Copy link
Member

jcupitt commented Feb 19, 2017

Ah OK, yes gaussblur(4) is a pretty large radius:

$ vips gaussblur k2.jpg x.jpg 4 --vips-info
VIPS-INFO: gaussblur mask width 15
VIPS-INFO: using vector path
VIPS-INFO: using vector path

So it'll need 15 scanlines to compute the blur, then if you've previously shrunk by x5 (for example), you need 15 * 5, or 75 lines from the input.

I would try adding a tilecache just before the blur. Set the tile width to the image width, the tile height to 1, and maxtiles to the number of lines you want it to keep. Perhaps 30? It depends how large you want the blur to be able to go. Set threaded to TRUE and access to sequential.

If you don't mind tying yourself to libvips 8.5, use thumbnail, instead of shrink and reduce. it'll be much faster.

@kleisauke
Copy link
Member Author

The resize logic is based on the sharp library (that library uses a tile cache in the resize function, see this and this). They are also investigating adding a tile cache before some operations, I'll link it for further reference: lovell/sharp#709

So if I understand correctly the vips_get_tile_size is not needed to calculate the scanlines count of a blur operation; the scanlines count of the blur operation is the mask width (15 in this case) multiplied by 2. I'll do some tests with adding a tile cache before the blur operation.

I’ve considered vips_thumbnail but it’s not possible to implement it in our current setting (correct me if I'm wrong):

  • We limit the image size (width x height) what’s processed through our image proxy. The check takes places after Image::newFromFile. If it exceeded our limit (71000000) an exception is called and the user is unable to process the image. With vips_thumbnail the image is shrunk on read, we never know what the original dimensions of the image were. (Maybe this is fixable with Image::newFromFile -> $image->writeToBuffer and then vips_thumbnail_buffer).
  • Our users can define an exact crop position using a focal point (this is defined using two offset percentages). The crop method of vips_thumbnail is always center-center
  • It's currently not possible with vips_thumbnail to fit the image constraining dimensions exactly; ignoring the aspect ratio.
  • Letterboxing (embed); not sure if this should incorporate in vips_thumbnail (this can easily be fixed afterwards)

@jcupitt
Copy link
Member

jcupitt commented Feb 19, 2017

Hi again, you can get the image size very quickly with newFromFile. It won't read any pixels until they are needed, it just reads the metadata. So I would read out the dimensions, and then decide whether to allow thumbnail to run.

You're right that the crop in thumbnail is always centre, and that it always maintains the aspect ratio.

You could steal some of the shrink-on-load logic from thumbnail. It can give a very dramatic speedup for many image formats.

kleisauke added a commit to weserv/images that referenced this issue Feb 19, 2017
This will speedup image processing for many image formats. See:
libvips/php-vips#33 (comment)
@kleisauke
Copy link
Member Author

Thanks for the tip about shrink-on-load. I've just implemented it and it looks like our images are now loading faster on our test environment. 👍

Is there another way to get/calculate the buffer height in scanlines (scanline count)? I'm trying to implement the logic here in our resize function.

@jcupitt
Copy link
Member

jcupitt commented Feb 24, 2017

Ooop, sorry for the pause.

I'd use linecache instead, it does this for you internally. You want threaded mode, and sequential access.

http://www.vips.ecs.soton.ac.uk/supported/current/doc/html/libvips/libvips-conversion.html#vips-linecache

kleisauke added a commit to weserv/images that referenced this issue Feb 24, 2017
A user can now decide which page he wants for an PDF and TIFF. It even
works for a multi-size ICO.
Also, I’ve added a linecache before some operations. It seems to work
(no `out of order read` error(s) anymore) but it will need some tweaking
to find the correct `tile_height` before we’re going into production.
See: libvips/php-vips#33
@kleisauke
Copy link
Member Author

I will do some experiments with vips_linecache ($image->linecache()) at the resize, sharpen and blur operation (need only to find out how many scanline(s) it will need for tile_height). Thanks again!

Outside the scope of this issue, but a function that I'm missing is vips_foreign_find_load. I'll need it to determine the image format before the Image::newFromFile operation to pass the correct options.

For example, I've just implemented a page option to let the user decide which page he wants (useful for PDFs, TIFFs, and multi-size ICOs). If a user decides to use this function for a JPEG (or any other loader that doesn't support the page property) it will fail: Message: jpegload: no property named 'page'.
This is fixable if we call Image::newFromFile first without options and then call $image->get('vips-loader') to see which loader it has been loaded and then call again Image::newFromFile with the correct options. But this feels odd.. :-(

@kleisauke
Copy link
Member Author

kleisauke commented Feb 25, 2017

@jcupitt Is there a method to clear the whole linecache (I tried Config::cacheSetMax(0); and Config::cacheSetMaxFiles(0);)? It looks like after I had enabled the linecache that our trim function is broken; it only trims the height and not the width. The weird thing here is that I've forced the access to random if we’re trimming.

See for the wrong trimmed image here and for the debug log here. From the debug log I can see that the ->profile() is returning always width 1 .

Update:
False alarm, seems someone optimized the PNG_transparency_demonstration_1.png which breaks the width trimming. I'll investigate why this particular image is only trimming in the height. Sorry for any inconvenience caused.

@kleisauke
Copy link
Member Author

kleisauke commented Mar 10, 2017

I'm going to close this issue. After testing with linecache I'm not getting any out of order read errors anymore. (Thus the vips_get_tile_size is not needed anymore). Not sure if linecache is needed anymore when libvips 8.5 is released (because of the new sequential mode).

Sorry for the last 2 messages (it's too offtopic). I will create separate issues to track the trim-problem (looks like the particular image is way too far optimized which breaks vips_profile() vips_getpoint()) and the vips_foreign_find_load enhancement.

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

No branches or pull requests

2 participants