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

WebP support #7

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

WebP support #7

wants to merge 4 commits into from

Conversation

inorichi
Copy link
Contributor

@inorichi inorichi commented Apr 3, 2018

This PR adds support for WebP images (animated too) and updates gradle and the SDK to the latest version available.

Motivation: some sites may be using webp images, and Skia already supports this, so I thought why not include it here too. I'll accept it if you don't want to support webp though :).

I've split this PR into as many commits as possible to help reviewing. I can remove some of them if desired or move to a different PR.

This PR uses the whole libwebp library (tag v0.6.1). I haven't found a "decoding only" fork, but the folders enc and mux could be deleted alongside some files inside dsp and utils. Those are not compiled anyways so it's probably not a problem.

The complete function in image_webp is unfinished, but I don't understand yet how it works (I saw in png you clear the frames' buffer starting from the second frame and start over, but I still don't understand it).

Also, I'm a bit concerned about memory management in webp_decode but I think it's fine.

@seven332
Copy link
Owner

seven332 commented Apr 4, 2018

It may take seconds to decode an animated image. That make users wait too much time.

My solution

  1. decode the first frame
  2. display the first frame
  3. decode other frames
  4. start animation

The step 3 is the complete function.

@seven332
Copy link
Owner

seven332 commented Apr 4, 2018

Please open a separate PR to update gradle and android sdk.

@inorichi
Copy link
Contributor Author

inorichi commented Apr 4, 2018

Done.

I'll try later to finish the complete function, and I can also give a try to #3 while I'm at it (in a separate PR).

@msm595
Copy link
Contributor

msm595 commented Aug 12, 2018

Any updates with this PR?

@inorichi
Copy link
Contributor Author

I'm not working on it yet because the image structures are going to change (#14) and the reply I got on #12.

Feel free to take over this PR, after all you should know better than me how to integrate it with the modular approach.

@inorichi
Copy link
Contributor Author

I ended up rebasing the PR, however performance in release mode is 2x slower than Skia which also uses libwebp, so I think neon optimizations or cpu features are not working properly.

I've tried to improve performance but the result was always the same. This also happened with the old Android.mk build instead of CMake... I'm out of ideas.

@seven332
Copy link
Owner

It helps.

bool webp_decode_buffer(...) {
    ...
    webp_config.options.use_cropping = clip;
    ...
    webp_config.options.use_scaling = ratio != 1;
    ...
}

@inorichi
Copy link
Contributor Author

Indeed... that was the reason, now I've got the same results as Skia. Thanks!

@seven332
Copy link
Owner

seven332 commented Sep 4, 2018

NEON is enabled on armeabi-v7a without runtime detection. But not all armeabi-v7a CPUs support NEON. Adding a cflag WEBP_HAVE_NEON_RTCD should be enough.

Please check here and here.

@seven332 seven332 mentioned this pull request May 20, 2019
@arkon
Copy link

arkon commented Jul 18, 2020

Was there any plans to merge this in?

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

4 participants