From 53ba82802419df4166450310dffcd9eb0804f762 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Tue, 3 Nov 2026 09:34:47 +0100 Subject: [PATCH 1/2] fix: Prevent duplicates and clean up remaining near duplicates Assisted-by: ClaudeCode:claude-opus-5.5 Signed-off-by: Marcel Klehr --- .../AbstractTaskProcessingClassifier.php | 22 +++ .../ImageFaceRecognitionClassifier.php | 35 +++++ lib/Db/FaceDetectionMapper.php | 25 ++++ .../RemoveDuplicateFaceDetections.php | 140 +++++++++++++----- lib/TaskProcessing/TaskResultListener.php | 15 ++ tests/RemoveDuplicateFaceDetectionsTest.php | 55 +++++++ 6 files changed, 259 insertions(+), 33 deletions(-) diff --git a/lib/Classifiers/AbstractTaskProcessingClassifier.php b/lib/Classifiers/AbstractTaskProcessingClassifier.php index f6f982f25..49f565ca5 100644 --- a/lib/Classifiers/AbstractTaskProcessingClassifier.php +++ b/lib/Classifiers/AbstractTaskProcessingClassifier.php @@ -51,6 +51,17 @@ public function classify(array $queueFiles): void { return; } + $filesToProcess = $this->filterQueueFiles($queueFiles); + if (count($filesToProcess) !== count($queueFiles)) { + $skipped = array_values(array_filter($queueFiles, static fn (QueueFile $qf): bool => !in_array($qf, $filesToProcess, true))); + $this->logger->debug('Skipping ' . count($skipped) . ' already processed files from ' . $this->getModelName() . ' queue'); + $this->dropFromQueue($skipped); + } + $queueFiles = $filesToProcess; + if (count($queueFiles) === 0) { + return; + } + $storageId = $queueFiles[0]->getStorageId(); $rootId = $queueFiles[0]->getRootId(); $userId = $this->findUserForStorage($storageId, $rootId); @@ -88,6 +99,17 @@ public function classify(array $queueFiles): void { $this->dropFromQueue($queueFiles); } + /** + * Hook for subclasses to skip queue files that don't need to be processed (again). + * Files that are filtered out are removed from the queue. + * + * @param list $queueFiles + * @return list + */ + protected function filterQueueFiles(array $queueFiles): array { + return $queueFiles; + } + private function findUserForStorage(int $storageId, int $rootId): ?string { $mounts = array_values(array_filter( $this->userMountCache->getMountsForStorageId($storageId), diff --git a/lib/Classifiers/TaskProcessing/ImageFaceRecognitionClassifier.php b/lib/Classifiers/TaskProcessing/ImageFaceRecognitionClassifier.php index 184aed6a2..2904c22ad 100644 --- a/lib/Classifiers/TaskProcessing/ImageFaceRecognitionClassifier.php +++ b/lib/Classifiers/TaskProcessing/ImageFaceRecognitionClassifier.php @@ -11,7 +11,13 @@ use OCA\Recognize\Classifiers\AbstractTaskProcessingClassifier; use OCA\Recognize\Classifiers\Images\ClusteringFaceClassifier; +use OCA\Recognize\Db\FaceDetectionMapper; +use OCA\Recognize\Db\QueueFile; +use OCA\Recognize\Service\QueueService; use OCA\Recognize\TaskProcessing\ImageFaceRecognitionTaskType; +use OCP\Files\Config\IUserMountCache; +use OCP\TaskProcessing\IManager as ITaskProcessingManager; +use Psr\Log\LoggerInterface; final class ImageFaceRecognitionClassifier extends AbstractTaskProcessingClassifier { public const MIN_FACE_RECOGNITION_SCORE = 0.5; @@ -24,6 +30,16 @@ final class ImageFaceRecognitionClassifier extends AbstractTaskProcessingClassif public const MAX_OVERLAP_NEW_CLUSTER = 0.1; public const MIN_OVERLAP_EXISTING_CLUSTER = 0.5; + public function __construct( + LoggerInterface $logger, + ITaskProcessingManager $taskProcessingManager, + IUserMountCache $userMountCache, + QueueService $queue, + private FaceDetectionMapper $faceDetections, + ) { + parent::__construct($logger, $taskProcessingManager, $userMountCache, $queue); + } + protected function getTaskTypeId(): string { return ImageFaceRecognitionTaskType::ID; } @@ -31,4 +47,23 @@ protected function getTaskTypeId(): string { protected function getModelName(): string { return ClusteringFaceClassifier::MODEL_NAME; } + + /** + * Skip files that already have face detections, so that re-crawling doesn't + * create a second set of detections for the same faces. + * + * @param list $queueFiles + * @return list + */ + protected function filterQueueFiles(array $queueFiles): array { + try { + $processed = array_flip($this->faceDetections->findFileIdsWithDetections( + array_map(static fn (QueueFile $qf): int => $qf->getFileId(), $queueFiles), + )); + } catch (\OCP\DB\Exception $e) { + $this->logger->warning('Could not look up existing face detections; scheduling all files', ['exception' => $e]); + return $queueFiles; + } + return array_values(array_filter($queueFiles, static fn (QueueFile $qf): bool => !isset($processed[$qf->getFileId()]))); + } } diff --git a/lib/Db/FaceDetectionMapper.php b/lib/Db/FaceDetectionMapper.php index 2651c67c8..f5906e1fa 100644 --- a/lib/Db/FaceDetectionMapper.php +++ b/lib/Db/FaceDetectionMapper.php @@ -132,6 +132,31 @@ public function findByFileId(int $fileId): array { return $this->findEntities($qb); } + /** + * Which of the given files already have at least one face detection (for any user) + * + * @param list $fileIds + * @return list + * @throws \OCP\DB\Exception + */ + public function findFileIdsWithDetections(array $fileIds): array { + $found = []; + foreach (array_chunk(array_values(array_unique($fileIds)), 1000) as $chunk) { + $qb = $this->db->getQueryBuilder(); + $qb->selectDistinct('file_id') + ->from('recognize_face_detections') + ->where($qb->expr()->in('file_id', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))); + $result = $qb->executeQuery(); + /** @var list $rows */ + $rows = $result->fetchAll(); + $result->closeCursor(); + foreach ($rows as $row) { + $found[] = (int)$row['file_id']; + } + } + return $found; + } + /** * @throws \OCP\DB\Exception * @return \OCA\Recognize\Db\FaceDetection[] diff --git a/lib/Migration/RemoveDuplicateFaceDetections.php b/lib/Migration/RemoveDuplicateFaceDetections.php index 7c21e6a85..f12cd927c 100644 --- a/lib/Migration/RemoveDuplicateFaceDetections.php +++ b/lib/Migration/RemoveDuplicateFaceDetections.php @@ -30,7 +30,21 @@ use OCP\Migration\IRepairStep; use Psr\Log\LoggerInterface; +/** + * Removes face detections that describe the same face in the same file for the same user. + * + * Processing a file more than once (e.g. after a recrawl) yields a second set of detections + * whose bounding boxes differ only slightly from the first one, so detections are considered + * duplicates when their bounding boxes overlap almost completely, not only when they are identical. + */ final class RemoveDuplicateFaceDetections implements IRepairStep { + /** + * Minimum intersection over union of two bounding boxes to consider them the same face. + * Two different faces in the same image practically never overlap this much. + */ + public const MIN_IOU = 0.9; + + private const DELETE_CHUNK_SIZE = 1000; public function __construct( private IDBConnection $db, @@ -44,44 +58,104 @@ public function getName(): string { public function run(IOutput $output): void { try { - $selectQuery = $this->db->getQueryBuilder(); - $selectQuery - ->select('file_id', 'user_id', 'x', 'y', 'height', 'width') - ->selectAlias($selectQuery->func()->min('id'), 'min_id') - ->from('recognize_face_detections') - ->groupBy('file_id', 'user_id', 'x', 'y', 'height', 'width'); - - $result = $selectQuery->executeQuery(); - - $deleteQuery = $this->db->getQueryBuilder(); - $deleteQuery->delete('recognize_face_detections') - ->andWhere( - $deleteQuery->expr()->neq('id', $deleteQuery->createParameter('min_id')), - $deleteQuery->expr()->eq('file_id', $deleteQuery->createParameter('file_id')), - $deleteQuery->expr()->eq('user_id', $deleteQuery->createParameter('user_id')), - $deleteQuery->expr()->eq('x', $deleteQuery->createParameter('x')), - $deleteQuery->expr()->eq('y', $deleteQuery->createParameter('y')), - $deleteQuery->expr()->eq('height', $deleteQuery->createParameter('height')), - $deleteQuery->expr()->eq('width', $deleteQuery->createParameter('width')) - ); - - while ( - /** @var array{min_id: int, file_id: int, user_id: string, x: float, y: float, height: float, width: float} $row */ - $row = $result->fetch() - ) { - $deleteQuery->setParameter('min_id', $row['min_id'], IQueryBuilder::PARAM_INT); - $deleteQuery->setParameter('file_id', $row['file_id'], IQueryBuilder::PARAM_INT); - $deleteQuery->setParameter('user_id', $row['user_id']); - $deleteQuery->setParameter('x', $row['x']); - $deleteQuery->setParameter('y', $row['y']); - $deleteQuery->setParameter('height', $row['height']); - $deleteQuery->setParameter('width', $row['width']); + $idsToDelete = $this->findDuplicateIds(); + foreach (array_chunk($idsToDelete, self::DELETE_CHUNK_SIZE) as $chunk) { + $deleteQuery = $this->db->getQueryBuilder(); + $deleteQuery->delete('recognize_face_detections') + ->where($deleteQuery->expr()->in('id', $deleteQuery->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))); $deleteQuery->executeStatement(); } - $result->closeCursor(); + if (count($idsToDelete) > 0) { + $output->info('Removed ' . count($idsToDelete) . ' duplicate face detections'); + $this->logger->info('Removed ' . count($idsToDelete) . ' duplicate face detections'); + } } catch (\Throwable $e) { $output->warning('Failed to automatically remove duplicate face detections for recognize.'); throw new \Exception('Failed to automatically remove duplicate face detections', previous: $e); } } + + /** + * @return list + * @throws \OCP\DB\Exception + */ + private function findDuplicateIds(): array { + $selectQuery = $this->db->getQueryBuilder(); + $selectQuery + ->select('id', 'file_id', 'user_id', 'x', 'y', 'height', 'width', 'cluster_id') + ->from('recognize_face_detections') + ->orderBy('file_id') + ->addOrderBy('user_id') + ->addOrderBy('id'); + $result = $selectQuery->executeQuery(); + + $idsToDelete = []; + $group = []; + $groupKey = null; + while ( + /** @var array{id: int|string, file_id: int|string, user_id: string, x: float|string, y: float|string, height: float|string, width: float|string, cluster_id: int|string|null}|false $row */ + $row = $result->fetch() + ) { + $key = $row['file_id'] . '/' . $row['user_id']; + if ($key !== $groupKey) { + array_push($idsToDelete, ...self::findDuplicatesInGroup($group)); + $group = []; + $groupKey = $key; + } + $group[] = [ + 'id' => (int)$row['id'], + 'x' => (float)$row['x'], + 'y' => (float)$row['y'], + 'width' => (float)$row['width'], + 'height' => (float)$row['height'], + 'clustered' => $row['cluster_id'] !== null, + ]; + } + array_push($idsToDelete, ...self::findDuplicatesInGroup($group)); + $result->closeCursor(); + return $idsToDelete; + } + + /** + * Given all detections of one user in one file, returns the ids of those that duplicate another one. + * Detections that are assigned to a cluster are preferred, then older ones. + * + * @param list $detections + * @return list + */ + private static function findDuplicatesInGroup(array $detections): array { + if (count($detections) < 2) { + return []; + } + usort($detections, static fn (array $a, array $b): int => [$b['clustered'], $a['id']] <=> [$a['clustered'], $b['id']]); + $kept = []; + $duplicates = []; + foreach ($detections as $detection) { + foreach ($kept as $keptDetection) { + if (self::isSameFace($detection, $keptDetection)) { + $duplicates[] = $detection['id']; + continue 2; + } + } + $kept[] = $detection; + } + return $duplicates; + } + + /** + * @param array{x: float, y: float, width: float, height: float, id?: int, clustered?: bool} $a + * @param array{x: float, y: float, width: float, height: float, id?: int, clustered?: bool} $b + */ + public static function isSameFace(array $a, array $b): bool { + $intersectionWidth = min($a['x'] + $a['width'], $b['x'] + $b['width']) - max($a['x'], $b['x']); + $intersectionHeight = min($a['y'] + $a['height'], $b['y'] + $b['height']) - max($a['y'], $b['y']); + $intersection = max(0.0, $intersectionWidth) * max(0.0, $intersectionHeight); + $union = $a['width'] * $a['height'] + $b['width'] * $b['height'] - $intersection; + if ($union <= 0.0) { + // Degenerate boxes without area: only identical ones are duplicates + return abs($a['x'] - $b['x']) < 1e-9 && abs($a['y'] - $b['y']) < 1e-9 + && abs($a['width'] - $b['width']) < 1e-9 && abs($a['height'] - $b['height']) < 1e-9; + } + return $intersection / $union >= self::MIN_IOU; + } } diff --git a/lib/TaskProcessing/TaskResultListener.php b/lib/TaskProcessing/TaskResultListener.php index 97f5764e8..7b9ea8c39 100644 --- a/lib/TaskProcessing/TaskResultListener.php +++ b/lib/TaskProcessing/TaskResultListener.php @@ -197,10 +197,25 @@ private function applyFaceResults(array $fileIds, array $results): void { // The mount tables are read for every file in this batch, so refresh them once here // rather than on every lookup. $this->userMountCache->clear(); + // Files may have been processed by more than one task, e.g. when a recrawl was + // triggered while tasks were still pending. Only the first result is stored, as + // re-detected faces differ slightly in their coordinates and would end up as duplicates. + try { + $alreadyProcessed = array_flip($this->faceDetections->findFileIdsWithDetections($fileIds)); + } catch (\Throwable $e) { + // Storing possible duplicates is better than losing the results; the + // RemoveDuplicateFaceDetections repair step can clean them up later + $this->logger->warning('Could not look up existing face detections', ['exception' => $e]); + $alreadyProcessed = []; + } foreach ($fileIds as $i => $fileId) { if (!isset($results[$i])) { continue; } + if (isset($alreadyProcessed[$fileId])) { + $this->logger->debug('File ' . $fileId . ' already has face detections, discarding new face results'); + continue; + } $raw = (string)$results[$i]; $userIds = $this->getUsersWithFileAccess($fileId); foreach (explode("\n", $raw) as $line) { diff --git a/tests/RemoveDuplicateFaceDetectionsTest.php b/tests/RemoveDuplicateFaceDetectionsTest.php index 037e65930..190948410 100644 --- a/tests/RemoveDuplicateFaceDetectionsTest.php +++ b/tests/RemoveDuplicateFaceDetectionsTest.php @@ -73,4 +73,59 @@ public function testRepairStep() : void { $count = $qb->select($qb->func()->count('*'))->from('recognize_face_detections')->executeQuery()->fetchOne(); $this->assertEquals(10 * 10 * 11 - 10 * 10 * 9, (int)$count); // 200 } + + public function testRepairStepRemovesNearDuplicates() : void { + $qb = $this->db->getQueryBuilder(); + $qb->delete('recognize_face_detections')->executeStatement(); + + // Values taken from https://github.com/nextcloud/recognize/issues/1603 + $original = $this->insertDetection('user1', 100, 0.295590, 0.269856, 0.037232, 0.070845, null); + $reDetected = $this->insertDetection('user1', 100, 0.295480, 0.269174, 0.037122, 0.071395, 1254); + $reDetectedAgain = $this->insertDetection('user1', 100, 0.295500, 0.269500, 0.037200, 0.071000, null); + $otherFace = $this->insertDetection('user1', 100, 0.5, 0.3, 0.04, 0.07, null); + $overlappingOtherFace = $this->insertDetection('user1', 100, 0.52, 0.3, 0.04, 0.07, null); + $otherUser = $this->insertDetection('user2', 100, 0.295590, 0.269856, 0.037232, 0.070845, null); + $otherFile = $this->insertDetection('user1', 101, 0.295590, 0.269856, 0.037232, 0.070845, null); + + Server::get(RemoveDuplicateFaceDetections::class)->run($this->createMock(\OCP\Migration\IOutput::class)); + + $qb = $this->db->getQueryBuilder(); + $result = $qb->select('id')->from('recognize_face_detections')->executeQuery(); + $remainingIds = []; + while (($id = $result->fetchOne()) !== false) { + $remainingIds[] = (int)$id; + } + $result->closeCursor(); + sort($remainingIds); + $expected = [$reDetected, $otherFace, $overlappingOtherFace, $otherUser, $otherFile]; + sort($expected); + $this->assertEquals($expected, $remainingIds); + $this->assertNotContains($original, $remainingIds, 'The unclustered duplicate should be removed in favour of the clustered one'); + $this->assertNotContains($reDetectedAgain, $remainingIds); + } + + public function testIsSameFace() : void { + $box = ['x' => 0.3, 'y' => 0.3, 'width' => 0.1, 'height' => 0.1]; + $this->assertTrue(RemoveDuplicateFaceDetections::isSameFace($box, $box)); + $this->assertTrue(RemoveDuplicateFaceDetections::isSameFace($box, ['x' => 0.301, 'y' => 0.299, 'width' => 0.1, 'height' => 0.101])); + $this->assertFalse(RemoveDuplicateFaceDetections::isSameFace($box, ['x' => 0.32, 'y' => 0.3, 'width' => 0.1, 'height' => 0.1])); + $this->assertFalse(RemoveDuplicateFaceDetections::isSameFace($box, ['x' => 0.6, 'y' => 0.6, 'width' => 0.1, 'height' => 0.1])); + $empty = ['x' => 0.3, 'y' => 0.3, 'width' => 0.0, 'height' => 0.1]; + $this->assertTrue(RemoveDuplicateFaceDetections::isSameFace($empty, $empty)); + $this->assertFalse(RemoveDuplicateFaceDetections::isSameFace($empty, ['x' => 0.31, 'y' => 0.3, 'width' => 0.0, 'height' => 0.1])); + } + + private function insertDetection(string $userId, int $fileId, float $x, float $y, float $width, float $height, ?int $clusterId): int { + $face = new \OCA\Recognize\Db\FaceDetection(); + $face->setUserId($userId); + $face->setFileId($fileId); + $face->setX($x); + $face->setY($y); + $face->setWidth($width); + $face->setHeight($height); + $face->setClusterId($clusterId); + $face->setThreshold(0.5); + $face->setVector([1, 2, 3, 4, 5, 6, 7, 8, 9, 0]); + return $this->faceDetectionMapper->insertWithoutDeduplication($face)->getId(); + } } From fff929607fd38baf58ec6aeb085f70e0b69e425c Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Tue, 3 Nov 2026 10:10:48 +0100 Subject: [PATCH 2/2] fix(RemoveDuplicateFaceDetections): Prioritize clustered faces + work in batches Assisted-by: ClaudeCode:claude-opus-5.5 Signed-off-by: Marcel Klehr --- .../RemoveDuplicateFaceDetections.php | 114 +++++++++++++----- tests/RemoveDuplicateFaceDetectionsTest.php | 90 ++++++++++++++ 2 files changed, 173 insertions(+), 31 deletions(-) diff --git a/lib/Migration/RemoveDuplicateFaceDetections.php b/lib/Migration/RemoveDuplicateFaceDetections.php index f12cd927c..7cde240ac 100644 --- a/lib/Migration/RemoveDuplicateFaceDetections.php +++ b/lib/Migration/RemoveDuplicateFaceDetections.php @@ -46,28 +46,47 @@ final class RemoveDuplicateFaceDetections implements IRepairStep { private const DELETE_CHUNK_SIZE = 1000; + /** + * Approximate number of detections loaded into memory at once + */ + private int $batchSize = 10000; + public function __construct( private IDBConnection $db, private LoggerInterface $logger, ) { } + /** + * @internal Only meant for tests + */ + public function setBatchSize(int $batchSize): void { + $this->batchSize = max(1, $batchSize); + } + public function getName(): string { return 'Remove duplicate face detections'; } public function run(IOutput $output): void { try { - $idsToDelete = $this->findDuplicateIds(); - foreach (array_chunk($idsToDelete, self::DELETE_CHUNK_SIZE) as $chunk) { - $deleteQuery = $this->db->getQueryBuilder(); - $deleteQuery->delete('recognize_face_detections') - ->where($deleteQuery->expr()->in('id', $deleteQuery->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))); - $deleteQuery->executeStatement(); - } - if (count($idsToDelete) > 0) { - $output->info('Removed ' . count($idsToDelete) . ' duplicate face detections'); - $this->logger->info('Removed ' . count($idsToDelete) . ' duplicate face detections'); + $removed = 0; + $lastFileId = -1; + do { + $upperFileId = $this->findBatchUpperBound($lastFileId); + $idsToDelete = $this->findDuplicateIds($lastFileId, $upperFileId); + foreach (array_chunk($idsToDelete, self::DELETE_CHUNK_SIZE) as $chunk) { + $deleteQuery = $this->db->getQueryBuilder(); + $deleteQuery->delete('recognize_face_detections') + ->where($deleteQuery->expr()->in('id', $deleteQuery->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))); + $deleteQuery->executeStatement(); + } + $removed += count($idsToDelete); + $lastFileId = $upperFileId; + } while ($upperFileId !== null); + if ($removed > 0) { + $output->info('Removed ' . $removed . ' duplicate face detections'); + $this->logger->info('Removed ' . $removed . ' duplicate face detections'); } } catch (\Throwable $e) { $output->warning('Failed to automatically remove duplicate face detections for recognize.'); @@ -76,58 +95,91 @@ public function run(IOutput $output): void { } /** + * Returns the file id that ends the batch of roughly batchSize detections starting after $lastFileId, + * or null if the remaining detections fit into one batch. Uses the index on file_id. + * + * @throws \OCP\DB\Exception + */ + private function findBatchUpperBound(int $lastFileId): ?int { + $qb = $this->db->getQueryBuilder(); + $qb->select('file_id') + ->from('recognize_face_detections') + ->where($qb->expr()->gt('file_id', $qb->createNamedParameter($lastFileId, IQueryBuilder::PARAM_INT))) + ->orderBy('file_id') + ->setFirstResult($this->batchSize) + ->setMaxResults(1); + $result = $qb->executeQuery(); + $fileId = $result->fetchOne(); + $result->closeCursor(); + return $fileId === false ? null : (int)$fileId; + } + + /** + * Finds duplicates among the detections with $lastFileId < file_id <= $upperFileId (no upper limit if null). + * Whole files are always contained in one batch, so all detections of a (file, user) group are compared. + * * @return list * @throws \OCP\DB\Exception */ - private function findDuplicateIds(): array { + private function findDuplicateIds(int $lastFileId, ?int $upperFileId): array { $selectQuery = $this->db->getQueryBuilder(); $selectQuery ->select('id', 'file_id', 'user_id', 'x', 'y', 'height', 'width', 'cluster_id') ->from('recognize_face_detections') - ->orderBy('file_id') - ->addOrderBy('user_id') - ->addOrderBy('id'); + ->where($selectQuery->expr()->gt('file_id', $selectQuery->createNamedParameter($lastFileId, IQueryBuilder::PARAM_INT))); + if ($upperFileId !== null) { + $selectQuery->andWhere($selectQuery->expr()->lte('file_id', $selectQuery->createNamedParameter($upperFileId, IQueryBuilder::PARAM_INT))); + } $result = $selectQuery->executeQuery(); - $idsToDelete = []; - $group = []; - $groupKey = null; + // Group in PHP instead of relying on the database's sort order, which depends on the collation of user_id + /** @var array> $groups */ + $groups = []; while ( /** @var array{id: int|string, file_id: int|string, user_id: string, x: float|string, y: float|string, height: float|string, width: float|string, cluster_id: int|string|null}|false $row */ $row = $result->fetch() ) { - $key = $row['file_id'] . '/' . $row['user_id']; - if ($key !== $groupKey) { - array_push($idsToDelete, ...self::findDuplicatesInGroup($group)); - $group = []; - $groupKey = $key; - } - $group[] = [ + $groups[$row['file_id'] . '/' . $row['user_id']][] = [ 'id' => (int)$row['id'], 'x' => (float)$row['x'], 'y' => (float)$row['y'], 'width' => (float)$row['width'], 'height' => (float)$row['height'], - 'clustered' => $row['cluster_id'] !== null, + 'priority' => self::getPriority($row['cluster_id']), ]; } - array_push($idsToDelete, ...self::findDuplicatesInGroup($group)); $result->closeCursor(); + + $idsToDelete = []; + foreach ($groups as $group) { + array_push($idsToDelete, ...self::findDuplicatesInGroup($group)); + } return $idsToDelete; } + /** + * Ranks which detection to keep among duplicates: one assigned to a cluster (the user's person assignment), + * then a rejected one (cluster_id = -1, so the user's rejection is retained), then an unclustered one. + */ + private static function getPriority(int|string|null $clusterId): int { + if ($clusterId === null) { + return 0; + } + return (int)$clusterId === -1 ? 1 : 2; + } + /** * Given all detections of one user in one file, returns the ids of those that duplicate another one. - * Detections that are assigned to a cluster are preferred, then older ones. + * Detections that are assigned to a cluster are preferred, then rejected ones, then older ones. * - * @param list $detections + * @param list $detections * @return list */ private static function findDuplicatesInGroup(array $detections): array { if (count($detections) < 2) { return []; } - usort($detections, static fn (array $a, array $b): int => [$b['clustered'], $a['id']] <=> [$a['clustered'], $b['id']]); + usort($detections, static fn (array $a, array $b): int => [$b['priority'], $a['id']] <=> [$a['priority'], $b['id']]); $kept = []; $duplicates = []; foreach ($detections as $detection) { @@ -143,8 +195,8 @@ private static function findDuplicatesInGroup(array $detections): array { } /** - * @param array{x: float, y: float, width: float, height: float, id?: int, clustered?: bool} $a - * @param array{x: float, y: float, width: float, height: float, id?: int, clustered?: bool} $b + * @param array{x: float, y: float, width: float, height: float, id?: int, priority?: int} $a + * @param array{x: float, y: float, width: float, height: float, id?: int, priority?: int} $b */ public static function isSameFace(array $a, array $b): bool { $intersectionWidth = min($a['x'] + $a['width'], $b['x'] + $b['width']) - max($a['x'], $b['x']); diff --git a/tests/RemoveDuplicateFaceDetectionsTest.php b/tests/RemoveDuplicateFaceDetectionsTest.php index 190948410..5350379c2 100644 --- a/tests/RemoveDuplicateFaceDetectionsTest.php +++ b/tests/RemoveDuplicateFaceDetectionsTest.php @@ -4,6 +4,7 @@ use OCA\Recognize\Migration\RemoveDuplicateFaceDetections; use OCP\IDBConnection; use OCP\Server; +use Psr\Log\LoggerInterface; use Test\TestCase; /** @@ -104,6 +105,95 @@ public function testRepairStepRemovesNearDuplicates() : void { $this->assertNotContains($reDetectedAgain, $remainingIds); } + public function testRepairStepPrefersClusteredOverRejectedOverUnclustered() : void { + $qb = $this->db->getQueryBuilder(); + $qb->delete('recognize_face_detections')->executeStatement(); + + // rejected (older) vs. clustered (newer): clustered wins + $rejected = $this->insertDetection('user1', 200, 0.295590, 0.269856, 0.037232, 0.070845, -1); + $clustered = $this->insertDetection('user1', 200, 0.295480, 0.269174, 0.037122, 0.071395, 1254); + // unclustered (older) vs. rejected (newer): rejected wins + $unclustered2 = $this->insertDetection('user1', 201, 0.295590, 0.269856, 0.037232, 0.070845, null); + $rejected2 = $this->insertDetection('user1', 201, 0.295480, 0.269174, 0.037122, 0.071395, -1); + + Server::get(RemoveDuplicateFaceDetections::class)->run($this->createMock(\OCP\Migration\IOutput::class)); + + $qb = $this->db->getQueryBuilder(); + $result = $qb->select('id')->from('recognize_face_detections')->executeQuery(); + $remainingIds = []; + while (($id = $result->fetchOne()) !== false) { + $remainingIds[] = (int)$id; + } + $result->closeCursor(); + sort($remainingIds); + $expected = [$clustered, $rejected2]; + sort($expected); + $this->assertEquals($expected, $remainingIds); + } + + public function testRepairStepAcrossBatches() : void { + $qb = $this->db->getQueryBuilder(); + $qb->delete('recognize_face_detections')->executeStatement(); + + $expected = []; + for ($fileId = 300; $fileId < 310; $fileId++) { + // Two different faces per file, each detected twice with slightly shifted boxes + $expected[] = $this->insertDetection('user1', $fileId, 0.295590, 0.269856, 0.037232, 0.070845, null); + $expected[] = $this->insertDetection('user1', $fileId, 0.5, 0.3, 0.04, 0.07, null); + $this->insertDetection('user1', $fileId, 0.295480, 0.269174, 0.037122, 0.071395, null); + $this->insertDetection('user1', $fileId, 0.5001, 0.3001, 0.04, 0.07, null); + } + + $repairStep = new RemoveDuplicateFaceDetections($this->db, Server::get(LoggerInterface::class)); + // A batch size that doesn't line up with file boundaries, so batch limits fall in the middle of a file + $repairStep->setBatchSize(3); + $repairStep->run($this->createMock(\OCP\Migration\IOutput::class)); + + $this->assertEquals($this->sorted($expected), $this->getRemainingIds()); + } + + public function testRepairStepWithFileLargerThanBatch() : void { + $qb = $this->db->getQueryBuilder(); + $qb->delete('recognize_face_detections')->executeStatement(); + + $expected = []; + $expected[] = $this->insertDetection('user1', 400, 0.1, 0.1, 0.05, 0.05, null); + for ($i = 0; $i < 5; $i++) { + $this->insertDetection('user1', 400, 0.1 + $i * 0.0001, 0.1, 0.05, 0.05, null); + } + $expected[] = $this->insertDetection('user1', 401, 0.1, 0.1, 0.05, 0.05, null); + $this->insertDetection('user1', 401, 0.1001, 0.1, 0.05, 0.05, null); + + $repairStep = new RemoveDuplicateFaceDetections($this->db, Server::get(LoggerInterface::class)); + $repairStep->setBatchSize(2); + $repairStep->run($this->createMock(\OCP\Migration\IOutput::class)); + + $this->assertEquals($this->sorted($expected), $this->getRemainingIds()); + } + + /** + * @return list + */ + private function getRemainingIds(): array { + $qb = $this->db->getQueryBuilder(); + $result = $qb->select('id')->from('recognize_face_detections')->executeQuery(); + $remainingIds = []; + while (($id = $result->fetchOne()) !== false) { + $remainingIds[] = (int)$id; + } + $result->closeCursor(); + return $this->sorted($remainingIds); + } + + /** + * @param list $ids + * @return list + */ + private function sorted(array $ids): array { + sort($ids); + return $ids; + } + public function testIsSameFace() : void { $box = ['x' => 0.3, 'y' => 0.3, 'width' => 0.1, 'height' => 0.1]; $this->assertTrue(RemoveDuplicateFaceDetections::isSameFace($box, $box));