ravachol / kew

A command-line music player
GNU General Public License v2.0
567 stars 21 forks source link

[Bug] Pressing F4 may render the player instance unusable. #128

Closed xplshn closed 2 months ago

xplshn commented 5 months ago

Info: Tracks are .flac, there are cover.jpg files in every sub folder, each folder is a different album from Artic Monkeys. At the root level of such folder there is another Cover.jpg.

Kew BEFORE pressing F4 2024-03-25-010542_1366x768_scrot

Kew AFTER pressing F4 2024-03-25-010447_1366x768_scrot

BEFORE F4 (strace) (Everything working correctly)

[pid  1065] 01:14:49 open("/dev/snd/controlC0", O_RDONLY|O_LARGEFILE|O_CLOEXEC) = 13
[pid  1065] 01:14:49 open("/dev/snd/controlC0", O_RDWR|O_LARGEFILE|O_CLOEXEC) = 13
/tmp/bigdl_cached/strace.bin: Process 1602 attached
[pid  1602] 01:14:49 execve("/usr/bin/notify-send", ["/usr/bin/notify-send", "-a", "kew", "Arctic Monkeys - 505", "--icon", "/tmp/kew/anto/coverlcpngv.jpg"], 0x7fffb18e0078 /* 72 vars */) = -1 ENOENT (No such file or directory)
[pid  1602] 01:14:49 +++ exited with 1 +++
[pid  1065] 01:14:49 --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=1602, si_uid=1000, si_status=1, si_utime=0, si_stime=0} ---

When I pressed F4 on a track which didn't present the problem

[pid  1065] 01:17:07 open("/tmp/kew/anto/coverlcpngv.jpg", O_RDONLY|O_LARGEFILE) = 13
[pid  1065] 01:17:10 open("/tmp/kew/anto/coverlcpngv.jpg", O_RDONLY|O_LARGEFILE) = 13

When I press F4 on a track which is problematic for Kew, it seems the problem is not present unless Kew runs inside of a pane in Wezterm?

/tmp/bigdl_cached/strace.bin: Process 8959 attached
[pid  8959] 01:19:38 open("/etc/passwd", O_RDONLY|O_LARGEFILE|O_CLOEXEC) = 13
[pid  8959] 01:19:38 stat("/tmp/kew", {st_mode=S_IFDIR|0700, st_size=60, ...}) = 0
[pid  8959] 01:19:38 stat("/tmp/kew/anto", {st_mode=S_IFDIR|0700, st_size=60, ...}) = 0
[pid  8959] 01:19:38 open("/home/anto/Music/Artic_Monkeys/(2007) Arctic Monkeys - Teddy Picker [16Bit-44.1kHz]/01. Teddy Picker.flac", O_RDONLY|O_LARGEFILE|O_CLOEXEC) = 13
[pid  8959] 01:19:38 open("/tmp/kew/anto/coverkvuxkr.jpg", O_WRONLY|O_CREAT|O_TRUNC|O_LARGEFILE, 0666) = 14
[pid  8959] 01:19:38 open("/tmp/kew/anto/coverkvuxkr.jpg", O_RDONLY|O_LARGEFILE) = 13
[pid  8959] 01:19:38 open("/tmp/kew/anto/coverkvuxkr.jpg", O_RDONLY|O_LARGEFILE) = 13
[pid  8959] 01:19:38 open("/home/anto/Music/Artic_Monkeys/(2007) Arctic Monkeys - Teddy Picker [16Bit-44.1kHz]/01. Teddy Picker.flac", O_RDONLY|O_LARGEFILE) = 13
[pid  8959] 01:19:38 open("/home/anto/Music/Artic_Monkeys/(2007) Arctic Monkeys - Teddy Picker [16Bit-44.1kHz]/01. Teddy Picker.flac", O_RDONLY|O_LARGEFILE) = 13
[pid  8959] 01:19:38 +++ exited with 0 +++

AFTER F4, when the bug happens, trying to kill it with CTRL+C (strace)

]/Music @ doas bigdl run strace -f -t -e trace=file -p 19081
Running 'strace' from cache...
/tmp/bigdl_cached/strace.bin: Process 19081 attached with 5 threads
[pid 19082] 01:10:34 --- SIGINT {si_signo=SIGINT, si_code=SI_KERNEL} ---
[pid 19081] 01:11:02 --- SIGWINCH {si_signo=SIGWINCH, si_code=SI_KERNEL} ---
[pid 19081] 01:11:10 --- SIGINT {si_signo=SIGINT, si_code=SI_KERNEL} ---
^C/tmp/bigdl_cached/strace.bin: Process 19081 detached
/tmp/bigdl_cached/strace.bin: Process 19340 detached
/tmp/bigdl_cached/strace.bin: Process 19085 detached
/tmp/bigdl_cached/strace.bin: Process 19082 detached
/tmp/bigdl_cached/strace.bin: Process 19083 detached

