Kosinkadink / ComfyUI-VideoHelperSuite

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

[Bug] VideoCombine.VALIDATE_INPUTS() got an unexpected keyword argument 'frame_rate' #114

Closed ricardofeynman closed 8 months ago

ricardofeynman commented 8 months ago

I'm fully updated, but am getting error messages like the one in the title in some workflows, and this one in others:

Failed to validate prompt for output 8:

I've deleted, and reinstalled ComfyUI-VideoHelperSuite from the manager; deleted the VideoCombine node and manually replaced it in the workflows; removed all custom nodes installed/updated in the last three days to see if any may have been conflicting. Unfortunately the issue persists.

There's been other issues over the last few days which I couldn't track down the cause of with the VideoCombine node, but it's at least throwing some error messages this time that might be helpful to report. Any idea what might be happening in this instance?

Thanks.

ricardofeynman commented 8 months ago

After updating to [36fea9f] commit 243, the error from my comment above has gone, and I am currently getting the following:

Failed to validate prompt for output 8:

AustinMroz commented 8 months ago

The first message appears to be particularly unhelpful error message. I pushed an update to fix what I thought the underlying issue was, but it takes some leaps of logic to actually link it to the error message.

full_type_name only exists as a helper function in ComfyUI's execution.py and is only called when an error has already occurred to try and print information in a more legible format. If execution.py were somehow edited or changed to remove this function and a node produces an error, then a new error is produced. Since validation is done recursively, this means the error would be mistakenly attributed a dependent node as well.

I'm still investigating the new issue. Are you doing anything fancy with converting inputs on Load Video? There's code about 3 lines before that error is produced that checks that the path is not None (which is needed for some delayed execution nodes) so I'm thinking the filepath recieved must somehow be the string "None"

ricardofeynman commented 8 months ago

The workflow I was just testing with (Workflow 2 - embedded in the image):

https://github.com/RenderRift/ComfyUI-RenderRiftNodes

ricardofeynman commented 8 months ago

I've just double checked again by booting up the server and running the workflow Immediately. In my case there weren't any other indicators 3 lines before. In the terminal I'm seeing this:

Starting server

To see the GUI go to: http://127.0.0.1:8188 got prompt Failed to validate prompt for output 8:

Also this may or may not be related, but I've just noticed that after selecting a video, VHS_VideoUpload doesn't load the video preview in that workflow I linked unless I manually insert a new VHS_VideoUpload node, which then immediately starts playing a preview of the clip that was loaded in the original node.

I tried to run the workflow after deleting and replacing the node but unfortunately I still got the identical error message.

AustinMroz commented 8 months ago

Not in the error message. The actual code that is producing the error is

def validate_path(path, allow_none=False, allow_url=True):
    if path is None:
        return allow_none
    if is_url(path):
        #Probably not feasible to check if url resolves here
        return True if allow_url else "URLs are unsupported for this path"
    if not os.path.isfile(path.strip("\"")):
        return "Invalid file path: {}".format(path)
    return True

In the workflow, the RR_VHS_LoadVideoPath_WithMeta nodes are actually 2 grouped nodes with the former passing an input path to the later. Because the video path is an input and not a widget, path should be None. What I don't understand, is how the error message "Invalid file path" can be reached with None when None is already explicitly handled.

I'm still doing tests with the provided workflow, but so far I've not been able to reproduce the issue.

ricardofeynman commented 8 months ago

Ah I completely misunderstood. Thanks for clarifying.

The RenderRift dev actually thought groups may have been a factor with the embedded workflow itself, and so provided an alternative version of the workflow:

https://github.com/RenderRift/ComfyUI-RenderRiftNodes/issues/1#issuecomment-1872520331

I'm not sure how relevant it will be as the same error is thrown for me with either version of the workflow.

AustinMroz commented 8 months ago

It all seems quite contradictory and with the earlier error in execution.py, it has me wondering if there's some sort of filesystem corruption that is happening, as unlikely as that seems.

Things are going to get a little involved here, but if you're up for it, I'd appreciate it if you could do a little bit of debugging on your end. Open videohelpersuite/utils.py and add the line breakpoint() on 120 so the full method becomes

def validate_path(path, allow_none=False, allow_url=True):
    if path is None:
        return allow_none
    breakpoint()
    if is_url(path):
        #Probably not feasible to check if url resolves here
        return True if allow_url else "URLs are unsupported for this path"
    if not os.path.isfile(path.strip("\"")):
        return "Invalid file path: {}".format(path)
    return True

That will cause the execution to pause for the (green) original Load Video and for the actual bugged case.

ricardofeynman commented 8 months ago

Thanks I'll get started, but I may be a little slow with my replies for the next hour (making dinner for the waifu).

ricardofeynman commented 8 months ago

Having added breakpoint() to line 120, when I restart the server and run the workflow with original green Load Video node, the following occurs:

Starting server

To see the GUI go to: http://127.0.0.1:8188 got prompt

/blah/ComfyUI/custom_nodes/ComfyUI-VideoHelperSuite/videohelpersuite/utils.py(121)validate_path() -> if is_url(path): (Pdb) print(path) None (Pdb) print(path is None) False (Pdb) type(path) <class 'str'>

AustinMroz commented 8 months ago

Fascinating. Even though I had guessed that this could be what's happening, I can't think of a way for it to occur. I'll dig through the code to try and find an explanation for how this state is reached, but could you check if adding an explicit check for a string "None" fixes the issue?

def validate_path(path, allow_none=False, allow_url=True):
    if path is None or path == "None":
        return allow_none
    if is_url(path):
        #Probably not feasible to check if url resolves here
        return True if allow_url else "URLs are unsupported for this path"
    if not os.path.isfile(path.strip("\"")):
        return "Invalid file path: {}".format(path)
    return True
ricardofeynman commented 8 months ago

After adding 'if path is None or path == "None":' I'm getting this:

!! Exception during processing !!! Traceback (most recent call last): File "/blah/ComfyUI/execution.py", line 154, in recursive_execute output_data, output_ui = get_output_data(obj, input_data_all) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/blah/ComfyUI/execution.py", line 84, in get_output_data return_values = map_node_over_list(obj, input_data_all, obj.FUNCTION, allow_interrupt=True) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/blah/ComfyUI/custom_nodes/ComfyUI_ezXY/autoCastPatch.py", line 299, in map_node_over_list return _map_node_over_list(obj, input_data_all, func, allow_interrupt) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/blah/ComfyUI/execution.py", line 77, in map_node_over_list results.append(getattr(obj, func)(**slice_dict(input_data_all, i))) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/blah/ComfyUI/custom_nodes/ComfyUI-VideoHelperSuite/videohelpersuite/load_video_nodes.py", line 173, in load_video raise Exception("video is not a valid path: " + kwargs['video']) Exception: video is not a valid path: File not found

Prompt executed in 0.02 seconds

Edit:

Removing ComfyUI_ezXY from custom nodes in case that played any role I still get this:

Starting server

To see the GUI go to: http://127.0.0.1:8188 got prompt !!! Exception during processing !!! Traceback (most recent call last): File "/blah/ComfyUI/execution.py", line 154, in recursive_execute output_data, output_ui = get_output_data(obj, input_data_all) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/blah/ComfyUI/execution.py", line 84, in get_output_data return_values = map_node_over_list(obj, input_data_all, obj.FUNCTION, allow_interrupt=True) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/blah/ComfyUI/execution.py", line 77, in map_node_over_list results.append(getattr(obj, func)(**slice_dict(input_data_all, i))) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/blah/ComfyUI/custom_nodes/ComfyUI-VideoHelperSuite/videohelpersuite/load_video_nodes.py", line 173, in load_video raise Exception("video is not a valid path: " + kwargs['video']) Exception: video is not a valid path: File not found

Prompt executed in 0.01 seconds

AustinMroz commented 8 months ago

Thank you for patience. We've finally made it to error messages that make sense to me.

If RR_VideoPathMetaExtraction is unable to find a file, it passes "File not found" as the file name to Load Video (Path). I'll open a pull request over there to have it raise an error instead so people get information on which node/filename is incorrect.

ricardofeynman commented 8 months ago

Thank you, for your time and energy.

TLDR You've been a great help. VideoCombine's working again (and even slightly improved..?) in my personal workflows :)

It's in some respects unfortunate that I chose to link a newly released custom nodes workflow that had several extra VHS nodes and contained an opaque issue itself for us to do the troubleshooting with. I chose that one as a timesaver because the issue appeared much faster in the execution process than with other complex animatediff workflows, where VideoCombine was also throwing slightly different errors.

However, as I commented in the other issue opened here from several days back, my best guess is that it maaaaay have had something to do with me transitioning ComfyUI over to Conda a few days ago, and that messing up an environmental path somewhere, perhaps relating to CUDA cupy, during the transition. First time using Conda so it's possible I set something up incorrectly along the way.

Trying another workflow requiring VideoCombine I got this:

!!! Exception during processing !!! Traceback (most recent call last): File "/blah/ComfyUI/execution.py", line 154, in recursive_execute output_data, output_ui = get_output_data(obj, input_data_all) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/blah/ComfyUI/execution.py", line 84, in get_output_data return_values = map_node_over_list(obj, input_data_all, obj.FUNCTION, allow_interrupt=True) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/blah/ComfyUI/execution.py", line 77, in map_node_over_list results.append(getattr(obj, func)(*slice_dict(input_data_all, i))) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/blah/ComfyUI/custom_nodes/ComfyUI-Frame-Interpolation/vfi_models/stmfnet/init.py", line 45, in vfi from .stmfnet_arch import STMFNet_Model File "/blah/ComfyUI/custom_nodes/ComfyUI-Frame-Interpolation/vfi_models/stmfnet/stmfnet_arch.py", line 28, in from vfi_models.ops import FunctionCorrelation, FunctionAdaCoF, ModuleSoftsplat File "/blah/ComfyUI/custom_nodes/ComfyUI-Frame-Interpolation/vfi_models/ops/init.py", line 21, in from .cupy_ops import softsplat, ModuleSoftsplat, FunctionSoftsplat, softsplat_func, costvol_func, sepconv_func, init, batch_edt, FunctionAdaCoF, ModuleCorrelation, FunctionCorrelation, _FunctionCorrelation File "/blah/ComfyUI/custom_nodes/ComfyUI-Frame-Interpolation/vfi_models/ops/cupy_ops/init.py", line 1, in from .costvol import File "/blah/ComfyUI/custom_nodes/ComfyUI-Frame-Interpolation/vfi_models/ops/cupy_ops/costvol.py", line 1, in from .utils import cuda_kernel, cuda_launch, cuda_int32 File "/blah/ComfyUI/custom_nodes/ComfyUI-Frame-Interpolation/vfi_models/ops/cupy_ops/utils.py", line 1, in import cupy ModuleNotFoundError: No module named 'cupy'

$ conda install -c conda-forge cupy has given me fresh package conflicts to contend with for less vital custom nodes, but whatever was going on with VideoCombine previously in my personal workflows has finally been resolved.

If it wasn't something to do with conda/cupy I'm stumped as to the exact cause. I didn't think to remove the changes to the line in utils.py to see if they were responsible for the !!! Exception during processing !!! Traceback helping me identify cupy, but that error wasn't being thrown in that workflow when I tried it before opening this issue and editing utils.py.

Assuming that tweak you provided to utils.py helped me identify another previously opaque issue in my conda setup, thanks for that helpful troubleshooting tip also.

The only thing that I've noticed which is different now (and this may have been something introduced in a commit over the last few days that I wouldn't have spotted due to my VideoCombine issues), is that where previously VideoCombine has never shown anything in the preview window for prores/mov outputs (video for h264 etc had always displayed in the preview for me), I'm now at least getting a still from the completed prores video.

An unexpected bonus improvement to VideoCombine's functionality. Woo!

AustinMroz commented 8 months ago

I've certainly caused a little bit of bumpiness with the size of the the last big update and am glad for any opportunity to smooth things over. Having a workflow is a huge help and those external nodes can frequently expose issues in the code I've written here.

I'll make a personal note to circle back on the "None" as a string issue. Even if that's somehow caused by a different set of custom nodes or nonstandard environment, it could still be an issue for others.

A new option was added for "Advanced Previews" if you click the settings button on the right of the webui. It's disabled by default, but having previews for all output options is one of the things it fixes.

ricardofeynman commented 8 months ago

Glad to have been of help, and grateful to get back to work on the projects involving AnimateDiff today.

I'd noticed "Advanced Previews" and had checked the option over the last couple days, guessing that explains why a preview image from the ProRes format output videos now appear when VideoCombine has successfully completed. Really handy update.

Thanks again!