Integrate sigfm algorithm

[0x00002a]

This integrates the sigfm algorithm into libfprint. It is now just the sigfm part, I've removed the goodixtls changes from the history

Results are pretty good, as long as you make sure to move your finger around on the scanner when enrolling so it gets lots of different shots, the acceptance rate is pretty high and I've so far got no false positives.

Where do I find the goodixtls branch with these changes?

Currently I have a branch that integrates this PR and #13 + a single commit to enable sigfm for the driver. Also there is an aur package

PR outline

C++/sigfm side

• sigfm is added as another library to be built, and put at the same level as the other algorithms (e.g. nbis) in the tree
• the sigfm library depends on opencv (and also doctest for the tests, handled by a wrap so its automatic install), and is built with c++. I'm not 100% sure which version of c++ is needed but I would guess at c++17, I've avoided c++20 as I don't want to restrict the compiler versions we can use too much
• the sigfm library offers a C API in sigfm.hpp (maybe should rename to .h?) which forward declares the SfmImgInfo struct and all the functions for dealing with the algorithm
• the SfmImgInfo struct stores the extracted keypoints and descriptors, and the C api deals in pointers to it. This lets the api communicate with C and be threaded through the libfprint internal api while still having access to c++ niceties like std::vector (and preventing us from having to convert the keypoints and descriptors to some C type)
• serialization and deserialization to binary is also supported (needed by libfprint to store enrolled fingerprints), it uses a homemade stream struct and some template magic (which would be less horrible with c++20 concepts)

C/libfprint side

• There is a new option for the image device class algorithm, which lets you pick between sigfm or nbis (defaulting to nbis), this is used to initalise prints which also store their algorithm (type)
• FpImage's now store an SfmImgInfo* alongside the minutiae, this will be uninitialised until fp_image_extract_sfm_info is called (mirroring minutiae and fp_image_detect_minutiae), this is called automatically by fpi_image_device_image_captured if the device uses sigfm (same as for nbis and minutiae)
• verification now checks the type of the print and either calls the bz3 matcher or the sfm one

From the perspective of drivers, all thats needed is to add <image device class ptr>->algorithm = FPI_DEVICE_ALGO_SIGFM and libfprint will do the rest.

Breaking changes that need discussion

• bz3_threshold is now also used for the threshold for sigfm scores. Ideally it would be renamed to like score_threshold or something but this would require refactoring all the existing drivers (which is piss easy with tooling but probably needs to be discussed with upstream)

[0x00002a]

I've merged the rootd changes (I originally did goodixtls but now I've also done sigfm). Note the example enroll and verify now timeout when running, theres an issue there with the timeout timer starting at some point where it shouldn't (guessing just before waiting on user input?), anyway it seems to work with fprintd but its an issue we might want to look into

[0x00002a]

btw we should be able to just split this branch into the goodixtls only changes and the sigfm changes by just making two new branches and then removing the non-goodixtls/goodixtls files from the commit history for each respectively

[mpi3d]

@0x00002a First of all, Thanks for your work!

This is what's in my mind right now:

• Branch sigfm should contain only SIGFM related stuff (and nothing about GoodixTLS stuff)
• Branch goodixtls should contain only GoodixTLS related stuff (same: nothing about SIGFM stuff)
• Branch master is just the one from libfprint (I may/should remove it)
• We may create a goodixtls-sigfm that contains both (Like actual 0x00002a:0x2a/feat/sigfm)
• I'm open to any proposition since you definitely have much more experience on Git than me.

[0x00002a]

@0x00002a First of all, Thanks for your work!

This is what's in my mind right now:

* Branch `sigfm` should contain only SIGFM related stuff (and nothing about GoodixTLS stuff)

* Branch `goodixtls` should contain only GoodixTLS related stuff (same: nothing about SIGFM stuff)

* Branch `master` is just the one from libfprint (I may/should remove it)

