Feature/manually mark faces - #841
Open
HerbertHorst wants to merge 11 commits into
Open
Conversation
added 9 commits
June 2, 2026 21:53
Add backend endpoints so a face region can be manually marked in an image and assigned to a person (new or existing), and so an existing face can be reassigned to another person. - ApiController: endpoints to list faces of a file, add a manual face, and reassign a face - routes.php: register the new API routes - FaceMapper / PersonMapper / ImageMapper: queries supporting manual faces - Face / Person entities: supporting fields - New migration Version0971Date20260416000000 Pairs with the Memories frontend modal for manual face tagging.
insertManualFace hardcodes is_groupable=false (ignoring the caller's value) and getGroupableFaces filters is_manual=false, so manually-added faces are excluded from clustering at two separate points regardless of what the user checks. Both issues are called out in the code so the connection is visible without digging.
insertManualFace was hardcoding is_groupable=false regardless of the caller's value, silently discarding the user's intent. getGroupableFaces was also filtering is_manual=false, excluding all manual faces from the clustering pipeline at a second point. Fix: store $face->isGroupable as-is and drop the is_manual constraint from getGroupableFaces so that manually-added faces with is_groupable=true are included in clustering runs. Faces without a valid descriptor (all manually-added ones until DLib re-processing is implemented) will be skipped naturally by the clustering algorithm.
getGroupableFaces() now filters out faces whose descriptor column is '[]' (empty JSON array). Without this guard a manually-added face with useForClustering=true but no DLib descriptor would either crash the background job via pdlib (wrong array length) or — on the pure-PHP Euclidean path — return distance 0 to every other face, merging all persons into one cluster. ApiController::addManualFace() now returns clusteringQueued:false so the frontend can tell the user that clustering intent is stored but will not take effect until a descriptor is available.
addManualFace() now rejects two previously unguarded inputs before any DB write: - file ids the user cannot access (verified via UrlService::getFileNode), preventing bogus facerecog_images rows for arbitrary/foreign file ids - rectangles that round down to a zero-area pixel box Adds ManualFaceApiTest covering validation and ownership paths for both addManualFace and reassignFace (disabled user, empty name, bad dimensions, out-of-bounds and zero-area rectangles, inaccessible file, missing face, foreign image, and the happy paths).
ImageMapper::imageProcessed() replaced an image's faces with the freshly detected ones via an unconditional DELETE. Manual faces (is_manual = true) carry no model descriptor and cannot be re-detected, so re-processing silently destroyed them. This hit the main use case: adding a face to a not-yet-scanned photo creates an unprocessed image that the background scan then processes, wiping the manual face. The DELETE now keeps rows with is_manual = true; the IS NULL branch covers legacy rows where the migration default did not apply. Genuine content changes still drop manual faces via the PostWrite/PostDelete listeners, which is intended (the stored pixel coordinates no longer match). Adds an integration regression test (ManualFacePreservationTest) that inserts a detected and a manual face, re-processes the image, and asserts the manual face survives while the old detected face is replaced.
The "use for clustering" option had no effect: a manual face carries no model descriptor, so clustering could never pick it up. This adds a background task (ManualFaceDescriptorTask) that, for each manual face the user flagged for clustering, crops the marked region from the original photo and runs face detection on just that crop. The crop is analysed at near-full resolution, so a small face that the downscaled full-image pass missed can now be detected, and its descriptor is comparable to detected faces (dlib aligns the face before computing it). If no face can be detected in the marked region, the face is simply excluded from clustering (is_groupable=false) and stays pinned to its person. No fallback descriptor is fabricated, and a single bad region never aborts the background job. The pending state needs no new column: is_manual=true AND is_groupable=true AND descriptor='[]' identifies faces awaiting extraction. addManualFace now returns clusteringQueued=true when queued. Note: once a manual face has a descriptor, the existing clustering may still reassign it to another person. Anchoring the user-set name is a follow-up. Adds unit coverage for the queued flag and an integration test for the descriptor task (positive and no-face cases).
A manual face (is_manual) pinned by the user to a named person could be reassigned by the clustering job. mergeClusters maps new chinese-whispers clusters to existing persons by majority vote, so a manual face landing in a cluster dominated by another person was moved there (its user-set name lost), and one split into a brand-new cluster got a null-name person. CreateClustersTask now builds a faceId->person map of manual faces (FaceMapper::findManualFacesWithPerson) and passes it to mergeClusters. Any new cluster that contains a manual face is anchored: each manual face keeps its own person and the remaining faces follow the dominant manual person, so the user-set name always survives and matching faces inherit it (use for recognition). Non-manual clusters keep the original majority path. Adds unit tests: anchor over majority, anchor in a would-be-new cluster, conflicting manual persons each keep their own, faces not in the run are ignored, and empty map == legacy behavior.
A manually marked face anchors clustering to its person. Previously the whole cluster it landed in was consolidated onto that person, so an auto face that already belonged to a DIFFERENTLY named person was relabeled and its group could end up emptied and deleted. Now the anchor only claims a face when it is unnamed (no person, or a person without a name) or already belongs to a person with the same name. A face owned by a differently named person stays with that person, so an existing named group is never renamed or removed. CreateClustersTask now passes a personId->name map into mergeClusters; resolveAnchoredFaces uses the new anchorMayClaim() check. An empty map preserves legacy behavior. Adds 5 MergeClustersTest cases covering claim/keep, unnamed clusters, same-name merges and conflicting anchors.
Owner
|
Hi @HerbertHorst |
…omputation When a manual face was flagged for clustering, the background task computed the descriptor from the CNN detection inside the user's cropped region but only stored the descriptor — the bounding box stayed as the original user rectangle. This meant box and descriptor could describe different faces (e.g. user marks a back, CNN finds a bystander in the 40% margin: the descriptor belonged to the bystander but the box still showed the user's rectangle). The crop offset and TempImage ratio are now used to map the CNN-detected box back to original-image pixels, mirroring the normalisation done for full-image detections. setManualFaceDescriptor() updates x/y/width/height together with the descriptor so they always describe the same face.
Author
|
Hi @matiasdelellis But I found a Bug in the logic while trying to understand all that, when the CNN finds a face in the +40% margin and decides to use that instead of the middle of the part that the user croped it will create a descriptor for that and still show the User created bounding box instead of the new CNN counding box that is actually used. If you still see problems in that logic maybe we can find a solution or I will remove it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add the option to tag a face manual using the memories gui. Fixes #168
You can check if you want to use the manual tag for clustering.
If checked yes it tries to get a face in your marked part and then checks if this is similar to an already found cluster.
It only checks with unnamed clusters or clusters with the same name that you provided so it can not get mixed up with another cluster from another person that it thinks looks similar. If no face is found for clustering (f.e. like in my provided picture with my cat) it silently ignors it for clustering)
I also upped the max version thats supported so I could test on my nextcloud to 33
This is the backend to this pr in memories pulsejet/memories#1678