search

awxkee / jxl-coder-coil

JPEG XL Decoder for Coil Android
Apache License 2.0
10 stars 1 forks source link

Crash when loading JXL #7

Open RobbWatershed opened 18 hours ago

RobbWatershed commented 18 hours ago

Tried to load multiple JXL files on an API29 device, using Coil 3.0.2 - I get a similar crash everytime :

A  Fatal signal 4 (SIGILL), code 1 (ILL_ILLOPC), fault addr 0x7210d2c1f8 in tid 734 (DefaultDispatch), pid 32148

Some of the sample files I tried : https://www.earth.org.uk/img/boiler/boiler-portrait-posterised-interlaced-256w.png.jxl https://github.com/libjxl/conformance/raw/refs/heads/master/testcases/animation_icos4d/input.jxl https://github.com/libjxl/conformance/raw/refs/heads/master/testcases/cafe/input.jxl

awxkee commented 15 hours ago

Thanks for the info. I can't actually reproduce this one, are you using emulator on x86 device? Or is it real android ARM device?

RobbWatershed commented 6 hours ago

It's a physical ARM device (hisilicon kirin 710, to be perfectly precise).

My app uses Coil to load a couple other file formats without any problem. I just added JxlDecoder.Factory() at init time just like you wrote on the readme.

Gotta test on an x86 emu to see what happens...

RobbWatershed commented 6 hours ago

It does the same thing on an emu hosted by a x86 PC.

RobbWatershed commented 4 hours ago

If that's of any help, images aren't loaded from their URL but loaded from the device's storage. And yes, I'm certain of my credentials and URIs.

RobbWatershed commented 4 hours ago

1/ I just found out that adding .allowHardware(false) to the Coil ImageLoader builder fixes the issue for still (non-animated) JXL images.

Is that mandatory to set .allowHardware(false)? If so , could you update your readme?

2/ I still get that same crash when trying to load animated JXLs.

awxkee commented 4 hours ago

Could you, please, also confirm that behaviour are reproducible in the sample app here that I just commited or from CI ImageToolbox

awxkee commented 4 hours ago

1/ I just found out that adding .allowHardware(false) to the Coil ImageLoader builder fixes the issue for still (non-animated) JXL images.

Is that mandatory to set .allowHardware(false)? If so , could you update your readme?

2/ I still get that same crash when trying to load animated JXLs.

Ah, ok. No .allowHardware(false) is not mandatory and should work.

Thanks for the info, I think I know what to check at least.

This is surprising since all HarwareBuffers API that I'm using should be available from 29 API, but on your device looks like it is not available.

RobbWatershed commented 4 hours ago

Could you, please, also confirm that behaviour are reproducible in the sample app here that I just commited or from CI ImageToolbox

I installed image-toolbox-3.1.0-rc01-foss-arm64-v8a-release-unsigned-signed.apk on my device and tested Tools > JXL Tools > JXL to JPEG

I confirm that trying to load a still or an animated JXL does crash the app.

PS : I love what you've done for the UI ❤️

awxkee commented 3 hours ago

Also, I'd like to ask run adb shell cat /proc/cpuinfo and paste here output

RobbWatershed commented 3 hours ago

Also, I'd like to ask run adb shell cat /proc/cpuinfo and paste here output

Processor       : AArch64 Processor rev 4 (aarch64)
processor       : 0
BogoMIPS        : 3.84
Features        : fp asimd evtstrm aes pmull sha1 sha2 crc32 cpuid
CPU implementer : 0x41
CPU architecture: 8
CPU variant     : 0x0
CPU part        : 0xd03
CPU revision    : 4

processor       : 1
BogoMIPS        : 3.84
Features        : fp asimd evtstrm aes pmull sha1 sha2 crc32 cpuid
CPU implementer : 0x41
CPU architecture: 8
CPU variant     : 0x0
CPU part        : 0xd03
CPU revision    : 4

processor       : 2
BogoMIPS        : 3.84
Features        : fp asimd evtstrm aes pmull sha1 sha2 crc32 cpuid
CPU implementer : 0x41
CPU architecture: 8
CPU variant     : 0x0
CPU part        : 0xd03
CPU revision    : 4

processor       : 3
BogoMIPS        : 3.84
Features        : fp asimd evtstrm aes pmull sha1 sha2 crc32 cpuid
CPU implementer : 0x41
CPU architecture: 8
CPU variant     : 0x0
CPU part        : 0xd03
CPU revision    : 4

processor       : 4
BogoMIPS        : 3.84
Features        : fp asimd evtstrm aes pmull sha1 sha2 crc32 cpuid
CPU implementer : 0x41
CPU architecture: 8
CPU variant     : 0x0
CPU part        : 0xd09
CPU revision    : 2

processor       : 5
BogoMIPS        : 3.84
Features        : fp asimd evtstrm aes pmull sha1 sha2 crc32 cpuid
CPU implementer : 0x41
CPU architecture: 8
CPU variant     : 0x0
CPU part        : 0xd09
CPU revision    : 2

processor       : 6
BogoMIPS        : 3.84
Features        : fp asimd evtstrm aes pmull sha1 sha2 crc32 cpuid
CPU implementer : 0x41
CPU architecture: 8
CPU variant     : 0x0
CPU part        : 0xd09
CPU revision    : 2

processor       : 7
BogoMIPS        : 3.84
Features        : fp asimd evtstrm aes pmull sha1 sha2 crc32 cpuid
CPU implementer : 0x41
CPU architecture: 8
CPU variant     : 0x0
CPU part        : 0xd09
CPU revision    : 2

Hardware        : Hisilicon Kirin710
awxkee commented 2 hours ago

Thanks! I think real issue is here, that fphp ( half precision float conversion ) cpu feature is missed. It is mandatory in android aarch64 specification so NDK always compiles everything like it is exists on CPU. So I think issue here probably not a hardware buffer but _RGBAF16 type, even I can do checks in my code on fphp if it is exists and do software emulation if it is not, I cannot completely guarantee then that libjxl won’t hit missing instructions. Even in the very best case that I do all the checks, there are jxl images encoded in ‘half precision float’ as a storage type, and they cannot be ever opened on such devices, because libjxl actually compiled with meaning that fphp is available.

awxkee commented 2 hours ago

And your crash is actually signaling that not existing CPU instructions is hit to be clear.

RobbWatershed commented 2 hours ago

Oh yeah, ILL_ILLOPC = illegal operation I guess.

I just ran adb shell cat /proc/cpuinfo on the emu and fphp is also missing from flags.

If fphp is mandatory in android aarch64, then why does Android Studio's emulator miss that instruction as well?

awxkee commented 1 hour ago

If fphp is mandatory in android aarch64, then why does Android Studio's emulator miss that instruction as well?

I don't know to be honest, fphp mandatory aarch64 part from 2017. Perhaps no one just worried about it, until AI time has come. Android emulator just uses qemu, it is possible to launch with any features, and my emulator works with fphp ( what is also strange ).

Anyway, I think I'll do soon all the checks, and software emulation and this perhaps should help.

RobbWatershed commented 1 hour ago

Thanks for taking the time to investigate this 😄

awxkee commented 1 hour ago

Yep, thanks for information, this is always surprising to know that something that must exists do not exists. I’ll try to recompile libjxl with forcing disable on half floats availability but since is libhwy is under the hood I doubt in success, but I’ll let you know.