Added more parameters and fix memory leak

[kylewang4929]

[bertsky]

@kylewang4929 could you please elaborate a little on your changes?

1. 0c6aaad essentially removes the 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?
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?

[bertsky]

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?

[kba]

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.      ]]

[bertsky]

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.

[kba]

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.