ueber-devel / ueberzug

Continuation of ueberzug
GNU General Public License v3.0
113 stars 6 forks source link

ueberzug stopped working. #10

Closed dharmx closed 1 year ago

dharmx commented 1 year ago

Hello, there!

First of all I want to say my thanks for picking up this project. This really makes me happy as I use ueberzug in almost all applications I use. Which is just Neovim, Ranger and FZF.

Arch recently (I think) switched to this fork. Which caused some issues when I was browsing for some images on neovim. I run ueberzug inside neovim by writing a JSON value into a file. Which gets tailed and piped into ueberzug.

This can be roughly translated to:

Here are the relevant parts:

  1. Part where ueberzug is being called. Here.
  2. Part where tail is being called. Here.
  3. The sender function. This parses a Lua dictionary (table) into JSON and then writes it to a FIFO file. So, whenever there is a new write, tail will pick it up and feed it to ueberzug. Here.

The JSON FIFO file looks like this.

{"path":null,"action":"add","identifier":"tele.media.fifo","y":1,"width":1,"height":1,"x":1}
{"path":"/home/maker/Pictures/wallpapers/abstract/o6nxhmkfsd791.png","action":"add","identifier":"tele.media.fifo","y":4,"width":111,"height":25,"x":85}
{"path":null,"action":"add","identifier":"tele.media.fifo","y":1,"width":1,"height":1,"x":1}
{"path":"/home/maker/Pictures/wallpapers/abstract/wallhaven-o33x55.png","action":"add","identifier":"tele.media.fifo","y":4,"width":111,"height":25,"x":85}

Investigating further I tried writing a minimal implementation of this. Still no luck.

#!/usr/bin/env python

from pathlib import PosixPath as Path
from time import sleep

from ueberzug.lib.v0 import Canvas, ScalerOption, Visibility, os

UEBERZUG_FIFO = Path(os.getenv("UEBERZUG_FIFO", "/tmp/ueberzug.fifo"))

with Canvas() as canv:
    place = canv.create_placement(
        identifier="tele.media.files",
        path="",
        x=0,
        y=0,
        scaler=ScalerOption.CONTAIN.value,
        visibility=Visibility.VISIBLE,
    )

    while True:
        with canv.lazy_drawing:
            message = UEBERZUG_FIFO.read_text(encoding="utf8").strip()
            if message == "EXIT":
                break
            message = message.split("\t")
            place.path = message[0]
            place.x = message[1]
            place.y = message[2]
            place.width = message[3]
            place.height = message[4]
        sleep(0.2)

# vim:filetype=python

Where the /tmp/ueberzug.fifo looks like this.

/home/maker/Pictures/wallpapers/abstract/1632112923543.jpg  0   0   100 50

I also tried re-installing but to no avail.

And, also noticed that ueberzug had changed its flags. Like layer was --layer IIRC?

Let me know your thoughts. Thanks.

dharmx commented 1 year ago

I do not what the hell happened but as soon as I posted this. It immediately started working.

Disregard.

dharmx commented 1 year ago

Nevermind. This is still happening. Ranger behaves this way as well. No image previews.

jstkdng commented 1 year ago

I'm trying to configure telescope-media but it's not working for some reason. Is this config not enough?

local telescope = require("telescope")
local canned = require("telescope._extensions.media.canned")

telescope.load_extension("media")
telescope.setup({
  extensions = {
    media = {
      backend = "ueberzug", -- "none"|"ueberzug"|"viu"|"chafa"|"jp2a"|"catimg"
      move = true, -- experimental GIF preview
      on_confirm = canned.single.copy_path,
      on_confirm_muliple = canned.multiple.bulk_copy,
      cache_path = "/tmp/tele.media.cache",
    }
  }
})
dharmx commented 1 year ago

That looks fine. I have this in mine.

telescope.setup({
  -- ...
  extensions = {
    -- ...
    media = {
      backend = "ueberzug",
    }
    -- ...
  }
  -- ...
})
telescope.load_extension("media")

Also, what is the error message?

jstkdng commented 1 year ago

There's no message, the preview window is empty.

image

unless I'm supposed to do/configure something else.

RayZ0rr commented 1 year ago

@dharmx @jstkdng How did you install ueberzug?

jstkdng commented 1 year ago

Tested on ueberzug from arch repositories and U++ from AUR.

