Discord breaks webp image on upload #159
Comments
|
(It'd generally be nice if animated webp would work like animated gif.) |
|
Also, it looks like the EXIF chunk is added at the front. Per https://developers.google.com/speed/webp/docs/riff_container#extended_file_format that's invalid: EXIF must follow image data (which for animated files is a sequence of ANMF chunks). |
nico
added a commit
to nico/serenity
that referenced
this issue
May 12, 2024
For example, for 7z7c.gif, we now store one 500x500 frame and then a 94x78 frame at (196, 208) and a 91x78 frame at (198, 208). This reduces how much data we have to store. We currently store all pixels in the rect with changed pixels. We could in the future store pixels that are equal in that rect as transparent pixels. When inputs are gif files, this would guaranteee that new frames only have at most 256 distinct colors (since GIFs require that), which would help a future color indexing transform. For now, we don't do that though. The API I'm adding here is a bit ugly: * WebPs can only store x/y offsets that are a multiple of 2. This currently leaks into the AnimationWriter base class. (Since we potentially have to make a webp frame 1 pixel wider and higher due to this, it's possible to have a frame that has <= 256 colors in a gif input but > 256 colors in the webp, if we do the technique above.) * Every client writing animations has to have logic to track previous frames, decide which of the two functions to call, etc. This also adds an opt-out flag to `animation`, because: 1. Some clients apparently assume the size of the last VP8L chunk is the size of the image (see discord/lilliput#159). 2. Having incremental frames is good for filesize and for playing the animation start-to-end, but it makes it hard to extract arbitrary frames (have to extract all frames from start to target frame) -- but this is mean tto be a delivery codec, not an editing codec. It's also more vulnerable to corrupted bytes in the middle of the file -- but transport protocols are good these days. (It'd also be an idea to write a full frame every N frames.) For https://giphy.com/gifs/XT9HMdwmpHqqOu1f1a (an 184K gif), output webp size goes from 21M to 11M. For 7z7c.gif (an 11K gif), output webp size goes from 2.1M to 775K. (The webp image data still isn't compressed at all.)
nico
added a commit
to nico/serenity
that referenced
this issue
May 12, 2024
For example, for 7z7c.gif, we now store one 500x500 frame and then a 94x78 frame at (196, 208) and a 91x78 frame at (198, 208). This reduces how much data we have to store. We currently store all pixels in the rect with changed pixels. We could in the future store pixels that are equal in that rect as transparent pixels. When inputs are gif files, this would guaranteee that new frames only have at most 256 distinct colors (since GIFs require that), which would help a future color indexing transform. For now, we don't do that though. The API I'm adding here is a bit ugly: * WebPs can only store x/y offsets that are a multiple of 2. This currently leaks into the AnimationWriter base class. (Since we potentially have to make a webp frame 1 pixel wider and higher due to this, it's possible to have a frame that has <= 256 colors in a gif input but > 256 colors in the webp, if we do the technique above.) * Every client writing animations has to have logic to track previous frames, decide which of the two functions to call, etc. This also adds an opt-out flag to `animation`, because: 1. Some clients apparently assume the size of the last VP8L chunk is the size of the image (see discord/lilliput#159). 2. Having incremental frames is good for filesize and for playing the animation start-to-end, but it makes it hard to extract arbitrary frames (have to extract all frames from start to target frame) -- but this is mean tto be a delivery codec, not an editing codec. It's also more vulnerable to corrupted bytes in the middle of the file -- but transport protocols are good these days. (It'd also be an idea to write a full frame every N frames.) For https://giphy.com/gifs/XT9HMdwmpHqqOu1f1a (an 184K gif), output webp size goes from 21M to 11M. For 7z7c.gif (an 11K gif), output webp size goes from 2.1M to 775K. (The webp image data still isn't compressed at all.)
trflynn89
pushed a commit
to SerenityOS/serenity
that referenced
this issue
May 14, 2024
For example, for 7z7c.gif, we now store one 500x500 frame and then a 94x78 frame at (196, 208) and a 91x78 frame at (198, 208). This reduces how much data we have to store. We currently store all pixels in the rect with changed pixels. We could in the future store pixels that are equal in that rect as transparent pixels. When inputs are gif files, this would guaranteee that new frames only have at most 256 distinct colors (since GIFs require that), which would help a future color indexing transform. For now, we don't do that though. The API I'm adding here is a bit ugly: * WebPs can only store x/y offsets that are a multiple of 2. This currently leaks into the AnimationWriter base class. (Since we potentially have to make a webp frame 1 pixel wider and higher due to this, it's possible to have a frame that has <= 256 colors in a gif input but > 256 colors in the webp, if we do the technique above.) * Every client writing animations has to have logic to track previous frames, decide which of the two functions to call, etc. This also adds an opt-out flag to `animation`, because: 1. Some clients apparently assume the size of the last VP8L chunk is the size of the image (see discord/lilliput#159). 2. Having incremental frames is good for filesize and for playing the animation start-to-end, but it makes it hard to extract arbitrary frames (have to extract all frames from start to target frame) -- but this is mean tto be a delivery codec, not an editing codec. It's also more vulnerable to corrupted bytes in the middle of the file -- but transport protocols are good these days. (It'd also be an idea to write a full frame every N frames.) For https://giphy.com/gifs/XT9HMdwmpHqqOu1f1a (an 184K gif), output webp size goes from 21M to 11M. For 7z7c.gif (an 11K gif), output webp size goes from 2.1M to 775K. (The webp image data still isn't compressed at all.)
|
@nico Thank you for reporting this issue, lots of great detail :) I'll take a look at this more closely in the next week. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
wow.zip
The webp file in the attached zip is an animated lossless webp that displays fine in all browsers I've tried.
If I upload the webp file in the attached zip, Discord:
a) fails to show it
b) turns it into a file that no longer renders in browsers
The webp has different rects for different animation frames – new frames don't store data that's identical to old frames.
For b), it looks like lilliput takes the width and height of the last animation frame and writes that into the vp8x header. That is incorrect, since the last frame is smaller than the animation.
For animated images, ideally the original size from the vp8x header would be written to the output unmodified. Failing that, the dimensions of the first animation frame are probably a better guess than the dimensions of the last animation frame.
It also looks like the flags byte in the vp8x header isn't quite right. On input, they are
0x12. In the file that comes back it's0x0a.0xameans "has exif, has animation". Discord added an (empty) EXIF chunk, so it's fine that that bit gets added. But the0x10bit, which meanshas alpha, got cleared. This animation definitely has alpha.If I manually fix up the dimensions in the vp8x header, the image starts playing in browsers. But e.g. Safari no longer shows the transparent pixels as transparent, because this bit got cleared.
The text was updated successfully, but these errors were encountered: