fix: CPU/ONNX face swapper runtime#1717
Open
kuroi17 wants to merge 3 commits intohacksider:mainfrom
Open
Conversation
Contributor
Reviewer's GuideAdjusts the face swapper CPU/ONNX model loading to gracefully handle missing/alternate model files while preserving GPU behavior, and adds one-time user-facing logs for model load and face-detection failures plus safer initialization of the InsightFace face analyser. Sequence diagram for face swapper ONNX model loading with CPU/GPU fallbacksequenceDiagram
actor User
participant App as AppRuntime
participant FaceSwapper as FaceSwapperModule
participant FS as FileSystem
participant HF as HuggingFace
participant IMZ as InsightFaceModelZoo
User->>App: Start swapping
App->>FaceSwapper: pre_check()
FaceSwapper->>FS: _get_existing_swapper_model_paths()
FS-->>FaceSwapper: List of existing model_paths (may be empty)
alt No existing models
FaceSwapper->>HF: conditional_download(inswapper_128_fp16.onnx)
HF-->>FaceSwapper: Download complete
end
FaceSwapper-->>App: pre_check result
App->>FaceSwapper: pre_start()
FaceSwapper->>FS: _get_existing_swapper_model_paths()
FS-->>FaceSwapper: model_paths
alt model_paths empty
FaceSwapper->>App: update_status Failed to load face swapper model: no inswapper model file found.
FaceSwapper-->>App: False
App-->>User: Swap aborted
else model_paths not empty
FaceSwapper-->>App: True
App->>FaceSwapper: get_face_swapper()
FaceSwapper->>FaceSwapper: Check FACE_SWAPPER cache
alt FACE_SWAPPER is None
FaceSwapper->>FaceSwapper: Build providers_config from execution_providers
loop For each model_path
FaceSwapper->>App: update_status Loading face swapper model from: model_path
FaceSwapper->>IMZ: get_model(model_path, providers_config)
alt Load success
IMZ-->>FaceSwapper: FACE_SWAPPER instance
FaceSwapper->>App: update_status Face swapper model loaded successfully.
else Load failure
IMZ-->>FaceSwapper: Exception
FaceSwapper->>FaceSwapper: Remember last_error
FaceSwapper->>FaceSwapper: FACE_SWAPPER = None
end
end
alt FACE_SWAPPER is None after all attempts
FaceSwapper->>App: update_status Failed to load face swapper model with last_error
FaceSwapper-->>App: None
else FACE_SWAPPER loaded
FaceSwapper-->>App: FACE_SWAPPER
App-->>User: Proceed with swapping
end
else FACE_SWAPPER already cached
FaceSwapper-->>App: Cached FACE_SWAPPER
App-->>User: Proceed with swapping
end
end
Sequence diagram for face detection and one-time no-face logging during frame processingsequenceDiagram
actor User
participant App as AppRuntime
participant FaceSwapper as FaceSwapperModule
participant Analyser as FaceAnalyser
User->>App: Process frame
App->>FaceSwapper: process_frame_v2(temp_frame)
FaceSwapper->>FaceSwapper: processed_frame = preprocess_frame(temp_frame)
FaceSwapper->>Analyser: get_many_faces(processed_frame)
Analyser-->>FaceSwapper: detected_faces (may be empty)
alt detected_faces nonempty
FaceSwapper->>FaceSwapper: _reset_no_target_faces_log()
alt modules.globals.many_faces is True
FaceSwapper->>FaceSwapper: source_face = default_source_face()
FaceSwapper->>FaceSwapper: Build source_target_pairs for many faces
else many_faces is False
FaceSwapper->>FaceSwapper: source_face = default_source_face()
FaceSwapper->>FaceSwapper: target_face = get_one_face(processed_frame, detected_faces)
FaceSwapper->>FaceSwapper: Append (source_face, target_face) to pairs
end
else detected_faces empty
FaceSwapper->>FaceSwapper: _log_no_target_faces_once()
FaceSwapper-->>App: Return original frame (no swaps)
App-->>User: Frame with no swaps
end
rect rgb(230,230,255)
Note over FaceSwapper: During swapping
loop For each source_target_pair
FaceSwapper->>FaceSwapper: swap_face(source_face, target_face, frame)
alt source_face is None or target_face is None
FaceSwapper->>FaceSwapper: _log_no_source_face_once() or _log_no_target_faces_once()
FaceSwapper-->>FaceSwapper: Return frame unchanged
else source_face and target_face present
FaceSwapper->>FaceSwapper: _reset_no_source_face_log()
FaceSwapper->>FaceSwapper: _reset_no_target_faces_log()
FaceSwapper-->>FaceSwapper: Return swapped frame
end
end
end
FaceSwapper-->>App: final_frame
App-->>User: Display swapped frame
Sequence diagram for InsightFace face analyser initialization with error handlingsequenceDiagram
actor User
participant App as AppRuntime
participant AnalyserModule as FaceAnalyserModule
participant InsightFace as InsightFaceLibrary
User->>App: Start face processing
App->>AnalyserModule: get_face_analyser()
AnalyserModule->>AnalyserModule: Check FACE_ANALYSER cache
alt FACE_ANALYSER is None
AnalyserModule->>AnalyserModule: Acquire FACE_ANALYSER_LOCK
AnalyserModule->>AnalyserModule: Recheck FACE_ANALYSER is None
alt Still None
AnalyserModule->>InsightFace: app.FaceAnalysis(name=buffalo_l, providers, allowed_modules)
alt Initialization success
InsightFace-->>AnalyserModule: FACE_ANALYSER instance
AnalyserModule->>InsightFace: FACE_ANALYSER.prepare(ctx_id=0, det_size=(640,640))
AnalyserModule-->>App: FACE_ANALYSER
App-->>User: Proceed with detection
else Initialization failure
InsightFace-->>AnalyserModule: Exception e
AnalyserModule->>AnalyserModule: print failure message with e
AnalyserModule->>AnalyserModule: FACE_ANALYSER = None
AnalyserModule-->>App: Raise exception
App-->>User: Show failure to initialize analyser
end
else Already initialized inside lock
AnalyserModule-->>App: Cached FACE_ANALYSER
end
else FACE_ANALYSER already cached
AnalyserModule-->>App: Cached FACE_ANALYSER
end
File-Level Changes
Possibly linked issues
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}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The NO_SOURCE_FACE_LOGGED/NO_TARGET_FACES_LOGGED globals are mutated from multiple code paths and may be accessed across threads; consider guarding them with a lock or scoping them per-session/request to avoid race conditions or cross-stream leakage of log suppression.
- In face_analyser.get_face_analyser, the failure path uses a bare print instead of the existing update_status/logging mechanisms used elsewhere in this PR; aligning this to the same status/logging API would keep error reporting consistent and easier to surface to users.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The NO_SOURCE_FACE_LOGGED/NO_TARGET_FACES_LOGGED globals are mutated from multiple code paths and may be accessed across threads; consider guarding them with a lock or scoping them per-session/request to avoid race conditions or cross-stream leakage of log suppression.
- In face_analyser.get_face_analyser, the failure path uses a bare print instead of the existing update_status/logging mechanisms used elsewhere in this PR; aligning this to the same status/logging API would keep error reporting consistent and easier to surface to users.
## Individual Comments
### Comment 1
<location path="modules/face_analyser.py" line_range="35-42" />
<code_context>
+ allowed_modules=['detection', 'recognition', 'landmark_2d_106']
+ )
+ FACE_ANALYSER.prepare(ctx_id=0, det_size=(640, 640))
+ except Exception as e:
+ print(
+ "[DLC.FACE-ANALYSER] Failed to initialize buffalo_l face analyser. "
+ "Ensure InsightFace model 'buffalo_l' is available. "
+ f"Error: {e}"
+ )
+ FACE_ANALYSER = None
+ raise
return FACE_ANALYSER
</code_context>
<issue_to_address>
**suggestion:** Using `print` for error reporting in the face analyser is inconsistent with the rest of the module’s status/logging mechanisms.
This error path writes directly to stdout/stderr instead of using the existing logging/`update_status` mechanism (as in the face swapper), which breaks consistency and makes integration with centralized logging or UI status updates harder. Please switch the `print` to the same logging/status approach used elsewhere so initialization failures are reported consistently.
Suggested implementation:
```python
except Exception as e:
# Use the module's logging/status mechanism instead of printing directly
try:
# Prefer a centralized status/logging mechanism if available
update_status = getattr(modules.globals, "update_status", None)
if callable(update_status):
update_status(
"face_analyser_init_failed",
(
"[DLC.FACE-ANALYSER] Failed to initialize buffalo_l face analyser. "
"Ensure InsightFace model 'buffalo_l' is available. "
f"Error: {e}"
),
)
else:
# Fallback to a logger if one is exposed in globals
logger = getattr(modules.globals, "logger", None)
if logger is not None:
logger.error(
"[DLC.FACE-ANALYSER] Failed to initialize buffalo_l face analyser. "
"Ensure InsightFace model 'buffalo_l' is available. "
f"Error: {e}"
)
finally:
FACE_ANALYSER = None
raise
```
To fully align with the existing logging/status conventions used elsewhere in the project (especially in the face swapper), you should:
1. Replace the generic `update_status` usage with the exact status function and signature already used for error reporting in the face swapper module (for example, if it is something like `modules.globals.update_status(message, level="error")`, adjust the call accordingly).
2. If the project uses a dedicated logger instance (for example, `modules.globals.LOGGER` or a module-level `logger`), update the fallback branch to call that concrete logger instead of using `modules.globals.logger`.
3. If no `update_status` helper exists and only a logger is used in the rest of this module, you can simplify the `except` block to a single `logger.error(...)` call, making sure a `logger = logging.getLogger(__name__)` is defined at module scope and `import logging` is present at the top of the file.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes the face-swapping feature so that photo and live swaps work correctly on CPU, even if only one ONNX model file is present. It also adds minimal logging to make model/face detection failures visible and easier to debug.
Key Changes
inswapper_128_fp16.onnxif present; otherwise falls back toinswapper_128.onnx.Failed to load face swapper modelNo source face detectedNo target faces detectedbuffalo_lfails to initializeWhy
Previously, the CPU path could fail silently if the downloaded ONNX file didn’t match the expected filename. This ensures swaps work reliably and failures are visible.
Testing Checklist
inswapper_128_fp16.onnx, leaveinswapper_128.onnx, runpython run.py, confirm swaps work.inswapper_128_fp16.onnxexists, run with GPU provider, confirm swaps work.Failed to load face swapper model.No source/target face detected.Branch / Commit
fix/swapper-runtimefe93537Summary by Sourcery
Improve reliability and visibility of face swapping on CPU and across runtimes by relaxing model file requirements and adding targeted error logging.
Bug Fixes:
Enhancements: