Feature identity recognition 2100793612207388938#157
Conversation
This change introduces face recognition capabilities to track specific people across project photos. - Added TensorFlow Lite dependencies and a dummy MobileFaceNet model. - Created `FaceRecognitionHelper` to generate face embeddings. - Updated `ProjectEntity` and database schema to store target embeddings. - Enhanced `ProjectViewModel` to support "Track This Person" and smart re-alignment using cosine similarity. - Updated UI to expose the new tracking feature. - Optimized threading (Dispatchers.Default/IO) and memory management (Bitmap recycling) for heavy TFLite operations. Co-authored-by: harrydbarnes <145344818+harrydbarnes@users.noreply.github.com>
This change introduces face recognition capabilities to track specific people across project photos. - Added TensorFlow Lite dependencies and a dummy MobileFaceNet model. - Created `FaceRecognitionHelper` to generate face embeddings. - Updated `ProjectEntity` and database schema to store target embeddings. - Enhanced `ProjectViewModel` to support "Track This Person" and smart re-alignment using cosine similarity. - Updated UI to expose the new tracking feature. - Optimized threading (Dispatchers.Default/IO) and memory management (Bitmap recycling, single load per photo) for heavy TFLite operations. - Refactored `FaceDetectorHelper` to support bitmap input for efficiency. Co-authored-by: harrydbarnes <145344818+harrydbarnes@users.noreply.github.com>
This change addresses review feedback by fixing a syntax error, preventing memory leaks, optimizing embedding storage, and improving code readability. - Fixed syntax error in `ProjectViewModel.kt` (extra brace). - Implemented `close()` in `FaceRecognitionHelper` and ensured `GpuDelegate` is closed to prevent memory leaks. - Optimized embedding storage: Changed `targetEmbedding` from String to BLOB (ByteArray) in Entity and Database, using `ByteBuffer` for efficient FloatArray conversion. Updated `MIGRATION_4_5`. - Refactored `ProjectViewModel.kt`: Split `processFacesInternal` into `processFacesWithTarget` and `processFacesSpatial` for better readability and maintainability. - Updated `Project` domain model to use `FloatArray` for embeddings. Co-authored-by: harrydbarnes <145344818+harrydbarnes@users.noreply.github.com>
This update addresses critical review feedback by fixing syntax errors, optimizing performance, and ensuring thread safety. - Fixed `ProjectViewModel` syntax (removed extra brace). - Refactored `FaceRecognitionHelper` to include a thread-safe `suspend fun close()` using `Mutex` for proper cleanup of TFLite resources. - Optimized database storage by switching `targetEmbedding` to `BLOB` (ByteArray) using `ByteBuffer`, replacing inefficient String storage. Updated migrations and mappers. - Parallelized face processing in `ProjectViewModel` using `coroutineScope` and `async/awaitAll` for improved performance on large datasets. - Refactored logic into `processFacesWithTarget` and `processFacesSpatial` for better readability. - Corrected status handling to ensure failed photo loads do not mark photos as processed. Co-authored-by: harrydbarnes <145344818+harrydbarnes@users.noreply.github.com>
This update addresses critical review feedback by fixing syntax errors, optimizing performance, and ensuring thread safety. - Fixed `ProjectViewModel` syntax (removed extra brace). - Refactored `FaceRecognitionHelper` to include a thread-safe `suspend fun close()` using `Mutex` for proper cleanup of TFLite resources. - Optimized database storage by switching `targetEmbedding` to `BLOB` (ByteArray) using `ByteBuffer`, replacing inefficient String storage. Updated migrations and mappers. - Parallelized face processing in `ProjectViewModel` using `coroutineScope` and `async/awaitAll` with a `Semaphore(4)` to improve performance on large datasets while preventing OOM. - Refactored logic into `processFacesWithTarget` and `processFacesSpatial` for better readability. - Corrected status handling to ensure failed photo loads do not mark photos as processed. - Removed `@Singleton` from `FaceRecognitionHelper` and implemented cleanup in `ProjectViewModel.onCleared()`. Co-authored-by: harrydbarnes <145344818+harrydbarnes@users.noreply.github.com>
This update addresses critical review feedback by fixing syntax errors, optimizing performance, and ensuring thread safety. - Fixed `ProjectViewModel` syntax (removed extra brace). - Refactored `FaceRecognitionHelper` to include a thread-safe `suspend fun close()` using `Mutex` for proper cleanup of TFLite resources. - Optimized database storage by switching `targetEmbedding` to `BLOB` (ByteArray) using `ByteBuffer`, replacing inefficient String storage. Updated migrations and mappers. - Parallelized face processing in `ProjectViewModel` using `coroutineScope` and `async/awaitAll` with a `Semaphore(4)` to improve performance on large datasets while preventing OOM. - Refactored logic into `processFacesWithTarget` and `processFacesSpatial` for better readability. - Corrected status handling to ensure failed photo loads do not mark photos as processed. - Removed `@Singleton` from `FaceRecognitionHelper` and implemented cleanup in `ProjectViewModel.onCleared()`. - Replaced `semaphore.withPermit` with `acquire`/`release` to support suspending calls. - Fixed `processFacesSpatial` loop compilation error. Co-authored-by: harrydbarnes <145344818+harrydbarnes@users.noreply.github.com>
This update addresses critical review feedback by fixing syntax errors, optimizing performance, and ensuring thread safety. - Fixed `ProjectViewModel` syntax (removed extra brace). - Refactored `FaceRecognitionHelper` to include a thread-safe `suspend fun close()` using `Mutex` for proper cleanup of TFLite resources. - Optimized database storage by switching `targetEmbedding` to `BLOB` (ByteArray) using `ByteBuffer`, replacing inefficient String storage. Updated migrations and mappers. - Parallelized face processing in `ProjectViewModel` using `coroutineScope` and `async/awaitAll` with a `Semaphore(4)` to improve performance on large datasets while preventing OOM. - Refactored logic into `processFacesWithTarget` and `processFacesSpatial` for better readability. - Corrected status handling to ensure failed photo loads do not mark photos as processed. - Removed `@Singleton` from `FaceRecognitionHelper` and implemented cleanup in `ProjectViewModel.onCleared()`. - Replaced `semaphore.withPermit` with `acquire`/`release` to support suspending calls. - Fixed `processFacesSpatial` loop compilation error. - Fixed initialization race condition in `FaceRecognitionHelper`. - Used `runBlocking` in `onCleared` to ensure TFLite cleanup. - Ensured loading state is reset when project data is missing. Co-authored-by: harrydbarnes <145344818+harrydbarnes@users.noreply.github.com>
…l.kt Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…l.kt Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…l.kt Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…reen.kt Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…er.kt Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
This update fixes the specific build errors reported in the previous run: - Defined `NUM_CPU_THREADS` in `FaceRecognitionHelper.kt` and used it. - Defined `MAX_CONCURRENT_FACE_PROCESSING` in `ProjectViewModel.kt` and used it. - Added `action_re_align_smart` to `strings.xml` and updated `ProjectDetailScreen.kt` to use it. - Verified compilation with `./gradlew assembleDebug`. This builds upon previous optimizations for memory, concurrency, and storage. Co-authored-by: harrydbarnes <145344818+harrydbarnes@users.noreply.github.com>
This update finalizes the Face Recognition feature implementation: - Fixed build errors by defining missing constants (NUM_CPU_THREADS, MAX_CONCURRENT_FACE_PROCESSING) and string resources (action_re_align_smart). - Cleaned up the repository by removing temporary build output files and the debug script count_braces.py. - Updated .gitignore to exclude build output files. - Refactored ProjectViewModel to clean up imports and usage of runBlocking. - Verified compilation with ./gradlew assembleDebug. This completes the optimization for memory, concurrency, and storage efficiency. Co-authored-by: harrydbarnes <145344818+harrydbarnes@users.noreply.github.com>
This update addresses final review comments: - Removed temporary build output files and debug script from version control. - Updated `.gitignore` to prevent build logs from being tracked. - Refactored `ProjectViewModel.onCleared()` to use `runBlocking` for ensuring native resource cleanup without unnecessary full qualification. - Verified that all components are correctly integrated and the project builds successfully. This concludes the optimization and refactoring of the Face Recognition feature. Co-authored-by: harrydbarnes <145344818+harrydbarnes@users.noreply.github.com>
Summary of ChangesHello @harrydbarnes, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the application's face alignment functionality by incorporating on-device face recognition. It enables users to select a specific face to track throughout a project, ensuring that the time-lapse video consistently focuses on that individual. This is achieved through the integration of TensorFlow Lite for generating and comparing face embeddings, providing a more robust and intelligent tracking experience. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a major new feature for identity recognition using TensorFlow Lite, enabling the app to track individuals across photos. While the implementation is generally solid, critical security concerns have been identified: face embeddings are stored unencrypted in the local database, and the image loading process lacks file size limits, potentially leading to disk exhaustion. Furthermore, the ViewModel logic could benefit from simplification to improve clarity and maintainability, specifically by addressing a redundant check and refining state management.
| val aspectRatio: String = Project.DEFAULT_ASPECT_RATIO | ||
| ) | ||
| val aspectRatio: String = Project.DEFAULT_ASPECT_RATIO, | ||
| val targetEmbedding: ByteArray? = null |
There was a problem hiding this comment.
The application stores face embeddings (targetEmbedding) in a local Room database without encryption. Face embeddings are sensitive biometric data that can be used to identify individuals. While the Android application sandbox provides a baseline level of protection, biometric data should ideally be encrypted at rest to prevent unauthorized access on compromised or rooted devices, or in cases where the database might be backed up insecurely.
| async { | ||
| semaphore.acquire() | ||
| try { | ||
| val loaded = imageLoader.loadOptimizedBitmap(Uri.parse(photo.originalUri), 1024, 1024) |
There was a problem hiding this comment.
The processFacesWithTarget function calls imageLoader.loadOptimizedBitmap, which internally copies the photo's content to a temporary file in the app's cache directory using imageLoader.copyToTemp. There is no limit on the size of the file being copied. If a project contains many large files or if a malicious app provides a URI to an extremely large stream, this could exhaust the device's disk space, leading to a Denial of Service (DoS).
| try { | ||
| val loaded = imageLoader.loadOptimizedBitmap(Uri.parse(photo.originalUri), 1024, 1024) | ||
| if (loaded != null) { | ||
| try { | ||
| val embedding = faceRecognitionHelper.getFaceEmbedding(loaded.bitmap, face) | ||
| if (embedding != null) { | ||
| val id = projectId | ||
| if (id == null) { | ||
| _isProcessing.value = false | ||
| return@withContext | ||
| } | ||
| val currentProject = repository.getProject(id) | ||
| if (currentProject == null) { | ||
| _isProcessing.value = false | ||
| return@withContext | ||
| } | ||
| repository.updateProject(currentProject.copy(targetEmbedding = embedding)) | ||
| processFacesInternal() | ||
| } | ||
| } finally { | ||
| if (!loaded.bitmap.isRecycled) loaded.bitmap.recycle() | ||
| } | ||
| } | ||
| } catch (e: Exception) { | ||
| Log.e("ProjectViewModel", "Error setting target person", e) | ||
| } finally { | ||
| _isProcessing.value = false | ||
| } |
There was a problem hiding this comment.
The try-finally block in setTargetPerson that manages the _isProcessing flag also wraps the call to processFacesInternal(), which has its own logic for managing the same flag. This creates redundant state updates and makes the code harder to reason about.
Consider refactoring to separate the two operations. First, set the target person and handle the processing state for that operation. If successful, then call processFacesInternal() to let it run and manage its own processing state. This will make the state management clearer.
For example:
fun setTargetPerson(photo: Photo, face: Face) {
viewModelScope.launch {
_isProcessing.value = true
val success = withContext(Dispatchers.Default) {
// Logic to get embedding and update project, returning true/false
}
if (success) {
processFacesInternal() // Manages its own _isProcessing state
} else {
_isProcessing.value = false // Set to false if first part failed
}
}
}| if (!photo.isProcessed) { | ||
| repository.updatePhoto(photo.copy(isProcessed = false)) | ||
| } |
There was a problem hiding this comment.
The if (!photo.isProcessed) check is redundant. This else block is only reachable if photo.isProcessed is false (due to the check on line 347), so the condition will always be true. You can remove the if statement for cleaner code.
repository.updatePhoto(photo.copy(isProcessed = false))
1 participant
No description provided.