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

avif: Ignore still picture rules for Image Sequences #41

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vigneshvg
Copy link
Contributor

This was updated in the AVIF specification version 1.1.0: AOMediaCodec/av1-avif#138

This was updated in the AVIF specification version 1.1.0:
AOMediaCodec/av1-avif#138
@rbouqueau
Copy link
Member

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

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),

  • It should have its still_picture flag set to 1.
  • It should have its reduced_still_picture_header flag set to 1.
    """

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

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

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants