Expose attention center when doing attention-based cropping.

[ejoebstl]

Hola,

Thanks for your work on libvips.

I'm currently trying to expose the region of interest when using smart/attention cropping. In libvips, only the crop offset is exposed, but having access to the center would be quite interesting.

However, I'm failing to implement this correctly. One of the unit tests crashes with the following backtrace:

#0  0x00007ffff775acc7 in __strchr_avx2 () at /lib64/libc.so.6
#1  0x00007ffff7f71623 in g_param_spec_pool_lookup () at /lib64/libgobject-2.0.so.0
#2  0x00007ffff7f68dbc in g_object_class_find_property () at /lib64/libgobject-2.0.so.0
#3  0x00007ffff7c003a5 in vips_object_get_argument
    (object=0x5004a0, name=0xffffffffffffffff <error: Cannot access memory at address 0xffffffffffffffff>, pspec=0x7ffff4b34fe8, argument_class=0x7ffff4b34fe0, argument_instance=0x7ffff4b34fd8) at ../libvips/iofuncs/object.c:773
#4  0x00007ffff7c04937 in vips_object_set_valist (object=0x5004a0, ap=0x7ffff4b35068)
    at ../libvips/iofuncs/object.c:2426
#5  0x00007ffff7c1c825 in vips_call_required_optional
    (operation=0x7ffff4b350d0, required=0x7ffff4b35100, optional=0x7ffff4b35210)
    at ../libvips/iofuncs/operation.c:905
#6  0x00007ffff7c1c99e in vips_call_by_name
    (operation_name=0x7ffff7c86052 "extract_area", option_string=0x0, required=0x7ffff4b35100, optional=0x7ffff4b35210) at ../libvips/iofuncs/operation.c:954
#7  0x00007ffff7c1d078 in vips_call_split (operation_name=0x7ffff7c86052 "extract_area", optional=0x7ffff4b35210)
    at ../libvips/iofuncs/operation.c:1058
#8  0x00007ffff7b96287 in vips_extract_area
    (in=0x50f960, out=0x7fffec00edf0, left=0, top=0, width=290, height=442) at ../libvips/conversion/extract.c:285
#9  0x00007ffff7a67085 in read_jpeg_image (jpeg=0x7fffec00be40, out=0x50f640)
    at ../libvips/foreign/jpeg2vips.c:925
#10 0x00007ffff7a67272 in vips__jpeg_read (jpeg=0x7fffec00be40, out=0x50f640, header_only=0)
    at ../libvips/foreign/jpeg2vips.c:989
#11 0x00007ffff7a6731a in vips__jpeg_read_source
    (source=0x50e480, out=0x50f640, header_only=0, shrink=1, fail_on=VIPS_FAIL_ON_NONE, autorotate=0, unlimited=0)
    at ../libvips/foreign/jpeg2vips.c:1014

I think it's a problem with how I implemented the additional arguments, but I fail to see the error.

Any guidance would appreciated - happy to finish this PR.

[jcupitt]

Hi @ejoebstl,

This looks interesting, but it's a huge API change. Could you explain the effect you are trying to achieve? Perhaps there's a less intrusive way of doing this.

Maybe just adding x_pos and y_pos as optional output arguments of the smartcrop operation would be enough?

[ejoebstl]

Hi,

Thank you for getting back.

I want to expose the center of attention computed by vips_smartcrop_attention via the API. I'm using vips via sharp to create thumbnails for a responsive website. The idea is to position thumbnails according to device size. And for this I'd use the center of attention provided by libvips.

I agree that adding x_pos and y_pos to the VImage class is a large change. Could you point me in the right direction on how to implement optional output arguments?

For this PR I followed what was done with Xoffset and Yoffset. The arguments are essentially added to the image after cropping.

[jcupitt]

max has optional output arguments for the position of the maximum:

https://github.com/libvips/libvips/blob/master/libvips/arithmetic/max.c#L444-L456

After computing the values in _build, set them with g_object_set():

https://github.com/libvips/libvips/blob/master/libvips/arithmetic/max.c#L232-L242

And then callers should be able to read them out.

[ejoebstl]

Hello @jcupitt - thank you for the guidance. This is way less intrusive now.

[ejoebstl]

Added unit test, this should be good for merging.

I did not find the documentation in the package, please feel free to point me in the right direction and I can update that as well.

[jcupitt]

Ah nice, yes, that's much neater.

The docs are in the comment block near the end:

https://github.com/libvips/libvips/blob/master/libvips/conversion/smartcrop.c#L445-L468

Just add your two new optional output params, plus a paragraph of text below saying what they are for.

Add a line to libvips/ChangeLog as well and credit yourself, and a date and one-line summary to the top of smartcrop.c.

[ejoebstl]

Done, hope that's good!

[jcupitt]

Great! Just two more tiny things ...

Thank you for doing this work, @ejoebstl, it'll be in 8.14, due RSN.

[ejoebstl]

Done 🙌

[jcupitt]

Great! Thanks again for this useful thing.