php-vips-ext icon indicating copy to clipboard operation
php-vips-ext copied to clipboard

feat: Add vips_image_get_fields

Open L3tum opened this issue 4 years ago • 5 comments

Hey, since we spoke about this in the issue I've gone ahead and added the method to the extension.

I'm not a C Dev, so apoligies if anything's amiss. The tests all pass (and I don't think I've seen a codestyler), so the PR should be okay.

The only thing I'm not entirely certain about is the gvalue field that contains the fields. I've tried to unref it with g_object_unref but that Segfaults. It currently doesn't report any memory leaks, so I guess it's alright?

Next Step is going to add it to the PHP Lib. After I've done that I may do an experiment over the holidays in regards to PHP FFI, but I can't promise anything yet.

L3tum avatar Dec 07 '21 13:12 L3tum

Oh nice! Thank you for doing this work. I'll have a read.

I started tinkering with php-ffi here:

https://github.com/libvips/php-vips/blob/switch-to-php-ffi/examples/vips-ffi.php

Just an experiment, currently unfinished, etc.

jcupitt avatar Dec 07 '21 14:12 jcupitt

Thanks, I'll take a look!

Do you plan on keeping the same API with the FFI Implementation?

L3tum avatar Dec 08 '21 08:12 L3tum

I was planning to redo it along the lines of pyvips --- so I'd make classes to wrap GValue and GObject, then use those to write calll(operation_name, args), then php-vips can sit on top of that.

php-vips would look the same to users of that interface, but have a new backend.

jcupitt avatar Dec 08 '21 09:12 jcupitt

I've done some first stuff in that vein (I hope) on my branch here. I'll try to extend that a bit, seems to be going well so far though. Not sure about FFI::new or FFI::string since they aren't entirely clear in the docs about their function.

On another note, did you have time to do a lookover for this PR? Or do you want me to create a PR for the change in php-vips first?

L3tum avatar Dec 21 '21 12:12 L3tum

I've mostly done a php ffi wrapper for libvips here:

https://github.com/libvips/php-vips/blob/switch-to-php-ffi/examples/vips-ffi.php

It needs breaking into a set of classes, some stubs fleshing out, and some tidying up, but it does seem to work. I see:

$ ./vips-ffi.php ~/pics/k2.jpg
vips_init: 0
found libvips version: 8.13.0
attempting to open: /home/john/pics/k2.jpg
selected loader: VipsForeignLoadJpegFile
description: load jpeg from file
flags: 0
n_args: 13
  filename:
    flags: 19
      REQUIRED
      CONSTRUCT
      INPUT
    blurb: Filename to load from
    type: gchararray
  out:
    flags: 35
      REQUIRED
      CONSTRUCT
      OUTPUT
    blurb: Output image
    type: VipsImage
  shrink:
    flags: 18
      CONSTRUCT
      INPUT
    blurb: Shrink factor on load
    type: gint
  autorotate:
    flags: 18
      CONSTRUCT
      INPUT
    blurb: Rotate image using exif orientation
    type: gboolean
  flags:
    flags: 34
      CONSTRUCT
      OUTPUT
    blurb: Flags for this file
    type: VipsForeignFlags
  memory:
    flags: 18
      CONSTRUCT
      INPUT
    blurb: Force open via memory
    type: gboolean
  access:
    flags: 18
      CONSTRUCT
      INPUT
    blurb: Required access pattern for this file
    type: VipsAccess
  fail_on:
    flags: 18
      CONSTRUCT
      INPUT
    blurb: Error level to fail on
    type: VipsFailOn
  sequential:
    flags: 82
      CONSTRUCT
      INPUT
      DEPRECATED
    blurb: Sequential read only
    type: gboolean
  fail:
    flags: 82
      CONSTRUCT
      INPUT
      DEPRECATED
    blurb: Fail on first warning
    type: gboolean
  disc:
    flags: 82
      CONSTRUCT
      INPUT
      DEPRECATED
    blurb: Open to disc
    type: gboolean
required input: filename
required output: out
optional input: shrink, autorotate, memory, access, fail_on, sequential, fail, disc
optional output: flags
member_x: 
method args: filename
setting arguments ...
building ...
in get() get object
result: 
shutting down ...

So that's picking a loader for a test JPG file, making the loader, introspecting to get the args, setting the filename, building the operation, and pulling out the constructed image.

I think the next step is to copy-paste chunks of that into the php binding to replace php-vips-ext.

jcupitt avatar Jan 12 '22 12:01 jcupitt