ImageOptim / gifski

GIF encoder based on libimagequant (pngquant). Squeezes maximum possible quality from the awful GIF format.
https://gif.ski
Other
4.83k stars 142 forks source link

Loop-count related build error with 32-bit toolchain #155

Closed robin2046 closed 3 years ago

robin2046 commented 3 years ago

Head is at

commit 000b40a2aef1e11af01f4be1feac5475984fcdca (HEAD -> main, origin/main, origin/HEAD)
Author: 0bmay <57501269+0bmay@users.noreply.github.com>
Date:   Thu Dec 17 09:53:08 2020 -0800

    Adds the ability to se the number of loops for an animation (#151)

    Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>

Tried to build with cargo ndk --platform 21 --target i686-linux-android build --lib --release

And got following error:

error[E0308]: mismatched types
  --> src/encodegifsicle.rs:84:54
   |
84 |                 Repeat::Finite(x) => gfs.loopcount = x as i64,
   |                                                      ^^^^^^^^ expected `i32`, found `i64`
   |
help: you can convert `gfs.loopcount` from `i32` to `i64`, matching the type of `x as i64`
   |
84 |                 Repeat::Finite(x) => i64::from(gfs.loopcount) = x as i64,
   |                                      ^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.
error: could not compile `gifski`.

64bit toolchain worked without any issue.

kornelski commented 3 years ago

Fixed.

robin2046 commented 3 years ago

Thanks a lot for the quick fix.

I managed to build the library release with cargo-ndk successfully (both .a and .so are generated for the common 4 archs). But when I tried to build it in my app with 32bit Android NDK toolchain I got another error:

src/main/cpp/gifski/libs/armeabi-v7a/libgifski.a(quantize.o):quantize.c:function colormap_image_floyd_steinberg: error: undefined reference to 'rand'
clang++: error: linker command failed with exit code 1 (use -v to see invocation)
ninja: build stopped: subcommand failed.

Trying to link with 32bit shared library libgifski.so gave the same error.

I googled a little bit and found it seems to be an infamous issue, a simple fix could be adding #include "stdlib.h" to those error files. However, how can I do it to quantize.c in this cargo build system? I am totally new to this fancy build system.

Please help to point out a solution, thanks a lot.

kornelski commented 3 years ago

The rand() call is from here:

https://github.com/ImageOptim/libimagequant/blob/197d141e37f7944f3a5a431a1ef828c3217f2661/kmeans.c#L64

which already includes <stdlib.h>.

It's built using this script:

https://github.com/ImageOptim/libimagequant/blob/197d141e37f7944f3a5a431a1ef828c3217f2661/rust-sys/build.rs#L55

but this is just a wrapper that in the end calls your regular C compiler (whatever it finds in the CC env var).

robin2046 commented 3 years ago

Thanks a lot for this comments, I finally get it built successfully by change NDK api level to 19 for 32bit targets.

By the way, I found with your new commit which updates the Rust gif crate, the output GIF is not correctly encoded. I always get decode error with the generated GIF (neither by my test code using libgifski.a, or by the executable binary generated directly by cargo build)

Use giflib to decode it and get error at:

DGifDecompressLine()

        if (CrntCode == EOFCode) {
            /* Note however that usually we will not be here as we will stop
             * decoding as soon as we got all the pixel, or EOF code will
             * not be read at all, and DGifGetLine/Pixel clean everything.  */
        GifFile->Error = D_GIF_ERR_EOF_TOO_SOON;

I also tried built an executable on my MACOS with the same PNG files, I also get un-decodable (with default preview app or Chrome) GIF results. I have to revert to 1.2.4 to get a correct result GIF. Please check if the new update breaks the GIF encoding routine. Thanks.