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

avif: Ignore still picture rules for Image Sequences #41

Open vigneshvg opened 1 year ago

vigneshvg commented 1 year ago

This was updated in the AVIF specification version 1.1.0: https://github.com/AOMediaCodec/av1-avif/pull/138

rbouqueau commented 1 year ago

I am not sure to follow you.

I think you want to do the opposite: only check for some rules (still pictures, ...) when you face an AV1 Image Item. Keep in mind that nothing prevents a file from containing both some AV1 Image Items and AV1 Image Sequence. From the spec: "When an item is of type av01, it is called an AV1 Image Item".

So you could check if findImageItems(root, FOURCC("av01")); is empty... which is basically what the code already does:

      auto const av1ImageItemIDs = findImageItems(root, FOURCC("av01"));

      for(auto itemId : av1ImageItemIDs)

What am I missing? What is your actual issue?

vigneshvg commented 1 year ago

Consider an animated AVIF file. According to the spec, it can also advertise itself as a single frame image by pointing to the first frame of the animation as the still image data.

The sequence header in this case will not have the reduced_still_picture and still_picture flags set to 0 (because the sequence header applies for the whole animation and not just the still image).

So for such dual function files, the AVIF specification makes an exception for these rules with the following sentence: """ If the AV1 Image Item Data consists of a single frame (i.e. when using a single layer),

So the current code checks for the still_picture flag and reduced_still_picture_header flag in all the cases. The new code checks it only for cases where the file contains only a single frame (which is what the specification tells).

Right now if you run compliance warden through an animated AVIF file, it will warn about these two fields being 0. With this PR, it won't.

Please let me know if that makes sense to you.

rbouqueau commented 1 year ago

The example helps.

The current code doesn't implement the latest version of the spec. I acknowledge this modification has not been implemented yet.

"If the AV1 Image Item Data consists of a single frame (i.e. when using a single layer)"

Is this equivalent to having one and only one OBU_FRAME? Or are we talking of some output/displayed frame (i.e. show_frame/show_existing_frame)?

Does the reference to "single layer" means we should also check the 'lsel' box when present (and then what about layer_id=0xffff? How many frames would that account for?) ?

vigneshvg commented 1 year ago

The current code doesn't implement the latest version of the spec. I acknowledge this modification has not been implemented yet.

I noticed that some of the variants of the latest version of the spec have been implemented, so i thought it was okay to make this change. :)

Is this equivalent to having one and only one OBU_FRAME? Or are we talking of some output/displayed frame (i.e. show_frame/show_existing_frame)?

My understanding is that, if the presented image is intended to be a still image item, then it will have exactly one OBU_FRAME because it will be a keyframe.

Does the reference to "single layer" means we should also check the 'lsel' box when present (and then what about layer_id=0xffff? How many frames would that account for?) ?

Perhaps we should. But that is outside the scope of this PR.

rbouqueau commented 1 year ago

If the AV1 Image Item Data consists of a single frame (i.e. when using a single layer),

Is the single layer the only condition (i.e. or e.g.?) ?

Do Image Sequences qualify as well (I also read "The AV1 Image Item Data shall be identical to the content of an AV1 Sample marked as sync, as defined in [AV1-ISOBMFF]." in the same section)?

CC @cconcolato