gpac / ComplianceWarden

A pluggable compliance checker (ISOBMFF, HEIF/MIAF/AVIF, AV1 HDR10+)
https://gpac.github.io/ComplianceWarden-wasm/
Other
16 stars 7 forks source link

HEIF - Validator should reject files that have essential bit set on descriptive properties other than those permitted #15

Closed cconcolato closed 1 year ago

cconcolato commented 4 years ago

There are essentially 3 classes of properties regarding the essential bit:

rbouqueau commented 4 years ago

No problem to add it. Where is this specified?

cconcolato commented 4 years ago

The following indicates that transformative properties can be essential or not:

NOTE When an image item has non-essential transformative item properties, the pixel values of the output image might depend on the reader's capability and/or choice of applying the non-essential transformative item properties.

The following indicates covers most descriptive properties:

Descriptive properties are non-essential, unless stated otherwise in their specification.

and this covers the rest: decoder configurations:

essential shall be equal to 1 for an 'hvcC' item property associated with an image item of type 'hvc1'. essential shall be equal to 1 for an 'lhvC' item property associated with an image item of type 'lhv1'. essential shall be equal to 1 for an 'avcC' item property associated with an image item of type 'avc1'. essential shall be equal to 1 for an 'jpgC' item property associated with an image item of type 'jpeg'.

and other descriptive properties:

essential shall be equal to 1 for an 'lsel' item property. essential shall be equal to 1 for an 'tols' item property.

rbouqueau commented 4 years ago

So I committed a check about the essential bit with the listed descriptive properties. However I'm unsure about a normative expectations for others to be at zero when I read this:

Descriptive properties are non-essential, unless stated otherwise in their specification.

cconcolato commented 4 years ago

Yes, you have to go through the spect text one by one to see if they are allowed to be essential. That's what I did above. So far, only the 5 above are essential.

rbouqueau commented 4 years ago

Yes that's what https://github.com/gpac/ComplianceWarden/commit/c253623e72bc89e85a5e5177ec3c72d6b367a0e4 does. So my perception is that we can close this issue.

cconcolato commented 4 years ago

I'm not sure.

You seem to be checking that clap, irot and imir are essential here: https://github.com/gpac/ComplianceWarden/commit/c253623e72bc89e85a5e5177ec3c72d6b367a0e4#diff-c13f959cf856284d748adbeba70b1365R1195 They may be essential, that's an author's choice. So you should not fail if they are not.

Also, can you confirm that the following file now fails (because of pixi wrongly marked essential): https://github.com/AOMediaCodec/av1-avif/blob/master/testFiles/Link-U/kimono.avif

rbouqueau commented 4 years ago

For clap, irot and imir, this is for MIAF (from the code):

"Section 7.3.9\n" "All transformative properties associated with coded and derived images required\n" "or conditionally required by this document shall be marked as essential, and\n" "shall be from the set that are permitted by this document or the applicable\n" "profile. No other essential transformative property shall be associated with\n" "such images.",

Also, can you confirm that the following file now fails (because of pixi wrongly marked essential): https://github.com/AOMediaCodec/av1-avif/blob/master/testFiles/Link-U/kimono.avif

No, cf ComplianceWarden output below.

HEIF:

rbouqueau@rose:~/works/ComplianceWarden$ bin/cw.exe heif kimono.avif 
No errors.

For MIAF:

rbouqueau@rose:~/works/ComplianceWarden$ bin/cw.exe miaf kimono.avif 
[Rule #38] File extension for "kimono.avif" doesn't match: expecting one of 'heif hif ', got 'avif'
1 error(s).

Specification description: MIAF (Multi-Image Application Format)
MPEG-A part 22 - ISO/IEC 23000-22 - w18260 FDIS - Jan 2019

Error rules description:

[Rule #38] Section 10.1
A MIAF file shall use the filename extensions specified by HEIF to identify the
presence of specific image coding formats

"Descriptive properties are non-essential, unless stated otherwise in their specification."

This part is problematic for two reasons:

Opinion?

rbouqueau commented 3 years ago

I'm trying to close open issues. I understand the current status is not prefect but I don't understand what could be improved.

First the following statement doesn't contain a "shall" statement: Descriptive properties are non-essential, unless stated otherwise in their specification. Secondly, if we considered it as a "shall" statement, it would break the separation of concerns as MIAF and now AVIF requires the essential bit on other boxes: HEIF could return that "av1C" shall be non essential while AVIF states the contrary.

As a consequence I think that the current status is a good compromise.

cconcolato commented 3 years ago

Can you recap the current status?

rbouqueau commented 3 years ago

Check for bit essential at 1:

cconcolato commented 3 years ago

Note regarding AVIF: https://github.com/AOMediaCodec/av1-avif/pull/144

rbouqueau commented 3 years ago

This has not been updated to avoid mixing versions of the spec. Is there a will from AOM to keep aligned on the latest version of the spec?

rbouqueau commented 1 year ago

Closing. Please re-open if there is news to share here.