The prior implementation made a best effort to continue in the event of
an error, but immediately threw an error afterwards since there was no
output from the subprocess.
As get_audio isn't called until the audio connection from a Load Video
is used, it's safe to raise an exception here and properly expose the
error message from ffmpeg as is done elsewhere.
See #250
cg-use-everywhere automatically creates links when when a graph is
executed. This is generally nice utility, but conflicts with the
paradigm of the new VAE inputs and is particularly problematic for Load
Video (where it can not be solved on the python side).
As a workaround, a wrapper is added around the graphToPrompt call that
cg-use-everywhere wraps which deletes any links from LoadVideo nodes
that do not actually exist.
The safety code to ignore the memory limit when it could not be
calculated was incorrectly applied after adding the vae changes.
They have been reapplied. See #240
Some external nodes have adopted the usage of VHS_AUDIO for user
convenience. If these nodes do not update, then a AudiotoVHSAudio node
must be added to perform the conversion. The code to check pulls up the
nodeDef and will not warn on old workflows if the target node has been
updated.
The conversion node is not automatically added. While this may be
feasible, I feel it would be overly destructive, error prone, and would
require logic for where to fit the conversion node into a workflow
visually. A gentle warning seems the safer option.
Known issues:
- need legacy node to convert from AUDIO to VHS_AUDIO
- add js to automate adding said node to workflows?
- need to parse number of channels when loading audio
Under unknown circumstances, the VAE input may be non-None even though
there is not actually a wire connected. In this circumstance, images are
still received as input and would throw an error when converted from a
dict to tensor. To resolve this, an extra check is added to make sure
images is a dict and, if not, the VAE input is overridden to None
Resolves#238
When an old workflow is loaded with converted widgets, those converted
widgets can take the vae slot and convert the image input to a latent
input. An explicit check is added to make sure the slot that is
connected has type VAE to fix these converted workflows.
Load Images didn't have return names defined, which gave a very
unhelpful description of INT to the frame count output. This has now
been fixed.
When Video Combine is used to output an image sequence consisting of a
single image, that single image is displayed instead of being converted
into a video
On Load Video, Size scaling had a major rewrite to support different
downscale ratios, but logic was flawed. This has been fixed
Image Sequence previews were broken by adding a framerate arg since the
input is a sequence file and not the images themself. This could be
added later by adding the speed information to the sequence file, but
has been removed for now since there isn't any infrastructure to set the
frame rate on the Load Images node itself
Added an 8bit-png output format. Lots of people seem to prefer
outputting image sequences. When the extra quality isn't required,
having a separate 8bit format allows for a significant speedup and less
disk space used.
Frame rate is now passed so image_sequqnces play back at proper rate.
Context menu options on an image sequence (open preview, save) now
operate on the first frame instead of throwing an error.
As part of the load_latent implementation, the code to rescale input
needed to be written to work on streams and properly align to different
downscale ratios.
Preset scaling dimensions were incorrectly set to perform no scaling and
a squeeze/unsqueeze were incorrectly added in the rescale method itself
ComfyUI does type validation that I by stoke of luck, completely missed
while working on implementation. As a work around, load video and video
combine have an extra input/outpu of the latent type that is hidden and
links are modified to point use those on serialization.
The logic for when links should be maintained has been improved.
latent links seem to always be broken on reload, likely because the link
type is incorrect at link creation time. Further investigation needed
Under some circumstances seemingly related to disabled or corrupt
performance counters on windows, the psutil.swap_memory() call can fail.
This is now handled a bit more gracefully by displaying warning and
simply disabling the memory limit. Will likely need revisiting later to
not swallow the brief hints on how to fix the issue.
See #224
Adds an optional vae input to Load Video and Video Combine. When this is
connected, the node is dynamically updated such that Load Video instead
has a latent output and Video combine instead has a latent input.
Fixed the number of frames being calculated after input has been
converted with map.
Removed duplicated code for looping which was likely caused by an
incorrect merge.
Resolves#228
There were still intermittent errors and console spam from a transcoded
advanced preview being interrupted before it could finish.Connection
errors are also caught now.
ffmpeg does not appear to handle transparency as expected for gif
creation. Transparent areas will show the previous frame, or a portion
of it. A deep dive into trying to find configuration options to fix this
did not result in solutions that functioned on my machine, but did
introduce me to the ffmpeg split filter.
Split allows for a single ffmpeg execution to generate a palette and
utilize the palette for creating the output gif without requiring a temp
file. However, the video data must be buffered in memory by ffmpeg.
Quick testing seems to show that ffmpeg uses 4 bytes per pixel
of buffered frame data.
The ramifications for memory usage are nuanced, but wind up preferable.
The increased memory per pixel of ffmpeg's buffering only results in
(approximated) 3% memory increase over the whole workflow, but remove
the need for a tempfile, a second subprocess call, and allow for
imperfect Meta Batch support which can only extend the total frame count
by 5-6x. It's a reduction in moving parts for underwhelming memory
characteristics in both the best and worst case. Additionally, it likely
actually addresses the issue the original requester saw in #160
The actual code for allowing pre-passes is left in for now and will
likely be re-evaluated for other uses or pruned at a later time.
Actually bump version.
All path stripping is now performed by a dedicated strip_path function.
First all whitespace is stripped, then a single double-quote is stripped
from the start and end of the path. Subsequent double quotes or white
space inside of the quotes is maintained.
Fixed a number of places paths received no stripping
- Load Video (Path) execution
- Path queries
- Advanced Previews
- Load Audio
See #224
in_place requires a fairly recent version of pillow and performing the
performance benefit of it is minimal.
Loading as float16 does break other nodes so the change is getting
reverted.
Resolves#226
Load Video has the same performance quirk of exceptionally slow
performance when performing a f16 divide. The same fix has been.
When utilizing Meta Batches, the progress bars didn't retain the total
step information when returning. It now displays a measure of progress
across all Meta Batch executions when possible.
Load Images, Load Video, Video Combine, and Batched VAE nodes have had
progress bars added.
For Load Video, a progress bar is only shown if frame_load_cap is set
and if the video has fewer frames than that cap, the length of the
progress bar may not be accurate.
Perform float16 divide with torch instead of numpy. It doesn't look
pretty, but this takes the load time for my 500 frame test from 55
seconds down to 35
Exif transposes are done in place. While the exif transpose operation
appears heavy, in depth profiling seems to show this is because the
actual image loading is differed until something uses it and the
transpose operation is assuming blame. However, designating the
operation as in place does eliminate an unnecessary copy
The gifski changes require a recent version of gifski and has a high
likelihood of breaking things for anyone who self-compiled gifski to get
ffmpeg support. The error message now suggests that users check their
gifski version
Because the gifski conversion process uses two processes, either of the
two can potentially still be running when another encounters an error.
Previously, this could result in a deadlock if python blocks on reading
stderr from a non-terminating process. stdin is now closed on ffmpeg to
ensure this does not happen.
Returned masks had incorrect shape
width and height were swapped when recalling to common size
The descaling had unusual behavior when performed after the mask has
been inverted, and is now performed before mask inversion
While I have no repro on the reported issue for this branch, I feel
merging it is the best path forward for getting information and ensuring
the added functionality of the branch doesn't stagnate forever
Minor tweaks are made to wrangle input.
The maximum available memory and swap are checked before loading a video
to ensure sufficient space exists to both load, and process the video
data.
Note that the load is attempted even if frame_load_cap is set to above
the calculated memory limit in case the actual video has fewer frames
than this cap.
When loading videos and images, the images are loaded as f16 instead of
f32. This results in a ~25% memory reduction for most workflows and
should ensure the slight overhead of the above safety checks does not
create any regression in what workflows are capable of being executed.
This change has been fairly extensive testing on many vanilla nodes, but
has a fairly high potential to cause issues and should be watched
carefully.
ffmpeg 7.0 seems to introduce a regression where shortest doesn't apply
until the entire audio stream has been processed. When combined with
apad, this means audio transcoding never completes.
As a workaround, a maximum duration for the audio with pad has been
added as well as framework to track the total number of frames written
across multiple meta batches.
See #213
Load Images has lagged behind to recent Load video updates and was due
for both a performance pass and improved handling of mixed resolutions.
- Meta Batches are now supported
- The x2 multiplier on memory usage on tensor creation is fixed
- Images are automatically rescaled to a common size
Performance testing has not yet been done. Of particular note,
determining alpha presence and median size (for rescaling) now
requires that a prepass of all images is performed which likely has
a measurable performance hit. Additionally, the hashing code has not yet
been updated probably adds a significant cost for Meta Batch executions.
Specific testing also needs to be done on if a contiguous tensor is
required for image data, with particular concern for when execution
occurs on cpu only (i.e. The image data does not have an opportunity to
be made contiguous when sent to the gpu)
Since updating to Python12, I've been graced with warnings about
improper escape sequences. This change should make no functional
difference, but resolves the warnings.
Gifski added support for input from pipes about a month ago, but I only
learned of this today.
The implementation was due for a rework, and brings numerous advantages:
- Gifski no longer needs to be compiled with ffmpeg support
- Gifski output now supports Meta Batches
- A temp file is no longer required
- The simplified intermediary format should increase performance/quality
This will require a recently updated version of gifski and there is not
currently code in place to notify the user of this.
Jedrzej Kosinski