Ignore descr props after transf props #2571
Conversation
| // Under any brand, the primary item (or an alternative if alternative support is required) | ||
| // shall be processable by a reader implementing only the required features of that brand. | ||
| // Specifically, given that each brand has a set of properties that a reader is required to | ||
| // support: the item shall not have properties that are marked as essential and are outside |
There was a problem hiding this comment.
[This should be fixed in HEIF.]
The colon (:) after "support" should be a comma (,).
Also reject non-essential transformative properties.
| @@ -35,6 +35,27 @@ License: [same as libavif](https://github.com/AOMediaCodec/libavif/blob/main/LIC | |||
| Source: `avifenc circle-trns-after-plte.png` with custom properties added in | |||
| `avifRWStreamWriteProperties()`: FullBox `1234`, Box `abcd` and Box `uuid`. | |||
|
|
|||
| ### File [clap_irot_imir_non_essential.avif](clap_irot_imir_non_essential.avif) | |||
There was a problem hiding this comment.
Here is the Compliance Warden report. Does it meet your expectations?
The only issue I see is that Compliance Warden should use the Amendment 2 version of [miaf][Rule #25] Section 7.3.9.
Compliance Warden, version v35-master-rev3-g1500613
+--------------------------------------+
| avif validation |
+--------------------------------------+
Specification description: AVIF v1.0.0, 19 February 2019
https://aomediacodec.github.io/av1-avif/
========================================
[avif] No errors.
========================================
+--------------------------------------+
| miaf validation |
+--------------------------------------+
Specification description: MIAF (Multi-Image Application Format)
MPEG-A part 22 - ISO/IEC 23000-22 - w18260 FDIS - Jan 2019
[miaf][Rule #25] Error: Property "clap" shall be marked as essential (item_ID=1)
[miaf][Rule #25] Error: Property "irot" shall be marked as essential (item_ID=1)
[miaf][Rule #25] Error: Property "imir" shall be marked as essential (item_ID=1)
========================================
[miaf] 3 error(s), 0 warning(s).
========================================
===== Involved rules descriptions:
[miaf][Rule #25] Section 7.3.9
All transformative properties associated with coded and derived images required
or conditionally required by this document shall be marked as essential, and
shall be from the set that are permitted by this document or the applicable
profile. No other essential transformative property shall be associated with
such images.
+--------------------------------------+
| heif validation |
+--------------------------------------+
Specification description: HEIF - ISO/IEC 23008-12 - 2nd Edition N18310
========================================
[heif] No errors.
========================================
+--------------------------------------+
| isobmff validation |
+--------------------------------------+
Specification description: ISO Base Media File Format
MPEG-4 part 12 - ISO/IEC 14496-12 - m17277 (6th+FDAM1+FDAM2+COR1-R4)
========================================
[isobmff] No errors.
========================================
There was a problem hiding this comment.
Here is the Compliance Warden report. Does it meet your expectations?
Yes. Thank you for checking.
The only issue I see is that Compliance Warden should use the Amendment 2 version of [miaf][Rule #25] Section 7.3.9.
Section 7.3.9 is part of w18260 "Revised text of ISO/IEC FDIS 23000-22 Multi-Image Application Format". Are you saying Amd2 was published since then and there is no need to refer to an FDIS output document?
There was a problem hiding this comment.
Section 7.3.9 is in both ISO/IEC 23000-22:2019 and ISO/IEC 23000-22:2019/Amd. 2:2021.
In ISO/IEC 23000-22:2019, the paragraph reads:
All transformative properties associated with coded and derived images required or conditionally required by this document shall be marked as essential, and shall be from the set that are permitted by this document or the applicable profile. No other essential transformative property shall be associated with such images.
In ISO/IEC 23000-22:2019/Amd. 2:2021, the paragraph is modified and reads:
All transformative properties associated with coded and derived images shall be marked as essential,
and shall be from the set defined in 7.3.6.7 or the applicable MIAF profile. No other essential
transformative property shall be associated with such images.
So Compliance Warden quotes the older version of the paragraph in the error message. It is better to quote the newer version of the paragraph in the error message.
In general Compliance Warden should not refer to an FDIS output document.
| @@ -55,6 +55,8 @@ The changes are relative to the previous release, unless the baseline is specifi | |||
| * Deprecate avifCropRectConvertCleanApertureBox() and | |||
| avifCleanApertureBoxConvertCropRect(). Replace them with | |||
| avifCropRectFromCleanApertureBox() and avifCleanApertureBoxFromCropRect(). | |||
| * Ignore descriptive properties associated after transformative ones. | |||
| * Reject non-essential transformative properties. | |||
There was a problem hiding this comment.
@farindk FYI.
We should audit the AVIF encoders in libavif and libheif and make sure they generate an ItemPropertyAssociationBox that meets the requirements checked in this pull request:
- All transformative properties (clap, irot, imir) associated with coded and derived images shall be marked as essential.
- If
property_indexis 0,essentialshall also be 0. - Should descriptive properties be listed before transformative properties in the
ItemPropertyAssociationBox? Otherwise they will be ignored by readers.
There was a problem hiding this comment.
All transformative properties (clap, irot, imir) associated with coded and derived images shall be marked as essential.
This should always be the case in libheif.
If property_index is 0, essential shall also be 0.
libheif never generates property_index=0. Is there any potential use case for this that I have missed?
Should descriptive properties be listed before transformative properties in the ItemPropertyAssociationBox? Otherwise they will be ignored by readers.
I will have to check this: strukturag/libheif#1443
BTW: what is the rationale for this rule? I cannot think about any case where it would be important to have the descriptive items before the transformative items.
There was a problem hiding this comment.
Dirk: Thanks for the quick reply!
If property_index is 0, essential shall also be 0.
libheif never generates property_index=0. Is there any potential use case for this that I have missed?
I also asked this question. Please see Yannis's reply in #2571 (comment).
Should descriptive properties be listed before transformative properties in the ItemPropertyAssociationBox? Otherwise they will be ignored by readers.
I will have to check this: strukturag/libheif#1443 BTW: what is the rationale for this rule? I cannot think about any case where it would be important to have the descriptive items before the transformative items.
This comes from the following in ISO/IEC 23008-12:2022 Section 6.5.1:
Readers shall allow and ignore descriptive properties following the first transformative or unrecognized property, whichever is earlier, in the sequence associating properties with an item.
Note that it says "allow and ignore", so it isn't a hard rule. I don't know what is the rationale is.
There was a problem hiding this comment.
Readers shall allow and ignore descriptive properties following the first transformative or unrecognized property, whichever is earlier, in the sequence associating properties with an item.
Note that it says "allow and ignore", so it isn't a hard rule. I don't know what is the rationale is.
But if a reader may ignore properties that are in the "wrong order", it effectively forces the writer to use the "correct order".
I also think that "Readers shall allow and ignore descriptive properties following the first transformative or unrecognized property" does not work well in practice because when encoders add new descriptive properties that are unknown to old readers, readers may ignore all descriptive properties following that new property, even if the new property was not essential (but a successive old property may be essential). I.e. images may be decoded differently by adding a non-essential property.
I thought that this is what the essential flag is for. When the reader sees an essential property that it does not understand, it knows that the decoded output might be incorrect. Non-essential properties can be ignored.
The separation of transformative and descriptive properties on-the-other hand makes little sense to me. Even more so since that means that the set of transformative properties may never be extended in future standards as old decoders have no way to classify them as transformative.
There was a problem hiding this comment.
Readers shall allow and ignore descriptive properties following the first transformative or unrecognized property, whichever is earlier, in the sequence associating properties with an item.
Note that it says "allow and ignore", so it isn't a hard rule. I don't know what is the rationale is.
But if a reader may ignore properties that are in the "wrong order", it effectively forces the writer to use the "correct order".
It is not "may" but "shall ignore". To me it is a hard rule.
I.e. images may be decoded differently by adding a non-essential property.
I disagree: MIAF* forbids non-essential transformative properties, and descriptive properties should not impact decoding.
*and maybe HEIF? that is unclear to me because it depends on profiles if I remember correctly.
There was a problem hiding this comment.
The colr box, for example, is a descriptive property. Still, it affects how the image will be displayed.
If the file now contains a descriptive property that the reader does not understand before the colr, the rule asks the reader to drop the colr box and show the image with wrong colors.
You may have the objection against the above because HEIF also says that "Writers should arrange the descriptive properties specified in 6.5 prior to any other properties in the sequence associating properties with an item.".
However, with every standard version, the set of properties in 6.5 is extended. Thus, we might have this situation:
A writer with HEIF-v2 knowledge writes this:
mdcv- added in HEIF-v2colr- from HEIF-v1
A reader with only HEIF-v1 knowledge sees this as:
mdcv- unknown propertycolr- known, but I have to drop it since it follows an unknown property
There was a problem hiding this comment.
Dirk: I agree with your analysis.
Yannis: Should we clarify in the source code comment that the following is what we actually do?
Readers shall allow and ignore descriptive properties following the first transformative in the sequence associating properties with an item.
Although there is already a clarifying comment:
// No need to check for unrecognized properties as they cannot be transformative according to MIAF.
it explains why we don't need to check for unrecognized transformative properties, but it doesn't explain why we don't need to check for unrecognized descriptive properties.
Also reject non-essential transformative properties.
Fixes #2525.