Support changing frame size during encoding #1069
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Yuan: I just merged my cleanup pull request #1070. Please rebase this pull request and then I will take a look at the crash. Thanks. |
tongyuantongyu
force-pushed
the
var-frame-size
branch
from
b46dfa9 to
054b138
Compare
tongyuantongyu
force-pushed
the
var-frame-size
branch
from
054b138 to
864c34e
Compare
|
I tried to run asan and here is the report: The function in question seems related to ssim, but if I set |
wantehchang
requested changes
Sep 1, 2022
There was a problem hiding this comment.
Yuan: I reviewed everything except the avifRWStreamWrite parts of src/write.c and the decoder part of the new test in avifchangesettingtest.cc. I believe your changes to src/codec_aom.c. So this looks like a libaom bug that certain arrays are allocated to the size of the initial values of cfg.g_w and cfg.g_h.
src/write.c
Outdated
| lastEncoder->minQuantizer = encoder->minQuantizer; | ||
| lastEncoder->maxQuantizer = encoder->maxQuantizer; | ||
| lastEncoder->minQuantizerAlpha = encoder->minQuantizerAlpha; | ||
| lastEncoder->maxQuantizerAlpha = encoder->maxQuantizerAlpha; | ||
| lastEncoder->tileRowsLog2 = encoder->tileRowsLog2; | ||
| lastEncoder->tileColsLog2 = encoder->tileColsLog2; | ||
| lastEncoder->speed = encoder->speed; |
There was a problem hiding this comment.
Delete this. This line has been moved to line 334.
| @@ -1046,6 +1046,9 @@ 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. | |||
| // * Width and height: width and height of encoded image. Default value 0 means infer from first frame. | |||
| // For grid image, this is the size of one cell. Value must not be smaller than the largest frame | |||
| // to be encoded. | |||
There was a problem hiding this comment.
I need to think about what this value means for grid images.
There was a problem hiding this comment.
This is the easiest choice: encoder->width and encoder-height overrides image->width and image->height, if present.
Alternatively this can be defined as size of the whole grid, and we verify and compute size of each cell internally.
src/write.c
Outdated
| imageWidth = imageMetadata->width * item->gridCols; | ||
| imageHeight = imageMetadata->height * item->gridRows; | ||
| imageWidth = imageWidth * item->gridCols; | ||
| imageHeight = imageHeight * item->gridRows; |
There was a problem hiding this comment.
Optional: Should we use the *= notation?
imageWidth *= item->gridCols;
imageHeight *= item->gridRows;
There was a problem hiding this comment.
This looks better. Done.
|
Yuan: Please test this libaom patch. It is likely to be incomplete (e.g., the VMAF and BUTTERAUGLI code may also need the same changes). |
|
I discussed this issue with my colleague Marco Paniconi, who works on real-time encoding and SVC in libaom. Marco told me that they don't use Although AVIF allows the images in an image sequence to have different sizes, the libavif encoder does not need to support that. (The libavif decoder supports that.) |
tongyuantongyu
commented
Sep 2, 2022
There was a problem hiding this comment.
Please test this libaom patch
Thaks for the patch. I confirm that after patch libaom encodes correctly.
let libaom downscale the frames internally for certain spatial layers
Actually I'm planning to support both. Use libaom internal scaler is more convenient for basic usage with only one input image, but if user wants more flexibility (e.g. using small blurred "thumbnail" as first layer) input image of different size can be both more efficient and convenient.
Earlier this year I reported two bugs in libaom's internal scaler. I see aomedia:3210 is fixed by a recent commit https://aomedia-review.googlesource.com/c/aom/+/161961. For aomedia:3203 I have an old CL, but I can't figure out how to update it because it's targeting master branch, so I created a new one instead.
| @@ -1046,6 +1046,9 @@ 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. | |||
| // * Width and height: width and height of encoded image. Default value 0 means infer from first frame. | |||
| // For grid image, this is the size of one cell. Value must not be smaller than the largest frame | |||
| // to be encoded. | |||
There was a problem hiding this comment.
This is the easiest choice: encoder->width and encoder-height overrides image->width and image->height, if present.
Alternatively this can be defined as size of the whole grid, and we verify and compute size of each cell internally.
# Conflicts: # include/avif/avif.h # src/write.c
tongyuantongyu
force-pushed
the
var-frame-size
branch
from
3eeb8c1 to
4011553
Compare
tongyuantongyu
force-pushed
the
var-frame-size
branch
from
4011553 to
d7fe311
Compare
|
With the latest tip of libaom all added test cases passes.
AVIF use case is different here: we want to use different images for each layer (like the low resolution blurry preview example I mentioned) - instead of sending the same frame many times to encode scalable video. So the SVC approach puts user into an awkward situation: they need to upscale the first layer to match frame size, and then libaom downscales it back. This is inefficient, and upscale-downscale roundtrip can be lossy. |
tongyuantongyu
force-pushed
the
var-frame-size
branch
from
7a7bbf1 to
4761eb5
Compare
tongyuantongyu
force-pushed
the
var-frame-size
branch
from
4761eb5 to
5e208db
Compare
# Conflicts: # include/avif/avif.h # src/codec_aom.c # src/codec_rav1e.c # src/write.c
cyh5272
pushed a commit
to cyh5272/aom
that referenced
this pull request
May 6, 2024
Based on the libavif avifchangedimensiontest AOMDecreasing/ChangeDimensionTest.EncodeDecode/0 in AOMediaCodec/libavif#1069 by Yuan Tong. Test: test_libaom --gtest_filter=*DimensionDecreasing \ --gtest_also_run_disabled_tests Bug: aomedia:3348 Change-Id: I8be3c092234f99402c02aa03340c916f629ce0f2
cyh5272
pushed a commit
to cyh5272/aom
that referenced
this pull request
May 6, 2024
Based on the libavif avifchangedimensiontest AOMDecreasing/ChangeDimensionTest.EncodeDecode/0 in AOMediaCodec/libavif#1069 by Yuan Tong. Test: test_libaom --gtest_filter=*DimensionDecreasing \ --gtest_also_run_disabled_tests Bug: aomedia:3348 Change-Id: I79aea87012cc3bea43d565911ac88816c860e737
cyh5272
pushed a commit
to cyh5272/aom
that referenced
this pull request
May 6, 2024
Based on the libavif avifchangedimensiontest AOMDecreasing/ChangeDimensionTest.EncodeDecode/0 in AOMediaCodec/libavif#1069 by Yuan Tong. Test: test_libaom --gtest_filter=*DimensionDecreasing \ --gtest_also_run_disabled_tests Bug: aomedia:3348 Change-Id: I79aea87012cc3bea43d565911ac88816c860e737
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the second part for #761.
@wantehchang, I wrote a simple test
ChangeSettingTest.ChangeDimensionthat encodes a 256x256 frame following by a 512x512 frame, with seq header dimension set to 512x512, but it crashes in theaom_codec_destroycall. Could you take a look if there's something wrong in my usage, or it's a bug in libaom?