Layered encoding support #1100

tongyuantongyu · 2022-09-12T08:14:34Z

This is the third part of #761.

Layered image can be encoded by calling avifEncoderAddImage() multiple times, one layer per call.

Rational to keep avifScalingMode: In very early version of Support for Progressive AVIF encoding #761 avifScalingMode is simply an alias of libaom's AOM_SCALING_MODE, and I later changed it to a struct representing a fraction. As @joedrago said directly using what libaom offers is not so good as libavif also supports other codecs, and available AOM_SCALING_MODE-s are also rather some arbitrary selected values instead of the only values supported by AV1. We may define some commonly used avifScalingMode as static constants for convenience, anyway.
AVIF_ADD_IMAGE_FLAG_LAYER: I found that I don't need this flag. We know whether we are dealing layer image by checking encoder->extraLayerCount.
Encoding of layered grid do works, and libavif decoder also decodes it correctly, but if you are concerned about corner cases, we can disable it for now.

@wantehchang:
This PR currently rely on libaom to scale the layers internally, so it's restricted to the modes defined by AOM_SCALING_MODE. Also there's still a bug in libaom's internal scaler that it doesn't scale YUV420 images with 4n+2 dimensions correctly. But other than that it looks good. Also once #1069 is done, directly encoding layers of different dimensions should also work.

y-guyon

Thanks for this PR.

Encoding of layered grid do works, and libavif decoder also decodes it correctly, but if you are concerned about corner cases, we can disable it for now.

Let's keep them and rely on fuzzing to detect issues.

y-guyon · 2022-09-12T09:45:50Z

include/avif/avif.h

@@ -1026,6 +1034,8 @@ struct avifCodecSpecificOptions;
 //   image in less bytes. AVIF_SPEED_DEFAULT means "Leave the AV1 codec to its default speed settings"./
 //   If avifEncoder uses rav1e, the speed value is directly passed through (0-10). If libaom is used,
 //   a combination of settings are tweaked to simulate this speed range.
+// * Extra layer: [0 - AVIF_MAX_AV1_LAYER_COUNT-1]. Non-zero value indicates a layered (progressive)
+//   image.


