kornelski / cavif-rs

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

Files with transparency doesn't work on Firefox #13

Closed Draghmar closed 3 years ago

Draghmar commented 3 years ago

Hi I think there is some issue with encoder that prevents files with alpha channel being displayed. I encoded this file: trp which resulted in this file. It works in Chrome but it doesn't in FF. My current FF is 86.0b2. I did use different encoder, an online one and their result worked just fine. I did try to use different parameters but the result is always the same.

kornelski commented 3 years ago

Fixed

Draghmar commented 3 years ago

Awesome, thanks for fast fix! :D BTW How soon I can expect this version to be available at cargo? (noob here in this regard :P)

kornelski commented 3 years ago

I've published it now

Draghmar commented 3 years ago

Hm...It still doesn't work. I tried this both on Linux and on Windows. This is the file I've got.

kornelski commented 3 years ago

Sigh. Firefox is very picky about the standard. I thought it's just a "brand" missing, but it's picky about even more details. It's so much easier to generate files for Chrome :(

kornelski commented 3 years ago

https://bugzilla.mozilla.org/show_bug.cgi?id=1689806

negge commented 3 years ago

Hi, I just ran into this. As some who works professionally in standards development, I'd like to understand the issue here.

Are the files being generated standards compliant? If Firefox is being "picky" by not displaying malformed AVIF files, then I don't think the solution is to file a bug against Firefox. As the author of this tool, you should care that the output is usable and I would ask you to please generate only standards compliant files so we don't need to "fix" all of the decoders out there who in fact are standards complaint.

Thank you.

kornelski commented 3 years ago

The issue is that AVIF is built on a tower crufty ISO specs, including a non-free one, and this whole spec sandwich has tons of dubious features with tons of requirements of varying relevance to images, and it's a pain in the ass to figure out the exact minimal subset that is required for compliance.

negge commented 3 years ago

Thanks Kornel, you raise some valid concerns. Let me look into this a little more and get back to you.

kornelski commented 3 years ago

I've worked with standards for a while too, so I understand it's annoying to deal with non-compliant tools.

OTOH AVIF is in practice a new format. HEIF is way too large and too complex to be supported fully on the Web. It's inevitable that there is going to emerge a "web-compatible" subset of it dictated by actual implementations. This is not necessarily a bad thing, and I wouldn't mind if that subset was minimal. So if libavif doesn't need the extra data, then one way to achieve interoperability is to make that data optional in Firefox too.

I think it would be worthwhile to create some kind of "AVIF web profile" spec that documents what browsers support, what is optional, etc.

negge commented 3 years ago

So MIAF is that reduced complexity subset of HEIF selected with a broad set of use cases in mind, including the web.

One thing to consider, potentially lots of devices, applications, hardware and software are going to be creating AVIF files and we would like them consumable everywhere. This includes on the web (Firefox, Chromium, Edge, etc.) in operating systems, for automation, computer vision and other use cases. There is no such thing as a "web-compatible subset ... dictated by actual implementations". Please produce standards compliant AVIF files, it is the only way to guarantee interoperability.

It sounds like the actual issue is that while the HEIF spec (ISO/IEC 23008-12:2017) is available for free here:

https://standards.iso.org/ittf/PubliclyAvailableStandards/index.html

the set of constraints described in the MIAF spec (ISO/IEC 23000-22:2019) is not free and only available for purchase here:

https://www.iso.org/standard/74417.html

I completely agree it would be worthwhile to have a public document that bridges this gap and aids implementors such as yourself.

kornelski commented 3 years ago

I've heard MIAF may end up being released for free. I'm waiting for that to happen, because I'm not going to use a commercial spec on principle.

This is huge obstacle to spec-compliance, because the AVIF spec defers with almost everything to MIAF, and without seeing MIAF I have no idea how much of HEIF is applicable through MIAF's lens.

So far I've been working around lack of MIAF by reverse-engineering other implementations, but as you see, that results in being compatible with libavif, and not the spec. If Firefox is aiming to be compatible with MIAF, and not libavif, then we're not implementing the same thing.

negge commented 3 years ago

Kornel, you may want to look at this AVIF file validator:

https://gpac.github.io/ComplianceWarden-wasm/avif.html

It is still under development, but I was able to submit PNG_transparency_demonstration_1.avif and it produced a list of errors with specific call outs to the sections in the spec where it was invalid, e.g,.

