comfyanonymous / ComfyUI

The most powerful and modular stable diffusion GUI, api and backend with a graph/nodes interface.
GNU General Public License v3.0
41.22k stars 4.38k forks source link

close image file after loading? #3477

Open liusida opened 1 month ago

liusida commented 1 month ago

The "Load Image" node opens an image file here: https://github.com/comfyanonymous/ComfyUI/blob/2de3b69b3074028fd0f850f801f4ca023489c692/nodes.py#L1460

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 commented 1 month ago

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

https://github.com/comfyanonymous/ComfyUI/blob/2de3b69b3074028fd0f850f801f4ca023489c692/nodes.py#L1460

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 commented 1 month ago

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