Things which I tested:

xplshn commented 5 months ago

Update: I can't reproduce the bug anymore, it only happened in the Wezterm window I had before. I will try to reproduce it again and leave instructions on how to do so

xplshn commented 5 months ago

Happened again. It might have to do with the covers being too big, pressing C to disable the cover, then pressing B, then pressing C triggered the bug, but I didn't do that before to trigger it, and it had happened four times already when trying to play songs from that folder. 2024-03-25-013137_1366x768_scrot

xplshn commented 5 months ago

Quick note: Apparently the ASCII version of the cover doesn't resize the cover before printing it to the terminal? And the Normal version doesn't either, so sometimes the covers are not centered, even though the aspect ratios are set correctly. (I am probably mistaken)

Jaoheah commented 5 months ago

I think I encountered this bug recently, though I might not be remembering it correctly. I remember while going through all of the FX keybindings, Kew suddenly quit itself. (By FX, I mean F2-F5) 

Though it might be a different bug, as you are saying, it still is open, just not rendering anything on the screen. Unless it's a difference in how my terminal (Konsole) handled the issue than how yours did.

(Edited it since replying through the email notification messed up the formatting.)

xplshn commented 5 months ago

Yeah, seems you found the same bug

ravachol commented 5 months ago

Might be something wrong with the new code for displaying cover art.

ravachol commented 5 months ago

Quick note: Apparently the ASCII version of the cover doesn't resize the cover before printing it to the terminal? And the Normal version doesn't either, so sometimes the covers are not centered, even though the aspect ratios are set correctly. (I am probably mistaken)

They are always resized to fit the window. But yes I notice in wezterm that if the window is small, it doesn't center. I have looked into this before and it's printing indentations before printing the image lines, but wezterm is simply ignoring them.

ravachol commented 5 months ago

I actually have a solution for that now that works in more window sizes, but it still doesn't center it for very small windows.

ravachol commented 5 months ago

I need a way to reproduce the bug in op. xplshn sent you an email.

ravachol commented 5 months ago

I've tried with very large covers but no "luck".

xplshn commented 5 months ago

Quick note: Apparently the ASCII version of the cover doesn't resize the cover before printing it to the terminal? And the Normal version doesn't either, so sometimes the covers are not centered, even though the aspect ratios are set correctly. (I am probably mistaken)

They are always resized to fit the window. But yes I notice in wezterm that if the window is small, it doesn't center. I have looked into this before and it's printing indentations before printing the image lines, but wezterm is simply ignoring them.

Then, its an issue with Wezterm?

ravachol commented 5 months ago

Quick note: Apparently the ASCII version of the cover doesn't resize the cover before printing it to the terminal? And the Normal version doesn't either, so sometimes the covers are not centered, even though the aspect ratios are set correctly. (I am probably mistaken)

They are always resized to fit the window. But yes I notice in wezterm that if the window is small, it doesn't center. I have looked into this before and it's printing indentations before printing the image lines, but wezterm is simply ignoring them.

Then, its an issue with Wezterm?

Yes, I think so. The centering issue that is. You can see it sets the indentation on the last line of the cover (the black block on the last line). That is incorrect, not what the code says.

ravachol commented 5 months ago

I can't seem to reproduce it still.

xplshn commented 5 months ago

I tried to understand how you print the covers, but I am not sure I get it tho.

Shouldn't everything be calculated using the terminal's size? From indentation to logo to player and cover?

ravachol commented 5 months ago

It is actually. It calculates the ideal image height given how many metatag lines there are and visualizerheight. and then send that height to: void printSquareBitmapCentered(FIBITMAP *bitmap, int baseHeight)

in chafafunc.c

xplshn commented 5 months ago

Oh, okay. Then, I am out of ideas then.

xplshn commented 5 months ago

You may as well just close this issue and open an issue in Wez/Wezterm

xplshn commented 5 months ago

The only reason I think it may not be an issue only with Wezterm is because @Jaoheah says he has dealt with this thing before

xplshn commented 5 months ago

If the ascii version is the same exact size as the Kitty protocol version... How come the indentation is not correct?

Hmmmmmmm, is there any way to use Sixel? Is Chafa using its "polite" mode? (looking at chafafunc it seems that, no, there is no possible way to use Sixels in a term which supports Kitty imgs, maybe some ENV variables/config options could be checked to allow for choosing behavior?)

ravachol commented 5 months ago

The centering thing is to do with wezterm I think. Not the bug you reported.

ravachol commented 5 months ago

Let's leave the issue open. If anyone finds a surefire way to get this bug to happen, please share it.

ravachol commented 2 months ago

Closing this as we haven't been able to reproduce it. We also have another wezterm issue. I have opened an issue with them as well.