sixel preview

[horriblename]

follow up to: #840

Some things have changed in this PR since the last one:

• The current implementation supports single-line sixels only
• the workaround used previously by splitting sixels into multiple lines is no longer needed (it doesn't work anymore).

TODO

• simplify renderPreviewLine to accept either text or sixel
• [ ] export terminal size in pixels to previewer script
• memory limitations: raise gPreviewerMaxLineSize or use custom Scanner or something else? (used io.ReadAll as a workaround)
• [ ] maybe file based caching
• bug where quitting while sixel is shown on screen will leave the terminal in bold

---

example previewer script:

#!/bin/bash
case "$1" in
  *.jpg|*.png|*.svg)
    # using chafa:
    chafa "$1" -f sixel -s "$2x$3" --animate false
    exit 1

  *) cat "$1"
esac

[horriblename]

Turns out sixel images can get really large, from my testing a sixel of around 900x600 using all 256 colors is about 1.5MB in size, if you have a bigger monitor the current size limit (gPreviewMaxLineSize=2MB) might not be enough. I'm not sure what size would be a "reasonable" limit so it might be better to expose the size limit as an option.

plus all this is cached in memory so memory consumption might actually become an issue

[AtomicFS]

After exiting lf with key q while looking at image, getting sixel preview, the terminal will have bold text afterwards.

Workaround is to add following to the configuration file:

cmd on-quit ${{
    tput init
}}

[horriblename]

will try to fix it soon, thanks for reporting

[joelim-work]

@horriblename Firstly, thanks for working on this - judging from the number of likes I can see that there are quite a few users interested in this feature.

However given the fact that version r29 was just released, I suspect this PR is not yet ready to be merged, and will probably remain that way unless the implementation is simplified. This is a fairly large change, both in the number of lines of code added, as well as the additions to the UI drawing logic. This makes the PR harder to accept beceause it will increase the maintenance burden on the project.

I have left a few comments, but I haven't reviewed the code in sixel.go in detail, and instead I have the following questions:

1. The renderPreviewLine function appears to support input that contains sixel data of multiple images intermingled with regular text, Is there a need for this complexity? I would imagine that most users simply want to display a single sixel image and nothing else.
2. I believe the reason you calculate the dimensions of the sixel image is to ensure that it will fit into the preview window without overflowing it. Instead, would it be possible to calculate the size of the preview window in pixels and export it out to the previewer script so that it img2sixel knows how large the image should be? From a safety point of view, it does make sense check the sixel input to ensure it doesn't overflow the preview window, but it adds a fair amount of code.
3. I am concerned about the potential memory usage if 2MB is allocated to cache the sixel data for each file, and this could become worse if the user has multiple instances of lf open. I'm also not sure whether it's a good idea to expose this as a configuration option to users. Would it be possible to instead save the sixel data to a file in the ~/.cache directory, similar to how image thumbnails would be saved for video files?

---

The above points are just suggestions, but my understanding is that implementing sixel in lf is really just the following:

1. Read the previewer script output and determine if the data is a sixel image or just plain text.
2. If the data is a sixel image, store it into a string variable to be displayed.
3. When drawing the UI, clear the preview window to erase any preview sixel images.
4. Set the cursor position using the \033[%d;%dH escape sequence, and then write the sixel image to the terminal.

This is a trimmed down version of the implementation that you have, but could possibly be more likely to be accepted.

[ilyagr]

It might also help if, once you follow @joelim-work's advice as best you can, you rebase it on top of the r29 release, and then people interested in sixel support spend some time using this code. They could report any bugs they find, how useful they find this, and perhaps will be able to suggest improvement.

I'd love for a small community of people familiar with sixels to form around this PR, benefit from it, and help with the review in the ways I described.

[gokcehan]

To give some context on this, the other patches had slowed down in the past couple of weeks so I decided it is a good time to release r29. Reading the comments here, my impression was that this PR still required some time to be ready, so I decided to save it for the next release.

[horriblename]

Thanks for the feedback!

1. The renderPreviewLine function appears to support input that contains sixel data of multiple images intermingled with regular text, Is there a need for this complexity? I would imagine that most users simply want to display a single sixel image and nothing else.

Yes, mixing between text and sixel was allowed to make it so that it mimics the result shown in an actual terminal as much as possible, I agree that not many people, if any, will care about this, so I'll remove this and make it either text or one sixel per line.

2. I believe the reason you calculate the dimensions of the sixel image is to ensure that it will fit into the preview window without overflowing it. Instead, would it be possible to calculate the size of the preview window in pixels and export it out to the previewer script so that it img2sixel knows how large the image should be? From a safety point of view, it does make sense check the sixel input to ensure it doesn't overflow the preview window, but it adds a fair amount of code.

Yes, if the sixel image is too high, it will scroll the terminal and break the entire ui. Exporting the pixel size is probably a good idea.

It's worth noting that chafa can get the pixel dimensions themselves, but 'lower-level' libraries like img2sixel and image-magick don't do it themselves.

3. I am concerned about the potential memory usage if 2MB is allocated to cache the sixel data for each file, and this could become worse if the user has multiple instances of lf open. I'm also not sure whether it's a good idea to expose this as a configuration option to users.

To clarify, 2MB is the size limit for each line, not the entire preview string.

I'm also hesitant about exposing it as a config option, but the current 2MB is most probably not enough. I do have an idea that I will try involving using a custom Scanner that handles sixels differently, but that would probably add a lot of complexity.

Would it be possible to instead save the sixel data to a file in the ~/.cache directory, similar to how image thumbnails would be saved for video files?

I'll look into this, I'll try to see if we can handle it within lf first, but I think a simpler solution would be to never cache previews that contain sixel in memory, and let the previewer script handle file caching.

[joelim-work]

It's worth noting that chafa can get the pixel dimensions themselves, but 'lower-level' libraries like img2sixel and image-magick don't do it themselves.

Good point about chafa, I did not consider it myself. If you're able to specify the image dimensions in terms of cells instead of pixels, then I think it's not necessary to export the preview window dimensions in pixels.

I'll look into this, I'll try to see if we can handle it within lf first, but I think a simpler solution would be to never cache previews that contain sixel in memory, and let the previewer script handle file caching.

Although this is my own suggestion, I have to point out that file caching for sixel images is more complex than the case of caching thumbnails for video files. For thumbnails, you only need to generate one thumbnail per video, but for sixel images you need to take the size into account as well, so a single image will result in many sixel cache files if the size changes (e.g. if you resize the terminal window).

Now that you mention it, what is the current behaviour for cached sixel data if you resize the terminal window? Suppose you load a sixel preview into the cache and then shrink the terminal, I can see the following possible results:

• The cache entries are automatically invalidated, and a new sixel preview will be generated to fit inside the shrunk preview window.
• The cache entries are not invalidated, and sixel preview will now overflow the shrunk preview window, messing up the UI.
• The cache entries are not invalidated, but there is 'bounds checking' to prevent the sixel preview from overflowing the shrunk preview window. Although this doesn't mess up the UI, the result is that the image will appear cropped, which is still undesirable but not as bad.

You would also have to consider what happens if the user doesn't resize the terminal window, but instead zooms in/out, which has a similar effect but this time the terminal dimensions in pixels remains the same but now the cell dimensions in pixels changes.

Of course, the simplest approach would be to just never cache sixel data at all, but that might not be desirable either. From my very limited time playing around with sixel (I am by no means an expert on this topic), I found that the majority of the time was spent generating the sixel data from the image, which took around 100-200ms, which is too slow for my liking, but you might have different results.

[horriblename]

Now that you mention it, what is the current behaviour for cached sixel data if you resize the terminal window?

The third one, somewhat. I didn't do anything special in regards of caching, lf already invalidates caches on vertical window resize but not horizontal ones, so the image might get too wide (goes off screen but doesn't break ui), but will never be "too tall" (which would've been bad)

That's a good point on resizing, I'll keep that in mind.

[joelim-work]

@horriblename I haven't looked at this in a long time, but thanks for continuing to show interest in this feature.

I'm still hesitant about approving this kind of patch (at least in its current state) for a few reasons:

• The size of the patch and the scope of what it is trying to do is fairly large. which will add a significant amount of maintenance burden. I find the size of the patch hard to justify, especially when this will affect only a subset of users that require image previews.
• The design philosophy of lf is minimalistic in nature - instead of trying to do everything by itself, lf should delegate certain responsibilities to other tools and work seamlessly with them. For the case of rendering sixels, I'm not sure whether it makes sense for lf to process the sixel data at all. It should be the user's responsibility to ensure that the sixel data isn't invalid or otherwise too large, as the dimensions of the preview window are passed to the previewer program.
• Caching is another feature that can be left as a responsibility for the user (similar to caching thumbnails for videos) and lf does not need to be involved with it at all. I have not tested this extensively and I could be wrong about this, but I would expect that the majority of the time taken is spent on converting the image to sixel format, as opposed to reading the sixel data.

Note that I am not against the idea of trying to display sixel image previews in lf. However I think that a better approach would be to start off small and add the bare minimum (i.e. read the sixel data from the previewer script and store it in a buffer, then display it in the UI), then consider additional functionality if it becomes necessary.

[joelim-work]

To everyone else who is interested in this PR, lf can already display sixel image previews even without this patch by writing the sixel data directly to the TTY.

Simple example:

#!/bin/sh

case $(file -bL --mime-type "$1") in
    image/*)
        chafa -f sixels -s "${2}x${3}" "$1" > /tmp/lfsixel  # can be slow
        tput cup "$5" "$4" > /dev/tty
        cat /tmp/lfsixel > /dev/tty
        exit 1
        ;;
esac

A slightly more complex example that includes clearing and caching sixel data:

#!/usr/bin/env bash

clear() {
    for ((i=0; i<"$3"; ++i)); do
        tput cup "$(($5 + i))" "$4"
        printf '%*s' "$2" ''
    done
}

hash() {
    printf '%s\0%s\0%s' "$2" "$3" "$(stat --printf '%n\0%i\0%Y' "$1")" |
    sha256sum |
    awk '{print $1}'
}

case $(file -bL --mime-type "$1") in
    image/*)
        cache="$HOME/.cache/lf/$(hash "$@").six"
        [[ -f "$cache" ]] || chafa -f sixels -s "${2}x${3}" "$1" > "$cache"
        clear "$@" > /dev/tty
        tput cup "$5" "$4" > /dev/tty
        cat "$cache" > /dev/tty
        exit 1
        ;;
esac

Although I do consider this a somewhat hacky solution and there are some caveats - writing directly to the TTY can conflict with lf itself, and the previews have to be cleared manually by using ctrl+l to redraw the screen when changing to a non-image file.

[horriblename]

one thing I forgot about: KDE konsole will only half-work(images can't be cleaned up automatically, only with ctrl+l).

In the official sixel manual, printing new characters over an image is undefined, but most terminals would overwrite the image; that's how I clean up images. Konsole is the only terminal that doesn't do that

[joelim-work]

Overall looks good, most of my comments are about minor cleanups.

Regarding KDE Konsole, I guess it's not a big deal and can be treated as a 'non-compliant' terminal along with other terminals that don't have any sixel support at all. This is something that can be mentioned in the wiki.

[joelim-work]

Looks good now, thanks for the PR. I will leave it up to @gokcehan to review and merge the patch.

[gokcehan]

It looks good to me as well. Thanks @horriblename for the patch and @joelim-work for the review. Judging by the emoji reactions in this thread, it seems that many people appreciate the work in this patch.

[MahouShoujoMivutilde]

First of all, thank you, this is definitely a long awaited feature, at least for me.

I've noticed one curious thing.

Sixel preview gets distorted (compressed about by a factor of 2 height wise) if chafa was called via setsid.

This might sound like an oddly specific nitpick, but the actual use case is a whole preview script being launched via bubblewrap sandbox as shown in the wiki.

Here is the preview script to reproduce the problem.

Preview

#!/bin/bash

WIDTH="$2"
HEIGHT="$3"

# notify-send "w = $WIDTH x h = $HEIGHT"

case "$(file -L -b --mime-type -- "$1")" in
image/*)

    # # CORRECT
    # chafa --dither ordered -f sixel \
    #     -s "${WIDTH}x${HEIGHT}" --animate false -- "$1"

    # # CORRECT, not sixel
    # chafa -f symbols --bg=white --fill=space --symbols=all \
    #     --dither ordered -s "${WIDTH}x${HEIGHT}" -- "$1" && exit 0

    # # still CORRECT, not sixel
    # setsid chafa -f symbols --bg=white --fill=space --symbols=all \
    #     --dither ordered -s "${WIDTH}x${HEIGHT}" -- "$1" && exit 0


    # # minimal repro
    # # distorted, width visual height compressed roughly twice
    # setsid chafa --dither ordered -f sixel \
    #     -s "${WIDTH}x${HEIGHT}" --animate false -- "$1"


    # distorted
    # no-op bwrap from arch wiki, narrowed down the problem to --new-session
    bwrap \
        --dev-bind / / \
        --new-session \
        chafa --dither ordered -f sixel \
        -s "${WIDTH}x${HEIGHT}" --animate false -- "$1"


    # # distorted
    # # Bubblewrap sandbox from
    # # https://github.com/gokcehan/lf/wiki/Previews#sandboxing-preview-operations
    # bwrap \
    #     --ro-bind /usr/bin /usr/bin \
    #     --ro-bind /usr/share/ /usr/share/ \
    #     --ro-bind /usr/lib /usr/lib \
    #     --ro-bind /usr/lib64 /usr/lib64 \
    #     --symlink /usr/bin /bin \
    #     --symlink /usr/bin /sbin \
    #     --symlink /usr/lib /lib \
    #     --symlink /usr/lib64 /lib64 \
    #     --proc /proc \
    #     --dev /dev \
    #     --ro-bind /etc /etc \
    #     --ro-bind ~/.config ~/.config \
    #     --ro-bind ~/.cache ~/.cache \
    #     --ro-bind "$PWD" "$PWD" \
    #     --new-session \
    #     --unshare-all \
    #     chafa --dither ordered -f sixel \
    #     -s "${WIDTH}x${HEIGHT}" --animate false -- "$1"

    exit 1
    ;;
*)
    echo no preview implemented
    ;;
esac

Here is how it looks:

It works fine outside of lf preview.

Outside

Any insight why that happens?

[horriblename]

My first guess is chafa can't read the terminal size from a bwrapped environment. if that's the case the only solution would be to read px size first then passing the size to the bwrapped script. chafa doesn't accept px units, so we'll have to use imagemagick's convert directly... I'll investigate later(in a few days), but if you wanna figure things out yourself,:

python script to get preview panel size in px (untested)

from fcntl import ioctl
from os import ctermid, getenv
from struct import pack, unpack
from termios import TIOCGWINSZ

with open(ctermid()) as fd:
    winsize = ioctl(fd, TIOCGWINSZ, pack("HHHH", 0, 0, 0, 0))
    # wc = width in cells
    hc, wc, wpx, hpx = unpack("HHHH", winsize)
    
    cellWidth = wpx/wc
    cellHeight = hpx/hc

    previewWidthPx = cellWidth * getenv('WIDTH')
    previewHeightPx = cellHeight * getenv('HEIGHT')
    print(previewWidthPx, previewHeightPx)

[MahouShoujoMivutilde]

I've played around a bit with it here is what I found.

Minimal repro outside of lf

package main

import (
	"bytes"
	"fmt"
	"os/exec"
	"strconv"
)

func main() {
	// from my comment above
	previewer := "/home/witch/.config/lf/chafa-sandbox.sh"
	path := "/home/witch/tmp/test/megumin-square.png"

	w := 54 / 2
	h := 25 / 2

	cmd := exec.Command(previewer, path,
		strconv.Itoa(w),
		strconv.Itoa(h))

	var out bytes.Buffer
	cmd.Stdout = &out

	cmd.Run()

	fmt.Print(out.String())
}

So apparently exec.Command and setsid and sixel don't mix well.

It is possible to get it to work even with setsid if using img2sixel and calculating precisely image size fitted in the preview pane in pixels, yes*, but that's annoying and I wanted to figure out why it is so allergic to as I now understand a combination of go's exec.Command and setsid.

* - btw, in your script change wc, hc, wpx, hpx = unpack("HHHH", winsize) to hc, wc, wpx, hpx = unpack("HHHH", winsize)

I've also noticed that with rendering sixel from setsid'ed command that while the image doesn't get distorted I would need to press enter to get back to the shell prompt.

demo

00.46.15.2023.mp4

It happens with both chafa and img2sixel. It doesn't happen when running anything non-sixel.

[frere-jacques]

Is there a recommendeed solution for the sixel distortion with the bwrapped script?

[MahouShoujoMivutilde]

Recommended? Idk.

But to make it work you could just drop the --new-session.

See the warning in the man page:

--new-session
Create a new terminal session for the sandbox (calls setsid()). This disconnects the sandbox from the controlling terminal which means the sandbox can't for instance inject input into the terminal.

Note: In a general sandbox, if you don't use --new-session, it is recommended to use seccomp to disallow the TIOCSTI ioctl, otherwise the application can feed keyboard input to the terminal

[joelim-work]

What's the use case of running the entire preview script inside a sandbox? According to the wiki section it seems to be for PDF vulnerabilities, and maybe it is less problematic to use bwrap for just pdftotext instead of everything.

[MahouShoujoMivutilde]

Mostly convenience for paranoia - there is no reason for image/video/archive/document previewer to edit/save anything outside $PWD and ~/.cache.

So it's disallowed to do so.

I have a few apps sandboxed, or, at least somewhat sandboxed (you know, swiss cheese and all that) like that.

Probably protects from nothing, but makes me feel better.

[joelim-work]

That's fine - what I'm really trying to say is that whoever wrote the bwrap example (not me) probably doesn't use sixel previews and therefore doesn't know about this issue. I imagine most users (including myself) just test setups on their own system only before adding them to the wiki.

Maybe it's fine to just add a note in the wiki to explain that bwrap uses a new session and therefore doesn't have access to the terminal cell size.

[MahouShoujoMivutilde]

Makes sense, I'm not complaining.