jstkdng / ueberzugpp

Drop in replacement for ueberzug written in C++
GNU General Public License v3.0
797 stars 30 forks source link

Kitty output seems broken when using kitty-0.30.0 #132

Closed ionenwks closed 1 year ago

ionenwks commented 1 year ago

Using ueberzugpp-2.9.1, displays no image with the (just released) kitty-0.30.0 and works normally if go back to kitty-0.29.2 or use other outputs like chafa and x11 (tested on Gentoo + Xorg).

First ran into this with ytfzf , but could reproduce with a simple:

ueberzug layer -o kitty
{"action":"add","identifier":"preview","max_height":0,"max_width":0,"path":"test.jpg","x":0,"y":0}

To be sure it wasn't entirely broken, also tried kitten icat test.jpg and ranger-1.9.3's kitty image previews, both were fine.

ueberzugpp itself gives no error, but can catch kitty giving (in the background if catching its output) a lot of repeated:

[261 09:01:48.535320] [PARSE ERROR] Failed to parse GraphicsCommand command payload with error: output buffer for base64_decode too small

Have not dug deeper into this, unsure if issue is in kitty or in the way ueberzugpp handles it that just happened to surface as an issue now.

kovidgoyal commented 1 year ago

This will be because the kitty graphics protocol command payloads are not chunked into pieces <= 4096 bytes as required by the protocol. kitty became a bit stricter about this in the latest release. Note that 4096 is th elimit on the size of the base64 encoded data not the size of the pre-encoding data. That means you first base64 encode the data then you split up the result of doing that into chunks of at most 4096 bytes

See https://sw.kovidgoyal.net/kitty/graphics-protocol/#remote-client

jstkdng commented 1 year ago

That means you first base64 encode the data then you split up the result of doing that into chunks of at most 4096 bytes

huh, currently ueberzugpp does it the other way around, it splits the image into 4096 byte chunks and then encodes those chunks. Does kitty rebuild the whole base64 file?

kovidgoyal commented 1 year ago

On Mon, Sep 18, 2023 at 11:02:21AM -0700, JustKidding wrote:

That means you first base64 encode the data then you split up the result of doing that into chunks of at most 4096 bytes

huh, currently ueberzugpp does it the other way around, it splits the image into 4096 byte chunks and then encodes those chunks. Does kitty rebuild the whole base64 file?

The protocol requires every chunk to be <= 4096 bytes long. That means the base64 encoded payload must be at most 4096 bytes. What youa re doing will result in chunks that are larger than 4096 bytes which is illegal as per the spec.

kovidgoyal commented 1 year ago

And to be clear you dont need to first base64 encode if you dont want to. The size of base64 encoded data is deterministic on the size of the input data, so you can chunk up the input data accordingly.

jstkdng commented 1 year ago

yeah, chunks of 3068 bytes result in <= 4096 bytes of base64 data. The issue should be fixed on master. Will make another release later.

jstkdng commented 1 year ago

thanks for the help @kovidgoyal , before you go, I've been wanting to support animated gifs with kitty but the existing documentation is kinda confusing and wasn't able to achieve it. Do you have/know some examples?

kovidgoyal commented 1 year ago

The kitty icat kitten supports animated gifs, you can see its source code for how it does it. I believe timg also supports them, dont recall if chafa does as well.

kovidgoyal commented 1 year ago

icat/native.go should be the simplest code path to follow. That converts GIFs into frames and see icat/transmit.go for how the frames are converted into graphics protocol commands.