libvips / build-win64-mxe

73 stars 15 forks source link

Add aom and libheif to 'web' group dependencies for AVIF support #16

Closed lovell closed 3 years ago

lovell commented 3 years ago

Resolves #13

lovell commented 3 years ago

@kleisauke Thanks for adding the libheif.pc patch. (I finally managed to get libheif's CI to pass with that change.)

kleisauke commented 3 years ago

Great! I'm currently debugging a segfault in SSE2 code that happens on Windows i686 in combination with GCC (v10.2 and 9.3) and aom (master and v2.0.0). The binaries produced by LLVM works without any problems.

Details ``` Access violation - code c0000005 (first chance) First chance exceptions are reported before any exception handling. This exception may be expected and handled. eax=0d5d7e70 ebx=00000072 ecx=0321e4f8 edx=00000630 esi=0d5d7e70 edi=00000000 eip=6a450370 esp=0321e460 ebp=0321e460 iopl=0 nv up ei pl nz na pe nc cs=0023 ss=002b ds=002b es=002b fs=0053 gs=002b efl=00010206 libvips_42!aom_dc_left_predictor_8x16_sse2+0x10: 6a450370 660f6f01 movdqa xmm0,xmmword ptr [ecx] ds:002b:0321e4f8=72727272727272727272727272727272 ```

I suspect this is an assembler bug, I'm now trying to compile GCC v10.2 with binutils v2.35.1 as the binutils provided by MXE is old (v2.28, released on 2017-03-06). If that doesn't work, I think we need to disable the SIMD optimizations for i686 (i.e. compile with -DAOM_TARGET_CPU=generic).

lovell commented 3 years ago

Did you see aom's -DENABLE_SSE2=0 flag? https://aomedia.googlesource.com/aom/+/master/build/cmake/cpu.cmake#70

kleisauke commented 3 years ago

Ah, I missed that. Upgrading binutils to the latest version didn't help, unfortunately. I disabled SSE2 with commit 050e4eb but it seems that this also disables the remaining optimizations (possible due to that disable_remaining_flavors flag).

i686 now almost passes the test suite, it fails with the last heifsave test here: https://github.com/libvips/libvips/pull/1845/files#diff-ad635814429f3e0cbd2aa94557b49443R1083-R1090

Failed to initialize encoder: Memory allocation error

(this needs a patch for libheif to improve the error reporting, see: 21ae186)

kleisauke commented 3 years ago

Oh, I just noticed that position-independent code is disabled by default within aom, see: https://aomedia.googlesource.com/aom/+/master/build/cmake/aom_config_defaults.cmake#81

Perhaps forcing the -fPIC flags here: https://github.com/libvips/build-win64-mxe/blob/f415e83d7ffcc402bf0a2e5a2abdbcd0b4484896/build/settings/gcc.mk#L12-L13 could collide with the CONFIG_PIC variable in aom's CMake config. I'll have a try to build aom with -DCONFIG_PIC=1 (Firefox and Chrome also seems to force PIC for 32-bit builds) and revert that SSE2 change, perhaps that will resolve the above mentioned segfault.

lovell commented 3 years ago

Perhaps aom's Not Invented Here approach is part of their patent defence program? ;)

kleisauke commented 3 years ago

The SSE2 segfault turns out to be a stack alignment bug, commit https://github.com/libvips/build-win64-mxe/pull/16/commits/755c615cf3e18444364c8048c4e0491347c0b292 adds a patch to fix this. I also incorporated patches from dav1d/webp with commit https://github.com/libvips/build-win64-mxe/pull/16/commits/afdf49bf586c5c919ad237bc74d9685aec0bac2e and backported a security fix with commit https://github.com/libvips/build-win64-mxe/pull/16/commits/36bfc16ce65f5663923e1be1de5c99c02a042199.

Windows x86 still has a OOM failure on the heifsave test suite, I'm not sure if there is an easy way to fix that. x64 passes the test suite without problems. I think this is ready to be included for the libvips 8.10.2 binaries, are you OK with that?

lovell commented 3 years ago

Fantastic, thank you Kleis, I'll let you press the big green merge button when you're ready.

kleisauke commented 3 years ago

For the record, test suite currently fails on Windows ARM64 when checking the absolute difference between loading and saving the same AVIF image. I could also reproduce this on the command line (on Windows ARM64), when I do this:

$ vips copy avif-orientation-6.avif x2.avif[compression=av1]

Output: https://t0.nl/x2.avif Expected output: https://github.com/kleisauke/libvips/blob/testsuite-avif/test/test-suite/images/avif-orientation-6.avif

Anyway, it works fine on Linux ARM64 (see https://github.com/libvips/libvips/pull/1845/commits/c4f9bdcea35d42e2797b4bfeb2951c5eb1df588a and Travis build), so this is ready for merging.