Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

close image file after loading? #3477

Open
liusida opened this issue May 14, 2024 · 2 comments
Open

close image file after loading? #3477

liusida opened this issue May 14, 2024 · 2 comments

Comments

@liusida
Copy link

liusida commented May 14, 2024

The "Load Image" node opens an image file here:

ComfyUI/nodes.py

Line 1460 in 2de3b69

img = node_helpers.pillow(Image.open, image_path)

and I think we should close it using img.close() before leaving the function.

This will release the handle of the file and allow other processes to access the image file. It's not critical but it feels better to do it, right?

P.S. I noticed this while trying to delete the image file in a later node. But this is not related to the issue since I have created special ImageLoader as well.

@shawnington
Copy link
Contributor

shawnington commented May 14, 2024

The "Load Image" node opens an image file here:

ComfyUI/nodes.py

Line 1460 in 2de3b69

img = node_helpers.pillow(Image.open, image_path)

and I think we should close it using img.close() before leaving the function.

This will release the handle of the file and allow other processes to access the image file. It's not critical but it feels better to do it, right?

P.S. I noticed this while trying to delete the image file in a later node. But this is not related to the issue since I have created special ImageLoader as well.

The problem with this is that the pillow Image.open function doesn't work like the normal with Open(file) as f:

The image isn't actually read until other operations are performed on it.

Also, Image appears to be self closing once it has loaded the data, Image.close() only applies when Image.open() has been called and no operations have been performed on the image per documentation.

https://pillow.readthedocs.io/en/stable/reference/Image.html

"This is a lazy operation; this function identifies the file, but the file remains open and the actual image data is not read from the file until you try to process the data (or call the [load() method]."

Closing the image to my understanding would prevent things like exif_transpose() or ImageSequence.Iterator(img) from working with the image.

So more appropriate if you want to free it from memory after use would be


if len(output_images) > 1 and img.format not in excluded_formats:
       output_image = torch.cat(output_images, dim=0)
       output_mask = torch.cat(output_masks, dim=0)
   else:
       output_image = output_images[0]
       output_mask = output_masks[0]

del img

return (output_image, output_mask)

I suppose the image could be closed at the end of the node but that wouldn't appear to actually do anything, but then it's already been saved to input as soon as you load it into the LoadImage node even before workflow execution.

Also, the return from node_helpers.pillow, is the same as the return from img = Image.open(image_path), so any closing can be done inside the LoadImage node itself. It's just a passthrough helper that applies a try except to set flags if the pil function fails, it accepts any pil function for injection.

@liusida
Copy link
Author

liusida commented May 15, 2024

Nice, del img will work, saving a bit more resources.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants