feat: MediaPipe face detection (CORE-235)#14009
Conversation
kijai
requested review from
Kosinkadink,
alexisrolland,
comfyanonymous,
guill and
rattus128
as code owners
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds a MediaPipe face-landmarker pipeline: a NumPy Procrustes-based 4×4 face transform solver; short- and full-range BlazeFace detectors with anchor decoding and weighted NMS; a FaceMesh model producing 478 canonical 3D landmarks; an MLP‑Mixer FaceBlendshapes regressor producing 52 coefficients; a FaceLandmarker orchestrator for batched detection, ROI warping, and inference; and ComfyUI nodes for model loading, detection with missing-frame fallbacks, mesh visualization, and mask generation. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
comfy_extras/mediapipe/face_landmarker.py (1)
566-571: ⚡ Quick winValidate
detector_variantwith a clear error message.If an invalid
detector_variantis passed,det_clswill beNoneand line 571 will raise an unhelpfulTypeError: 'NoneType' object is not callable. A validation check provides clearer feedback.💡 Proposed improvement
def __init__(self, device=None, dtype=None, operations=None, detector_variant: str = "short"): super().__init__() - det_cls = {"short": BlazeFace, "full": BlazeFaceFullRange}.get(detector_variant) + det_cls_map = {"short": BlazeFace, "full": BlazeFaceFullRange} + if detector_variant not in det_cls_map: + raise ValueError(f"detector_variant must be 'short' or 'full', got {detector_variant!r}") + det_cls = det_cls_map[detector_variant] self.detector_variant = detector_variant self.detector = det_cls(device=device, dtype=dtype, operations=operations)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@comfy_extras/mediapipe/face_landmarker.py` around lines 566 - 571, The __init__ currently maps detector_variant to det_cls and directly instantiates it, which yields a confusing TypeError if detector_variant is invalid; add an explicit validation after det_cls assignment in face_landmarker.__init__ that checks if det_cls is None and raises a ValueError with a clear message like "Invalid detector_variant: {detector_variant}; expected 'short' or 'full'". Reference the existing symbols det_cls, detector_variant, BlazeFace, BlazeFaceFullRange and perform the check before calling det_cls(...) so self.detector is only created when a valid class is selected.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@comfy_extras/nodes_mediapipe.py`:
- Around line 377-386: The code sets B from img_np.shape[0] when image is
provided but then indexes frames[bi], causing IndexError if the two batch sizes
differ; update the logic in the mesh-drawing block (the variables image,
face_landmarks, frames, B, img_np, _image_to_uint8, and _draw_mesh) to ensure
both arrays are the same length before looping: either validate and raise a
clear error when img_np.shape[0] != len(frames), or compute B =
min(img_np.shape[0], len(frames)) and iterate only up to B (ensuring out is
sized accordingly) so you never index past frames. Ensure the chosen fix adjusts
out shape and progress bar (comfy.utils.ProgressBar) consistently.
---
Nitpick comments:
In `@comfy_extras/mediapipe/face_landmarker.py`:
- Around line 566-571: The __init__ currently maps detector_variant to det_cls
and directly instantiates it, which yields a confusing TypeError if
detector_variant is invalid; add an explicit validation after det_cls assignment
in face_landmarker.__init__ that checks if det_cls is None and raises a
ValueError with a clear message like "Invalid detector_variant:
{detector_variant}; expected 'short' or 'full'". Reference the existing symbols
det_cls, detector_variant, BlazeFace, BlazeFaceFullRange and perform the check
before calling det_cls(...) so self.detector is only created when a valid class
is selected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 968cf85a-d162-4cca-9715-248398a7b40a
📒 Files selected for processing (5)
comfy_extras/mediapipe/face_geometry.pycomfy_extras/mediapipe/face_landmarker.pycomfy_extras/nodes_mediapipe.pyfolder_paths.pynodes.py
|
can MediaPipe face detection detect gender? or any other nodes that can do |
You can probably use Gemma 4 to detect gender in an image. It is natively supported in ComfyUI. You could also combine it with SAM3 for segmenting / tracking / extracting faces. |
* Initial mediapipe face detection support * Update face_geometry.py * Account for diff sized batch input * Model folder placeholder
4 participants
This PR adds dependency free re-write of Google's MediaPipe's Face detection and land marker.
mediapipe_example.json
Model has been extracted as a .safetensors:
https://huggingface.co/Comfy-Org/mediapipe/blob/main/mediapipe/mediapipe_face_fp32.safetensors