search

AOMediaCodec / av1-avif

AV1 Image File Format Specification - ISO-BMFF/HEIF derivative
https://aomediacodec.github.io/av1-avif/
BSD 2-Clause "Simplified" License
463 stars 40 forks source link

Fix issue 150 #202

Closed leo-barnes closed 3 weeks ago

leo-barnes commented 1 year ago

Microsoft/Chimera_cropped.avif:

Link-U/kimono*.crop.avif:

This closes issue #150

leo-barnes commented 1 month ago

@wantehchang @y-guyon Do you want to sanity check the files? If not I'll merge this PR tomorrow.

y-guyon commented 1 month ago

I ran avifdec at https://github.com/AOMediaCodec/libavif/commit/3e4b9c88761aab919cbf98377447b7315dd834b3 compiled with

    "AVIF_ENABLE_EXPERIMENTAL_YCGCO_R": "ON",
    "AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP": "ON",
    "AVIF_ENABLE_EXPERIMENTAL_MINI": "ON",
    "AVIF_ENABLE_EXPERIMENTAL_SAMPLE_TRANSFORM": "ON",

It fails on kimono.crop.avif, kimono.mirror-vertical.rotate270.crop.avif and Chimera_8bit_cropped_480x256.avif with the error:

ERROR: Failed to parse image: BMFF parsing failed
Diagnostics:
 * [Strict] crop rect Y offset must be even due to this image's YUV subsampling

It works on Chimera_10bit_cropped_to_1920x1008.avif.

It fails on Chimera_10bit_cropped_to_1920x1008_with_HDR_metadata.avif with the error:

Conversion to RGB failed
maryla-uc commented 1 month ago

I looked into Chimera_10bit_cropped_to_1920x1008_with_HDR_metadata.avif which gives the Conversion to RGB failed error. This is because the image has matrix coeffecients value 10, i.e. AVIF_MATRIX_COEFFICIENTS_BT2020_CL, which is currently unsupported https://github.com/AOMediaCodec/libavif/blob/3e4b9c88761aab919cbf98377447b7315dd834b3/src/reformat.c#L108 avifdec tries to convert from YUV to RGB to save the output image and fails. So the file is valid, it's just that avifdec doen't handle it.

leo-barnes commented 3 weeks ago

It fails on kimono.crop.avif, kimono.mirror-vertical.rotate270.crop.avif and Chimera_8bit_cropped_480x256.avif with the error:

ERROR: Failed to parse image: BMFF parsing failed
Diagnostics:
 * [Strict] crop rect Y offset must be even due to this image's YUV subsampling

Ah. This is the old restriction on grid/clap odd offset/dimensions when subsampled. This got relaxed in the latest MIAF spec due to the issues it caused for AVIF.

@y-guyon: We should probably open an issue for libavif to relax this check. The new MIAF text says this:

—   If any of the following conditions hold true, the image is first implicitly upsampled to 4:4:4:
    —   chroma is subsampled horizontally (i.e., 4:2:2 and 4:2:0) and cleanApertureWidth is odd
    —   chroma is subsampled horizontally (i.e., 4:2:2 and 4:2:0) and left-most pixel is on an odd position
    —   chroma is subsampled vertically (i.e., 4:2:0) and cleanApertureHeight is odd
    —   chroma is subsampled vertically (i.e., 4:2:0) and topmost line is on an odd position
y-guyon commented 3 weeks ago

Ah. This is the old restriction on grid/clap odd offset/dimensions when subsampled. This got relaxed in the latest MIAF spec due to the issues it caused for AVIF.

Should the MIAF version in the AVIF spec be bumped before this goes into effect?

It does not even seem versioned at the moment:

https://github.com/AOMediaCodec/av1-avif/blob/aa906e00c9d8655a8b97718aa16d379970678663/index.bs#L65

leo-barnes commented 3 weeks ago

I don't think we're versioning any of the specs at the moment. I guess there are pros and cons to both approaches, but in general we try to make sure that MIAF changes don't cause issues for dependent specs.

wantehchang commented 3 weeks ago

@leo-barnes wrote:

@y-guyon: We should probably open an issue for libavif to relax this check. The new MIAF text says this:

— If any of the following conditions hold true, the image is first implicitly upsampled to 4:4:4:
    — chroma is subsampled horizontally (i.e., 4:2:2 and 4:2:0) and cleanApertureWidth is odd
    — chroma is subsampled horizontally (i.e., 4:2:2 and 4:2:0) and left-most pixel is on an odd position
    — chroma is subsampled vertically (i.e., 4:2:0) and cleanApertureHeight is odd
    — chroma is subsampled vertically (i.e., 4:2:0) and topmost line is on an odd position

Is the new MIAF text you quoted in

leo-barnes commented 3 weeks ago

I was looking at ISO/IEC FDIS 23000-22, but can't remember if it actually got added to the earlier amendments or not.

(I also thought the latest edition had proceeded further than the current stage... ISO is kind of slow.)

wantehchang commented 3 weeks ago

Thank you, Leo. Since the two amendments were published in 2021, they do not have the new text you quoted in https://github.com/AOMediaCodec/av1-avif/pull/202#issuecomment-2339926253.

wantehchang commented 3 weeks ago

@leo-barnes Leo: These two conditions for implicitly upsampling to 4:4:4 seem too strong for AVIF, because AV1 allows odd widths for 4:2:2 and 4:2:0 and odd heights for 4:2:0:

—   If any of the following conditions hold true, the image is first implicitly upsampled to 4:4:4:
    —   chroma is subsampled horizontally (i.e., 4:2:2 and 4:2:0) and cleanApertureWidth is odd
    ...
    —   chroma is subsampled vertically (i.e., 4:2:0) and cleanApertureHeight is odd
    ...
y-guyon commented 3 weeks ago

These two conditions for implicitly upsampling to 4:4:4 seem too strong for AVIF, because AV1 allows odd widths for 4:2:2 and 4:2:0 and odd heights for 4:2:0

That seemed strange to me too, there is usually no issue with odd dimensions. However it matters for the offset coordinates so I did not think further about it.

leo-barnes commented 3 weeks ago

These two conditions for implicitly upsampling to 4:4:4 seem too strong for AVIF, because AV1 allows odd widths for 4:2:2 and 4:2:0 and odd heights for 4:2:0

That seemed strange to me too, there is usually no issue with odd dimensions. However it matters for the offset coordinates so I did not think further about it.

Pedantic interpretation: While it's true that AV1 supports odd dimensions for 4:2:X, the AV1 spec doesn't really tell you how to do further cropping of subsampled content (at least I assume so given that AV1 does not internally have cropping). I assume that AV1 will internally pad odd subsampled content, but something like that can't be assumed at the container level. So this kind of still applies to AV1.

Pragmatically: The spec doesn't say how to do upsampling. So an implementation that decides to implement the above snippet for handling a crop with even offset and odd dimensions for AV1 would be perfectly permitted in implicitly upsampling with nearest neighbor, doing the crop and then downsampling again.

wantehchang commented 3 weeks ago

@leo-barnes The issue of how to crop subsampled content to odd widths or heights also occurred to me. I worked out the details and concluded that the naive way to do that works, whether we will upsample by nearest neighbor or bilinear interpolation after cropping naively. Note: With clap, we crop the decoded image, so it does not matter whether AV1 internally pads odd subsampled content.

I understand why MIAF has those conditions on cleanApertureWidth and cleanApertureHeight. And I agree with you that the wording "the image is first implicitly upsampled to 4:4:4" leaves room for interpretation. We can take advantage of that as you outlined.

wantehchang commented 3 weeks ago

@leo-barnes I just realized that I assumed the "center" chroma sample position in my analysis. I am not sure if my conclusions also apply if the chroma sample position is "left" or "top left".