-
Notifications
You must be signed in to change notification settings - Fork 62
-
The issue
I am comparing web page screenshots for testing purposes. The workflow is like this:
- obtain the screenshot blob from chromium and load it via
Image.new_from_buffer - assert the similarity with the reference.jpg asset and if it's significant, fail the test
- if in fact there was no error, update the asset, manually rewriting it with the saved blob
The issue is that even after I renew the asset file it fails again (with the perceptual difference of 11), BUT if I save the current blob to another file and then load it back, the perceptual difference is zero!
Why? Because of some unexpected (for me and for practical reason, because it looks stupid to fix it via save/load) difference that exists between Vips::Image#thumbnail_image and Vips::Image::thumbnail here:
image = if input.is_a? Vips::Image input.thumbnail_image(size, height: size, size: :force) else Vips::Image.thumbnail(input, size, height: size, size: :force) end
The load-from-buffer feature was wanted by users two, times, and you would not notice the issue if all your fingerprinting was either from Image or from buffer, but in my workflow I need to compare practically the same blob loaded in both ways, and that's where the difference occures.
How to observe it?
blob = File.binread "vk_video.jpg"
puts Vips::Image.new_from_buffer(blob, "").thumbnail_image(8, height: 8, size: :force). colourspace("b-w")[0].to_a.map{ |_| _.map{ |_| "%3d" % _ }.join }
47 42 47 50 47 45 46 47
4 5 3 3 4 4 4 4
0 0 0 0 0 0 0 0
1 1 1 2 5 1 1 1
0 0 0 0 0 0 0 0
103102 97101 99 99102111
248243243247251251250254
246238239254253253253252
puts Vips::Image.thumbnail("vk_video.jpg", 8, height: 8, size: :force). colourspace("b-w")[0].to_a.map{ |_| _.map{ |_| "%3d" % _ }.join }
45 42 45 47 46 44 45 46
4 4 4 3 3 4 4 4
0 0 0 0 1 0 0 0
4 4 4 5 8 4 4 4
0 0 0 0 0 0 0 0
104102 99103 98 99101111
250247248250255255254254
245239241253253253253252
Or use this debug script:
require "dhash-vips" ha = DHashVips::IDHash.fingerprint Vips::Image.new_from_buffer File.binread("vk_video.jpg"), "" hb = DHashVips::IDHash.fingerprint "vk_video.jpg" puts "distance: #{d1 = DHashVips::IDHash.distance ha, hb}" size = 2 ** 3 shift = 2 * size * size ai = ha >> shift ad = ha - (ai << shift) bi = hb >> shift bd = hb - (bi << shift) _127 = shift - 1 _63 = size * size - 1 d2 = 0 a, b = [[ad, ai], [bd, bi]].map do |xd, xi| puts "" hor = Array.new(size){Array.new(size){" "}} ver = Array.new(size){Array.new(size){" "}} _127.downto(0).each_with_index do |i, ii| if i > _63 y, x = (_127 - i).divmod size else x, y = (_63 - i).divmod size end if xi[i] > 0 target, c = if i > _63 [ver, %w{ v ^ }[xd[i]]] else [hor, %w{ > < }[xd[i]]] end target[y][x] = c end if ai[i] + bi[i] > 0 && ad[i] != bd[i] d2 += 1 target = if i > _63 ver else hor end target[y][x] = "\e[7m#{target[y][x]}\e[27m" end end hor.zip(ver).each{ |_| puts _.join " " } end
Expected behavior
The difference to be ≤5.
Screenshots
Environment:
- vips-8.11.3-Wed Aug 11 09:29:27 UTC 2021
- ruby-vips 2.2.1
I could rework the fingerprinting algorithm specifically for such case (the case of non-photorealistic images with huge blank lines) by calculating the "significance median" not two times (for vertical and horizontal half of fingerprinting) but once for both added, and maybe I will do it but not soon. I just think you might want to double-check if there is something unexpected happening in libvips.
Beta Was this translation helpful? Give feedback.
All reactions
Replies: 1 comment 4 replies
-
Hi @Nakilon,
Yes, thumbnail and thumbnail_image will give different results since they work in different ways. thumbnail_image is only there for emergencies and should really not be used -- it will often be much, much slower.
I would use thumbnail_buffer instead -- it's as fast as thumbnail_image and should give exactly the same pixels.
The main docs warn against thumbnail_image:
https://www.libvips.org/API/current/libvips-resample.html#vips-thumbnail-image
Though maybe the ruby docs should have this warning too :(
The dev checklist in the next version of the docs has some more info:
https://github.com/libvips/libvips/blob/master/doc/Developer-checklist.md
Beta Was this translation helpful? Give feedback.
All reactions
-
I would use
thumbnail_bufferinstead -- it's as fast asthumbnail_imageand should give exactly the same pixels.
Just so I understand this correctly - thumbnail_image is not ideal, and you recommend thumbnail_buffer instead? In what way is thumbnail_buffer preferable if it's not actually any faster and gives you the same pixels? I do understand why you would want to use a thumbnail method that can leverage shrink-on-load, but if the use case means that's not an option, would it still be preferable to use thumbnail_buffer over thumbnail_image?
Though maybe the ruby docs should have this warning too :(
FWIW I did run into this footgun in a few cases and solved a lot of problems by switching to thumbnail_source.
Beta Was this translation helpful? Give feedback.
All reactions
-
It would be very helpful to have that warning in the Ruby Vips documentation as well.
Beta Was this translation helpful? Give feedback.
All reactions
-
Hi @taylorthurlow, sorry, I missed your question.
Just so I understand this correctly - thumbnail_image is not ideal, and you recommend thumbnail_buffer instead?
It's to do with where the source pixels come from.
thumbnail and thumbnail_buffer work on compressed images. You give them a file or a string containing (for example) the bytes of the JPEG file. They do open, decompress and resize all in one operation, so they can take a lot of shortcuts.
thumbnail_image works on a vips image. The image has already been opened and decompressed, so all the thumbnail_image operation can do is resize. Occasionally that's a useful thing to be able to do, but performance will be much worse and, for formats like PDF and SVG, quality can be worse too.
The dev checklist is now here:
https://www.libvips.org/API/8.17/developer-checklist.html
It has this example of the difference it can make:
$ vipsheader nina.jpg
nina.jpg: 6048x4032 uchar, 3 bands, srgb, jpegload
$ /usr/bin/time -f %M:%e vips thumbnail nina.jpg x.jpg 600
75444:0.09
$ /usr/bin/time -f %M:%e vips thumbnail_image nina.jpg x.jpg 600
254360:0.21
So thumbnail_image needs three times as much RAM and is half the speed.
It's actually worse than that, since libvips has a base startup and RAM cost.
$ /usr/bin/time -f %M:%e vips > /dev/null
32104:0.03
Subtract 32mb and 30ms from the numbers and thumbnail_image is 5x more memory and 3x slower. You'll see a larger difference with bigger images.
Beta Was this translation helpful? Give feedback.
All reactions
-
❤️ 1 -
👀 1
-
You're right Jesse. The problem is that the ruby docs for the thumbnail methods are generated automatically, so we'd need to generate the warning automatically too.
Right now the ruby-vips docs are mostly very minimal and generated from the libvips runtime introspection data, which cannot warn about things like this.
I think we'd need to take extra docs from the Vips-8.0.gir XML file that is built during the build of libvips. It has all this extra information extracted from the C source code:
<method name="thumbnail_image" c:identifier="vips_thumbnail_image" introspectable="0"> <doc xml:space="preserve" filename="libvips/resample/thumbnail.c" line="1814">Exactly as [ctor@Image.thumbnail], but read from an existing image. This operation is not able to exploit shrink-on-load features of image load libraries, so it can be much slower than [ctor@Image.thumbnail] and produce poorer quality output. Only use this operation if you really have to. ::: tip "Optional arguments" * @height: `gint`, target height in pixels * @size: [enum@Size], upsize, downsize, both or force * @no_rotate: `gboolean`, don't rotate upright using orientation tag * @crop: [enum@Interesting], shrink and crop to fill target * @linear: `gboolean`, perform shrink in linear light * @input_profile: `gchararray`, fallback input ICC profile * @output_profile: `gchararray`, output ICC profile * @intent: [enum@Intent], rendering intent * @fail_on: [enum@FailOn], load error types to fail on ::: seealso [ctor@Image.thumbnail].</doc> <source-position filename="libvips/include/vips/resample.h" line="91"/> <return-value transfer-ownership="none"> <doc xml:space="preserve" filename="libvips/resample/thumbnail.c" line="1842">0 on success, -1 on error.</doc> <type name="gint" c:type="int"/> </return-value> <parameters> <instance-parameter name="in" transfer-ownership="none"> <doc xml:space="preserve" filename="libvips/resample/thumbnail.c" line="1816">input image</doc> <type name="Image" c:type="VipsImage*"/> </instance-parameter> <parameter name="out" direction="out" caller-allocates="0" transfer-ownership="full"> <doc xml:space="preserve" filename="libvips/resample/thumbnail.c" line="1817">output image</doc> <type name="Image" c:type="VipsImage**"/> </parameter> <parameter name="width" transfer-ownership="none"> <doc xml:space="preserve" filename="libvips/resample/thumbnail.c" line="1818">target width in pixels</doc> <type name="gint" c:type="int"/> </parameter> <parameter name="..." transfer-ownership="none"> <doc xml:space="preserve" filename="libvips/resample/thumbnail.c" line="1819">`NULL`-terminated list of optional named arguments</doc> <varargs/> </parameter> </parameters> </method>
It's used to make the C docs:
https://www.libvips.org/API/8.17/method.Image.thumbnail_image.html
We could automatically pull out chunks of that XML and paste it into the Ruby doc comments. Someone just needs to get around to it.
Beta Was this translation helpful? Give feedback.