dharmx commented 1 year ago

@jstkdng Yes! Same thing is happening for me as well. @RayZ0rr I also installed it from Arch repository.

jstkdng commented 1 year ago

ohh, well, it seems your plugin is not starting a ueberzug instance. I'm checking the U++ log and it is not being updated.

dharmx commented 1 year ago

Yes. It seems that way. I just opened my laptop from sleep and it started working again. Then closed Neovim and re-opened it. It didn't open.

https://user-images.githubusercontent.com/80379926/237012064-06974d91-4f06-4c87-9dd2-c32952c97244.mp4

Same happened with the script I shared as well.

RayZ0rr commented 1 year ago

Earlier this week I had issues with image-preview on ueberzug with vifm. Vifm would freeze when it was showing a preview.

Right now, I am using ueberzug without issues on Vifm. Here's what I did to fi it.

I had an /usr/bin/ueberzug which was not working properly. From my memory I installed it from pip. But it didn't show association to any package managers.

It didn't show up when I did pip list or sudo pacman -Qq

So I deleted that file :P :

sudo trash /usr/bin/ueberzug

I used trash-cli instead of rm just in case

Then I reinstalled ueberzug from official arch repos:

sudo pacman -S ueberzug

Then everything was back to normal

eylles commented 1 year ago

So perhaps a broken link lingering from a prvious install may be the issue here.

dharmx commented 1 year ago

Hello, again.

Sorry, for the delay. I was busy with university work.

This is the resulting error if it helps:

image

Could this be related to the window manager? Other people have reported this to me as well. Again, note that its not just my plugin but ranger and fzf previews as well which is not working.

Any, insight is appreciated.

Thanks.

jstkdng commented 1 year ago

It seems that you are caching images but are not waiting for the caching to end before trying to display it, causing ueberzug to try to read an incomplete image and thus fail.

Edit: nvm, it was an issue on my side, it works now (on ueberzugpp). That being said, I'd recommend you to send a clear command before showing a new picture otherwise the image will stay there even if Telescope can't display it.

dharmx commented 1 year ago

Hello, again.

Dug for an entire day. Finally managed to get a different error message.

# ueberzug exited with code `1` and signal `0`.

Traceback (most recent call last):
  File "/usr/bin/ueberzug", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/usr/lib/python3.11/site-packages/ueberzug/__main__.py", line 55, in main
    module.main(options)
  File "/usr/lib/python3.11/site-packages/ueberzug/layer.py", line 236, in main
    with windows, image_loader:
  File "/usr/lib/python3.11/site-packages/ueberzug/batch.py", line 147, in __enter__
    return BatchList([instance.__enter__() for instance in self])
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/ueberzug/batch.py", line 147, in <listcomp>
    return BatchList([instance.__enter__() for instance in self])
                      ^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/ueberzug/ui.py", line 144, in __enter__
    self.draw()
  File "/usr/lib/python3.11/site-packages/ueberzug/ui.py", line 155, in draw
    self.parent_info.calculate_sizes(
  File "/usr/lib/python3.11/site-packages/ueberzug/terminal.py", line 68, in calculate_sizes
    self.__calculate_sizes(self.pty, fallback_width, fallback_height)
  File "/usr/lib/python3.11/site-packages/ueberzug/terminal.py", line 74, in __calculate_sizes
    cols, rows, xpixels, ypixels = TerminalInfo.get_size(fd_pty)
                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/ueberzug/terminal.py", line 19, in get_size
    fretint = fcntl.ioctl(fd_pty, termios.TIOCGWINSZ, farg)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
OSError: [Errno 25] Inappropriate ioctl for device

I'd recommend you to send a clear command before showing a new picture otherwise the image will stay there even if Telescope can't display it.

In the plugin I cache the image then display that by sending text in a fifo file in JSON format. The payload looks like the following:

{
  "path": "/tmp/media/0D52889FD0DFED7F7381248EB1071C0DB7146A9FF57DAB68321688E19CC4B96CA58FA3E4E6358087F270826B6E5D56A9B9EB4C0FA212B26F92D37DA5F410FA8D.jpg",
  "y": 2,
  "action": "add",
  "identifier": "media",
  "x": 84,
  "width": 111,
  "height": 25
}

Behavior: The image is now displayed.

For hiding the ueberzug window I send the following payload:

{
  "path": null,
  "y": 1,
  "action": "add",
  "identifier": "media",
  "x": 1,
  "width": 1,
  "height": 1
}

This process is briefly described here.

Thank you.

jstkdng commented 1 year ago

to hide the image send this command instead,

{
  "action":"remove",
  "identifier":"media"
}

OSError: [Errno 25] Inappropriate ioctl for device

well that's bad, ueberzug is no longer properly obtaining the terminal sizes. Will debug that. Maybe the neovim folks changed something and their terminal no longer works.

dharmx commented 1 year ago

Hello.

to hide the image send this command instead,

{
  "action":"remove",
  "identifier":"media"
}

Thanks. Will use that instead.

OSError: [Errno 25] Inappropriate ioctl for device

well that's bad, ueberzug is no longer properly obtaining the terminal sizes. Will debug that. Maybe the neovim folks changed something and their terminal no longer works.

Perhaps. But the weird thing is that ueberzug starts working whenever I kill my window manager and relogin. Then after opening and closing a few terminals (I am not quite sure.) it stops working.

This behavior varies by terminals.

Note that my $TERMINAL is set to tym.

Thanks.

dharmx commented 1 year ago

Another detail.

Right now ueberzug has stopped working in Tym and gnome-terminal. But, it is working fine in Kitty (not in nvim).

Wrote a minimal script for testing this:

#!/usr/bin/env python

from time import sleep
from ueberzug.lib.v0 import Canvas, ScalerOption, Visibility

with Canvas() as canv:
    place = canv.create_placement(
        identifier="media",
        path="/home/maker/Pictures/wallpapers/aerial/MoarBeach.jpg",
        x=0,
        y=0,
        width=500,
        height=300,
        scaler=ScalerOption.CONTAIN.value,
        visibility=Visibility.VISIBLE,
    )

    while True:
        with canv.lazy_drawing:
            sleep(0.2)

See the following list. Check means it works:

Measured with btop.

jstkdng commented 1 year ago

I think there is an issue on all VTE based terminals, they don't answer ioctl calls correctly so ueberzug must fallback to another value. Does it work on mate terminal?

dharmx commented 1 year ago

mate-terminal behaves the same way as Kitty.

eylles commented 1 year ago

hmmm weird, just for sanity's sake does vifmimg work? i'm on devuan ceres (debian unstable)

image

mate terminal   1.26.0-2 
libvte-2.91-0   0.70.3-1
libgtk-3-0      3.24.37-2

will check the fzf image previewers and telescope-media later and see what the hell is going on

wonder if this has something to do with a change in libvte considering they have been working on sixel support for a while.

aclist commented 1 year ago

I can confirm that I am seeing the same ioctl error as reported by @dharmx above after upgrading from 18.1.9-4 -> 18.2.0-2. This is using rxvt-unicode and fzf preview window, with JSON being written into a FIFO.

Using a naive implementation of ueberzug to just draw images from a list, it sometimes works, but in a more complex implementation (i.e., forming a part of a much larger program and being invoked through shared functions), the error always occurs. I have not been able to drill down to a minimum reproducible example for my use case yet, due to how intricately intertwined it is with other components, but I confirm that the error is verbatim identical to the one posted in https://github.com/ueber-devel/ueberzug/issues/10#issuecomment-1544553602

alacritty: not working kitty: not working urxvt: not working xterm: not working

gnome-terminal: working

jstkdng commented 1 year ago

It could be a bug seebye introduced (by accident I hope) before killing OG ueberzug.

https://github.com/ueber-devel/ueberzug/commit/83a0e9937dbe0653f30c82b0deda5e93f54b131f#diff-0b5c6bcc846a0924797a1c8ca2cdcdedc9cb49179d4269aa4a9e1d647820bb2bR126

reintroducing this old way could fix the issue.

also, could I ask you @aclist and @dharmx if this also happens with ueberzugpp ? there's an lf and fzf script there.

aclist commented 1 year ago

@jstkdng

I worked up a minimum reproducible example. Invoke this with argument 1 as the path to some directory containing images and argument 2 being working or broken. working demonstrates working functionality and passes the test in the above terminals. broken demonstrates broken functionality and throws the error, failing in those terminals mentioned.

I dropped all geometry/columns calculation logic in the fzf preview to keep the example small, so the images will be just thrown into the preview off-center. I just set it to X of 85 and Y of 0. You'll probably need the terminal emulator in fullscreen to actually see the images.

In the broken example, the root cause is invocation through a subshell, namely returning the results of the fzf picker to a variable.

In gnome-terminal, both the working and broken test pass.

poc.sh.txt

jstkdng commented 1 year ago

I'm using zsh and your script doesn't seem to work.

I think current ueberzug is trying to read terminal sizes from standard output and that doesn't always work. Even worse if it is running in a subshell.

I wrote this script to test fzf and I think it does the same as yours (kinda?). It seems to also work with kitty's output method. (only works with u++)

#!/bin/bash

UB_PID_FILE="/tmp/.$(uuidgen)"
ueberzugpp layer --no-stdin --silent --use-escape-codes --pid-file $UB_PID_FILE
UB_PID=$(cat $UB_PID_FILE)

export SOCKET=/tmp/ueberzugpp-$UB_PID.socket
export X=$(($(tput cols) / 2 + 1))

# run fzf with preview
export MYPATH=$1
sel=$(ls -1 "$1" | fzf --preview='ueberzugpp cmd -s $SOCKET -i fzfpreview -a add -x $X -y 1 --max-width $FZF_PREVIEW_COLUMNS --max-height $FZF_PREVIEW_LINES -f $MYPATH/{}' --reverse -q png)

echo $sel

ueberzugpp cmd -s $SOCKET -a exit

do test it please

aclist commented 1 year ago

I can't speak to zsh; you'd have to use a standard bash shell due to the bashisms in the script. But I think I got the idea across.

I did compile and try u++ yesterday at your prompting but was unable to get the fzub script working as intended when pointing it to a set of images, so I shelved it to try again today. I will test it again and report.

Are you thinking of backporting some functionality from u++? I am currently targeting ueberzug in my application due to it being more widely available in distributions.

aclist commented 1 year ago

do test it please

Yes, I confirm that the sample above does work as intended in every terminal emulator I pointed it at.

jstkdng commented 1 year ago

Are you thinking of backporting some functionality from u++

uhh, new feature I don't think so, this code's logic is quite complex. Bugfixes I can do.

aclist commented 1 year ago

I think I worded my comment poorly. I wanted to know if the purpose of the test was to see if u++'s method of handing ioctl can be used for this bugfix in ueberzug. In any case, the answer seems to be yes. Thank you for looking into this issue and let me know if you need anything else.

jstkdng commented 1 year ago

Turns out it had nothing to do with ioctl, and I'm still not sure what the issue is, but applying this patch seems to have fixed the terminal detection. Maybe something changed in python 3.11 with lists.

diff --git a/ueberzug/xutil.py b/ueberzug/xutil.py
index cd96e8e..4723a86 100644
--- a/ueberzug/xutil.py
+++ b/ueberzug/xutil.py
@@ -138,8 +138,8 @@ def get_parent_window_infos(display: X.Display):
             try:
                 window_pid, window_id = next(iter(sort_by_key_list(
                     ppid_window_id_map, ppids)))
-                window_children_pids = ppids[:ppids.index(window_pid)][::-1]
-                pty = get_first_pty(window_children_pids)
+                # window_children_pids = ppids[:ppids.index(window_pid)][::-1]
+                pty = get_first_pty(ppids)
                 window_infos.append(TerminalWindowInfo(window_id, pty))
             except StopIteration:
                 # Window needs to be mapped,
aclist commented 1 year ago

I can confirm that it reverts to normal behavior after applying the patch.

eylles commented 1 year ago

Weird, that shouldn't have to cause the issue but i guess there's some spaghetti we aren't seeing under the hood at work...

jstkdng commented 1 year ago

I guess seebye was trying to optimize this part by not checking all parent pids for a valid terminal. I'm not sure if this change merits a version bump.

eylles commented 1 year ago

Well, since it is a bugfix i'd say bump the patch version, 18.2.1

Open a PR, we need to be sure this doesn't break anything and after that it will be good to release.

jstkdng commented 1 year ago

just push to master bruh /s

eylles commented 1 year ago

well, the bugfix release is out, took too much time for whatever reason but i will try to dedicate more time to this stuff in the future.

dharmx commented 1 year ago

Forgot about this. Both ueberzug and ueberzugpp works now. Thank you for your work.