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

Fix issue 150 #202

Merged
merged 1 commit into from
Sep 10, 2024
Merged

Fix issue 150 #202

merged 1 commit into from
Sep 10, 2024

Conversation

leo-barnes
Copy link
Collaborator

@leo-barnes leo-barnes commented Nov 14, 2022

Microsoft/Chimera_cropped.avif:

  • Truncated crop to integers
  • Stripped optional config OBUs

Link-U/kimono*.crop.avif:

  • Reordered property associations so 'ispe' comes before transform properties
  • Rounded up crop to integers (that matches the PNGs)
  • Added NCLX with values 1,13,9,0
  • Stripped optional config OBUs

This closes issue #150

Microsoft/Chimera_*_cropped_*.avif:
- Truncated crop to integers
- Stripped optional config OBUs

Link-U/kimono*.crop.avif:
- Reordered property associations so 'ispe' comes before transform properties
- Rounded up crop to integers (that matches the PNGs)
- Added NCLX with values 1,13,9,0
- Stripped optional config OBUs
@leo-barnes
Copy link
Collaborator Author

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

@y-guyon
Copy link
Collaborator

y-guyon commented Aug 21, 2024

I ran avifdec at AOMediaCodec/libavif@3e4b9c8 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
Copy link

maryla-uc commented Aug 21, 2024

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.

@podborski podborski added the conformance Related to conformance suite. E.g. file updates, asserts, etc. label Aug 28, 2024
@leo-barnes
Copy link
Collaborator Author

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

@leo-barnes leo-barnes merged commit 27cce80 into master Sep 10, 2024
@leo-barnes leo-barnes deleted the u/lbarnes/fix_clap_content branch September 10, 2024 09:03
@y-guyon
Copy link
Collaborator

y-guyon commented Sep 10, 2024

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:

"isoNumber": "ISO/IEC DIS 23000-22"

@leo-barnes
Copy link
Collaborator Author

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
Copy link
Collaborator

@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
Copy link
Collaborator Author

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
Copy link
Collaborator

Thank you, Leo. Since the two amendments were published in 2021, they do not have the new text you quoted in #202 (comment).

@wantehchang
Copy link
Collaborator

@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
Copy link
Collaborator

y-guyon commented Sep 12, 2024

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
Copy link
Collaborator Author

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
Copy link
Collaborator

@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
Copy link
Collaborator

@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".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conformance Related to conformance suite. E.g. file updates, asserts, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants