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

Support AVIF (AV1 Image FIle Format) #1152

Closed
jansol opened this issue Feb 20, 2020 · 21 comments · Fixed by #1314 or #1398
Closed

Support AVIF (AV1 Image FIle Format) #1152

jansol opened this issue Feb 20, 2020 · 21 comments · Fixed by #1314 or #1398

Comments

@jansol
Copy link
Contributor

jansol commented Feb 20, 2020

Spec at https://aomediacodec.github.io/av1-avif/

Since there is already a PR for FLIF support, it probably makes sense to have at least a tracking issue for AVIF as well.

@kornelski
Copy link
Contributor

kornelski commented Sep 14, 2020

For AVIF encoding, have a look at https://github.com/kornelski/cavif-rs (https://docs.rs/ravif)

It's a 100% pure Rust integration with rav1e.

@HeroicKatora
Copy link
Member

HeroicKatora commented Sep 16, 2020

I tried using the encoder with the bindings and imagemagick thinks the produced file is not valid:

$ cargo run --example convert --features=avif -- tests/reference/tiff/testsuite/mandrill.tiff.694c777d.png mandrill.avif
$ file mandrill.avif
mandrill.avif: ISO Media
$ convert -verbose mandrill.avif mandrill.jpeg
convert: Invalid input: No 'hdlr' box `mandrill.avif' @ error/heic.c/IsHeifSuccess/136.
convert: no images defined `mandrill.jpeg' @ error/convert.c/ConvertImageCommand/3285

I believe this refers to this statement in the specification.
There are no mentions of pict in avif-serialize. Are there any reference, roundtrip or self-tests to execute?

@kornelski
Copy link
Contributor

kornelski commented Sep 16, 2020

avif-serialize is verified to be compatible with libavif (Chrome) and mp4parse (Firefox).

I do not support AVIF-sequence format in avif-serialize (hdlr is for marking video/audio tracks), only AVIF still images. These are two different flavors of MP4.

@est31
Copy link
Contributor

est31 commented Sep 18, 2020

Should this be reopened? There is no decoding support yet.

@HeroicKatora HeroicKatora reopened this Sep 18, 2020
@foresterre
Copy link
Contributor

foresterre commented Oct 4, 2020

@kornelski

I'm not always experiencing success with encoding AVIF in https://github.com/foresterre/sic which uses the latest https://github.com/image-rs/image (at time of writing v0.23.10) using the current implementation. Or, I am not sure whether it works or not, because Firefox seems unable to decode the resulting files (Firefox 81, x64 on Ubuntu):

image

with about:config:

image

If I disable use-dav1d I get the same results.

I've made a minimal example using the image crate which can be found here: https://github.com/foresterre/avifexample/blob/main/src/main.rs#L24

I used the wh1616.png sample file: avif_samples.zip (includes my .avif results).

Is this the above usage for the minimal example correct or did I miss something?

If there is something I can do to help (further), please let me know!

Hoping this is an OK place to place to ask about this 😅, I'm a bit unsure whether to post here, since I did use the image crate, however, I also experience difficulties when using your cli tool with cavif wh1616.png, and the image crate doesn't really do much magic in between as far as I a aware... I used cavif 0.6.2, latest version, installed from crates.io

@est31
Copy link
Contributor

est31 commented Oct 4, 2020

@foresterre chrome opens the avif files you attached just fine. In firefox I can reproduce the errors. Maybe avif support is not ready yet in Firefox? You could try filing a Firefox bug.

@kornelski
Copy link
Contributor

kornelski commented Oct 4, 2020

This is expected. Implementation in Firefox is still very incomplete and does not support alpha channel. I'm waiting for this to be merged:

mozilla/mp4parse-rust#239

@kleisauke
Copy link

kleisauke commented Nov 13, 2020

avif-serialize is verified to be compatible with libavif (Chrome) and mp4parse (Firefox).

I do not support AVIF-sequence format in avif-serialize (hdlr is for marking video/audio tracks), only AVIF still images. These are two different flavors of MP4.

Note that not including the hdlr-box could break future strict decoding modes in libavif and libheif. See AOMediaCodec/libavif#407 and strukturag/libheif#371.

For example, checking a cavif-rs encoded AVIF image against ComplianceWarden currently results in this error:

[Rule #2] Error: 'hdlr' not found in MetaBox
[heif] 1 error(s).
[heif] 0 warning(s).

Specification description: HEIF - ISO/IEC 23008-12 - First edition 2017-12

Involved rules descriptions:

[heif][Rule #2] Section 6.2
The handler type for the MetaBox shall be 'pict'.
[isobmff] No errors.

@BezPowell
Copy link
Contributor

BezPowell commented Nov 25, 2020

I've just been having a play around with this, trying to add support for avif to Zola, and have to say it's looking pretty amazing so far. The work people have been putting into supporting such a new format so soon after browsers are only just wiring it in is incredibly impressive.

I noticed it's not currently possible to specify the quality while encoding, with it defaulting to 100%. Is this something that's going to be implemented in the future, or are there some technical hurdles preventing it?

I'm happy to have a crack at adding specifying the quality if it's not already in progress. I realize this is incredibly new, so apologies for pestering; the potential benefits for the web are so big I'm really eager to start using it.

@est31
Copy link
Contributor

est31 commented Nov 25, 2020

I'm happy to have a crack at adding specifying the quality if it's not already in progress. I realize this is incredibly new, so apologies for pestering

@BezPowell it should be easy to add a function like JpegEncoder's new_with_quality function. Right now, the quality is hardcoded.

Not a maintainer but I think a quality param would be really helpful.

@BezPowell
Copy link
Contributor

BezPowell commented Nov 26, 2020

I'm happy to have a crack at adding specifying the quality if it's not already in progress. I realize this is incredibly new, so apologies for pestering

@BezPowell it should be easy to add a function like JpegEncoder's new_with_quality function. Right now, the quality is hardcoded.

Not a maintainer but I think a quality param would be really helpful.

That sounds like a really good idea. I wonder whether a good solution would then be to convert the ImageOutputFormat::Avif variant into a tuple with a quality parameter, like the existing behaviour of the ImageOutputFormat::Jpeg(u8) variant?

Zola uses write_to() which this would fit well with, but I'm not sure how that would impact other api interactions. Does anyone have any objections to me having a crack at wiring this up?

@paolobarbolini
Copy link
Contributor

paolobarbolini commented Nov 26, 2020

I think it should also be possible to set the encoder speed parameter. Rav1e can go from 10 fastest, 0 slowest.

@est31
Copy link
Contributor

est31 commented Nov 26, 2020

@BezPowell changing the enum would be a semver breaking change so it should wait until the next breaking update of the image crate. Not sure when it's planned. Adding a function is entirely semver compatible.

@BezPowell
Copy link
Contributor

BezPowell commented Nov 26, 2020

@BezPowell changing the enum would be a semver breaking change so it should wait until the next breaking update of the image crate. Not sure when it's planned. Adding a function is entirely semver compatible.

Absolutely. Sorry, I forgot to mention that in my reply, it would of course have to wait for a breaking update. I was more thinking of doing it in two stages; adding the function first and changing the enum later if that was considered a worthwhile change.

@BezPowell
Copy link
Contributor

BezPowell commented Nov 26, 2020

I think it should also be possible to set the encoder speed parameter. Rav1e can go from 10 fastest, 0 slowest.

Perhaps a new_with_speed_quality function?

@est31
Copy link
Contributor

est31 commented Nov 26, 2020

@BezPowell both seem to be worthwhile changes to me. One could also think about removing the quality option from ImageFormat and instead create a struct ImageFormatWithQuality or something, changing the write_to function signature. Edit: nvm it's a bad idea as quality parameters aren't comparable across libraries. I guess having to break the format every time a param is added is something one has to live with.

@paolobarbolini
Copy link
Contributor

paolobarbolini commented Nov 26, 2020

Another solution that comes to mind for the next version could be something like this:

pub enum ImageOutputFormat {
    Png(PngEncoderOptions),
    Jpeg(JpegEncoderOptions),
    Pnm(PngEncoderOptions),
    Gif(GifEncoderOptions,
    Ico(IcoEncoderOptions),
    Bmp(BmpEncoderOptions),
    Farbfeld(FarbfeldEncoderOptions),
    Tga(TgaEncoderOptions),
    Avif(AvifEncoderOptions),
}

// NOTE: This already exists in the current version, but using `From`
// and having a variant in `ImageOutputFormat` for unsupported formats.
impl TryFrom<ImageFormat> for ImageOutputFormat {
    type Error = UnsupportedImageOutputFormat;

    fn try_from(fmt: ImageFormat) -> Result<Self, Self::Error> {
        match fmt {
            ImageFormat::Png => Ok(Self::Png(Default::default())),
            ImageFormat::Jpeg => Ok(Self::Jpeg(Default::default())),
            ImageFormat::Pnm => Ok(Self::Pnm(Default::default())),
            ImageFormat::Gif => Ok(Self::Gif(Default::default())),
            ImageFormat::Ico => Ok(Self::Ico(Default::default())),
            ImageFormat::Bmp => Ok(Self::Bmp(Default::default())),
            ImageFormat::Farbfeld => Ok(Self::Farbfeld(Default::default())),
            ImageFormat::Tga => Ok(Self::Tga(Default::default())),
            ImageFormat::Avif => Ok(Self::Avif(Default::default())),
            f => Err(UnsupportedImageOutputFormat(f)),
        }
    }
}

@BezPowell
Copy link
Contributor

BezPowell commented Nov 26, 2020

Another solution that comes to mind for the next version could be something like this:

I really like that suggestion, it feels very intuitive.

@jansol
Copy link
Contributor Author

jansol commented Nov 26, 2020

I really like that suggestion, it feels very intuitive.

And, perhaps more importantly, I like the flexibility it leaves for future additions. Should reduce the need for further API changes when adding new stuff for quite a while.

@HeroicKatora
Copy link
Member

HeroicKatora commented Nov 26, 2020

Please move the more general discussion to #1380 and keep this specific to AVIF.

@linkmauve
Copy link
Contributor

linkmauve commented Jan 28, 2021

AVIF decoding support is now in master! \o/

@HeroicKatora HeroicKatora moved this from New Features to Done in Version 0.23 Feb 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Version 0.23
  
Done
9 participants