Use pyav to load images instead of pillow (CORE-95)#13594
Conversation
On failure (ex: animated webp files) fallback to old pillow code.
📝 WalkthroughWalkthroughThe 🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nodes.py`:
- Around line 1719-1722: Wrap the call to
InputImpl.VideoFromFile(image_path).get_components() in a try/except so failures
from VideoFromFile.get_components() (e.g., pyav errors on animated WebP) are
caught and allow the code path to continue to the existing Pillow-based loader;
specifically, try to assign components =
InputImpl.VideoFromFile(image_path).get_components() and on exception (catch
broad Exception as e) skip returning the VideoFromFile result so the subsequent
Pillow fallback executes, optionally logging the caught exception for debug
context while keeping behavior unchanged when components is valid.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| components = InputImpl.VideoFromFile(image_path).get_components() | ||
| if components.images.shape[0] > 0: | ||
| return (components.images, 1.0 - components.alpha[..., -1] if components.alpha is not None else torch.zeros((components.images.shape[0], 64, 64), dtype=torch.float32, device="cpu")) | ||
|
|
There was a problem hiding this comment.
Missing exception handling prevents fallback to Pillow.
The PR description states that on failure (e.g., animated WebP files) the code should fall back to Pillow-based loading. However, there's no try-except block here. If VideoFromFile.get_components() raises an exception (such as when pyav cannot parse a file), the method will crash instead of falling back to the PIL code path below.
🐛 Proposed fix to add exception handling for fallback
- components = InputImpl.VideoFromFile(image_path).get_components()
- if components.images.shape[0] > 0:
- return (components.images, 1.0 - components.alpha[..., -1] if components.alpha is not None else torch.zeros((components.images.shape[0], 64, 64), dtype=torch.float32, device="cpu"))
-
+ try:
+ components = InputImpl.VideoFromFile(image_path).get_components()
+ if components.images.shape[0] > 0:
+ return (components.images, 1.0 - components.alpha[..., -1] if components.alpha is not None else torch.zeros((components.images.shape[0], 64, 64), dtype=torch.float32, device="cpu"))
+ except Exception:
+ pass
+
img = node_helpers.pillow(Image.open, image_path)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| components = InputImpl.VideoFromFile(image_path).get_components() | |
| if components.images.shape[0] > 0: | |
| return (components.images, 1.0 - components.alpha[..., -1] if components.alpha is not None else torch.zeros((components.images.shape[0], 64, 64), dtype=torch.float32, device="cpu")) | |
| try: | |
| components = InputImpl.VideoFromFile(image_path).get_components() | |
| if components.images.shape[0] > 0: | |
| return (components.images, 1.0 - components.alpha[..., -1] if components.alpha is not None else torch.zeros((components.images.shape[0], 64, 64), dtype=torch.float32, device="cpu")) | |
| except Exception: | |
| pass | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nodes.py` around lines 1719 - 1722, Wrap the call to
InputImpl.VideoFromFile(image_path).get_components() in a try/except so failures
from VideoFromFile.get_components() (e.g., pyav errors on animated WebP) are
caught and allow the code path to continue to the existing Pillow-based loader;
specifically, try to assign components =
InputImpl.VideoFromFile(image_path).get_components() and on exception (catch
broad Exception as e) skip returning the VideoFromFile result so the subsequent
Pillow fallback executes, optionally logging the caught exception for debug
context while keeping behavior unchanged when components is valid.
There was a problem hiding this comment.
Real-world reproduction of the missing exception handling:
I can confirm this issue. After updating to commit dae3d34, loading a large static JPEG image (13841×18537 px) in the Load Image node throws the following error instead of falling back to Pillow:
text
av.error.MemoryError: [Errno 12] Cannot allocate memory
File "nodes.py", line 1719, in load_image
components = InputImpl.VideoFromFile(image_path).get_components()
File "video_types.py", line 293, in get_components_internal
img = frame.to_ndarray(format=image_format)
File "av/video/reformatter.pyx", line 207, in _reformat
new_frame._init(dst_format, width, height)
The av.error.MemoryError is not caught, so the fallback to Pillow never triggers. This worked correctly on v0.19.5 (commit c88cf44) which used Pillow directly.
Suggested fix: wrap VideoFromFile.get_components() in a try-except that also catches av.error.MemoryError (and ideally MemoryError) to trigger the Pillow fallback path.
There was a problem hiding this comment.
@laolaoruqin, thank you for the real-world reproduction — that's a very clear and useful confirmation of the bug. The av.error.MemoryError from PyAV is indeed a subclass of Exception, so the broad except Exception in my original suggestion would catch it. Python's built-in MemoryError is also a subclass of Exception in Python 3, so both are covered.
Here's a slightly improved version of the fix that also adds a debug-level log to preserve observability:
try:
components = InputImpl.VideoFromFile(image_path).get_components()
if components.images.shape[0] > 0:
return (components.images, 1.0 - components.alpha[..., -1] if components.alpha is not None else torch.zeros((components.images.shape[0], 64, 64), dtype=torch.float32, device="cpu"))
except Exception as e:
logging.debug(f"pyav failed to load image, falling back to Pillow: {e}")
img = node_helpers.pillow(Image.open, image_path)This ensures:
av.error.MemoryError(large JPEG OOM case you hit) → caught, falls back to Pillow ✅- Python's built-in
MemoryError→ also a subclass ofException, caught ✅ - Any other pyav parse/decode errors (e.g., animated WebP) → caught ✅
- A debug log is emitted so developers can diagnose fallback triggers without silently swallowing errors ✅
🧠 Learnings used
Learnt from: pythongosssss
Repo: Comfy-Org/ComfyUI PR: 12555
File: comfy_extras/nodes_glsl.py:719-724
Timestamp: 2026-02-21T14:01:41.482Z
Learning: In PyOpenGL, bare Python scalars can be accepted for 1-element array parameters by NumberHandler. This means you can pass an int/float directly to OpenGL texture deletion (e.g., glDeleteTextures(tex)) without wrapping in a list. Verify function-specific expectations and ensure types match what the OpenGL call expects; use explicit lists only when the API requires an array.
On failure (ex: animated webp files) fallback to old pillow code.
This should fix the extra precision in high bit depth images (like 16 bit PNG) being discarded and add support for more image formats.