discord / lilliput

Resize images and animated GIFs in Go
https://discord.com/blog/how-discord-resizes-150-million-images-every-day-with-go-and-c
Other
1.96k stars 123 forks source link

Discord breaks webp image on upload #159

Closed nico closed 2 months ago

nico commented 6 months ago

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's 0x0a.

0xa means "has exif, has animation". Discord added an (empty) EXIF chunk, so it's fine that that bit gets added. But the 0x10 bit, which means has 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.

nico commented 6 months ago

(It'd generally be nice if animated webp would work like animated gif.)

nico commented 6 months ago

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).

skidder commented 6 months ago

@nico Thank you for reporting this issue, lots of great detail :) I'll take a look at this more closely in the next week.

nico commented 5 months ago

Let me know if I can help answer any questions. I know a bit about webp format internals.

skidder commented 3 months ago

Quick update, I'm making some improvements & fixes to Lilliput's support for animated image formats, and hope to cover this animated WebP issue as part of that work in the coming weeks.

6446plm commented 3 months ago

كيف اديرها

skidder commented 3 months ago

@nico I too was able to reproduce this issue consistently in Discord. Although Lilliput currently lacks support for transformations of animated WebP content, Discord should not be corrupting animated WebP files uploaded by users.

The root-cause of the corruption is actually external to Lilliput. Discord uses the Piexif library to remove EXIF data from media uploads. The corruption stems from two defects I found in Piexif:

I will close this issue out once those defects have been corrected in our fork of Piexif.

skidder commented 2 months ago

@nico this has been fixed in Discord, you should now see animated WebP uploads presented as a single frame in Discord, and viewing the original animated WebP in browser (or any other WebP viewer) should work. We're working on full animated WebP support in Discord, soon 😄

Thanks again for raising this, and please file an issue should anything else come up!

nico commented 2 months ago

Super cool, thanks! I can confirm that uploaded animated webps are no longer getting corrupted.

Looking forward to full support! :)

nico commented 2 months ago

(ps: Is there an issue I could follow to get updates for full support?)

skidder commented 2 months ago

(ps: Is there an issue I could follow to get updates for full support?)

Definitely! Earlier this week I merged in full support for animated WebP support to Lilliput, and we're in the process of integrating those changes with the rest of Discord infrastructure.

I plan on partial integration of animated WebP (a subset of use-cases (e.g. only attachments), likely as an experiment) by end of September. Naturally we want to be cautious rolling out such a big change, as well as ensure we can measure any changes in performance (client & server). Most images generated by Discord are non-animated WebP, and it's important that we not break that behavior.

The Discord engineering blog has a monthly patch-notes post with recent changes. Animated WebP support will be reported in the 'Media' section. I'll also try to tag you on this Github issue once fully rolled out.

Thanks again!

skidder commented 1 month ago

@nico fyi you should see animated WebP attachments display on web & Discord desktop clients! We plan to enable retrieval of animated WebP attachments on mobile clients very soon. Please open an issue against Lilliput if you see any issues or areas to improve.

goodusername123 commented 1 month ago

@skidder Hello. 3 weeks ago when animated WebP support was rolled out to desktop clients it functioned almost flawlessly even after testing it for an hour or so against a few various test cases it came out on top with the only sole issue I could notice being they would not animate in the "GIF" picker, However a few days or so later support for animated WebPs became crippled in functionality with them proceeding to only animate if they where a directly uploaded attachment in a message and would no longer function when linked to and also lost the favorite star button ability as well. an example of a version of Discord that worked fine was stable 331573 (1886297) while an example of a version that does not function is stable 333307 (1c1ea47) (maybe not exactly when it started) and anything else recent.

Is this new less/barely functional animated WebP support intended? or has full animated WebP support been temporarily disabled until support is ready for mobile clients in which case a full rollout across platforms is planned maybe?

skidder commented 1 month ago

@goodusername123 first off, thank you for the feedback - this is a new capability, and hearing from users helps ensure things work intuitively and correctly.

Here's a list of the issues I see:

  1. Linked animated WebP attachments don't animate
  2. Animated WebP attachments cannot be favorited for the GIF picker
  3. Previously, animated WebP attachments in GIF picker didn't animate

As for how/why things regressed, with the initial rollout of animated WebP support we didn't signal to the Discord client whether it's an animated or still WebP. Previously the Discord client could assume all WebP files were still, which is no longer the case. That led to all static WebP files being shown as animated, even those that were still, which isn't right.

We've since begun signaling to the client whether a WebP attachment is animated, and the client requests an animated WebP by including the animated=true query parameter; without that parameter, a still WebP is returned. This relates to the first bug listed above, where links need to include the animated=true query parameter to animate.

I totally acknowledge all of these, and we'll tackle them all soon. Our first goal with this change was to support animated WebP attachments that had been completely unsupported (aside from just showing a still image). We're in a good spot there, but these client-facing issues remain.

The Discord client app is a complex bit of software, and these client facing changes are nuanced and can take time to uncover. 😅

I assure you, we'll fix these issues in short order, and I appreciate you reporting them!