Chatterino / chatterino2

Chat client for https://twitch.tv
MIT License
2.07k stars 448 forks source link

3rd party emotes can hit memory limit #4145

Open kornes opened 2 years ago

kornes commented 2 years ago

Checklist

Describe your issue

chatterino image size check is failing for some emotes because of width * height * frames * 4 formula doesn't reflect real size too accurately (which is correct as explained by Nerixyz below) https://github.com/Chatterino/chatterino2/blob/8fa89b40731c169e52a386e29f9bd6971fee7ae2/src/messages/Image.cpp#L501

example: https://7tv.app/emotes/6347ab8f8b11c5b2c95e5a39 loads on 2x (861 KB) but fails to load 3x for tooltip (1.3 MB)

chatterino.http: "GET [CACHED] 200 https://cdn.7tv.app/emote/6347ab8f8b11c5b2c95e5a39/3x.webp"
chatterino.image: image too large in RAM

138 96 492 (frames) * 4 = 26 072 064 Image::maxBytesRam is set to 20 971 520

i think we could just use cached file size instead? and maybe add some width/height safety constraints

Screenshots

No response

OS and Chatterino Version

Chatterino 2.3.5 DEBUG (commit 8fa89b40 modified) built with Qt 5.15.6, Windows SDK, MSVC 192930147

Nerixyz commented 2 years ago

because width * height * frames * 4 formula doesn't reflect real size too accurately [...] i think we could just use cached file size instead?

I don't think this is the right approach. The formula is used because that's how images are stored in memory. Each uncompressed frame has width * height pixels, so width * height * 4 bytes for all channels (RGBA), thus width * height * 4 * frames bytes to store the image sequence in memory.

Maybe there could be a setting to adjust the maximum image size, with a tooltip warning users of higher memory usage. Chatterino should ideally show the smaller version in the emote-tooltip if the larger version doesn't fit in the limit.

To be clear: This isn't a problem with 7TV itself, you can get the same with BTTV emotes (for example https://betterttv.com/emotes/6370bd6bb9076d0aaebbdea9). It's just that on 7TV, the limits aren't as strict as on BTTV.

pajlada commented 2 years ago

https://doc.qt.io/qt-6/qimagereader.html#allocationLimit might come in handy when we're on Qt6

kornes commented 2 years ago

wrongly assumed frames are not uncompressed without reading the code, my bad double checked sum of frame.sizeInBytes() for said emote and it really takes 26 MB of ram

QImage has few 24bit formats https://doc.qt.io/qt-5/qimage.html#Format-enum I tested converting each frame to QImage::Format_ARGB8565_Premultiplied and results doesn't look outright bad while saving 25% memory per pixel (26 MB -> 19.55 MB), might be worth further testing for side effects (probably cpu/ram trade).

Other thing i found is ETC2_EAC compression for openGL textures https://www.qt.io/blog/2018/05/07/compressed-textures-in-qt-5-11 which is said to use 1 byte per pixel. But that would be basically rework of whole emotes rendering, although there is QOpenGLWidget which looks promising.

fourtf commented 2 years ago

The limit was set in place for images in the link preview which are displayed much larger than emotes in chat. E.g. a greatly compressed PNG image can turn out to be a 100000×100000px single color image taking more RAM than the user has available (although only being a couple kb compressed). The fact that regular emotes from 7tv (although in 3×) are hitting that limit is alarming. If you have multiple of those in channels it can be a real issue. ~500 frames for an emote seems excessive. Perhaps it would be a good idea to limit gif emotes to 2× in chat to reduce usage.

Furthermore changing the image format might be a decent way to reduce the ram usage by a bit as long as it's ensured that the images don't lose transparency and not unreasonably much quality.

kornes commented 2 years ago

i did some tests last week, so dont have screens at hand now, but:

alpha worked fine in both cases (tested on windows).

Worth to note that qt docs says rendering of some formats is more optimized:

Rendering is best optimized to the Format_RGB32 and Format_ARGB32_Premultiplied formats, and secondarily for rendering to the Format_RGB16, Format_RGBX8888, Format_RGBA8888_Premultiplied, Format_RGBX64 and Format_RGBA64_Premultiplied formats

if above would be confirmed to not be a problem, all frames could be stored in 3-bytes format and we could add "low quality" setting to use 2 bytes format, disabled by default but maybe auto enabled for low RAM PCs (8gb or lower).