kornelski / cavif-rs

AVIF image creator in pure Rust
https://lib.rs/cavif
BSD 3-Clause "New" or "Revised" License
575 stars 27 forks source link

FireFox 92.0b2 - avif from cavif won't work #37

Closed Draghmar closed 2 years ago

Draghmar commented 3 years ago

So the FF92 branch has dropped and with that official AVIF support (turned on by default) but at the same time they changed something because all the files that were working in 91 doesn't work right now - opening them directly shows info that image had some errors and can't be displayed. Doesn't matter if the source was JPG or PNG with transparency. They were generated with cavif 1.3.0 as this is the last one at cargo. I also tried 1.3.1 but the result was the same.

Edit: For the record - squoosh produces files valid for FF92.

kornelski commented 3 years ago

The latest version should be compatible. Were you using Windows by any chance? #36

Draghmar commented 3 years ago

And yet it's not for some reason. No, I'm using this on ArchLinux. I've made simple test: original, cavif 1.3.0 (cargo), cavif 1.3.1, squoosh. Both files from cavif won't show up in FF.

BTW The same goes for mobile FF 93.0a1 (nightly).

nocduro commented 3 years ago

Oops I should have mentioned in #36 that the produced avifs didn't work in firefox 92.0b3 with cavif 1.3.1, wanted to keep the released binary issue separate from the firefox problem

kornelski commented 3 years ago

Do you have AVIF set to strict in about:config?

Draghmar commented 3 years ago

Yes, I do. That's the default and it was like that from 90 or 91 - don't remember now.

nocduro commented 3 years ago

image

Yes, I think it's the default now?

Changing image.avif.compliance_strictness to 0 allows the avifs to load

Compliance Warden output for the linked cavif 1.3.1 image from above ``` Compliance Warden, version v29-master-rev0-geb35e21. +--------------------------------------+ | avif validation | +--------------------------------------+ Specification description: AVIF v1.0.0, 19 February 2019 https://aomediacodec.github.io/av1-avif/ [avif][Rule #7] Error: [ItemId=1] The values of the AV1CodecConfigurationBox shall match the Sequence Header OBU in the AV1 Image Item Data: AV1CodecConfigurationBox: seq_profile=0 seq_level_idx_0=0 seq_tier_0=0 high_bitdepth=0 twelve_bit=0 mono_chrome=0 chroma_subsampling_x=0 chroma_subsampling_y=0 chroma_sample_position=0 Sequence Header OBU in the AV1 Image Item Data: seq_profile=1 seq_level_idx_0=31 seq_tier_0=0 high_bitdepth=0 twelve_bit=0 mono_chrome=0 chroma_subsampling_x=0 chroma_subsampling_y=0 chroma_sample_position=0 [avif][Rule #9] Error: Transformative property "av1C" shall be marked as essential (item_ID=1) ======================================== [avif] 2 error(s), 0 warning(s). ======================================== ===== Involved rules descriptions: [avif][Rule #7] Section 2.2.1 The values of the fields in the AV1CodecConfigurationBox shall match those of the Sequence Header OBU in the AV1 Image Item Data. [avif][Rule #9] Section 2.2.1 AV1 Item Configuration Property [...] shall be marked as essential. +--------------------------------------+ | miaf validation | +--------------------------------------+ Specification description: MIAF (Multi-Image Application Format) MPEG-A part 22 - ISO/IEC 23000-22 - w18260 FDIS - Jan 2019 [miaf][Rule #2] Error: compatible_brands list shall contain 'miaf' (not found) and 'mif1' (found) ======================================== [miaf] 1 error(s), 0 warning(s). ======================================== ===== Involved rules descriptions: [miaf][Rule #2] Section 7.2.1.2 The FileTypeBox shall contain, in the compatible_brands list, the following (in any order): 'mif1' (specified in ISO/IEC 23008-12) [...] Files conforming to the general restrictions in clause 7 shall include the brand 'miaf' in the compatible_brands in the FileTypeBox. /!\ this rule doesn't look for 'mif1' and 'miaf' brands rule-conformance: if a brand is absent then its conformance rules won't be checked here /!\ +--------------------------------------+ | 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][Rule #13] Error: 'hdlr box': reserved1 shall be 0 but value is 1383428980 ======================================== [isobmff] 1 error(s), 0 warning(s). ======================================== ===== Involved rules descriptions: [isobmff][Rule #13] Section 8.4.3.1 'hdlr box': pre_defined = 0, reserved = 0, 'name' field shall be null-terminated. ```
kukulich commented 3 years ago

I've just tested it too...

FF 91, compliance_strictness = 1: Image works FF 920b3, compliance_strictness = 1 (default): Image does not work

Image generated with 1.3.1 on Windows (downloaded today).

baumanj commented 3 years ago

Do you have AVIF set to strict in about:config?

Yes, I do. That's the default and it was like that from 90 or 91 - don't remember now.

A little bit of extra context here, since setting image.avif.compliance_strictness isn't "strict", it's "normal":

From the definition

# How strict we are in accepting/rejecting AVIF inputs according to whether they
# conform to the specification
# 0 = Permissive: accept whatever we can simply, unambiguously interpret
# 1 = Normal: reject violations of "shall" specification directives
# 2 = Strict: reject violations of "should" specification directives

1 (Normal) is and always has been the default. The only images it intends to reject are those which violate requirements of the specifications, any unfulfilled recommendations are accepted.

The reason both cavif 1.3.0 (cargo) and cavif 1.3.1 are rejected is due to the invalid HandlerBox (hdlr). As reported by Compliance Warden:

[isobmff][Rule #13] Error: 'hdlr box': reserved1 shall be 0 but value is 1383428980
[isobmff][Rule #13] Section 8.4.3.1
'hdlr box': pre_defined = 0, reserved = 0, 'name' field shall be null-terminated.
kornelski commented 3 years ago

I'm having a déjà vu here. I think I've already added an extra byte for the (unused) name:

https://github.com/kornelski/avif-serialize/commit/53a751e260731970bfea3fe6220892d19f8e5e83

kornelski commented 3 years ago

I've found the cause. It turns out I've used ISO 14496-1 as a reference, and Firefox wants ISO 14496-12 conformance. Specifically, it doesn't tolerate non-0 manufacturer field.

I've stopped testing against outdated mp4parse version, and updated the serializer to match, so next version of cavif should be compatible again.

Unfortunately, all existing AVIF files will be a problem if Firefox 92 ships with image.avif.compliance_strictness=1.

Draghmar commented 3 years ago

Unfortunately, all existing AVIF files will be a problem if Firefox 92 ships with image.avif.compliance_strictness=1.

I'm pretty sure it will ship like that. Mozilla states that current beta has avif turned on: "Firefox now supports the new AVIF image format, which is based on the modern and royalty free AV1 video codec. It offers significant bandwidth savings for sites compared to existing image formats. Additionally it supports transparency and other advanced features." Which means it will go like that into stable release.

I've already done few passes of converting files to avif because there were few times where compatibility was an issue (for both FF and Chrome) so I think I'll manage to survive another. ;)

kukulich commented 3 years ago

I've already done few passes of converting files to avif because there were few times where compatibility was an issue (for both FF and Chrome) so I think I'll manage to survive another. ;)

Agreed :)

baumanj commented 3 years ago

I'm having a déjà vu here. I think I've already added an extra byte for the (unused) name:

kornelski/avif-serialize@53a751e

You're right. I think the 'name' field shall be null-terminated error in Compliance Warden is a false positive and the only issue is the non-zero reserved fields.

nikonthethird commented 3 years ago

For what it's worth, I created a simple tool to fix problematic AVIF images without re-encoding them: https://github.com/nikonthethird/avif-fixer

It uses kornelski's avif-parse and avif-serialize to parse the file passed to stdin, and passes the fixed file out to stdout.

This saved me from hours of re-encoding.

jsf84ksnf commented 3 years ago

Firefox Version 93.0b5 (Developer Edition) has avif enabled and set to strict. However, it shows the Avif files generated with cavif 1.3.1

I also tested nikonthethirds avif-fixer. which worked well for me. But since Firefox 92 has Avif disabled by default and Firefox 93 seems to show them again (as they are), a fix might not be needed.

baumanj commented 3 years ago

Firefox Version 93.0b5 (Developer Edition) has avif enabled and set to strict.

As mentioned previously a value of 1 for image.avif.compliance_strictness isn't "strict", it's "normal".

I can't foresee a scenario where strict (2) will ever be the default, but it may be useful for testing that inputs are as conformant as possible.

jsf84ksnf commented 3 years ago

Firefox Version 93.0b5 (Developer Edition) has avif enabled and set to strict.

As mentioned previously a value of 1 for image.avif.compliance_strictness isn't "strict", it's "normal".

Excuse me. Let me reformulate.

Firefox Version 93.0b5 (Developer Edition) has avif enabled and strict set to 1. However, it shows the Avif files generated with cavif 1.3.1

I also tested nikonthethirds avif-fixer. which worked well for me. But since Firefox 92 has Avif disabled by default and Firefox 93 seems to show them again (as they are), a fix might not be needed.

kukulich commented 2 years ago

Is there a reason why the fix has not been released?

baumanj commented 2 years ago

I think this has already been fixed the the latest version. What error are you seeing and with what versions of cavis-rs and/or Firefox?

kukulich commented 2 years ago

@baumanj Latest release is from 30 Jul 2021, this issue is from 12 Aug.

baumanj commented 2 years ago

Oh, you want a new GitHub release? As of now, the latest crates.io release was 26 October 2021.

kornelski commented 2 years ago

I'm in a process of moving to an ARM Mac. I'll build x86-64 exe for you once I set everyting up, including cross-compilation.

kornelski commented 2 years ago

Released