I suggest [0..AVIF_MAX_AV1_LAYER_COUNT[ or [0:AVIF_MAX_AV1_LAYER_COUNT[ or [0..AVIF_MAX_AV1_LAYER_COUNT) or [0:AVIF_MAX_AV1_LAYER_COUNT) or [0..AVIF_MAX_AV1_LAYER_COUNT - 1] or [0:AVIF_MAX_AV1_LAYER_COUNT - 1]

Same in src/write.c

I'm following style of other fields above. Do you think [0 - AVIF_MAX_AV1_LAYER_COUNT) is OK?

I'm following style of other fields above.

Ah right, I missed these.

Do you think [0 - AVIF_MAX_AV1_LAYER_COUNT) is OK?

Yes.

include/avif/avif.h

src/codec_aom.c

src/write.c

tests/gtest/avifprogressivetest.cc

y-guyon · 2022-09-13T08:27:42Z

tests/gtest/avifprogressivetest.cc

+    }
+
+    // TODO Check decoder->image and image are similar,
+    //  and better quality layer is more similar.


Extra space

This space tells my IDE that this line is the continuation of the TODO above. I can remove it anyway.

I suggest to align the text then:

// TODO Check decoder->image and image are similar, // and better quality layer is more similar.

y-guyon · 2022-09-13T09:04:44Z

src/write.c

-    }
-    if (memcmp(&lastEncoder->heightScale, &encoder->heightScale, sizeof(avifScalingMode)) != 0) {
-        *encoderChanges |= AVIF_ENCODER_CHANGE_HEIGHT_SCALE;
+    if (memcmp(&lastEncoder->scaleMode, &encoder->scaleMode, sizeof(avifScalingMode)) != 0) {


Note: This is a false positive in case lastEncoder->scaleMode = { {1, 1}, {1, 1} } and lastEncoder->scaleMode = { {2, 2}, {3, 3} } but this is not important, leave it as is.

src/write.c

wantehchang · 2022-09-15T16:21:14Z

Yannis: Thank you for reviewing this pull request. I'd like to review this PR, too. Please wait for my review.

Yuan: Could you confirm that this PR does not require #1069?

tongyuantongyu · 2022-09-15T16:44:03Z

Yuan: Could you confirm that this PR does not require #1069?

Yes I can confirm this PR does not require #1069, as the unit tests shows. I believe real usage wants #1069 as well, anyway.

Originally reported by Yuan Tong in AOMediaCodec#1100.

Originally reported by Yuan Tong in #1100.

wantehchang

LGTM. Thank you very much for your contribution.

I did the code review at https://aomedia-review.googlesource.com/c/libavif/+/167643.

wantehchang · 2023-01-20T20:08:44Z

tests/gtest/avifprogressivetest.cc

+TEST_F(ProgressiveTest, DimensionChange) {
+  if (avifLibYUVVersion() == 0) {
+    GTEST_SKIP() << "libyuv not available, skip test.";
+  }


@tongyuantongyu Yuan: I guess the reason we need to skip this test when libyuv is not available is because this test needs the avifImageScale() function, which is only implemented if libyuv is available. Correct?

Yes.
I just noticed the assertion failure when using libaom head as well. Seems like a regression after v3.5.0 release.

I have only tested with libaom v3.5.0 and v3.6.0-rc1. If it's a regression, I can do a git bisect.

git bisect says:

fbebe9d771f485272d6dd9f3829d9389160d89e1 is the first bad commit commit fbebe9d771f485272d6dd9f3829d9389160d89e1 Author: chiyotsai <[email protected]> Date: Mon May 23 13:31:41 2022 -0700 RESIZE_MODE: Fix incorrect strides being used for motion search Currently during motion search, the encoder always assumes that the ref frame's stride is the same as that of source frame. But this assumption breaks when resize mode/superres feature is turned on, leading to unexpected motion search result and invalid memory access. This commit actively checks for the ref stride with what's stored in MotionVectorSearchParams::search_site_config, and creates a new search_site_config if there is a mismatch. BUG=aomedia:3283 Change-Id: Ia99a88326bf716027b5a652ec519fb13cfa0a345 av1/av1.cmake | 1 + av1/encoder/block.h | 10 ++++ av1/encoder/encoder.c | 13 ----- av1/encoder/mcomp.c | 2 + av1/encoder/mcomp.h | 111 ++++++++++--------------------------- av1/encoder/mcomp_structs.h | 83 +++++++++++++++++++++++++++ av1/encoder/motion_search_facade.c | 35 ++++++++---- av1/encoder/motion_search_facade.h | 22 ++++++++ av1/encoder/nonrd_pickmode.c | 9 ++- av1/encoder/temporal_filter.c | 5 +- 10 files changed, 182 insertions(+), 109 deletions(-) create mode 100644 av1/encoder/mcomp_structs.h

But that is the commit that added the assertion that fails. I will ask Chi Yo (the author of that commit) about that assertion.

y-guyon reviewed Sep 12, 2022

View reviewed changes

y-guyon approved these changes Sep 13, 2022

View reviewed changes

y-guyon mentioned this pull request Nov 10, 2022

Support for Progressive AVIF encoding #761

Draft

wantehchang added a commit to wantehchang/libavif that referenced this pull request Jan 11, 2023

Fix two comment typos

6ac89b5

Originally reported by Yuan Tong in AOMediaCodec#1100.

wantehchang mentioned this pull request Jan 11, 2023

Fix two comment typos #1262

Merged

wantehchang added a commit to wantehchang/libavif that referenced this pull request Jan 11, 2023

Fix two comment typos

b8281f6

Originally reported by Yuan Tong in AOMediaCodec#1100.

wantehchang added a commit that referenced this pull request Jan 12, 2023

Fix two comment typos

e1c7b0b

Originally reported by Yuan Tong in #1100.

Layered encoding support

18651e1

tongyuantongyu force-pushed the layered_encoding branch from 2bf0c5d to 57542d6 Compare January 20, 2023 13:26

extras

9604e4b

tongyuantongyu force-pushed the layered_encoding branch from 57542d6 to 9604e4b Compare January 20, 2023 13:34

skip dimension change test if libyuv not available

343db27

wantehchang approved these changes Jan 20, 2023

View reviewed changes

wantehchang merged commit 5d16f1f into AOMediaCodec:main Jan 20, 2023

wantehchang reviewed Jan 20, 2023

View reviewed changes

tongyuantongyu deleted the layered_encoding branch September 13, 2023 12:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Layered encoding support #1100

Layered encoding support #1100

tongyuantongyu commented Sep 12, 2022

y-guyon left a comment

y-guyon Sep 12, 2022

tongyuantongyu Sep 13, 2022

y-guyon Sep 13, 2022

y-guyon Sep 13, 2022

tongyuantongyu Sep 13, 2022

y-guyon Sep 13, 2022

tongyuantongyu Sep 13, 2022

y-guyon Sep 13, 2022

wantehchang commented Sep 15, 2022

tongyuantongyu commented Sep 15, 2022

wantehchang left a comment

wantehchang Jan 20, 2023

tongyuantongyu Jan 21, 2023

wantehchang Jan 21, 2023

wantehchang Jan 23, 2023

Layered encoding support #1100

Layered encoding support #1100

Conversation

tongyuantongyu commented Sep 12, 2022

y-guyon 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wantehchang commented Sep 15, 2022

tongyuantongyu commented Sep 15, 2022

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