Feature identity recognition 2100793612207388938#155
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>
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 enhances the face alignment feature by introducing identity recognition capabilities. By leveraging TensorFlow Lite, users can now select a specific face to track, ensuring that the alignment process focuses on that individual throughout a project. This provides a more accurate and user-controlled method for generating time-lapse videos, moving beyond simple spatial face detection to intelligent person tracking. 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 identity recognition for face tracking, leveraging TensorFlow Lite for on-device inference and a new FaceRecognitionHelper to generate embeddings, alongside updates to the database schema and view model logic. While the changes are well-structured, there are critical security and stability concerns: the unencrypted storage of biometric data (face embeddings) and a potential app crash due to improper bitmap recycling logic. Additionally, a logic issue exists in the updated spatial processing, and unnecessary files are included in the pull request.
| 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 (biometric data) in the targetEmbedding field, which is persisted in a local Room database. Biometric templates are sensitive personal data. Storing them unencrypted poses a privacy risk if the device's storage is accessed by an unauthorized party. Consider using an encrypted database like SQLCipher or encrypting this specific field before storage.
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential app crash due to improper bitmap recycling. Bitmap.createBitmap may return the source bitmap instance if the requested subset matches the source dimensions. In such cases, calling cropped.recycle() will also recycle the source bitmap, leading to an IllegalStateException when the source bitmap is accessed again (e.g., in the processing loop in ProjectViewModel).
if (cropped != null && cropped != bitmap && !cropped.isRecycled) {
cropped.recycle()
}| @@ -0,0 +1 @@ | |||
| dummy model No newline at end of file | |||
There was a problem hiding this comment.
This appears to be a dummy placeholder file for the TFLite model. Committing a text file with the content "dummy model" instead of a binary .tflite file can cause issues with asset packaging and loading at runtime. It's recommended to either remove this placeholder and add the real model in a subsequent PR, or replace it with a minimal but valid TFLite model file. If this file is for local testing only, it should be added to .gitignore.
| if (!photo.isProcessed) { | ||
| repository.updatePhoto(photo.copy(isProcessed = false)) | ||
| } |
There was a problem hiding this comment.
The logic for handling cases where no face is found in processFacesSpatial seems inconsistent and inefficient.
- If
photo.isProcessedisfalse, this code results in a redundant database write, updating the photo withisProcessed = falsewhen that's already its state. - If
photo.isProcessedistrue(e.g., from a manual selection), this block does nothing, leaving the photo marked as processed even though the algorithm didn't find a face.
This is inconsistent with processFacesWithTarget, which correctly marks the photo as unprocessed in this scenario. To ensure consistent behavior and avoid unnecessary database operations, it would be better to unconditionally set isProcessed to false when no face is found.
repository.updatePhoto(photo.copy(isProcessed = false))|
|
||
| override fun onCleared() { | ||
| super.onCleared() | ||
| kotlinx.coroutines.runBlocking { |
There was a problem hiding this comment.
Using runBlocking in onCleared is acceptable for running suspend functions during ViewModel cleanup. However, using the fully qualified name kotlinx.coroutines.runBlocking is a bit verbose. It would be cleaner to add import kotlinx.coroutines.runBlocking at the top of the file.
runBlocking {| @@ -0,0 +1,51 @@ | |||
| Configuration on demand is an incubating feature. | |||
There was a problem hiding this comment.
Build output files should not be committed to the version control system. These files are generated locally and can vary between machines and builds, leading to unnecessary merge conflicts and repository bloat. Please remove this file from the pull request and add *.txt (or a more specific pattern like build_output*.txt) to your .gitignore file.
| @@ -0,0 +1,51 @@ | |||
| Configuration on demand is an incubating feature. | |||
There was a problem hiding this comment.
Build output files should not be committed to the version control system. These files are generated locally and can vary between machines and builds, leading to unnecessary merge conflicts and repository bloat. Please remove this file from the pull request and ensure your .gitignore file is configured to exclude them.
| @@ -0,0 +1,51 @@ | |||
| Configuration on demand is an incubating feature. | |||
There was a problem hiding this comment.
Build output files should not be committed to the version control system. These files are generated locally and can vary between machines and builds, leading to unnecessary merge conflicts and repository bloat. Please remove this file from the pull request and ensure your .gitignore file is configured to exclude them.
| @@ -0,0 +1,51 @@ | |||
| Configuration on demand is an incubating feature. | |||
There was a problem hiding this comment.
Build output files should not be committed to the version control system. These files are generated locally and can vary between machines and builds, leading to unnecessary merge conflicts and repository bloat. Please remove this file from the pull request and ensure your .gitignore file is configured to exclude them.
| @@ -0,0 +1,17 @@ | |||
| with open('app/src/main/java/com/facelapse/app/ui/project/ProjectViewModel.kt', 'r') as f: | |||
There was a problem hiding this comment.
This Python script appears to be a temporary debugging or utility tool. Such files should generally not be part of the main application's source history. If it's a one-off script, please remove it. If it's a tool that other developers might find useful, consider moving it to a dedicated scripts/ or tools/ directory.
1 participant
No description provided.