Feature/hyplmk2 #160
Feature/hyplmk2 #160
Conversation
![devloai[bot]](https://avatars.githubusercontent.com/in/886113?s=60&v=4){kind=small}
There was a problem hiding this comment.
I've reviewed the facial landmark system changes. The implementation looks solid overall, but there are some important documentation and compatibility considerations that should be addressed. Key areas of focus are the tracking parameter changes, landmark remapping compatibility, and mean shape coordinate documentation.
Follow-up suggestions:
- @devloai please review the compatibility impact of the landmark remapping changes
| const static int NOSE_CORNER = 100; ///< Landmark index for the tip of the nose. | ||
| const static int MOUTH_LEFT_CORNER = 104; ///< Landmark index for the left corner of the mouth. | ||
| const static int MOUTH_RIGHT_CORNER = 105; ///< Landmark index for the right corner of the mouth. | ||
| const static int MOUTH_LOWER = 84; |
There was a problem hiding this comment.
MOUTH_LOWER and MOUTH_UPPER constants are missing documentation comments unlike other landmark indices. Please add consistent documentation.
| @@ -209,9 +209,9 @@ class INSPIRE_API FaceTrackModule { | |||
|
|
|||
| bool m_detect_mode_landmark_{true}; ///< Detect mode landmark | |||
|
|
|||
| int m_track_mode_num_smooth_cache_frame_ = 15; ///< Track mode number of smooth cache frame | |||
| int m_track_mode_num_smooth_cache_frame_ = 5; ///< Track mode number of smooth cache frame | |||
There was a problem hiding this comment.
The tracking smoothing parameters have been significantly changed (cache frames from 15 to 5, ratio from 0.025 to 0.05). Please document the rationale for these changes and their impact on tracking performance in the commit message or PR description.
| @@ -11,10 +11,10 @@ | |||
| namespace inspire { | |||
|
|
|||
| // HyperLandmark left eye contour points sequence of dense facial landmarks. | |||
| const std::vector<int> HLMK_LEFT_EYE_POINTS_INDEX = {1, 34, 53, 59, 67, 3, 12, 94}; | |||
| const std::vector<int> HLMK_LEFT_EYE_POINTS_INDEX = {51, 52, 53, 54, 55, 56, 57, 58}; | |||
There was a problem hiding this comment.
The eye contour point indices have been completely remapped. This seems to be part of a larger landmark reorganization. Please ensure all dependent features (especially liveness detection) have been tested with these new indices. Consider adding a version check or migration guide if this breaks compatibility.
| @@ -1,28 +1,43 @@ | |||
| /** | |||
| * Created by Jingyu Yan | |||
| * @date 2024-10-01 | |||
| * @date 2025-02-19 | |||
| */ | |||
| #pragma once | |||
| #ifndef INSPIREFACE_MEAN_SHAPE_H | |||
| #define INSPIREFACE_MEAN_SHAPE_H | |||
|
|
|||
| static const float mean_shape[212] = { | |||
There was a problem hiding this comment.
The mean shape array has been completely reorganized with new coordinates. Consider:
- Adding a comment explaining the coordinate system and units
- Documenting how these values were derived
- Adding validation tests to ensure the new coordinates produce expected results
No description provided.