Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LibGfx+animation: Only store changed pixels in animation frames #24288

Merged
merged 6 commits into from May 14, 2024

Conversation

nico
Copy link
Collaborator

@nico nico commented 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 breaks webp image on upload 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.)

@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label May 12, 2024
nico added 5 commits May 11, 2024 21:35
The function's implementation makes these assumptions, so check
that they are true instead of silently doing the wrong this when
they're not true.
Has (no-op) the effect of adding a few missing default-initializers for
the copy in WebPLoader.cpp.

No behavior change.
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 trflynn89 merged commit e2336e2 into SerenityOS:master May 14, 2024
10 checks passed
@github-actions github-actions bot removed the 👀 pr-needs-review PR needs review from a maintainer or community member label May 14, 2024
@nico nico deleted the webp-anim-inc branch May 14, 2024 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants