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

Add lsel (layer selection) property to progressive images. #2429

Closed
wants to merge 3 commits into from

Conversation

maryla-uc
Copy link
Collaborator

My understanding from the discussions in #939 and the AVIF specification is that libavif treats the absence of the 'lsel' property the same as 'lsel' being present with the value 0xFFFF, but this is done for backward compatibility only and encoders are supposed to add the 'lsel' property on layered images.

src/write.c Outdated
avifBoxMarker lsel;
AVIF_CHECKRES(avifRWStreamWriteBox(&dedup->s, "lsel", AVIF_BOX_SIZE_TBD, &lsel));
// Layer Selection Property
// Section 2.3.1 of of AV1 Image File Format specification v1.1.0:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"of of"

@tongyuantongyu
Copy link
Contributor

tongyuantongyu commented Sep 12, 2024

I'd like to mention that Firefox was rejecting AVIF images with 'lsel' property, so I didn't add 'lsel' property to images when I was implementing progressive encoding.

I just tested that Firefox (131.0b5) still rejects AVIF images with 'lsel' property, unless you set image.avif.compliance_strictness to 0 in about:config (because 'lsel' is an essential property but is unsupported by Firefox, so per spec the decoding fails).

Currently avifenc produces progressive images that Firefox accepts (because 'a1lx' is non-essential, so it can be ignored), but adding 'lsel' will prevent these images from being decoded even non-progressively.

I would suggest landing this change at least after we ensure major decoder implementations all have supported 'lsel'. I know the following decoders do not support 'lsel', but there may be more:

  • mp4parse-rust (Firefox)
  • Windows with AV1 Video Extension
    • It rejects images with only 'a1lx' as well, so this one may not count.

And even after that, this will still upset early adopters of progressive AVIFs because images will appear broken on old version of browsers, instead of losing progressive but still work.

@maryla-uc
Copy link
Collaborator Author

Thanks @tongyuantongyu for the valuable background information.
I agree with you that it's best not to add this property for now. I will add some comments in the code instead, and ideally we should work on getting this property supported in major decoders...

@maryla-uc maryla-uc closed this Sep 12, 2024
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.

3 participants