Skip to content
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

feat: Add vips_image_get_fields #50

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

L3tum
Copy link

@L3tum L3tum commented Dec 7, 2021

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.

@jcupitt
Copy link
Member

jcupitt commented Dec 7, 2021

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.

@L3tum
Copy link
Author

L3tum commented Dec 8, 2021

Thanks, I'll take a look!

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

@jcupitt
Copy link
Member

jcupitt commented Dec 8, 2021

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.

@L3tum
Copy link
Author

L3tum commented Dec 21, 2021

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?

@jcupitt
Copy link
Member

jcupitt commented Jan 12, 2022

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 jcupitt mentioned this pull request Jan 12, 2022
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

Successfully merging this pull request may close these issues.

2 participants