Kosinkadink / ComfyUI-VideoHelperSuite

Nodes related to video workflows
GNU General Public License v3.0
583 stars 100 forks source link

Remove unnecessary underscore from saved file names #45

Closed kijai closed 1 year ago

kijai commented 1 year ago

Some Windows software seem to have issues loading filenames that end with underscore, and it otherwise seems unnecessary and remnant of something Comfy did in the past...

AustinMroz commented 1 year ago

The underscores have been a nuisance to me as well, but I'm conflicted about the change after seeing that it's required as a separator for the counter.

It seems wrong to desyncronize the name of image and the video, and it would cause things to break if the image file were to be phased out now that metadata is saved in the video itself.

kijai commented 1 year ago

The underscores have been a nuisance to me as well, but I'm conflicted about the change after seeing that it's required as a separator for the counter.

It seems wrong to desyncronize the name of image and the video, and it would cause things to break if the image file were to be phased out now that metadata is saved in the video itself.

Yeah I underestimated the underscore! What if we just replace the counter code with our own?

counter = 1  
        for existing_file in os.listdir(full_output_folder):
            if existing_file.startswith(f"{filename}_") and existing_file.endswith(".png"):
                counter += 1

By quick testing it seems to work, can you anticipate issues with that? I'll keep testing anyway.

AustinMroz commented 1 year ago

Creating our own implementation does seem to be required, but I know this suggested implementation can return the counter of an existing file if any file is deleted. This could occur if someone goes back and prunes an output they don't like, or in my case, I usually output to temp and manually save results I do like, but sometimes forget to switch off the default

I think a more robust implementation would need to take the max of all digits returned by a regex like re.fullmatch(f"{filename}_(\d+)_?\.[a-zA-Z0-9]+", existing_file).group(1), but I'm not completely confident I'm not missing any edge cases