[miaf][Rule #1] Error: The HandlerBox shall be the first contained box within the MetaBox
[miaf][Rule #2] Error: compatible_brands list shall contain 'miaf' (not found) and 'mif1' (not found)
[miaf][Rule #4] Error: 'hdlr' not found in MetaBox
[miaf][Rule #4] Error: 'hdlr' not found in MetaBox
...
adworacz commented 3 years ago

I just started running into similar issues after attempting to convert a PNG. My source is attached, and cavif produces a file that Firefox and imv refuse to load properly.

Image: 6d38d1b1dc9fdf0700

Output log of the validation tool linked above: https://bin.snopyta.org/?904fd4716e8ef9d2#Aa9cJKCGqCZ82cftwxYRZwRLWRz3mk1R2nZeXGk3w6rj

Draghmar commented 3 years ago

I'm adding this info as a "bonus": FF 90b1 broke displaying even JPGs. There's image.avif.compliance_strictness flag hidden that is set to 1 by default. With that it won't work for your tool as well as few others...Sqoosh works from what I can see but that's it.

baumanj commented 3 years ago

FF 90b1 broke displaying even JPGs

This is the first I've heard of that. Can you provide some more info?

There's image.avif.compliance_strictness flag hidden that is set to 1 by default

That's in mp4parse-rust, but hasn't been added to Firefox (even Nightly) yet. It should be there soon. The hope is that it will provide a valuable signal to AVIF writers when their files are in violation of the spec, giving them time to fix things up before AVIF becomes widespread, resulting in an inconsistent patchwork of support, while still allowing users the flexibility to see what can be rendered (although less efficiently/correctly depending on the nature of the input).

kornelski commented 3 years ago

I've fixed issues that mp4parse-rust flagged under Normal compliance level.

I've missed these issues before, because stricter parsing in mp4parse isn't released yet, and I was testing against the version on crates.io.

I'm not happy about AVIF parsers growing in complexity. HEIF is a kitchen sink, and the ISO specs with video heritage have many features that are redundant and overengineered for a still image format. I hoped AVIF could be trimmed down to minimum, and be a simpler alternative to JPEG XL.

Draghmar commented 3 years ago

FF 90b1 broke displaying even JPGs

This is the first I've heard of that. Can you provide some more info?

I'm not sure what info you'd like to get here. After updating to 90b1 I've noticed that all the AVIF images converted from JPGs won't display with the image.avif.compliance_strictness set to 1, which is a default state. I'm reading what I've wrote and maybe I simply wasn't clear enough here, suggesting that the JPG itself has issues when I was simply referring to the fact that this topic was about PNG.

baumanj commented 3 years ago

Re @kornelski:

I've fixed issues that mp4parse-rust flagged under Normal compliance level.

Thanks for that!

I've missed these issues before, because stricter parsing in mp4parse isn't released yet, and I was testing against the version on crates.io.

Ah, my apologies if updating crates.io would've been more helpful. When testing against cavif-rs, I've been building from source, so I guess I assumed you were doing the same with mp4parse-rust. If you like, I can make a point of updating on crates.io more frequently, and especially with any changes around compliance strictness.

I'm not happy about AVIF parsers growing in complexity

I sympathize with your concerns. Basing AVIF on HEIF and BMFF definitely has advantages in terms of being able to reuse a lot of existing tooling as well as disadvantages of the inherent complexity involved. At this point, I think the best thing we can do is try to ensure the readers and writers are at least consistent about adhering to the specs so that if tools like ComplianceWarden are happy, those files should display correctly on all compliant renderers.

baumanj commented 3 years ago

Re @Draghmar

I'm not sure what info you'd like to get here. After updating to 90b1 I've noticed that all the AVIF images converted from JPGs won't display with the image.avif.compliance_strictness set to 1, which is a default state.

Ah, I misunderstood "broke displaying even JPGs" to mean Firefox wasn't rendering JPEGs themselves, not AVIFs created from JPEGs. It looks like the compliance-related issues have been fixed in https://github.com/kornelski/avif-serialize, so if you're still seeing failures with the most current version of cavif-rs, I'd be curious to see some example images.

Draghmar commented 3 years ago

Hm...I didn't know there's a newer version than 0.6.6 from release page. And here at cargo you can find 1.1.1. So after installing it and testing a little bit I see that not only files without transparency works as they should but also those with transparency. Great job! :) @kornelski you should really put some info on the github page about having this newer version. You even have there message: ➡️ Download the latest release ⬅️ which points to the 0.6.6 one. I for one didn't bother to check it at cargo as I'm not using it too often. :P

kornelski commented 3 years ago

I've made a new build.