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

AVIF encoding is broken on browsers without multi-threaded support #1397

Closed
jamsinclair opened this issue Jan 26, 2024 · 13 comments · Fixed by #1409
Closed

AVIF encoding is broken on browsers without multi-threaded support #1397

jamsinclair opened this issue Jan 26, 2024 · 13 comments · Fixed by #1409

Comments

@jamsinclair
Copy link
Contributor

jamsinclair commented Jan 26, 2024

Describe the bug
When the browser does not support SharedArrayBuffer/Multi-threading, Squoosh fails to encode AVIF images.

To Reproduce
Steps to reproduce the behavior:

  1. Clone the repository
  2. Remove the Cross-Origin-Opener-Policy and Cross-Origin-Embedder-Policy to force the dev environment to use the single threaded logic
  3. Run npm run dev
  4. Open the dev page at http://localhost:5000
  5. Upload an image. Choose the AVIF compress option.
  6. Observe the following error is thrown in the console and the image is not encoded to AVIF
Uncaught (in promise) RuntimeError: indirect call to null
    invoke_iiiiiiiiii http://localhost:5000/c/avif_enc-8f7d01d5.js:10
    invoke_iii http://localhost:5000/c/avif_enc-8f7d01d5.js:10
    encode http://localhost:5000/c/avif_enc-8f7d01d5.js line 10 > Function:11
    encode http://localhost:5000/c/features-worker-2348e279.js:276

Expected behavior
The image would still be encoded to AVIF with single threaded browser.

Version:

  • OS w/ version: MacOS Sonoma 14.1.1
  • Browser w/ version: Chrome 120.0.6099.234
  • Node version: 16 LTS
  • npm version: 8.14.0

Additional context, screenshots, screencasts
This issue was introduced in the recent update of the AVIF version #1381

@jakearchibald
Copy link
Collaborator

Thanks for the report. I'm trying to figure out the impact of this bug. How did you encounter it? Is there an environment where this is impacting users that aren't deliberately disabling features in their browser?

@jamsinclair
Copy link
Contributor Author

jamsinclair commented Jan 26, 2024

Thanks @jakearchibald. I didn't expect such a prompt reply. I think the impact of this bug is quite small, it's definitely low priority.

How did you encounter it?

I was just playing around with it 😅 . I have a hobby project that shares these codecs as individual ES modules (with licenses and correct attributions).

Is there an environment where this is impacting users that aren't deliberately disabling features in their browser?

No, I don't think that's possible. A small number of older browsers might be affected, but I doubt you see much traffic for them on squoosh.app.

@jakearchibald
Copy link
Collaborator

Makes sense. We'll work on a fix, but it doesn't seem to warrant a rollback.

Thanks for playing around with the project and finding the issue!

@AshleyScirra
Copy link

Oof, just lost a couple of hours to trying to figure out why avif_enc.wasm (the single threaded version) wasn't working in a different project - this looks like the cause. The previous version seems to work fine. For the benefit of anyone else searching, the error message I get is null function or function signature mismatch.

@aryanpingle
Copy link
Contributor

Quick way to reproduce this - In avifEncode.ts, change:

if (await checkThreadsSupport()) {
  const avifEncoder = await import('codecs/avif/enc/avif_enc_mt');
  return initEmscriptenModule<AVIFModule>(avifEncoder.default);
}

to:

if (false && await checkThreadsSupport()) {
  const avifEncoder = await import('codecs/avif/enc/avif_enc_mt');
  return initEmscriptenModule<AVIFModule>(avifEncoder.default);
}

which enables only single-threaded encoding.

image

image


Looking into this 👍

@tyaqing
Copy link

tyaqing commented Apr 2, 2024

I have encountered the same problem.

@aryanpingle
Copy link
Contributor

@wantehchang I've narrowed this issue down to this line in avif_enc.cpp:

avifResult encodeResult = avifEncoderWrite(encoder.get(), image.get(), &output);

encoder.get() and image.get() are not returning null pointers, so I believe it's something with the avifEncoderWrite function itself. Any idea what it might be?

@wantehchang
Copy link

Aryan: We will need to debug it. You can start by logging the return value of avifEncoderWrite(). If the return value is not AVIF_RESULT_OK, log the diagnostic message encoder->diag.error.

@jamsinclair
Copy link
Contributor Author

I could be doing something wrong, but when I enabled some debugging flags like -g it seems to have helped reveal some of the method names in the stack trace. The last 5 calls being:

indirect call to null
av1_predict_intra_block@http://localhost:9000/6e2d8511161e40ca5061.wasm:wasm-function[485]:0x57f65
av1_predict_intra_block_facade@http://localhost:9000/6e2d8511161e40ca5061.wasm:wasm-function[487]:0x58444
intra_model_rd@http://localhost:9000/6e2d8511161e40ca5061.wasm:wasm-function[920]:0x108514
av1_rd_pick_intra_mode_sb@http://localhost:9000/6e2d8511161e40ca5061.wasm:wasm-function[949]:0x11c2b6
pick_sb_modes@http://localhost:9000/6e2d8511161e40ca5061.wasm:wasm-function[1126]:0x184c60

It seems to fail before the AVIF_RESULT_OK check and there's no string data for encoder->diag.error.

I am sure there's more debugging to be done, but perhaps there's something up with how av1 is being compiled?

Full stack trace
indirect call to null
av1_predict_intra_block@http://localhost:9000/6e2d8511161e40ca5061.wasm:wasm-function[485]:0x57f65
av1_predict_intra_block_facade@http://localhost:9000/6e2d8511161e40ca5061.wasm:wasm-function[487]:0x58444
intra_model_rd@http://localhost:9000/6e2d8511161e40ca5061.wasm:wasm-function[920]:0x108514
av1_rd_pick_intra_mode_sb@http://localhost:9000/6e2d8511161e40ca5061.wasm:wasm-function[949]:0x11c2b6
pick_sb_modes@http://localhost:9000/6e2d8511161e40ca5061.wasm:wasm-function[1126]:0x184c60
av1_rd_pick_partition@http://localhost:9000/6e2d8511161e40ca5061.wasm:wasm-function[1149]:0x193397
av1_rd_pick_partition@http://localhost:9000/6e2d8511161e40ca5061.wasm:wasm-function[1149]:0x193e49
av1_rd_pick_partition@http://localhost:9000/6e2d8511161e40ca5061.wasm:wasm-function[1149]:0x193e49
av1_rd_pick_partition@http://localhost:9000/6e2d8511161e40ca5061.wasm:wasm-function[1149]:0x193e49
av1_encode_sb_row@http://localhost:9000/6e2d8511161e40ca5061.wasm:wasm-function[1050]:0x15ab46
av1_encode_tile@http://localhost:9000/6e2d8511161e40ca5061.wasm:wasm-function[1044]:0x1565ba
encode_frame_internal@http://localhost:9000/6e2d8511161e40ca5061.wasm:wasm-function[1161]:0x1b3b93
av1_encode_frame@http://localhost:9000/6e2d8511161e40ca5061.wasm:wasm-function[1160]:0x1af4d1
encode_with_recode_loop_and_filter@http://localhost:9000/6e2d8511161e40ca5061.wasm:wasm-function[1469]:0x20f322
av1_encode@http://localhost:9000/6e2d8511161e40ca5061.wasm:wasm-function[1179]:0x1c739d
av1_encode_strategy@http://localhost:9000/6e2d8511161e40ca5061.wasm:wasm-function[1170]:0x1c0550
invoke_iiiiiiiiii@webpack://with-webpack-example/../../packages/avif/dist/codec/enc/avif_enc.js?:3410:36
av1_get_compressed_data@http://localhost:9000/6e2d8511161e40ca5061.wasm:wasm-function[1042]:0x15598f
invoke_iii@webpack://with-webpack-example/../../packages/avif/dist/codec/enc/avif_enc.js?:3421:36
encoder_encode@http://localhost:9000/6e2d8511161e40ca5061.wasm:wasm-function[1504]:0x220a15
aom_codec_encode@http://localhost:9000/6e2d8511161e40ca5061.wasm:wasm-function[1435]:0x1fcdfc
aomCodecEncodeImage@http://localhost:9000/6e2d8511161e40ca5061.wasm:wasm-function[142]:0x13524

@jamsinclair
Copy link
Contributor Author

Aha! Well that was a fun one to debug 😅

Looks like the issue was with the compilation of libsharpyuv. We need to add -DCMAKE_DISABLE_FIND_PACKAGE_Threads=1 in the makefile for single thread builds. Single thread encodes work successfully with that flag.

Do we need to compile different versions of libsharpyuv? Or would it be ok to use a single threaded copy for both?

@surma
Copy link
Collaborator

surma commented Apr 4, 2024

Brilliant work, @jamsinclair. Thanks so much.

We should definitely compile a separate copy of libsharpyuv, as we might have to add more permutations in the future. In fact, IIRC, we already do this for libaom in the helper.Makefile.

Do you have the time and would you be willing to whip up a PR for the fix at hand?

@wantehchang
Copy link

@jamsinclair wrote:

Looks like the issue was with the compilation of libsharpyuv. We need to add -DCMAKE_DISABLE_FIND_PACKAGE_Threads=1 in the makefile for single thread builds.

Please try libsharpyuv's (libwebp's) cmake option -DWEBP_USE_THREAD=OFF instead.

@jzern FYI.

@jamsinclair
Copy link
Contributor Author

Do you have the time and would you be willing to whip up a PR for the fix at hand?

Sure thing, I'll whip that up later 🖖

Please try libsharpyuv's (libwebp's) cmake option -DWEBP_USE_THREAD=OFF instead.

Thank you! I'll give that a go.

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 a pull request may close this issue.

7 participants