Kosinkadink / ComfyUI-VideoHelperSuite

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

Add LoadAudioUpload -node #192

Closed kijai closed 2 months ago

kijai commented 2 months ago

Adds a node to allow audio file upload in same way as video upload. I find this necessary when working remotely with audio stuff, and in my opinion it easily integrates here.

Tested to be working with .mp3, .wav and .ogg from Firefox on Linux.

Still not 100% sure of all the javascript stuff especially, I don't have much js experience at all, so @AustinMroz if you could check if there's something missing or if I made a mistake.

AustinMroz commented 2 months ago

It's always a delight to see your continued contributions. I don't consider myself experienced in javascript either, so it makes it all the more commendable to wade through code that probably isn't the most legible.

The only potential issue I see that makes a functional difference is the manual selection for imageio-ffmpeg which sidesteps all the selection logic/user overrides. Were you seeing some issue with the default ffmpeg selection/ it's usage in get_audio()?

And some minor notes that don't make any functional difference:

kijai commented 2 months ago

Alright, thank you for the very informative response, that answered all my questions!

There actually were no issues with the existing get_audio function, I just forgot about it as I initially copied this code from my VoiceCraft nodes where I have made some bridging nodes to convert VHS audio to torch audio tensors.

Removed the redundant code bits too so it should be cleaner now.

AustinMroz commented 2 months ago

I pull the updated code and am immediately corrected by personal changes I made to execution.py: IS_CHANGED is called, VALIDATE_INPUTS is not. Errors to IS_CHANGED are normally swallowed, though which resulted in the node always being marked stale.

I've applied fixes to the minor issues this raised (resolving audio to a path) and am merging.

Sorry for the confusion and thanks again.