* We may create a `goodixtls-sigfm` that contains both (Like actual [`0x00002a:0x2a/feat/sigfm`](https://github.com/0x00002a/libfprint/tree/0x2a/feat/sigfm))

* I'm open to any proposition since you definitely have much more experience on Git than me.

Yeah that is neat, I like it. What I can do then is like I said remove the goodixtls changes from this branch and make a goodixtls-sigfm using those. If we make this branch the sigfm one I can do it destructively (removing the files from each commit in the history) or non-destructively (adding a new commit that reverts the changes). Destructively is potentially cleaner overall but would require a force push to this branch (or a new PR)

[mpi3d]

@0x00002a First of all, Thanks for your work!
This is what's in my mind right now:

* Branch `sigfm` should contain only SIGFM related stuff (and nothing about GoodixTLS stuff)

* Branch `goodixtls` should contain only GoodixTLS related stuff (same: nothing about SIGFM stuff)

* Branch `master` is just the one from libfprint (I may/should remove it)

* We may create a `goodixtls-sigfm` that contains both (Like actual [`0x00002a:0x2a/feat/sigfm`](https://github.com/0x00002a/libfprint/tree/0x2a/feat/sigfm))

* I'm open to any proposition since you definitely have much more experience on Git than me.

Yeah that is neat, I like it. What I can do then is like I said remove the goodixtls changes from this branch and make a goodixtls-sigfm using those. If we make this branch the sigfm one I can do it destructively (removing the files from each commit in the history) or non-destructively (adding a new commit that reverts the changes). Destructively is potentially cleaner overall but would require a force push to this branch (or a new PR)

I'm for the destructive method since it's cleaner

[0x00002a]

@mpi3d OK, this branch should now just have the sigfm stuff on it

I'm going to make a new PR with just the goodix changes, then another one on top of that with the goodix-sigfm branch, sound good?

[mpi3d]

Ok thanks. Do you want me to merge it right now or you prefer to keep this open?

[0x00002a]

Since I don't have push access to this repo its probably best to keep it open since it may need further changes I'm not sure

[mpi3d]

Ok. But I may add some changes too (When I have time of course 🙂). So first I'll ask @rootd to give you write access.

[mpi3d]

• serialization and deserialization to binary is also supported (needed by libfprint to store enrolled fingerprints), it uses a homemade stream struct and some template magic (which would be less horrible with c++20 concepts)

Can you explain this a bit more please. I don't understand what binary serialization/deserialization is.

[mpi3d]

• the sigfm library offers a C API in sigfm.hpp (maybe should rename to .h?) which forward declares the SfmImgInfo struct and all the functions for dealing with the algorithm

I think that libfprint will prefer to have everything written in pure C and only have bindings to C++.
Is it possible to have only pure C?

[mpi3d]

• bz3_threshold is now also used for the threshold for sigfm scores. Ideally it would be renamed to like score_threshold or something but this would require refactoring all the existing drivers (which is piss easy with tooling but probably needs to be discussed with upstream)

I think we can ignore this in a first time.

[mpi3d]

Just for you to know I didn't read the code yet.

[mpi3d]

Wait. Last commit should be cleaned too.

[0x00002a]

• the sigfm library offers a C API in sigfm.hpp (maybe should rename to .h?) which forward declares the SfmImgInfo struct and all the functions for dealing with the algorithm

I think that libfprint will prefer to have everything written in pure C and only have bindings to C++. Is it possible to have only pure C?

depends what you mean by "bindings"? libfprint main is still pure C and compiler by the c compiler, sigfm is compiled by the c++ compiler and needs to be c++ to use opencv

[mpi3d]

• the sigfm library offers a C API in sigfm.hpp (maybe should rename to .h?) which forward declares the SfmImgInfo struct and all the functions for dealing with the algorithm

I think that libfprint will prefer to have everything written in pure C and only have bindings to C++. Is it possible to have only pure C?

depends what you mean by "bindings"? libfprint main is still pure C and compiler by the c compiler, sigfm is compiled by the c++ compiler and needs to be c++ to use opencv

That should be ok then.

[mpi3d]

What does sfm stand for in fonction names?

sigfm

I think that we should use sigfm entirely. sfm brings confusion and sigfm is just one more letter compared to nbis. Two more compared to bz3 but that's fine. We shouldn't be that lazy 😉.

[mpi3d]

I saw that a lot of lines were formatted. Do you mind that we let the format as is? I can do it if you're too lazy grinning. But I think it will help a lot if we try to minimize commits.

yeah I have a clang-format file I can send to you if you want, I couldn't find any existing one for the project unfortunately but if there is a style guide and I've missed it that would be ideal

Oh wait did you mean remove the formatted lines from the history?

I don't think they have a styling guide. They just have a script: scripts/uncrustify.sh to check basic formatting issues. And yeah, removing formatted lines from the history would be great. 👍

[0x00002a]

I saw that a lot of lines were formatted. Do you mind that we let the format as is? I can do it if you're too lazy grinning. But I think it will help a lot if we try to minimize commits.

yeah I have a clang-format file I can send to you if you want, I couldn't find any existing one for the project unfortunately but if there is a style guide and I've missed it that would be ideal
Oh wait did you mean remove the formatted lines from the history?

I don't think they have a styling guide. They just have a script: scripts/uncrustify.sh to check basic formatting issues. And yeah, removing formatted lines from the history would be great. +1

Thats just what I was looking for, thanks!

[0x00002a]

OK I've run uncrustify using their config over the history and it seems to have worked

[rootd]

Ok. But I may add some changes too (When I have time of course 🙂). So first I'll ask @rootd to give you write access.

Done

[mpi3d]

I found some remaining sfm strings. I hope I didn't forget one. Can you update that please?

[0x00002a]

That should've got them all, I just did sed -i s/sfm/sigfm/g, sed is case sensitive and so was the grep tool I was using (ripgrep), I've done s/Sfm/Sigfm/g now and searched case-insensitively so I think we're good

[mpi3d]

Everything seems fine but I still need to check some things here and there.

[Dinolek]

Works great with elan 0c6e(sensor size 150x52), I was even able to switch from swiping to touch.
This sensor is completely unusable with nbis.

[mpi3d]

@0x00002a Can you review commit 7aedccb just to make sure everything is ok.

Updates since

[0x00002a]

@0x00002a Can you review commit 7aedccb just to make sure everything is ok.

Yeah looks correct

[mpi3d]

We will need to remove formatting of non SIGFM related lines (unmodified lines) in commits. Otherwise libfprint devs will reject them. New and edited lines should use the recommended format. But we shouldn't try to format the entire libfprint code (at least not on this branch).

[mpi3d]

I'll merge. Then I need to edit things before the libfprint merge.