2014-09-30 3:06 GMT+02:00 Vishesh Handa <[email protected]>: > Hey Tobias > > Some comments about the code - > > 1. The code seems to be licensed under GPL. In order to make it into a > framework, it will need to be re-licensed. This library seems like an ideal > candidate for becoming a framework.
libkface have been writted in same way than libkipi, libkexiv2, and libkdcraw, already in KDEGraphics. https://projects.kde.org/projects/kde/kdegraphics/libs/libkipi https://projects.kde.org/projects/kde/kdegraphics/libs/libkexiv2 https://projects.kde.org/projects/kde/kdegraphics/libs/libkdcraw > > 2. The copyright header seems to say "Part of the Digikam Project". You may > want to change that. Idem here. libkface follow exactly the same way than libkipi, libkexiv2, libkdcraw. > > 3. There is an empty TODO file yes, this need to be filled. > > 4. The coding style uses seems a little unorthdox. Could you perhaps add a > link to where one can know what style is being followed? Maybe this could go > in the README file. coding style follow instructions from digiKam project : https://projects.kde.org/projects/extragear/graphics/digikam/repository/revisions/master/entry/HACKING It's the same coding style that libkexiv2, libkipi, and libkdcraw. > > 5. Identity ABI - The Identity class seems to be missing a d pointer. > yes, this need to be fixed. libkface ABI / API id need to be increased. > 6. From what I understand, facial recognition is not particularly cheap. > However, the APIs appear to be blocking. What is the correct way to use > them? Spawn a new thread? Run under a QRunnable? in digiKam, Face Recognition run in separated threads through QThread. Best Gilles Caulier
