Add SimQ quality assessment library to SimFace #17
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds the SimQ quality assessment library to the SimFace project, replacing the existing custom face quality calculation with a more comprehensive quality assessment system using OpenCV. The SimQ library evaluates faces based on alignment, blur, brightness, contrast, and eye openness metrics.
Key Changes
- Replaced basic quality calculation logic in
MlKitFaceDetectionProcessorwith calls to the new SimQ library - Added the SimQ library module with OpenCV-based quality analysis for alignment, blur, brightness, and contrast
- Created comprehensive test coverage using Robolectric shadows to mock OpenCV functionality
Reviewed Changes
Copilot reviewed 27 out of 29 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/com/simprints/biometrics/simface/detection/MlKitFaceDetectionProcessor.kt | Refactored to use SimQ library for quality calculation, passing full bitmap instead of dimensions |
| simq/src/main/java/com/simprints/simq/SimQ.kt | Main quality calculation API using weighted scores from multiple analysis modules |
| simq/src/main/java/com/simprints/simq/analysis/*.kt | Analysis modules for alignment, blur, brightness, and contrast using OpenCV |
| simq/src/main/java/com/simprints/simq/utils/QualityUtils.kt | Utility functions for scoring, cropping, and resizing bitmaps |
| simq/src/main/java/com/simprints/simq/QualityWeights.kt | Configurable weights for different quality metrics |
| simq/src/main/java/com/simprints/simq/QualityParameters.kt | Configurable thresholds for quality assessments |
| simq/src/test/java/com/simprints/simq/shadows/*.kt | Robolectric shadow implementations for OpenCV classes |
| simq/src/test/java/com/simprints/simq/*.kt | Comprehensive test coverage for quality calculations |
| simq/build.gradle.kts | Module configuration with OpenCV dependency |
| gradle/libs.versions.toml | Updated dependencies including OpenCV 4.10.0 |
| settings.gradle.kts | Updated project structure to include simq module |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
luhmirin-s
left a comment
luhmirin-s
left a comment
There was a problem hiding this comment.
Left couple of improvement suggestions to make it more kotlin-like and to align better with other Android repos that we have.
I would also recommend installing ktlint plugin to keep the code-style consistent.
src/main/java/com/simprints/biometrics/simface/detection/MlKitFaceDetectionProcessor.kt
Outdated
Show resolved
Hide resolved
| rootProject.name = "Biometrics-SimFace" | ||
| include(":Biometrics-SimFace") | ||
| rootProject.name = "simface" | ||
| include(":simq") |
There was a problem hiding this comment.
I have a suspicion that this removes the "simface" from the project; both should be included.
I would suggest moving all of the simFace-related stuff into a subfolder first, tho.
|
These are great, I’ve mostly incorporated the changes now! I did not wrap the OpenCV library for now as it seemed a bit of an overkill for not much benefit, since we are not mocking anymore. I also did not create a sub-folder for SimFace as I remember having gradle issues with that before. I will look into it as part of a different PR to create GitHub actions for deployment. |
simq/src/androidTest/java/com/simprints/simq/QualityWeightsTest.kt
Outdated
Show resolved
Hide resolved
| @Test | ||
| fun defaultWeightsSumTo1() { | ||
| val weights = QualityWeights.DEFAULT | ||
| val sum = |
There was a problem hiding this comment.
Unless there is a hidden logic that enforces weight sum to be 1, this is only testing addition in Kotlin :D
There was a problem hiding this comment.
You are right, technically it doesn't really matter if it adds up to 1, although it is more convenient when balancing weights in your head 👍
simq/src/main/java/com/simprints/simq/analysis/AlignmentAnalysis.kt
Outdated
Show resolved
Hide resolved
Wrapping 3rd party library is never overkill, especially if it allows you to make it more usable. |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 31 out of 34 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/main/java/com/simprints/biometrics/simface/SimFaceConfig.kt
Outdated
Show resolved
Hide resolved
src/androidTest/java/com/simprints/biometrics/simface/embedding/EmbeddingProcessorTest.kt
Show resolved
Hide resolved
src/androidTest/java/com/simprints/biometrics/simface/embedding/CustomModelTest.kt
Outdated
Show resolved
Hide resolved
luhmirin-s
left a comment
luhmirin-s
left a comment
There was a problem hiding this comment.
Solution-wise LGTM.
Ping me after the merge, I will add the KtLint config and run the auto-formatting; it is a bit ridiculous in some places. :D
| faceWeights = | ||
| QualityWeights( | ||
| alignment = 1.0, | ||
| blur = 0.0, |
There was a problem hiding this comment.
Something exploded here.
|
|
||
| val perfectScore = | ||
| simQ.calculateFaceQuality( | ||
| bitmap = bitmap, |
| QualityWeights( | ||
| alignment = 0.9, | ||
| blur = 0.025, | ||
| brightness = 0.025, |
There was a problem hiding this comment.
It is better to set all indents to 4 spaces in IDE, otherwise this happens.
No description provided.