Feature identity recognition 2100793612207388938#156
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>
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 application's face alignment capabilities by integrating TensorFlow Lite for advanced face recognition. Users can now select a specific face to track across all photos in a project, ensuring more accurate and consistent alignment for time-lapse videos. This moves beyond simple spatial tracking to incorporate identity-based recognition, providing a more intelligent and user-controlled face processing workflow. 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
|
There was a problem hiding this comment.
Code Review
This pull request introduces a significant new feature for identity recognition, allowing the app to track a specific person across multiple photos using face embeddings. The changes include adding TensorFlow Lite dependencies, a new FaceRecognitionHelper for model inference, and updating the data models, database, and UI accordingly. The implementation of the new face tracking logic is robust, particularly the concurrent processing in processFacesWithTarget. My review includes a few suggestions to address a potential ANR risk, improve performance in the legacy face alignment logic, and clean up some temporary files from the commit.
| kotlinx.coroutines.runBlocking { | ||
| faceRecognitionHelper.close() | ||
| } |
There was a problem hiding this comment.
Using runBlocking in onCleared can block the main thread, potentially causing an Application Not Responding (ANR) error. This can happen if faceRecognitionHelper.close() needs to wait for a lock that is held by another long-running operation. While resource cleanup is important, blocking the main thread should be avoided. Consider alternative, non-blocking approaches for resource cleanup that are not tied to the ViewModel's lifecycle or ensure that the close() operation is guaranteed to be fast and non-blocking.
| fun calculateCosineSimilarity(embed1: FloatArray, embed2: FloatArray): Float { | ||
| if (embed1.size != embed2.size) return 0f | ||
| var dot = 0f | ||
| var mag1 = 0f | ||
| var mag2 = 0f | ||
| for (i in embed1.indices) { | ||
| dot += embed1[i] * embed2[i] | ||
| mag1 += embed1[i] * embed1[i] | ||
| mag2 += embed2[i] * embed2[i] | ||
| } | ||
| val mag = (sqrt(mag1) * sqrt(mag2)) | ||
| return if (mag > 0) dot / mag else 0f | ||
| } |
There was a problem hiding this comment.
The implementation of calculateCosineSimilarity is correct, but it can be written more idiomatically using Kotlin's collection processing functions. This would make the code more concise and potentially more readable.
fun calculateCosineSimilarity(embed1: FloatArray, embed2: FloatArray): Float {
if (embed1.size != embed2.size) return 0f
val dotProduct = embed1.zip(embed2) { a, b -> a * b }.sum()
val mag1 = sqrt(embed1.map { it * it }.sum())
val mag2 = sqrt(embed2.map { it * it }.sum())
val mag = mag1 * mag2
return if (mag > 0f) dotProduct / mag else 0f
}| private suspend fun processFacesSpatial(photos: List<Photo>) { | ||
| var previousFaceCenter: PointF? = null | ||
|
|
||
| for (photo in photos) { | ||
| val result = faceDetectorHelper.detectFaces(Uri.parse(photo.originalUri)) | ||
| val faces = result.faces | ||
| val width = result.width | ||
| val height = result.height | ||
|
|
||
| previousFaceCenter = calculateNormalizedCenter( | ||
| bestFace.boundingBox.left.toFloat(), | ||
| bestFace.boundingBox.top.toFloat(), | ||
| bestFace.boundingBox.width().toFloat(), | ||
| bestFace.boundingBox.height().toFloat(), | ||
| if (width == 0 || height == 0) { | ||
| repository.updatePhoto(photo.copy(isProcessed = false)) | ||
| continue | ||
| } | ||
| if (photo.isProcessed) { | ||
| val fx = photo.faceX | ||
| val fy = photo.faceY | ||
| val fw = photo.faceWidth | ||
| val fh = photo.faceHeight | ||
|
|
||
| previousFaceCenter = if (fx != null && fy != null && fw != null && fh != null) { | ||
| calculateNormalizedCenter(fx, fy, fw, fh, width, height) | ||
| } else { | ||
| null | ||
| } | ||
| } else { | ||
| val bestFace = if (previousFaceCenter == null) { | ||
| faces.maxByOrNull { it.boundingBox.width() * it.boundingBox.height() } | ||
| } else { | ||
| val prevCenter = checkNotNull(previousFaceCenter) | ||
| faces.minByOrNull { face -> | ||
| val center = calculateNormalizedCenter( | ||
| face.boundingBox.left.toFloat(), | ||
| face.boundingBox.top.toFloat(), | ||
| face.boundingBox.width().toFloat(), | ||
| face.boundingBox.height().toFloat(), | ||
| width, | ||
| height | ||
| ) | ||
| } else { | ||
| repository.updatePhoto(photo.copy(isProcessed = true)) | ||
| hypot(center.x - prevCenter.x, center.y - prevCenter.y) | ||
| } | ||
| } | ||
|
|
||
| if (bestFace != null) { | ||
| val updatedPhoto = photo.copy( | ||
| isProcessed = true, | ||
| faceX = bestFace.boundingBox.left.toFloat(), | ||
| faceY = bestFace.boundingBox.top.toFloat(), | ||
| faceWidth = bestFace.boundingBox.width().toFloat(), | ||
| faceHeight = bestFace.boundingBox.height().toFloat() | ||
| ) | ||
| repository.updatePhoto(updatedPhoto) | ||
|
|
||
| previousFaceCenter = calculateNormalizedCenter( | ||
| bestFace.boundingBox.left.toFloat(), | ||
| bestFace.boundingBox.top.toFloat(), | ||
| bestFace.boundingBox.width().toFloat(), | ||
| bestFace.boundingBox.height().toFloat(), | ||
| width, | ||
| height | ||
| ) | ||
| } else { | ||
| if (!photo.isProcessed) { | ||
| repository.updatePhoto(photo.copy(isProcessed = false)) | ||
| } | ||
| } | ||
| } | ||
| _isProcessing.value = false | ||
| } | ||
| } |
There was a problem hiding this comment.
The processFacesSpatial function processes photos sequentially, which can be inefficient for projects with many photos. For better performance and consistency with the new processFacesWithTarget function, consider refactoring this to process photos concurrently. You could use a similar pattern with a Semaphore and async/awaitAll to parallelize the work while limiting resource consumption.
| with open('app/src/main/java/com/facelapse/app/ui/project/ProjectViewModel.kt', 'r') as f: | ||
| content = f.read() | ||
|
|
||
| count = 0 | ||
| for i, char in enumerate(content): | ||
| if char == '{': | ||
| count += 1 | ||
| elif char == '}': | ||
| count -= 1 | ||
|
|
||
| if count < 0: | ||
| print(f"Extra closing brace at char {i}") | ||
|
|
||
| if count > 0: | ||
| print(f"Missing {count} closing braces") | ||
| elif count == 0: | ||
| print("Braces are balanced") |
There was a problem hiding this comment.
This file, along with build_output_2.txt, build_output_3.txt, and build_output_final.txt, appears to be a temporary script or build output. Such files should not be committed to the version control system. Please remove them from this pull request and add their patterns (e.g., *.py, build_output_*.txt) to your project's .gitignore file to prevent them from being accidentally committed in the future.
No description provided.