Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

imgcat: add option to resample/resize images #3264

Open
AnonymouX47 opened this issue Mar 16, 2023 · 3 comments
Open

imgcat: add option to resample/resize images #3264

AnonymouX47 opened this issue Mar 16, 2023 · 3 comments
Labels
enhancement New feature or request

Comments

@AnonymouX47
Copy link

AnonymouX47 commented Mar 16, 2023

Is your feature request related to a problem? Please describe.

I have some high-res images (JPEGs and PNGs at least) for which wezterm simply displays nothing (using wezterm imgcat) even though the escape sequence is actually emitted.

I actually expect this behaviour from any terminal emulator, since imgcat transmits the image file content as-is.
The following was logged:

18:06:13.999  ERROR  wezterm_term::terminalstate::iterm > Ignoring image data 9000x6357 because 228852000 bytes > max allowed 100000000

So, I understand why.

Describe the solution you'd like

imgcat should downscale and re-encode high-resolution images before writing the escape sequence.

Describe alternatives you've considered

imgcat could check the image size and emit an error message if it's above the maximum for wezterm.

Additional context

Sample image: high-res.jpg (9,000 × 6,357 px)

Every other [reputable] tool I know for displaying images in terminals does exactly what I suggested (i.e downscaling and re-encoding) as a solution.
Personally, I'm not really affected by this and I don't see it as high priority. I'm just concerned about users that might be confused in such situations.

@AnonymouX47 AnonymouX47 added the enhancement New feature or request label Mar 16, 2023
@wez wez changed the title imgcat: Display high resolution images imgcat: add option to resample/resize images Mar 28, 2023
wez added a commit that referenced this issue Jul 17, 2023
This is primarily to improve the chances of displaying an arbitrary
image without resorting to additional external tools, that may be
difficult or impossible to install.

refs: #3716
refs: #3264
@AnonymouX47
Copy link
Author

AnonymouX47 commented Jul 17, 2023

I see you've addressed this in e048410 and #3716 (comment).

The options and their defaults look good.

The only "odd" thing is the reduction of animated images to a single frame but I guess there're reasons for that. Anyways, an animated image with a resolution above the default maximum should probably be an abomination IMO!
That said, I think the specific frame such would be reduced to should also be stated, except if it's not defined by the dependency resonsible.

I'll test it out later today and get back to you.

Thanks.

@wez
Copy link
Owner

wez commented Jul 17, 2023

That limitation is imposed by the image library: while it can consume certain animated formats, it doesn't offer a to way re-encode them. In addition, the processing time will be non-trivial; it already takes the better part of 1 second to process the image you provided as an example.

I consider re-encoding animations to be out of scope for imgcat.

The frame is likely the first frame, but that is expressly undefined by wezterm: if you need specific behavior for content that is already out of spec of the terminal, then you will need to be responsible for pre-processing it into the correct parameters ahead of time; the purpose of this resampling is a best effort to show an image that would otherwise not be possible to display.

@AnonymouX47
Copy link
Author

Thanks, that's all understandable.


I've tested out the options and they all work as described and expected with a couple exceptions:

1. Varying downscale resolutions with the same --max-pixels

Screenshot_2023-07-18_13-48-18

--max-pixels is described thus:

Images will be scaled down so that they do not exceed this size, ...

The behaviour is definitely in line with this description but seems to me like there's way more room available for the second image in the screenshot above.
I was wondering what the determination process of the downscale resolution could be to result in this behaviour and whether it was intended to be so.

2. --resize crops the image when the destination resolution differs in aspect ratio from the source resolution.

Screenshot_2023-07-18_15-27-02

This behaviour goes against both description (since it doesn't mention "cropping") and expectation. Normally, I would expect a "resize" operation with fixed destination height and width to scale, not crop, the source image to the aspect ratio of the destination resolution.

3. --resample-format input (default) does not support WebP

The following error is emitted when resizing/resampling a WebP image with input format:

18:13:09.457  ERROR  wezterm > encoding resampled image as WebP: The image format `WebP` is not supported; terminating

I tested a couple different formats I had available and this was the only one for which this error occurred. I suggest that this error should be handled (not just for WebP, assuming there could be others) and PNG (since it'd preserve way more information than JPEG) should be used as a fallback format.


Thanks once again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants