fix(image): grayscale image support for save image advanced#14748
fix(image): grayscale image support for save image advanced#14748yousef-rafat wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe change modifies 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
comfy_extras/nodes_images.py (1)
1105-1106: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMisleading comment: this checks tensor rank, not channel count.
img_tensor.ndim == 2means the tensor has no channel axis at all (a true 2D(H, W)grayscale), not "2 channels." As written, the comment could easily be misread as referring to a 2-channel (e.g., luma+alpha) tensor.✏️ Suggested comment fix
- if img_tensor.ndim == 2: # 2 channel grayscales + if img_tensor.ndim == 2: # (H, W) grayscale with no channel axis img_tensor = img_tensor.unsqueeze(-1)🤖 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/nodes_images.py` around lines 1105 - 1106, The inline comment in the image tensor handling logic is misleading because the `img_tensor.ndim == 2` check in `nodes_images.py` is about tensor rank, not channel count. Update the comment near the `img_tensor.ndim` branch to clearly state that this case handles a 2D grayscale tensor with shape like `(H, W)` and that `unsqueeze(-1)` adds the missing channel axis; keep the wording aligned with the `img_tensor` condition so it cannot be read as referring to 2-channel input.
🤖 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_images.py`:
- Around line 1105-1125: The grayscale handling in the image export path still
passes a 3D array with a singleton channel into av.VideoFrame.from_ndarray,
which PyAV grayscale formats do not accept. Update the code in the grayscale
branch around img_tensor/img_np processing so that when num_channels == 1 you
remove the trailing channel dimension before creating the frame, or avoid adding
it earlier; keep the existing format selection logic in _FORMAT_SPECS and the
frame creation flow aligned with a 2D (H, W) grayscale input.
---
Nitpick comments:
In `@comfy_extras/nodes_images.py`:
- Around line 1105-1106: The inline comment in the image tensor handling logic
is misleading because the `img_tensor.ndim == 2` check in `nodes_images.py` is
about tensor rank, not channel count. Update the comment near the
`img_tensor.ndim` branch to clearly state that this case handles a 2D grayscale
tensor with shape like `(H, W)` and that `unsqueeze(-1)` adds the missing
channel axis; keep the wording aligned with the `img_tensor` condition so it
cannot be read as referring to 2-channel input.
🪄 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 Plus
Run ID: abdb4b42-07bb-4814-b547-cb4037a08a3d
📒 Files selected for processing (1)
comfy_extras/nodes_images.py
Adds a channel in case of 2d channel grayscales.
Changes the grayscale format depending on dtype.
Before and after screenshots: