Add macOS/Windows setup scripts and update modules for enhanced funct… #1323
Conversation
…ionality - Add clone_or_update scripts for cross-platform repo management - Introduce exclude.txt to prevent syncing .git, models, and binary files - Add install and run scripts for macOS/Windows environments - Improve error handling and add docstrings in utilities.py - Enhance robustness in video processing functions - Update core modules (face_analyser, globals, ui, etc.) for consistency The changes implement cross-platform setup automation while improving code quality through better error handling, documentation, and synchronization control. Key modules and scripts were updated to ensure stable execution across different operating systems.
Reviewer's GuideImplements cross-platform setup automation via new shell and batch scripts (with exclusion control) while enhancing core modules (UI, face processing, utilities, globals) with consistent docstrings, error handling, and robust cross-OS logic for stable execution. Sequence Diagram for Toggling Virtual CamerasequenceDiagram
actor User
participant UI_Button as "UI Button"
participant toggle_virtual_cam_func as "toggle_virtual_cam()"
participant vcm as "virtual_cam_manager (VirtualCamManager)"
participant StatusUpdate as "update_status()"
User->>UI_Button: Clicks "Toggle Virtual Cam"
UI_Button->>toggle_virtual_cam_func: Invoke
toggle_virtual_cam_func->>toggle_virtual_cam_func: Check PYVIRTUALCAM_AVAILABLE
alt pyvirtualcam not installed
toggle_virtual_cam_func->>StatusUpdate: "pyvirtualcam not installed..."
else pyvirtualcam available
toggle_virtual_cam_func->>toggle_virtual_cam_func: Get virtual_cam_enabled state
alt Virtual cam is currently disabled
toggle_virtual_cam_func->>vcm: start(PREVIEW_DEFAULT_WIDTH, PREVIEW_DEFAULT_HEIGHT, 30)
vcm-->>toggle_virtual_cam_func: Started successfully
toggle_virtual_cam_func->>toggle_virtual_cam_func: Set virtual_cam_enabled = True
toggle_virtual_cam_func->>StatusUpdate: "Virtual camera enabled."
else Virtual cam is currently enabled
toggle_virtual_cam_func->>vcm: stop()
vcm-->>toggle_virtual_cam_func: Stopped successfully
toggle_virtual_cam_func->>toggle_virtual_cam_func: Set virtual_cam_enabled = False
toggle_virtual_cam_func->>StatusUpdate: "Virtual camera disabled."
end
end
Sequence Diagram for Webcam Frame Processing and Virtual Camera OutputsequenceDiagram
participant WebcamPreviewLoop as "create_webcam_preview() loop"
participant VideoCapture as "cap.read()"
participant FrameProcessors
participant vcm as "virtual_cam_manager (VirtualCamManager)"
participant UIDisplay as "UI Image Display"
loop Each Frame in Webcam Preview
WebcamPreviewLoop->>VideoCapture: Read new frame
VideoCapture-->>WebcamPreviewLoop: Frame data
alt Frame successfully read
WebcamPreviewLoop->>WebcamPreviewLoop: Copy frame for processing
WebcamPreviewLoop->>FrameProcessors: Process frame (face swap, enhance, etc.)
FrameProcessors-->>WebcamPreviewLoop: Processed frame
alt Virtual camera enabled AND face_swap_enabled
WebcamPreviewLoop->>vcm: send(processed_frame)
end
WebcamPreviewLoop->>UIDisplay: Update preview with processed_frame
else Frame read failed
WebcamPreviewLoop->>WebcamPreviewLoop: Break loop
end
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @rehanbgmi - I've reviewed your changes - here's some feedback:
Blocking issues:
- Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'. (link)
General comments:
- Consider extracting the VirtualCamManager, toggle_virtual_cam, and related globals out of ui.py into a dedicated virtual_cam module to reduce global state and keep UI code focused.
- The application metadata in modules/metadata.py was changed to name='Chrome', version='1.0.0', edition=''; please verify those overrides are intentional.
- There’s an inconsistent mix of print statements and logging calls for error reporting—standardize on your logging framework to improve maintainability.
Here's what I looked at during the review
- 🟡 General issues: 10 issues found
- 🔴 Security: 1 blocking issue
- 🟢 Testing: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @@ -797,75 +813,61 @@ def webcam_preview(root: ctk.CTk, camera_index: int): | |||
| ) | |||
|
|
|||
|
|
|||
| virtual_cam_manager = VirtualCamManager() | |||
| virtual_cam_enabled = False # Use a global variable for clarity | |||
There was a problem hiding this comment.
suggestion: Avoid module-global virtual_cam_enabled state
Encapsulate the enabled state within VirtualCamManager or a controller class to prevent global namespace pollution and reduce concurrency risks.
Suggested implementation:
virtual_cam_manager = VirtualCamManager()You must update the VirtualCamManager class (likely in another file) to add an enabled attribute (e.g., self.enabled = False in __init__).
Replace all references to virtual_cam_enabled in this file and elsewhere with virtual_cam_manager.enabled or, better, with methods like virtual_cam_manager.is_enabled() and virtual_cam_manager.set_enabled(value) for encapsulation.
| def send(self, frame): | ||
| if self.cam and self.enabled: | ||
| try: | ||
| # pyvirtualcam expects RGB |
There was a problem hiding this comment.
issue (bug_risk): Convert frames from BGR to RGB before sending
Add a color conversion step (e.g., cv2.cvtColor(frame, cv2.COLOR_BGR2RGB)) before sending the frame to ensure correct color representation.
| try: | ||
| self.cam = pyvirtualcam.Camera(width=width, height=height, fps=fps, print_fps=False) | ||
| self.enabled = True | ||
| print("Virtual camera started.") |
There was a problem hiding this comment.
suggestion: Use update_status or logging instead of print in start()
Printing directly to the console can lead to inconsistent notifications. Use update_status or a logging framework to ensure consistent UI updates.
Suggested implementation:
self.update_status("Virtual camera started.")
except Exception as e:
self.update_status(f"Failed to start virtual camera: {e}")
self.cam = None
self.enabled = FalseIf update_status is not already defined in this class or inherited from a parent, you will need to implement it or ensure it is available. If you prefer to use a logging framework instead, replace self.update_status(...) with appropriate logging calls (e.g., logger.info(...) or logger.error(...)).
|
|
||
|
|
||
| def find_cluster_centroids(embeddings, max_k=10) -> Any: | ||
| def find_cluster_centroids(embeddings: List[Any], max_k: int = 10) -> Any: |
There was a problem hiding this comment.
issue: Handle empty embeddings list in find_cluster_centroids
Add a check to return an empty list if embeddings is empty to prevent indexing errors in cluster_centroids.
| try: | ||
| os.rename(temp_output_path, output_path) | ||
| except Exception as e: | ||
| print(f"Error moving temp output: {e}") |
There was a problem hiding this comment.
suggestion (bug_risk): Use shutil.move instead of os.rename for moving temp output
os.rename can fail across file systems, while shutil.move handles these cases reliably.
| try: | |
| os.rename(temp_output_path, output_path) | |
| except Exception as e: | |
| print(f"Error moving temp output: {e}") | |
| try: | |
| shutil.move(temp_output_path, output_path) | |
| except Exception as e: | |
| print(f"Error moving temp output: {e}") |
| for i in range(len(centroids)): | ||
| pass # Implement as needed |
There was a problem hiding this comment.
suggestion (code-quality): Remove nested block which has no effect (remove-empty-nested-block)
| temp_directory_path = get_temp_directory_path(modules.globals.target_path) | |
| for i in range(len(centroids)): | |
| pass # Implement as needed | |
| temp_directory_path = get_temp_directory_path(modules.globals.target_path) |
| @@ -119,8 +124,8 @@ 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: | |||
| def process_frame_v2(temp_frame: Any, temp_frame_path: str = "") -> Any: | |||
There was a problem hiding this comment.
issue (code-quality): We've found these issues:
- Remove redundant conditional [×3] (
remove-redundant-if) - Use named expression to simplify assignment and conditional (
use-named-expression) - Hoist nested repeated code outside conditional statements (
hoist-similar-statement-from-if) - Low code quality found in process_frame_v2 - 13% (
low-code-quality)
Explanation
The quality score for this function is below the quality threshold of 25%.
This score is a combination of the method length, cognitive complexity and working memory.
How can you solve this?
It might be worth refactoring this function to make it shorter and more readable.
- Reduce the function length by extracting pieces of functionality out into
their own functions. This is the most important thing you can do - ideally a
function should be less than 10 lines. - Reduce nesting, perhaps by introducing guard clauses to return early.
- Ensure that variables are tightly scoped, so that code using related concepts
sits together within the function rather than being scattered.
| target_frame = cv2.imread(target_path) | ||
| result = process_frame(source_face, target_frame) | ||
| if np.array_equal(result, target_frame): | ||
| logging.warning("No face detected in target image. Skipping output.") | ||
| return False | ||
| cv2.imwrite(output_path, result) |
There was a problem hiding this comment.
issue (code-quality): Hoist repeated code outside conditional statement [×2] (hoist-statement-from-if)
| else: | ||
| # Face swap disabled, just show the frame | ||
| pass | ||
|
|
There was a problem hiding this comment.
suggestion (code-quality): Remove redundant pass statement (remove-redundant-pass)
| else: | |
| # Face swap disabled, just show the frame | |
| pass |
| devices = self.graph.get_input_devices() | ||
| if self.device_index >= len(devices): | ||
| raise ValueError( | ||
| f"Invalid device index {device_index}. Available devices: {len(devices)}" | ||
| ) | ||
|
|
||
| def start(self, width: int = 960, height: int = 540, fps: int = 60) -> bool: | ||
| """Initialize and start video capture""" | ||
| """Initialize and start video capture.""" | ||
| try: | ||
| if platform.system() == "Windows": |
There was a problem hiding this comment.
issue (code-quality): Extract code out into method (extract-method)
…ionality
The changes implement cross-platform setup automation while improving code quality through better error handling, documentation, and synchronization control. Key modules and scripts were updated to ensure stable execution across different operating systems.
Summary by Sourcery
Add cross-platform setup automation and improve core functionality by introducing new setup scripts, virtual camera and preview toggles, and strengthening code quality with enhanced error handling, documentation, and processing robustness across all modules.
New Features:
Enhancements:
Chores: