Added support for multiple faces #566
Conversation
Reviewer's Guide by SourceryThis pull request adds support for mapping multiple faces in image, video, and webcam targets. The functionality can be enabled by toggling 'Map faces' on the UI or using the '--map-faces' command-line argument. The implementation involves significant changes to the face swapping process, UI, and introduces new clustering and face mapping algorithms. File-Level Changes
Tips
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @pereiraroland26 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟡 Documentation: 1 issue found
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| from typing import Any | ||
|
|
||
|
|
||
| def find_cluster_centroids(embeddings, max_k=10) -> Any: |
There was a problem hiding this comment.
suggestion (performance): Optimize the cluster centroid finding algorithm
The current implementation runs K-means for every K up to max_k, which could be inefficient for large datasets. Consider using a more efficient method for determining the optimal number of clusters, such as the elbow method or silhouette analysis, without running K-means for every K.
from typing import List, Tuple
from sklearn.cluster import KMeans
from sklearn.metrics import silhouette_score
def find_cluster_centroids(embeddings, max_k=10) -> List[Tuple[int, List[float]]]:
best_k = 2
best_score = -1
for k in range(2, max_k + 1):
kmeans = KMeans(n_clusters=k, random_state=42).fit(embeddings)
score = silhouette_score(embeddings, kmeans.labels_)
if score > best_score:
best_score, best_k = score, k
final_kmeans = KMeans(n_clusters=best_k, random_state=42).fit(embeddings)
return list(enumerate(final_kmeans.cluster_centers_.tolist()))
| @@ -65,26 +66,98 @@ def process_frame(source_face: Face, temp_frame: Frame) -> Frame: | |||
| return temp_frame | |||
|
|
|||
|
|
|||
| def process_frame_v2(temp_frame: Frame, temp_frame_path: str = "") -> Frame: | |||
There was a problem hiding this comment.
suggestion: Refactor process_frame_v2 to reduce complexity
This function has a high cyclomatic complexity due to nested conditions. Consider breaking it down into smaller functions for each case (image, video, live) to improve readability and maintainability.
def process_frame_v2(temp_frame: Frame, temp_frame_path: str = "") -> Frame:
return process_frame_based_on_input(temp_frame, temp_frame_path)
def process_frame_based_on_input(frame: Frame, frame_path: str) -> Frame:
if is_image(modules.globals.target_path):
return process_image(frame)
elif is_video(modules.globals.target_path):
return process_video(frame)
else:
return process_live(frame)
| else: | ||
| select_output_path(start) | ||
|
|
||
| def create_source_target_popup(start: Callable[[], None], root: ctk.CTk, map: list) -> None: |
There was a problem hiding this comment.
suggestion: Separate UI logic from business logic in popup creation
This function mixes UI creation with business logic (e.g., face detection). Consider separating these concerns by moving the face detection logic to a separate function or class, leaving this function to handle only UI-related tasks.
def create_source_target_popup(start: Callable[[], None], root: ctk.CTk, map: list) -> None:
ui_handler = UIHandler(root)
face_detector = FaceDetector()
ui_handler.create_popup(start, map, face_detector)
| @@ -146,6 +146,7 @@ options: | |||
| --keep-audio keep original audio | |||
| --keep-frames keep temporary frames | |||
| --many-faces process every face | |||
| --map-faces map source target faces | |||
There was a problem hiding this comment.
suggestion (documentation): Consider clarifying the description of the --map-faces option
The current description 'map source target faces' is a bit ambiguous. Consider rephrasing to 'map source to target faces' for improved clarity while maintaining brevity.
| --map-faces map source target faces | |
| --map-faces map source to target faces |
| @@ -124,6 +151,109 @@ def create_root(start: Callable[[], None], destroy: Callable[[], None]) -> ctk.C | |||
|
|
|||
| return root | |||
|
|
|||
| def analyze_target(start: Callable[[], None], root: ctk.CTk): | |||
There was a problem hiding this comment.
issue (complexity): Consider refactoring the UI code to improve maintainability and readability.
The addition of face mapping functionality has significantly increased the complexity of the UI code. While the feature itself is complex, the current implementation could be improved to enhance maintainability and readability. Consider the following refactoring suggestions:
-
Separate face mapping logic:
Move face mapping-related functions to a separate module to reduce clutter in the UI code.# face_mapping.py def analyze_target(target_path): # Face mapping logic here pass def create_source_target_popup(root, map): # Popup creation logic here pass # ui.py import face_mapping def on_start_button_click(): if modules.globals.map_faces: face_mapping.analyze_target(modules.globals.target_path) else: select_output_path(start)
-
Reduce global state:
Use a class to encapsulate UI state instead of relying on global variables.class UIState: def __init__(self): self.popup = None self.popup_live = None self.source_label_dict = {} self.target_label_dict = {} ui_state = UIState() def create_source_target_popup_for_webcam(root, map): ui_state.popup_live = ctk.CTkToplevel(root) # Rest of the function
-
Simplify nested functions:
Move nested functions to the module level to improve readability and reusability.def on_submit_click(popup): if has_valid_map(): popup.destroy() simplify_maps() create_webcam_preview() else: update_pop_live_status("At least 1 source with target is required!") def create_source_target_popup_for_webcam(root, map): popup = ctk.CTkToplevel(root) # ... other setup code ... close_button = ctk.CTkButton(popup, text="Submit", command=lambda: on_submit_click(popup))
These changes will help maintain the new functionality while improving code organization and reducing complexity. Remember to update all references to global variables and nested functions throughout the code when implementing these changes.
|
This is one of the best commits I've seen so far! Thanks bro! |
|
@pereiraroland26 Hi there bro, do you have a paypal account? can you drop your paypal account here? Thanks! |
Added support for multiple faces
Added support for multiple faces
Sorry, I don't use paypal. Maybe you can send me an amazon gift card? |
|
Will try to send you one this week once I get back from the vacation Thanks for the contribution |
|
Can you send me an email? My address is on my profile. |
Added support for multiple faces
*Added ability to map unique source face to each unique target faces.
*Functionality works for image, video and webcam targets.
*Functionality can be enabled by toggling 'Map faces' on the ui.
Summary by Sourcery
Add functionality to map unique source faces to unique target faces for images, videos, and webcam inputs. This feature can be toggled via the UI and involves new UI components and clustering logic to handle face mapping efficiently.
New Features:
Enhancements:
Documentation: