hpjansson / chafa

📺🗿 Terminal graphics for the 21st century.
https://hpjansson.org/chafa/
GNU Lesser General Public License v3.0
2.97k stars 64 forks source link

Sixel and tmux passthrough over ssh not rendering jpegs, ignoring sizes, and other chaos #217

Closed stevenwalton closed 2 months ago

stevenwalton commented 2 months ago

I've been trying to get chafa to work well with fzf (on wezterm) so I can remotely preview images on my servers. When sshing in I seem to be getting an issue where I require using the -f sixels command.

Here's an example when I'm just sshing and there is no fzf nor tmux involved (the command is similar to what I have in my fzf configuration).

(ssh)

Screenshot 2024-09-09 at 3 58 02 PM

There's two things that seem off to me 1) The size is not respected. This is even true if I use --size (full command being chafa -s 120x80 20240320_073017.jpg) 2) The image quality is quite poor. This issue does not exist when I'm sitting at a monitor in front of my server, only over ssh

This can be resolved through -f sixels but this is where things get weird. Outside of tmux, things work as expected (yay, problem solved?) but once in tmux jpeg images just don't render. Let me demonstrate

(I'll put most images in dropdowns with default close for easier reading of the issue)

Example with sixels enabled where we see high quality image (`ssh` + `-f sixels`)

Screenshot 2024-09-09 at 4 11 07 PM

Example within `tmux` over `ssh` (`tmux` + `ssh`)

Screenshot 2024-09-09 at 4 02 35 PM

So image quality dramatically decreases.

Adding Passthrough (`tmux` + `ssh `+ `--passthrough auto`)

Screenshot 2024-09-09 at 4 09 28 PM

(specifying `tmux` makes no difference)

Now let's add our -f sixels flag (tmux + ssh + -f sixels)

Screenshot 2024-09-09 at 4 04 15 PM

And we get nothing!!!

`tmux` + `ssh` + `-f sixels` + `--passthrough auto`

Screenshot 2024-09-09 at 4 17 30 PM

Okay, but what about a png? (`ssh` + `tmux` + `-f sixels`)

Screenshot 2024-09-09 at 4 07 47 PM

I didn't even need the passthrough! (do I ever?) If I try to scroll then the image disappears so I can't include the command in the screenshot :(

So I'm not quite sure what is actually going on here and what the issue is. But it appears to me that when using tmux and ssh jpegs do not render but pngs do.

When not sshing I only have issues when using tmux and/or fzf

Maybe I've missed something in the docs but I tried looking and didn't see any issue related to this.

Relevant info: Host machine:

$ inxi -S
System:
  Host: foo Kernel: 6.10.8-arch1-1 arch: x86_64 bits: 64
  Console: pty pts/19 Distro: EndeavourOS
$ chafa --version
Chafa version 1.14.2

Client machine: MacBook Air M2 Sonoma 14.6.1

related fzf config lines (but this is tangential to the issue since this is all without fzf, but is the end goal)

Let me know if there is more information needed or if I've missed something.

hpjansson commented 2 months ago

Hi, thanks for the report! It looks like there are a couple of interacting issues here.

tl;dr: Try with recent versions of the terminal and Chafa, and use chafa -f kitty --passthrough tmux on the command line.

If you find any issues with image scaling in the latest version, I can look at samples (to make it as clear as possible, you could try at 30x30 inside a terminal window that's 80x80). I'm interested in making this as frictionless as possible.

AnonymouX47 commented 2 months ago

Concerning tmux, I recall sixel support was added in v3.4. Though, I should note that the release notes say "if built with --enable-sixel"; so, you might wanna check with your package source.

With this, I think it'll be a good idea to use Passthrough::NONE in the combination of sixel + tmux + --pasthrough=auto. 🤔

AnonymouX47 commented 2 months ago

I believe the tmux version can be determined via the TERM_PROGRAM_VERSION env var (and the XTVERSION query, for when those are implemented).

@stevenwalton, do you mind confirming your tmux version and testing with -f sixel --passthrough=none, if you have v3.4?

stevenwalton commented 2 months ago

@hpjansson, thanks, this is some good info. I'll give these things a try and report back.

Regarding the SendEnv/AcceptEnv part, does this not represent a security risk? My understanding is that this leaves you vulnerable to the LD_PRELOAD exploit described here. I'll test but even if it does work this unfortunately won't be a viable solution for me if my understanding is correct about this vuln.

Most of those variables do not exist for me. But TERM is xterm-256color inside and outside of tmux. My tmux conf sets the default terminal this way, should I try any of the other settings? I've also tried with -2 in (it is my default) and of course I've tried \tmux.

re Kitty: I'm on wezterm and not a particular fan of kitty. The developed doesn't seem to like tmux and I think a common pain point has been that my need (as many others) is about persistence on remote machines, not cutting up terminals. I'd love an alternative besides screen lol

chafa -f kitty --passthrough tmux caused a crash on my version. I'll check the new sub-sub version (I built from arch repo)

For --scale 1 and --scale max I think I might be misunderstanding. I thought this combination would try to do the original image size but be no larger than --view-size. But I was also a bit confused by --size and the short description, so I was just trying stuff and that seemed to work. What's the proper way to achieve this? I don't want my thumbnails to be scaled up but I do want my large photos from my phone or elsewhere to shrink (i.e. no upscale, yes downscale). Perhaps we could improve the language in the man pages for illiterates like me lol

re fzf: I'm happy to be a guinea pig and I can bridge conversations with them because I think many users would like this feature. But I think more important than getting it working with fzf is just in the terminal. It would still be a huge benefit even if it doesn't work under fzf.

I'm interested in making this as frictionless as possible.

If I have any skill in life, it is being unfortunate enough to walk into any possible bug haha. I appreciate this philosophy and the project. Being able to render images at all in the terminal is a big help these days.

@AnonymouX47

TERM_PROGRAM_VERSION gives 3.4 (I should have reported this. tmux -V returns 3.4 and I checked via pacman which says 3.4-10). XTVERSION is unset.

-f sixel --passthrough=none When done remotely over ssh inside tmux this returns nothing (like the picture above). On a png image, it works. Outside tmux I get a nice high quality picture.

To all:

I was also doing some further testing and found yesterday that it appears sometimes jpgs work and sometimes they don't. I'm not sure exactly what the pattern is but I think it might be either the image size or filesize. I believe it has not worked for any of the images that I took from my phone. I did chafa -f sixel --passthrough=none *.jpg in a folder with a mix of them and most images did not render but a few did and all but 1 were definitely not. Unfortunately scrollback makes the images disappear so I ran find . -maxdepth 1 -name "*.jpg" -exec bash -c "echo {} && chafa -f sixel --passthrough=none {}" \; to watch the stream and I could see the empty spaces via scrollback, and no photo that rendered was taken from my phone, but some photos that did were taken by a smartphone, but were much smaller in size. If it helps, I reran to get the file size and the largest was 792K but many images much smaller than this failed to render (find . -maxdepth 1 -name "*.jpg" -exec bash -c "echo {} && du -h {} && chafa -f sixel --passthrough=none {} && sleep 1" \; works quite well if you want to replicate). The pattern is still unclear for what renders and what doesn't. I also redid with png and there were zero failures, so this seems jpg specific (I only had 2 jpegs in that location even after removing depth and neither rendered)

Would some examples help?

hpjansson commented 2 months ago

chafa -f kitty --passthrough tmux caused a crash on my version. I'll check the new sub-sub version (I built from arch repo)

Crashes are my highest priority - could you please file a new issue for this so we can follow up there? It could be a variant of #210, but it's hard to tell. FWIW I can't trigger it (but I also don't think my slightly older WezTerm version supports Kitty passthrough).

I was also doing some further testing and found yesterday that it appears sometimes jpgs work and sometimes they don't. I'm not sure exactly what the pattern is but I think it might be either the image size or filesize.

I doubt it's related to png vs. jpeg, but I ran some tests just now and was reminded that tmux has a hard upper limit on the size of the image data that can be transferred -- it will discard image data that is too big. Maybe they will all come through with smaller output sizes, e.g. with -s 40? See also https://github.com/tmux/tmux/issues/3959.

re fzf: I'm happy to be a guinea pig and I can bridge conversations with them because I think many users would like this feature.

Great! Could you open a separate bug for improved fzf support? Once we have a handle on the issues outside fzf, we can deal with that there.

For --scale 1 and --scale max I think I might be misunderstanding. I thought this combination would try to do the original image size but be no larger than --view-size. But I was also a bit confused by --size and the short description, so I was just trying stuff and that seemed to work. What's the proper way to achieve this?

I think it should do what you're trying to by default, without any options. --scale 1 is the default. --size will force it to use a specific size irrespective of scale and view (it always "wins"). You're right that the documentation could be better; if you file a separate issue for this, I'll follow up there (I need to draw a diagram as this is best explained visually).

hpjansson commented 2 months ago

Regarding the SendEnv/AcceptEnv part, does this not represent a security risk? My understanding is that this leaves you vulnerable to the LD_PRELOAD exploit described here. I'll test but even if it does work this unfortunately won't be a viable solution for me if my understanding is correct about this vuln.

If you select the environment variables carefully, this is not a risk. But note that a malicious user on the server could be able to see the environment variables you pass (along with anything else you do in the server-side session).

As an example, some Linux distributions come pre-configured with a SendEnv/AcceptEnv list that includes LANG and LC_* variables.

stevenwalton commented 2 months ago

Sorry for the late reply.

Crashes are my highest priority

I'm having a difficult time reproducing a full crash but this is the output I'm currently getting and there's a lag but subsequent runs don't have the lag. I did do an update since the issue was opened so idk if that resolved things or there was a fluke. But I suspect there was some things that got fixed (more later). I can still open a new issue for this if you'd like.

SSH (no tmux) SSH + tmux

Screenshot 2024-09-16 at 3 15 54 PM

Screenshot 2024-09-16 at 3 12 30 PM

If I run the command in front of the machine, not over ssh then I get the same output as the SSH (no tmux) but my desktop gives me a warning about fonts and an error message about \u{10eeee}

Font problem
No fonts contain glyphs for these codepoints: \u{10eeee}.
Placeholder glyphs are being displayed instead.
You may wish to install additional fonts, or adjust
your configuration so that it can find them.

find . -maxdepth 1 -name "*.jpg" -exec bash -c "echo {} && du -h {} && chafa -f sixel --passthrough=none {} && sleep 1" \;

So I did this again. Over ssh, no issues. Image quality isn't the best but it isn't bad. Like text is readable but not crisp. SSH + tmux we get missing images again. Changing --passthrough=tmux we miss every single image. Adding -s 40 does not resolve the issue. Fwiw, I decided to try the screen passthrough and it will render black or gray boxes that are the images.

So this is why I think the upgrade may have thrown a wrench in the debugging.

I decided to try another experiment btw. That screenshot I keep testing is 578x304

exiftool output ```bash $ exiftool screenshot.png ExifTool Version Number : 12.96 File Name : screenshot.png Directory : . File Size : 112 kB File Modification Date/Time : 2024:08:16 16:07:56-07:00 File Access Date/Time : 2024:08:16 16:07:56-07:00 File Inode Change Date/Time : 2024:09:02 12:33:07-07:00 File Permissions : -rw-r--r-- File Type : PNG File Type Extension : png MIME Type : image/png Image Width : 578 Image Height : 304 Bit Depth : 8 Color Type : RGB Compression : Deflate/Inflate Filter : Adaptive Interlace : Noninterlaced Significant Bits : 8 8 8 Image Size : 578x304 Megapixels : 0.176 ```

So I ran chafa -f sixel -s N screenshot.png with a few different values of N. It didn't work until I brought it down to 400, which in this case it goes way off the screen

Screenshot 2024-09-16 at 4 23 03 PM

It won't fit on screen until -s 194

Result with -s 100

Screenshot 2024-09-16 at 4 24 42 PM

I think I understand this one. Rereading the docs I notice it says columns, not pixels. Discussing documentation I think it could help adding (not pixels) just because I think despite the docs being right that it's an easy misread haha.

But this brings me to the part of my original confusion. What's the conversion of columns and rows to pixels? I notice in this particular case --exact-size=on is the same as -s 39. This is the goal of what I was trying to achieve above where we don't upscale an image if it hits its max.

$ chafa -f sixel --exact-size=auto -s 50 screenshot.png
# yields size 50 image
$ chafa -f sixel --exact-size=on -s 50 screenshot.png
# yields no image
$ chafa -f sixel --exact-size=on --view-size 50 screenshot.png
# this is what I want?

Do I have this right? (I don't think so: see edit)

With tmux I fail to render with anything above -s 115 (screen passthrough does give a black box of the right size though). -f kitty and -f iterm gives no image, no -f gives an image when using higher -s but it's junk at any size. Outside tmux -f {iterm,kitty,sixel} works up to -s 450. At this size that bottom line cuts off in the 5 on 18:56.

ssh config Okay that makes more sense. Still something I'd like to avoid. I will test but haven't had a chance yet. (sorry, trying to balance my PhD work and some other projects). I do have a vested interest in getting this all working fwiw, so I'll spend some time in the docs and code as well to try to get a better understanding.

And I apologize I'm unraveling things haha. I'll get the fzf issue open.

[EDIT]

Okay, I'm more confused about $ chafa -f sixel --exact-size=on --view-size 50 screenshot.png. While working on #218 I am more than confused than ever

Screenshot 2024-09-16 at 6 56 18 PM

There is a possible conflict though as when --exact-size on --view-size N was being run while dropping in and out of tmux frequently I started getting errors from starship

starship_zle-keymap-select-wrapped:1: maximum nested function level reached; increase FUNCNEST?

BUT exiting ssh and reconnecting cleared the starship error but not this chafa one. There were no updates and only thing that happened between me commenting under this issue and opening the new issue was I walked away from my computer to do the dishes.

AnonymouX47 commented 2 months ago

Concerning Wezterm + Tmux...

There's no point trying to use -f kitty --passthrough={tmux | auto} because Wezterm currently doesn't support virtual placements (unicode placeholders).

See the list at https://github.com/wez/wezterm/issues/986.

Wezterm is simply ignoring the U key in the sequence because it doesn't understand it yet and goes on to create a real (non-virtual) placement (which is the image you see in the screenshot without tmux), after which it tries to display the diacritics which you see as "question marks in a box".

These codepoints are displayed as such (and you get a warning) because they are within Unicode PUA and normal fonts don't map any glyphs to them.

The reason you aren't seeing the image when running tmux is most likely because tmux overwrites it in the course or displaying the diacritics i.e the image is probably actually displayed but before you see it, it's been overwritten by the diacritics for the placeholders.


EDIT:

As for the rest of your comment, I'm honestly not sure what exactly is being addressed but concerning image size with sixels, I'd like to remind you that tmux has a limit for sixel payloads.

hpjansson commented 2 months ago

@stevenwalton I think the latest commits to the master branch should address most of your concerns. It allows WezTerm to be detected inside tmux, and disables passthrough for tmux 3.4+, so you get native sixels instead.

It's technically possible to work around the tmux sixel limit too, but that'd be a lot more complex. I'd rather addess that in a new, longer-term issue. Likewise detecting sixel capability over ssh without access to the env vars. Sound ok?

stevenwalton commented 2 months ago

Okay after upgrading there is a SIGNIFICANT increase in the number of successful files when using that find command (with -s 40). Like >90%! And fwiw, we got it working in fzf too!

For what pictures aren't working, this is still unclear but I think it is tmux. I was able to display them using a smaller size, so I guess it is the aspect ratio (900 x 1600, 736x1600, and 1200x1600 was a mixed bag but mostly successful).

A minor glitch is when in tmux and running fzf (using my command) the left side does not get cleared. Additionally without fzf if I scrollback the images are not persistent. But I think these both are not issues with chafa.

So I'm more than happy to close the issue. Thanks for all the help!

EDIT:

In case anyone visits this issue in the future looking to do similar things here is the relevant section in my dotfiles.

At time of comment ```bash function export_fzf_defaults() { FZF_DEFAULT_OPTS='--ansi --preview ' # If we have chafa installed then let's display pictures # For this to work properly we need chafa to be >= 1.14.4 # https://github.com/hpjansson/chafa/issues/217 if ( _exists chafa ); then FZF_DEFAULT_OPTS+='"if file --mime-type {} | grep -qF image; then chafa ' FZF_DEFAULT_OPTS+='--passthrough none -f sixels --size ' # tmux can't display as large of images so we'll need to reduce them to # 30 as this is the max (determined by testing) if [[ $(env | grep tmux) ]]; then FZF_DEFAULT_OPTS+='30 ' else FZF_DEFAULT_OPTS+='${FZF_PREVIEW_COLUMNS}x${FZF_PREVIEW_LINES} ' fi # Note the 'elif' for the bat lines FZF_DEFAULT_OPTS+='{}; elif ' else FZF_DEFAULT_OPTS+='"if ' fi FZF_DEFAULT_OPTS+='file --mime-type {} | grep -aF -e directory; then ' if (_exists lsd); then FZF_DEFAULT_OPTS+='lsd --color always --icon always --almost-all ' FZF_DEFAULT_OPTS+='--oneline --classify --long {}; ' else FZF_DEFAULT_OPTS+='ls --color=always -Al {}; ' fi if (_exists strings); then FZF_DEFAULT_OPTS+='elif file --mime-type {} | grep -aF -e binary; then ' FZF_DEFAULT_OPTS+='strings {} | ' if ( _exists bat ); then FZF_DEFAULT_OPTS+='bat ' else FZF_DEFAULT_OPTS+='batcat ' fi FZF_DEFAULT_OPTS+='--color always --language c; ' fi # If it is a text or json file we'll read it with bat # We could probably fizzbuzz this and just check that it isn't a bin FZF_DEFAULT_OPTS+='elif file --mime-type {} | grep -aF -e text -e json -e ' FZF_DEFAULT_OPTS+='empty; then ' # Support batcat with older Ubuntu if ( _exists bat ); then FZF_DEFAULT_OPTS+='bat ' else FZF_DEFAULT_OPTS+='batcat ' fi FZF_DEFAULT_OPTS+='--color=always --theme=Dracula --style=numbers,grid ' FZF_DEFAULT_OPTS+='--line-range :500 {}; fi"' export FZF_DEFAULT_OPTS="${FZF_DEFAULT_OPTS}" } export_fzf_defaults ```