Update libavif #1381

aryanpingle · 2023-09-15T09:44:30Z

Update libavif for improved compression and speed
v1.0.0 deprecates usage of min and max-quantizers; we use quality and qualityAlpha instead
Renamed maxSpeed to MAX_EFFORT for better readability

* Update libavif for improved compression and speed * v1.0.0 deprecates usage of min and max-quantizers; we use `quality` and `qualityAlpha` instead * Renamed `maxSpeed` to `MAX_EFFORT` for better readability

surma

Minor nit to avoid magic numbers, otherwise LGTM!

src/features/encoders/avif/client/index.tsx

wantehchang

Hi Aryan,

I will review this pull request tomorrow. Please wait for my review.

wantehchang

Hi Aryan,

I reviewed all the files that look human readable. For avif_enc.cpp, I read the entire file.

I suggest some changes. Note that where I suggest the name qualityAlpha, you may replace it with alphaQuality. (qualityAlpha is the name used in avif/avif.h.)

codecs/avif/Makefile

codecs/avif/enc/avif_enc.cpp

src/features/encoders/avif/client/index.tsx

aryanpingle · 2023-09-27T05:24:02Z

@wantehchang I'll go through your reviews now, thanks for replying so quickly 😄

src/features/encoders/avif/client/index.tsx

Changes `cqlevel` to `quality`, and `cqAlphaLevel` to `qualityAlpha`

wantehchang · 2023-10-17T16:56:57Z

Hi Aryan,

I am receiving notifications of changes to this pull request. I am afraid that I may miss something, so please ping me when you'd like me to review this pull request again. Thanks.

aryanpingle · 2023-10-17T17:03:17Z

@wantehchang Yep I'm just resolving conversations as and when I push changes, maybe that's why you're getting so many notifications. My mid-terms just got over so I finally have some time to work on the PR - I'll send you a review request when I finish!

aryanpingle · 2023-10-18T16:55:37Z

@wantehchang thanks for your reviews, I've fixed and refactored the code you suggested. Also, you mentioned on the AV1 Discussion forum:

Off the top of my head, I think you can expose the AVIF_CHROMA_DOWNSAMPLING_SHARP_YUV option for avifImageRGBToYUV()

I went through the avif.h documentation for the avifChromaDownsampling options, I didn't understand why the automatic / fastest / best quality downsampling options are marked as (same as AVERAGE). Does this mean they all serve the same purpose as of now?

If so, then maybe a checkbox called "Enable sharp YUV chroma downsampling" can be used to toggle between AVIF_CHROMA_DOWNSAMPLING_AUTOMATIC and AVIF_CHROMA_DOWNSAMPLING_SHARP_YUV? Let me know what you think.

aryanpingle · 2023-10-28T07:50:00Z

@wantehchang I think we got AVIF_CHROMA_DOWNSAMPLING_SHARP_YUV working now! Here's the deploy preview.

wantehchang

Hi Aryan,

I read avif_enc.cpp carefully. I tried to read index.tsx and meta.ts as much as I could, but I only skimmed through the parts in index.tsx that you didn't modify.

codecs/avif/Makefile

wantehchang · 2023-10-28T23:10:36Z

codecs/avif/Makefile

-$(OUT_NODE_ENC_JS) $(OUT_NODE_ENC_MT_JS): ENVIRONMENT=node
-$(OUT_NODE_ENC_JS) $(OUT_ENC_JS): $(OUT_ENC_CPP) $(CODEC_DIR)/CMakeLists.txt $(LIBAOM_DIR)/CMakeLists.txt
+# ST-Encoding
+$(OUT_ENC_JS): $(OUT_ENC_CPP) $(CODEC_DIR)/CMakeLists.txt $(LIBAOM_DIR)/CMakeLists.txt $(LIBSHARPYUV)


I noticed you pass -DCONFIG_AV1_HIGHBITDEPTH=0 to libaom (e.g., in line 47). Do you only support bit depth 8 when encoding AVIF?

Yep, for all codecs on Squoosh.

codecs/avif/enc/avif_enc.cpp

src/features/encoders/avif/client/index.tsx

wantehchang · 2023-11-01T18:02:22Z

Aryan wrote:

I went through the avif.h documentation for the avifChromaDownsampling options, I didn't understand why the automatic / fastest / best quality downsampling options are marked as (same as AVERAGE). Does this mean they all serve the same purpose as of now?

Some of those "(same as AVERAGE)" should be changed to "(same as AVERAGE in the current implementation)". The reason is that the average filter is the only downsampling filter that libavif uses automatically. The Sharp YUV downsampling filter was added later and we are not sure if it should be one of the automatically-used downsampling filters.

NOTE: AVIF_CHROMA_DOWNSAMPLING_FASTEST will always be the same as AVERAGE.

If so, then maybe a checkbox called "Enable sharp YUV chroma downsampling" can be used to toggle between AVIF_CHROMA_DOWNSAMPLING_AUTOMATIC and AVIF_CHROMA_DOWNSAMPLING_SHARP_YUV? Let me know what you think.

Agreed.

wantehchang

LGTM! Just a few final minor issues.

wantehchang · 2023-11-01T18:07:51Z

codecs/avif/enc/avif_enc.cpp

+  do {                                    \
+    if (val1 == val2)                     \
+      return val::null();                 \
+  } while (false)


Optional: You can merge these two macros into RETURN_NULL_IF_FALSE that takes a conditional expression as the argument.

That's so much better than the current macro LOL, thank you!

Note: I probably would not use a macro that saves only one line. But some people like to use this kind of macro so that the main flow of the code is not cluttered with error handling code.

Yep that was my line of thinking. I'm pretty sure we'll be repeating that line again for other optimizations, so it's better to just have a macro for it.

codecs/avif/Makefile

codecs/avif/enc/avif_enc.cpp

wantehchang · 2023-11-01T18:28:08Z

codecs/avif/enc/avif_enc.cpp

+  // 100 = lossless
+  int quality;
+  // As above, but -1 means 'use quality'
+  int qualityAlpha;


In a follow-up pull request, we should look into using the new autoTiling option of the AVIF encoder.

I suggest Squoosh set autoTiling to AVIF_TRUE by default. (The default of autoTiling in libavif is AVIF_FALSE for backward compatibility.) The Squoosh UI can have a "manual tiling" box, which when checked, shows the tileRowsLog2 and tileColsLog2 fields (both default to 0, meaning no tiling).

Gotcha, I'll try to add it to this PR or at least make raise an issue for it. I assume autoTiling automatically chooses the best tileRowsLog2 and tileColsLog2, is there any more documentation about it that I can link to?

Unless this requires just simple changes, I suggest filing an issue for this and implementing this in a follow-up pull request. The review of this pull request has been going on for a while. We need to watch out for "code review fatigue."

You understand autoTiling correctly. The autoTiling algorithm may be tweaked in the future, so we didn't document the current algorithm in the public header avif/avif.h. You can find the current algorithm in libavif/src/write.c:

if (encoder->autoTiling) { // Use as many tiles as allowed by the minimum tile area requirement and impose a maximum // of 8 tiles. const int threads = 8; avifSetTileConfiguration(threads, firstCell->width, firstCell->height, &encoder->data->tileRowsLog2, &encoder->data->tileColsLog2); }

I do think all the automatic tiling algorithms have some common elements, such as imposing a minimum tile area or tile width and height, and potentially imposing a maximum number of tiles. But I am not sure if you need to mention this. You can, however, say that the use of tiles makes it easier for the decoder to decode an AVIF image with multiple threads in parallel.

codecs/avif/enc/avif_enc.cpp

aryanpingle · 2023-11-03T20:43:38Z

@wantehchang implemented the changes, please give it a once-over if you find the time.
Totally agree about the code review fatigue so I've made a new issue referencing that comment thread.

aryanpingle · 2023-11-03T20:48:09Z

Enabling the ERROR_ON_UNDEFINED_SYMBOLS=0 flag in the Makefile was a hacky solution to get rid of this error:

error: undefined symbol: setjmp (referenced by top-level compiled C/C++ code)
warning: Link with `-s LLD_REPORT_UNDEFINED` to get more information on undefined symbols
warning: To disable errors for undefined symbols use `-s ERROR_ON_UNDEFINED_SYMBOLS=0`
warning: _setjmp may need to be added to EXPORTED_FUNCTIONS if it arrives from a system library

It might be a compiler bug.
Regardless, @surma found that bumping the Emscripten version to 3.1.47 fixes this issue. Might be worth updating the version for all codec binaries.

wantehchang

LGTM!

z11h · 2024-01-03T05:14:52Z

any idea about when this will get merged and pushed to squoosh.app? thanks!

grandpa1946 · 2024-01-22T15:24:06Z

any idea about when this will get merged and pushed to squoosh.app? thanks!

you can always check at the bottom of the page where it says "changes approved" & you should click on "Details" to the right of deploy/netlify

jakearchibald · 2024-01-24T07:30:18Z

@wantehchang it won't block this PR, but is the experimental progressive stuff in avifenc worth exposing? Also, is there any compression benefit to premultiplying alpha, or is it just an input option?

wantehchang · 2024-01-25T00:10:03Z

@jakearchibald Hi Jake,

You can expose the encoding of progressive (layered) images if it is clearly marked as experimental in the UI.

There could be compression benefit to premultiplying alpha (see Note below). But the main benefit that after the image is decoded, we don't need to multiply it with alpha. We just need to multiply the backgroud with 1 - alpha. So the decoder has less work to do.

Note: If the alpha channel has a lot of zero or near-zero values, the alpha-premultiplied image will have a lot of zero or near-zero values, so it should result in a smaller compressed file.

jamsinclair · 2024-01-26T07:14:44Z

Seems like single threaded AVIF encoding may have broken with this change. From my limited debugging it looks like a memory issue. The output handle seems to be no longer available when passing the value back to JS? 🤔

TypeError: emval_handle_array[handle] is undefined

jakearchibald · 2024-01-26T08:33:15Z

@jamsinclair thank you. We'll look into it.

jakearchibald · 2024-01-26T08:34:18Z

@wantehchang Thanks for the details! Do any browsers support rendering of progressive AVIF? (as in, rendering before the file has fully downloaded)

wantehchang · 2024-01-26T19:57:36Z

@jakearchibald Jake: Chrome and Microsoft Edge (Edge 121 supports AVIF) support progressive rendering of AVIF layered images.

Firefox doesn't support progressive rendering of AVIF layered images, based on inspection of its source code (by searching for "layer" and "progressive" in image/decoders/nsAVIFDecoder.{h,cpp}).

I will get back to you on Safari's support.

leo-barnes · 2024-01-26T20:41:34Z

@jakearchibald Jake: Chrome and Microsoft Edge (Edge 121 supports AVIF) support progressive rendering of AVIF layered images.

Firefox doesn't support progressive rendering of AVIF layered images, based on inspection of its source code (by searching for "layer" and "progressive" in image/decoders/nsAVIFDecoder.{h,cpp}).

I will get back to you on Safari's support.

It should be supported in Safari to the best of my knowledge.

jakearchibald · 2024-01-26T20:44:25Z

Chrome support at least makes it interesting 😀

Source: GoogleChromeLabs/squoosh#1381

aryanpingle added 2 commits September 15, 2023 15:10

Update libavif (v1.0.0-main)

47f8746

* Update libavif for improved compression and speed * v1.0.0 deprecates usage of min and max-quantizers; we use `quality` and `qualityAlpha` instead * Renamed `maxSpeed` to `MAX_EFFORT` for better readability

Update libavif (v1.0.1-main)

f8804b1

aryanpingle changed the title ~~Update libavif (v1.0.0-main)~~ Update libavif Sep 15, 2023

surma approved these changes Sep 15, 2023

View reviewed changes

src/features/encoders/avif/client/index.tsx Outdated Show resolved Hide resolved

src/features/encoders/avif/client/index.tsx Outdated Show resolved Hide resolved

Refactor variable names for clarity

82658c7

wantehchang reviewed Sep 26, 2023

View reviewed changes

wantehchang reviewed Sep 27, 2023

View reviewed changes

src/features/encoders/avif/client/index.tsx Show resolved Hide resolved

src/features/encoders/avif/client/index.tsx Outdated Show resolved Hide resolved

Update libaom (v3.7.0)

642ac6d

aryanpingle force-pushed the avif-update branch from 29cc9bc to 642ac6d Compare September 28, 2023 14:20

aryanpingle added 3 commits October 17, 2023 13:57

Add checks for API return values

96da59f

Rename variables for readability

a5a3f63

Changes `cqlevel` to `quality`, and `cqAlphaLevel` to `qualityAlpha`

Minor patches in logic

f1f4030

aryanpingle added 2 commits October 18, 2023 22:03

Minor patches in lossless calculation

3ed34e1

Add chroma subsampling options to AVIF

625eee2

aryanpingle requested a review from wantehchang October 18, 2023 16:44

aryanpingle and others added 7 commits October 23, 2023 16:03

Add skeleton for sharp downsampling param

1b3f791

Try to use libsharpyuv

f27d524

Encoder working, decoder isnt

47cd47e

Make sure sharpyuv is disabled for decoder

25ea8b9

Add AVIF_LOCAL flags for sharp yuv

0413c66

Get AVIF sharp YUV working

ab79caf

Clean up AVIF makefiles

41c4b30

AVIF: Make sharpyuv conditional on subsample

faa1467

wantehchang reviewed Oct 29, 2023

View reviewed changes

aryanpingle added 4 commits October 31, 2023 01:50

AVIF: Flags to speed up sharpyuv build

fd53823

AVIF: Minor refactoring in enc.cpp

aad51d3

AVIF: Minor refactoring & renaming

699b9e0

AVIF: Use smart pointers to prevent memory leaks

4cdc246

wantehchang approved these changes Nov 1, 2023

View reviewed changes

aryanpingle added 2 commits November 4, 2023 01:50

AVIF: Minor refactoring

7099d5e

AVIF: Revert defaultoptions logic change

aa89d7c

aryanpingle mentioned this pull request Nov 3, 2023

Expose the AVIF autoTiling option #1387

Open

wantehchang approved these changes Nov 3, 2023

View reviewed changes

jamsinclair mentioned this pull request Jan 6, 2024

Improve @jquash/avif decode speed jamsinclair/jSquash#43

Open

Merge branch 'dev' into avif-update

795e395

jakearchibald merged commit e217740 into GoogleChromeLabs:dev Jan 24, 2024
7 checks passed

jamsinclair mentioned this pull request Jan 26, 2024

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

Closed

jamsinclair added a commit to jamsinclair/jSquash that referenced this pull request Apr 3, 2024

chore: implement upstream libavif upgrade changes

4cb9664

Source: GoogleChromeLabs/squoosh#1381

jamsinclair mentioned this pull request Apr 23, 2024

Upgrade AVIF library jamsinclair/jSquash#56

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update libavif #1381

Update libavif #1381

aryanpingle commented Sep 15, 2023

surma left a comment

wantehchang left a comment

wantehchang left a comment

aryanpingle commented Sep 27, 2023

wantehchang commented Oct 17, 2023

aryanpingle commented Oct 17, 2023

aryanpingle commented Oct 18, 2023

aryanpingle commented Oct 28, 2023

wantehchang left a comment

wantehchang Oct 28, 2023

aryanpingle Oct 29, 2023

wantehchang commented Nov 1, 2023

wantehchang left a comment

wantehchang Nov 1, 2023

aryanpingle Nov 2, 2023

wantehchang Nov 2, 2023

aryanpingle Nov 3, 2023

wantehchang Nov 1, 2023

aryanpingle Nov 2, 2023 •

edited

Loading

wantehchang Nov 2, 2023

aryanpingle commented Nov 3, 2023

aryanpingle commented Nov 3, 2023 •

edited

Loading

wantehchang left a comment

z11h commented Jan 3, 2024

grandpa1946 commented Jan 22, 2024

jakearchibald commented Jan 24, 2024 •

edited

Loading

wantehchang commented Jan 25, 2024

jamsinclair commented Jan 26, 2024

jakearchibald commented Jan 26, 2024

jakearchibald commented Jan 26, 2024

wantehchang commented Jan 26, 2024

leo-barnes commented Jan 26, 2024 •

edited

Loading

jakearchibald commented Jan 26, 2024

Update libavif #1381

Update libavif #1381

Conversation

aryanpingle commented Sep 15, 2023

surma left a comment

Choose a reason for hiding this comment

wantehchang left a comment

Choose a reason for hiding this comment

wantehchang left a comment

Choose a reason for hiding this comment

aryanpingle commented Sep 27, 2023

wantehchang commented Oct 17, 2023

aryanpingle commented Oct 17, 2023

aryanpingle commented Oct 18, 2023

aryanpingle commented Oct 28, 2023

wantehchang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wantehchang commented Nov 1, 2023

wantehchang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aryanpingle Nov 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aryanpingle commented Nov 3, 2023

aryanpingle commented Nov 3, 2023 • edited Loading

wantehchang left a comment

Choose a reason for hiding this comment

z11h commented Jan 3, 2024

grandpa1946 commented Jan 22, 2024

jakearchibald commented Jan 24, 2024 • edited Loading

wantehchang commented Jan 25, 2024

jamsinclair commented Jan 26, 2024

jakearchibald commented Jan 26, 2024

jakearchibald commented Jan 26, 2024

wantehchang commented Jan 26, 2024

leo-barnes commented Jan 26, 2024 • edited Loading

jakearchibald commented Jan 26, 2024

aryanpingle Nov 2, 2023 •

edited

Loading

aryanpingle commented Nov 3, 2023 •

edited

Loading

jakearchibald commented Jan 24, 2024 •

edited

Loading

leo-barnes commented Jan 26, 2024 •

edited

Loading