tramhao / termusic

Music Player TUI written in Rust
GNU General Public License v3.0
1.03k stars 43 forks source link

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

Open Porkepix opened 1 year ago

Porkepix commented 1 year ago

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 commented 1 year ago

May I ask which terminal you're using?

tramhao commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

Porkepix commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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: https://github.com/jstkdng/ueberzugpp/issues/4 and https://github.com/zellij-org/zellij/issues/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 commented 1 year ago

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 commented 1 year ago

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

CRAG666 commented 1 year ago

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

CRAG666 commented 1 year ago

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