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

Cover display can still shows a part of the previous one in case of a different picture ratio #115

Open
Porkepix opened this issue Feb 20, 2023 · 11 comments
Assignees
Labels
bug Something isn't working

Comments

@Porkepix
Copy link
Contributor

I'm taking the guess here that the picture encoded in the files have the same ratio as the .jpg included in the directory (btw, does it pick those, if no pictures are included in the audio file themselves?)

I had one song playing with a 640x640 cover, then following was a song with a cover of size 770x700.
The first cover wasn't completely replaced, and a part of it was remaining, leading to a mixed picture, see this:

Screenshot from 2023-02-20 16-54-26-mod

@tramhao
Copy link
Owner

tramhao commented Feb 21, 2023

May I ask which terminal you're using?

@tramhao
Copy link
Owner

tramhao commented Feb 21, 2023

There are several situations:

  1. For kitty, the image is cleared before draw a new one. It should be fine.
  2. For iterm & wezterm(they use same protocol), the image is not cleared(I don't know how to clear them).
  3. For other terms like alacritty, I use ueberzug, and it should be fine too. However, ueberzug only support x11.

@Porkepix
Copy link
Contributor Author

There are several situations:

1. For kitty, the image is cleared before draw a new one. It should be fine.

2. For iterm & wezterm(they use same protocol), the image is not cleared(I don't know how to clear them).

3. For other terms like alacritty, I use ueberzug, and it should be fine too. However, ueberzug only support x11.

It was from Wezterm. I'm using Wayland sessions and unfortunately Alacritty isn't supporting what's required yet.

@tramhao
Copy link
Owner

tramhao commented Feb 22, 2023

Please try if kitty works. For wezterm/iterm, I need to figure out how to clear the image.

@Porkepix
Copy link
Contributor Author

Please try if kitty works. For wezterm/iterm, I need to figure out how to clear the image.

So, yeah, kitty doesn't reproduce the issue. That also means that when going from a track with a cover to a track without one, wezterm doesn't clear at all the cover, and therefore keeps the previous one displayed.

I thought sixels were something pretty standardized from what I was explained in another issue, but from what I understand things, you have to handle things in custom ways depending on the terminal, actually?

Interestingly I also note rendering difference between Kitty and wezterm or Alacritty, like the Title column in the playlist using bolder font, and the direction icon after "Add to:" that's glitchy in Kitty (default conf).

@tramhao
Copy link
Owner

tramhao commented Feb 23, 2023

Currently I'm using two library to draw images. Firstly is viuer, which support kitty, iterm2 protocol(including wezterm). If not supported, fall back to ueberzug(need feature cover compiled). For sixel, I heard of it, but I'm not sure how to handle it. For ueberzug, as the author cleared the repo, it's not maintained and it doesn't support wayland. I'm thinking of remove ueberzug completely but currently I don't have better solution to draw the photo.

@Porkepix
Copy link
Contributor Author

Porkepix commented Feb 23, 2023

Currently I'm using two library to draw images. Firstly is viuer, which support kitty, iterm2 protocol(including wezterm). If not supported, fall back to ueberzug(need feature cover compiled). For sixel, I heard of it, but I'm not sure how to handle it. For ueberzug, as the author cleared the repo, it's not maintained and it doesn't support wayland. I'm thinking of remove ueberzug completely but currently I don't have better solution to draw the photo.

Okay, I wasn't aware of the library and terminals with their own protocols. As I understand it, removing ueberzug would then let without solution every terminal not having this kind of homemade protocol, unless viuer is planning on adding others and standardized protocols?

For Ueburzug, yeah, I unfortunately saw the story and what happened. Someone did a copy of the repo at Codeberg just in case (https://codeberg.org/speedie/ueberzug) but that's all and I don't think anyone used that for the time being.
However I discovered through ArchLinux package manager (it's providing the same binary) a complete rewrite in C++ thought out as a drop-in replacement: uerberzugpp (https://github.com/jstkdng/ueberzugpp).

That's how I heard about sixels (I don't even know what it exactly is, not very knowledgeable on the topic), because I reported an issue in their repo for unsupported terminals, as well as wezterm covers being broken if used within Zellij (and that can be explained if it's using another protocol… ; maybe kitty would break too).

ueberzugpp added sixel support which wasn't in the original ueberzug, I believe. That's in these issues I heard of https://www.arewesixelyet.com/

For the record, the reported issues: jstkdng/ueberzugpp#4 and zellij-org/zellij#2168 (maybe the last one should be corrected if in fact sixel was never used…)

PS: I just tested termusic within zellij on kitty. It's even worse: termusic isn't even running, it doesn't draw anything of the UI and gets completely stuck. I'm not even sure if that's an issue that belongs to Kitty, Zellij or Termusic…
(termusic runs well within Zellij on many other terminals, minus the loss of the cover feature with wezterm)

EDIT: Maybe crates cover the sixel use, don't know about that. I'm guessing zellij and wezterm could find some.

@jstkdng
Copy link

jstkdng commented Feb 23, 2023

Hello, I'm the maintainer of ueberzugpp, I'd encourage you to switch to ueberzugpp as it has more features than the original and works on more terminals.
In the case of zellij (and some(? VTE based terminals), it could be needed to use tcp sockets to communicate with ueberzug++ in case your program interacts with stdin.

@tramhao
Copy link
Owner

tramhao commented Feb 27, 2023

It's great to know ueberzugpp and I'll try it.

@tramhao tramhao self-assigned this Apr 11, 2023
@tramhao tramhao added the bug Something isn't working label Apr 11, 2023
@CRAG666
Copy link

CRAG666 commented Apr 29, 2023

@tramhao Currently wezterm supports this option enable_kitty_graphics = true.But termusic does not show any cover.

@CRAG666
Copy link

CRAG666 commented Apr 29, 2023

I attach the image of lf using kitty protocol in wezterm
imagen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants