Skip to content

fix: magick preview of multiframe files#4024

Open
Omenabaka wants to merge 2 commits into
sxyazi:mainfrom
Omenabaka:magick-fix
Open

fix: magick preview of multiframe files#4024
Omenabaka wants to merge 2 commits into
sxyazi:mainfrom
Omenabaka:magick-fix

Conversation

@Omenabaka

Copy link
Copy Markdown
Contributor

Rationale of this PR

What changed

If the magick previewer is used without the --flatten argument, it now processes only the first frame of the file. For example, instead of running magick with input.jpg, it will run it with input.jpg[0] (which discards all frames but first and for files with only single frame it changes nothing).

Why do you need this?

Two reasons.

1st reason. If you set the yazi previewer to magick and try to preview a file with several frames (layered image or animation), it generates preview JPEGs for each frame and names them <hash>-0, <hash>-1, etc. As a result, preview still won't be displayed (because no file named just <hash> was created). It seems that this logic has no use case and simply punishes a user who forgot to add --flatten argument to the magick previewer, by creating a bunch of unnecessary files and not showing preview anyway.

2nd reason. Moreover sometimes for a useful preview you need the first frame instead of a flattened image.

Example 1.
Animated .avif with transparent background.
On the left: preview generated after this PR with the previewer magick --bg=gray.
On the right: preview generated with magick --flatten --bg=gray (you forced to use --flatten before this PR, otherwise a valid preview file would not be generated at all as described above).

fish.avif[0] --bg=gray fish.avif --flatten --bg=gray
6d13eb6fd8a9b31329844a65bccced74(avif 0  --bg=gray) 6d13eb6fd8a9b31329844a65bccced74(avif --flatten --bg=gray)

So before this PR you could only get a preview of all frames mingled together.
(Furthermore, the --bg argument is not respected. This is likely because in magick.lua the background color is set after flattening, but that's another issue that I don't touch here.)

Example 2.
Layered .psd file with usage of layer masks.

lorem.psd[0] lorem.psd --flatten
90705e64c14be5eb549defd27c09913e 90705e64c14be5eb549defd27c09913e flatten

ImageMagick's flatten does not respect layer masks (which should hide parts of a layer). But you don't really even need to flatten .psd files because they already have a preview which is stored as the first frame.

Files used for examples: examples.zip

Checklist

Comment thread yazi-plugin/preset/plugins/magick.lua Outdated
cmd:arg("-flatten")
cmd:arg(tostring(job.file.path)):arg("-flatten")
else
cmd:arg(tostring(job.file.path) .. "[0]")

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch!

Appending [0] after the file path causes an error when image filenames already contain [0], like a.jpg[0] or b.jpg[0][0]:

`magick` exited with error code: 1

without manually appending [0], they can be previewed normally with the magick previewer.

Perhaps some kind of escaping is needed for the file path here?

@Omenabaka Omenabaka Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This turned out to be more difficult than I thought. There a combination of two problems.

  1. Apparently, magick does not provide a way to escape individual characters in a file path. Moreover, you cannot even pass an input path to magick in such a way that it does not internally process the characters in it at all. For the output path, there is a setting that allows this (-define filename:literal=true), but looks like its implementation for the input path was not added: Expand -define filename:literal=true to also encompass inputs ImageMagick/ImageMagick#8035 (comment)
  2. You can only select a frame by adding square brackets to the end of the path. There is no alternative argument for this operation.

So, as I understand it, magick works with filenames as follows:

  • If a file exists with exactly the name that was passed as an argument, it uses that file. It doesn't matter whether it contains special characters or not, everything is interpreted as part of the path. Therefore, files like a.jpg[0] are now handled correctly.
  • If a file with exactly that name does not exist, magick tries to parse the options passed in [], but it also parse the [] that were part of the filename, which leads to an error.

In other words, you can avoid using magick's functions that activated by special characters in the filename, and everything will be fine with any filenames. But if you use at least one of these functions (and we need Frame Selection, which is only available by adding [] to the filename), then ALL special characters in the filename start being treated as special. There is no option in between, like escaping some characters and having special use for others.

At first, I thought about checking path for a trailing ] (that was the only source of issues with square brackets). But then I found that magick also have special meaning for :, @, and %. And having these in a filename while trying to use Frame Selection ([0]) trips up magick too, because it now tries to interpret them. It becomes too complicated to deal with, so maybe it's better not to use Frame Selection at all (maybe sometime in the future if magick adds a proper argument for it).

Alternative way to get the first frame

-delete 1--1 argument discards all frames from the second (1) to (-) the last (-1).

The only concern is performance. I'm not good at benchmarking (perhaps someone else should run more tests) but here are my observations:

For a multiframe file:

  • [0] is the fastest. As I understand it, it does not depend on the number of frames, because it doesn't even load them for processing, only the first one. But we cannot use this.
  • -delete 1--1 is significantly slower, because it loads all frames and then deletes the extras.
  • no arguments is even slower than the previous option (but not for much). That's how it works now, creates one file per frame — not what we need.
  • -flatten takes about the same time as the previous option.

For a single-frame file:

  • no arguments is how it works now, nothing is done with frames.
  • [0] takes the same time as above.
  • -delete 1--1 takes the same time as above.
  • -flatten is slightly slower.

So if we use -delete 1--1, performance for single-frame files should not change. For multi-frame files, performance will not be the maximum that magick could achieve, but still it will be the same/faster than now (and working previews will be generated instead of a bunch of numbered frames).

@Omenabaka Omenabaka requested a review from sxyazi June 8, 2026 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants