Kosinkadink / ComfyUI-VideoHelperSuite

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

ERROR:root:* VHS_VideoCombine 330: #108

Closed gateway closed 8 months ago

gateway commented 8 months ago

getting an error now with the latest build


ERROR:root:  - Value not in list: format: 'video/h265-mp4' not in ['image/gif', 'image/webp', 'video/ProRes', ['video/av1-webm', [['pix_fmt', ['yuv420p10le', 'yuv420p']], ['crf', 'INT', {'default': 23, 'min': 0, 'max': 100, 'step': 1}], ['input_color_depth', ['8bit', '16bit']], ['save_metadata', 'BOOLEAN', {'default': True}]]], ['video/h264-mp4', [['pix_fmt', ['yuv420p', 'yuv420p10le']], ['crf', 'INT', {'default': 19, 'min': 0, 'max': 100, 'step': 1}], ['save_metadata', 'BOOLEAN', {'default': True}]]], ['video/h265-mp4', [['pix_fmt', ['yuv420p10le', 'yuv420p']], ['crf', 'INT', {'default': 22, 'min': 0, 'max': 100, 'step': 1}], ['save_metadata', 'BOOLEAN', {'default': True}]]], ['video/webm', [['crf', 'INT', {'default': 20, 'min': 0, 'max': 100, 'step': 1}], ['save_metadata', 'BOOLEAN', {'default': True}]]]]
ERROR:root:Output will be ignored```

![image](https://github.com/Kosinkadink/ComfyUI-VideoHelperSuite/assets/519563/f0feaee1-f7eb-4139-991f-ff73fe6added)
Kosinkadink commented 8 months ago

The problem appears to be a change in how comfyui handles validating inputs, the relevant comfyui update being 2 hours ago https://github.com/comfyanonymous/ComfyUI/commit/04b713dda1c4109f84386b17b0f7c25722f0ae15

Kosinkadink commented 8 months ago

@AustinMroz looks like the root cause is the formats not all being strings in the format list - I think this would always 'technically' be an issue with comfy, but the VALIDATE_INPUTS function changed to not ignore validating specific inputs unless there is an argument in the VALIDATE_INPUTS function with the same name as the input. I will push out an update that will fix the problem by simply adding 'format' as one of the args for the VALIDATE_INPUTS function, and we can worry about properly validating that the format exists later (i don't think our code was checking for it anyway with how it was written before).

AustinMroz commented 8 months ago

Was just starting to read into things, but your description sounds like an appropriate solution.

AustinMroz commented 8 months ago

Having done a thorough read through, I can properly vouch for the correctness of the fix.

Overall the change to how input validation is handled seems to be a healthy one. It makes it very legible which arguments have special validation logic and allows for introduction of inputs to a node without requiring updates to VALIDATE_INPUT or resorting to kwargs as before. Going through and updating all the VALIDATE_INPUT methods is likely beneficial, but low priority. It has also given me an answer to a related issue I had: Load Audio never has it's VALIDATE_INPUT method called because it's an optional input and only the graph of required inputs have their input validated. Consequentially, there is not a guarantee that a file will be checked for existence before IS_CHANGED is called to hash files.