libvips / lua-vips

Lua binding for the libvips image processing library
MIT License
125 stars 10 forks source link

Bad performance when iterating over individual pixels #56

Closed rrrnld closed 10 months ago

rrrnld commented 10 months ago

I am running into some issues when iterating over the pixels of an image. The image I'm dealing with is 1920x1080px, and iterating over a single row takes 8s -- without doing any real work. Iterating over all 1080 rows by extension would take much longer than an hour.

local img = Image.new_from_file("image.jpg")
local start = os.clock()
for x = 0, (img:width() - 1) do
  local _ = img(x, 0)
end
print(string.format("Elapsed time: %.02f ms", (1000 * (os.clock() - start))) # Elapsed time: 7646.47 ms

I am trying to work with the image pixels directly to experiment with different clustering algorithms in Lua. Is this something that I can improve or sidestep somehow? I was assuming the approach in https://github.com/libvips/lua-vips/blob/a050f50c/example/array.lua might be faster, but running this fails:

# lua -v
LuaJIT 2.1.1693350652 -- Copyright (C) 2005-2023 Mike Pall. https://luajit.org/

# lua array.lua
lua: array.lua:14: bad storage class near 'typedef'
stack traceback:
    [C]: in function 'typeof'
    array.lua:14: in main chunk
    [C]: at 0x004062d0
jcupitt commented 10 months ago

Hi @heyarne,

Yes, image(x, y) is only there for getting single points, it's extremely slow. You need to render a large block of pixels to an array and then loop over that.

Ooop, sorry the example failed, I'll check. Maybe remove the typedef?

rrrnld commented 10 months ago

Sorry, I'm not really familiar with C, coming from way higher level languages. Here's what I tried:

local ptype = ffi.typeof("typedef unsigned short int[$][?]", im:bands())
lua: array.lua:14: bad storage class near 'typedef'
stack traceback:
    [C]: in function 'typeof'
    array.lua:14: in main chunk
    [C]: at 0x004062d0
- local ptype = ffi.typeof("typedef unsigned short int[$][?]", im:bands())
+ local ptype = ffi.typeof("unsigned short int[$][?]", im:bands())
lua: array.lua:14: size of C type is unknown or too large
stack traceback:
    [C]: in function 'typeof'
    array.lua:14: in main chunk
    [C]: at 0x004062d0
- local ptype = ffi.typeof("typedef unsigned short int[$][?]", im:bands())
+ local ptype = ffi.typeof("unsigned short int[?][$]", im:bands())
lua: array.lua:17: bad argument #1 to 'cast' (invalid C type)
stack traceback:
    [C]: in function 'cast'
    array.lua:17: in main chunk
    [C]: at 0x004062d0
- local ptype = ffi.typeof("typedef unsigned short int[$][?]", im:bands())
+ local ptype = ffi.typeof("unsigned short int[$]", im:bands())
lua: array.lua:17: bad argument #1 to 'cast' (invalid C type)
stack traceback:
    [C]: in function 'cast'
    array.lua:17: in main chunk
    [C]: at 0x004062d0
- local ptype = ffi.typeof("typedef unsigned short int[$][?]", im:bands())
+ local ptype = ffi.typeof("unsigned short int[$][$]", im:bands(), im:width() * im:height())
lua: array.lua:17: bad argument #1 to 'cast' (invalid C type)
stack traceback:
    [C]: in function 'cast'
    array.lua:17: in main chunk
    [C]: at 0x004062d0

So from what I can tell [$] simply is a placeholder and the $ gets replaced by the number of bands. What exactly does [?] mean? That it's an array of unknown length? As I said, I'm not familiar with C, but I should be able to wrap my head around the memory layout at any point in time, but I don't really know what it should look like.

And I guess I can use float just as well when dealing with other image types, is that correct?

Thanks in advance!

rrrnld commented 10 months ago

I'm a bit confused. I found that casting works like this:

- local ptype = ffi.typeof("typedef unsigned short int[$][?]", im:bands())
+ local ptype = ffi.typeof("unsigned short int")

But then the program fails like so:

lua: array.lua:24: 'unsigned short' cannot be indexed with 'number'
stack traceback:
    array.lua:24: in main chunk
    [C]: at 0x004062d0

Which I guess makes sense, because why would something that's not an array allow indexing via array notation. But why does it allow casting like that in the first place? The target data type is way too small to hold all the data. C is a strange beast :upside_down_face:

Do you have any idea how I could adapt array.lua to make this work? I'd like to have an array of floats (or doubles), so I can work comfortably with lch / lab colours.

jcupitt commented 10 months ago

Yeah, I'm trying to fix this, I'll update it later today. I think I must have committed this example by mistake, sorry about that.

jcupitt commented 10 months ago

Right, I fixed the example. This code:

#!/usr/bin/luajit

-- turn a vips image into a lua array

vips = require "vips"
ffi = require "ffi"

-- make a tiny two band u16 image whose pixels are their coordinates
local im = vips.Image.xyz(3, 2):cast("ushort")

-- write as a C-style memory array, so band-interleaved, a series of scanlines
--
-- "data" is a pointer of type uchar*, though the underlying memory is really
-- pairs of int16s, see above
local data = im:write_to_memory()

-- the type of each pixel ... a pair of shorts
ffi.cdef [[
  typedef struct {
      unsigned short x;
      unsigned short y;
  } pixel;
]]
-- and cast the image pointer to a 1D array of pixel structs
local ptype = ffi.typeof("pixel*")
local array = ffi.cast(ptype, data)

-- and print! ffi arrays number from 0
for y = 0, im:height() - 1 do
    for x = 0, im:width() - 1 do
        local i = x + y * im:width()

        print("x = ", x, "y = ", y, "value = ", array[i].x, array[i].y)
    end
end

Prints:

$ ./array.lua 
x =     0   y =     0   value =     0   0
x =     1   y =     0   value =     1   0
x =     2   y =     0   value =     2   0
x =     0   y =     1   value =     0   1
x =     1   y =     1   value =     1   1
x =     2   y =     1   value =     2   1

Which I think is correct. Sorry about the mixup, I feel dumb.

rrrnld commented 10 months ago

It's fantastic you fixed this, this is exactly what I needed. Thank you so much for your help! :)