Open kylewang4929 opened 3 years ago
2. 48c34d2 looks like it exposes all the internal parameters of the LSD library (and standalone CLI) to the C API. Would that enable me to override them via Python bindings? How does that work concretely – just add a kwarg to
lsdGet
, wrapping it via ctypes?
Note: I have tried doing that...
def lsd(src, scale=0.8, sigma_scale=0.6, ...):
...
lsdlib.lsdGet(src, ctypes.c_int(rows), ctypes.c_int(cols), fname_bytes,
scale=ctypes.c_double(scale),
sigma_scale=ctypes.c_double(sigma_scale),
...)
Also merging #12 to get painless pip
installation and Python 3 behaviour, this does still compile and run. But I do not see any differences in the result.
I do get different results when using non-default parameters with the standalone CLI from the LSD page.
@kba, do you happen to have any idea what's missing?
It works for me. Are you sure you recompiled the library and copied it to pylsd/lib/linux/liblsd.so
? C.f. https://github.com/kba/pylsd/tree/recompile-16 which contains the API change you hinted at and a recompiled liblsd.so:
Using the example_PIL.py
script, for scale=0.1
I get:
[[306.669372 288.27688 156.025136 250.871433 36.213681]
[805.080155 37.270572 514.735951 47.520257 42.444689]
[375.44709 201.671813 645.574298 183.57009 27.978672]
[ 34.447406 286.237638 205.080144 362.42364 55.764194]
[676.155366 301.46789 554.672829 323.168453 30.239204]
[275.4019 208.473788 125.099842 225.86298 15.270342]
[201.355268 357.591009 289.662401 314.149758 31.478007]
[165.138144 172.510045 365.077518 183.602788 46.586021]
[805.157343 324.13985 683.732657 301.928201 25.07174 ]
[354.781142 240.377863 484.49033 234.236127 43.266029]
For scale=1
I get
[[364.477322 162.710564 343.505184 160.451869 2.952225]
[530.834595 10.410942 528.415064 28.488632 1.919767]
[303.419478 292.75489 202.679975 260.930295 4.772108]
...
[225.5 39.5 225.5 18.5 1. ]
[359.576656 51.500746 359.34308 75.508203 1. ]
[340.515895 182.399117 354.484246 184.599984 1. ]]
Are you sure you recompiled the library and copied it to
pylsd/lib/linux/liblsd.so
?
Duh! Thanks, that's the missing ingredient. I was under the wrong impression pip install
took care of that.
Maybe we should add such rules, e.g. via Cython.Distutils.build_ext
. Care for a PR against your fork/branch, @kba?
BTW, in contrast to your implementation, I did use the kwargs formulated above – it was my understanding that these would be compiled as C++ default arguments. I kept the same ordering, but this apparently does something different (although the compiler does not mock it). The values are all kept uninitialized (i.e. zero) with that notation.
Maybe we should add such rules, e.g. via Cython.Distutils.build_ext. Care for a PR against your fork/branch, @kba?
Sure, the more complete #12 is, the better. My first idea would have been a Makefile but if it can be done within the python tooling, it should.
BTW, in contrast to your implementation, I did use the kwargs formulated above – it was my understanding that these would be compiled as C++ default arguments. I kept the same ordering, but this apparently does something different (although the compiler does not mock it). The values are all kept uninitialized (i.e. zero) with that notation.
I cannot quite follow but whatever works for you is fine by me, my implementation was just a proof-of-concept.
@kylewang4929 could you please elaborate a little on your changes?
calloc
for the image struct's data ptr. Your commit message suggests that allocation is redundant (causing a leak). Help me understand: Who does the allocation then?lsdGet
, wrapping it via ctypes?