AOMediaCodec / av1-avif

AV1 Image File Format Specification - ISO-BMFF/HEIF derivative
https://aomediacodec.github.io/av1-avif/
BSD 2-Clause "Simplified" License
450 stars 40 forks source link

Please add some AVIF testfiles with matrix_coefficients == 0 #195

Open novomesk opened 1 year ago

novomesk commented 1 year ago

I think it would be useful to have testfiles with zero MatrixCoefficients, so that various AVIF implementations can test before release.

For example following file is rendered differently in various applications, because they perform unnecessary YUV->RGB conversion: avif-rgb-8bit.zip

Safari on iPhone (iOS 16.0): iOS16-Safari

AV1 Extension on Windows: BTW, there is some weird stripe on the right side. Microsoft-AV1_Video_Extension

It looks OK in the following apps (background color may be different but that's expected):

Opera browser: Opera

qimgv viewer: qimgv

GIMP: GIMP

leo-barnes commented 1 year ago

There is actually already test content using identity matrix in the repo. Here is one example: https://github.com/AOMediaCodec/av1-avif/blob/master/testFiles/Netflix/avif/hdr_cosmos01000_cicp9-16-0_lossless.avif

The reason why the file you attached renders differently in various places is because it's actually kind of ambiguous. It contains an ICC profile but no nclx color box. ICC profiles don't actually specify the YCbCr matrix. It looks like some implementations are peeking into the actual elementary stream to figure out what the matrix should be.

The latest wording in MIAF attempts to clear this up, but I don't think that version has been published yet. The gist of it is:

  1. You can now have both an ICC profile and an NCLX color box. ICC profile has precedence. This means you can use the ICC profile as usual, but use the NCLX box to specify full/video-range and YCbCr matrix.
  2. If there is no color box, it defaults to primaries 1, transfer characteristics 13, matrix coefficients 5 or 6 (they are the same), full range 1.

There is also a note saying:

Any colour information in the bitstream is ignored by the MIAF reader and MIAF renderer processing models. The colour information property whether explicit or default, takes precedence over any colour information in the image bitstream.

I still think the wording is slightly unclear on what should happen if you only have an ICC profile. @cconcolato, can you remember what the decision was? Since the ICC profile does not override the default matrix coefficients, should the default be used rather than what might be in the elementary stream?

novomesk commented 1 year ago

The colour information property whether explicit or default, takes precedence over any colour information in the image bitstream.

The clarification would be useful. Because I understand that "takes precedence" doesn't mean to ignore completely the info from bitstream.

leo-barnes commented 1 year ago

The clarification would be useful. Because I understand that "takes precedence" doesn't mean to ignore completely the info from bitstream.

Well, it does actually explicitly say Any colour information in the bitstream is ignored by the MIAF reader. A strict interpretation of that would be that if YCbCr matrix is not specified in an nclx box (or potentially in the av1C in the optional config OBUs), the default value shall be used.

I still think the spec could spell out exactly what is meant to happen when you only have an ICC profile just to make it abundantly clear what the expectation is.

novomesk commented 1 year ago

It means that files with ICC only will be handled like matrix coefficients 5,6

But it will break lot of older files which use coefficients 1 in the bitstream. MC == 0 is not so frequent but MC == 1 is used. Those old files will be decoded with slightly different colors, people will not be happy that.

baumanj commented 1 year ago

Per @cconcolato 's comment here, if there is no colr box with colour_type 'nclx', then if the AV1 bitstream has color_description_present set in the sequence header and the value of MC isn't 2 (unspecified), then the MC value from the AV1 bitstream is used.

I believe that's what was agreed to in https://github.com/AOMediaCodec/av1-avif/issues/84 and is what I implemented in Firefox. I think the files you're referring to with ICC embedded in a colr box and MC == 1 in the AV1 bitstream should be handled correctly, @novomesk. That said, I'm not sure there are any test files like that in this repo, so maybe it would be good to add some.

leo-barnes commented 1 year ago

Per @cconcolato 's comment here, if there is no colr box with colour_type 'nclx', then if the AV1 bitstream has color_description_present set in the sequence header and the value of MC isn't 2 (unspecified), then the MC value from the AV1 bitstream is used.

That's not what the closing statement from @cconcolato says though:

The MIAF group discussed this issue and resolved to say in the upcoming amendment text that:

  • the color property (whether defaulted or explicit) always takes precedence over the bitstream value
  • when it's defaulted it means 1/13/5 (or 6)/1. RGB requires an explicit color box (not a possible default anymore, resolving this issue).
  • we still have to discuss what 2/2/2 means if used in the colour property.

@baumanj

I believe that's what was agreed to in #84 and is what I implemented in Firefox. I think the files you're referring to with ICC embedded in a colr box and MC == 1 in the AV1 bitstream should be handled correctly, @novomesk. That said, I'm not sure there are any test files like that in this repo, so maybe it would be good to add some.

Hmm... I'm not completely sure that's what we agreed upon in the meeting, but I'll try to dig up the meeting notes to check. It's not the text that is going into the as-of-yet unpublished version of MIAF at least (it explicitly tells you that the bitstream shall be ignored).

HEIF and especially MIAF has from the start been trying to make sure that you should never need to reach into the video bitstream itself to figure out color stuff since that is not codec agnostic. For HEVC, it was considered ok (but not mandated) to check in the codec config box, since that is not part of the bitstream and the codec config is defined in ISOBMFF. But with AV1, this information is usually not present in the codec config (most likely since it can be described with a codec agnostic colr box). Unfortunately the AV1 ISOBMFF bindings did not mandate that a colr box is mandatory if the info is not present in the codec config.

So the exact behaviour for AVIF files with only an ICC profile with the old wording in MIAF is actually undefined. Which @novomesk actually noted here: https://github.com/AOMediaCodec/av1-avif/issues/84#issuecomment-669990448 The new wording in the MIAF spec tries to fix this by defining the expected behaviour. Which to my eyes is to always ignore what is in the bitstream.

So the example file here is relying on undefined behaviour. Like in C, anything might happen when you do that. :)

novomesk commented 1 year ago

Writing two boxes (ICC+NCLX) helps in iOS 16.0 to get expected rendering. It doesn't help for the Windows extension yet.

I also observed that AVIF with 12bit depth is not displayed on iOS 16.0.

I think web developers need to know what AVIF features they can use (or what to avoid) so that their files work across the browsers. Android 12 used to support less AVIF files than Google Chrome on the same phone.

baumanj commented 1 year ago

HEIF and especially MIAF has from the start been trying to make sure that you should never need to reach into the video bitstream itself to figure out color stuff since that is not codec agnostic.

Why is that? I agree it's more convenient and consistent to get it from the colr box(es) and that it is potentially useful for overriding a bitstream with incorrect CICP values, but in the case that there's only an ICC profile in the colr box and the bitstream MC explicitly differs from the default, it seems more likely that using the default will result in incorrect rendering relative to the writer's intent.

Why should we try so hard to avoid getting this data from the bitstream? To me, it seems most likely to be accurate to how the image was encoded, and by the time a renderer is applying the MC, the bitstream has already been decoded, right? I'm all for codec agnosticism, but something at this point has to be aware of the codec type just to call the right decode function.

I could get behind the idea that this case is too ambiguous and that it's a "shall not", so readers are encouraged to reject the input, but I find it highly unlikely that we're going to see this handled consistently in practice if the standard says to ignore the bitstream's explicit signal in favor of the implied default from the container.

As for putting it in the codec config, do you mean in the configOBUs of the av1c box ? I don't think I've ever seen that. While technically possible, that seems to make the situation even more confusing since now there would be 3 places (4 with defaults) to derive the information. Where does it fall in terms of precedence?

baumanj commented 1 year ago

I think web developers need to know what AVIF features they can use (or what to avoid) so that their files work across the browsers. Android 12 used to support less AVIF files than Google Chrome on the same phone.

Given the incredibly wide variety of features possible with HEIF, it may be interesting to devise some sort of syntax to communicate this information via additional parameters on the MIME type, right? That should probably be a whole separate issue/discussion, though.

leo-barnes commented 1 year ago

@baumanj

Why is that? I agree it's more convenient and consistent to get it from the colr box(es) and that it is potentially useful for overriding a bitstream with incorrect CICP values, but in the case that there's only an ICC profile in the colr box and the bitstream MC explicitly differs from the default, it seems more likely that using the default will result in incorrect rendering relative to the writer's intent. Why should we try so hard to avoid getting this data from the bitstream? To me, it seems most likely to be accurate to how the image was encoded, and by the time a renderer is applying the MC, the bitstream has already been decoded, right? I'm all for codec agnosticism, but something at this point has to be aware of the codec type just to call the right decode function.

YCbCr <-> RGB conversion is typically not handled by the codec but rather at the container level, in the same way that color primaries and transfer function typically is not really applied or used by the codec. I would guess the reason it's possible to signal it at the codec level is twofold:

Given that this is happening at the container level rather than at the codec level, I would argue that it's actually more likely that the container level has the correct values and that whatever is in the bitstream is incorrect. But all of this is actually kind of beside the point. :)


It would be fine to check the bitstream for CICP if the spec mandated that this is the required behaviour. But the spec doesn't (or rather didn't) say anything about it at all, which means the behaviour is undefined. If we want to make the behaviour defined, we have two choices:

  1. Mandate that matrix coefficients and/or full/video-range settings shall be extracted from bitstream if missing
  2. Mandate that CICP and/or full/video-range settings shall be inferred from defaults if missing

NOTE: Notice that I do not include extracting full CICP from bitstream in option 1. The MIAF spec has mandated from the start that if there is no colr box of any kind, sRGB + full-range is the default and trumps whatever is in the bitstream. Any parser that does not do this is not compliant with either the old or the new wording of the spec, since this was was fully defined.

Option 1 may sound perfectly reasonable for a simple single-item file. But as soon as you introduce a simple grid item you have problems.

MIAF added restrictions on grid items to allow easier and more performant parsing and rendering. The idea was to make sure that decoding could be done into a single canvas buffer of the native format of the bitstream. Why is this important?

So how does MIAF add restrictions to make sure that all tiles can be decoded into a shared canvas?

For all other codecs than AV1, this is enough. But the AV1 codec config does not specify matrix coefficients or full/video-range. So I can create a file like the following:

Given that items 1 and 2 share av1C, we can be guaranteed that they have the same subsampling and bit-depth. We can also know that they have the same color primaries and transfer function, or at least that the container overrides whatever is in the elementary stream by providing an ICC profile.

How do we know that they have the same matrix coefficients and range? We don't.

We can't decode into a shared canvas unless we first check the bitstreams for all the tiles. This means we can't do streamed decoding (typically pretty important on the web) since we need to wait until we have the final tile.

Things only get more complicated as you start to create more complex files. Hence why I filed this issue: https://github.com/AOMediaCodec/av1-avif/issues/194

How does this compare with mandating that the container level explicitly or implicitly overrides whatever is in the bitstream? Once you have the meta box, you know everything you need to know about the file.

As for putting it in the codec config, do you mean in the configOBUs of the av1c box ? I don't think I've ever seen that. While technically possible, that seems to make the situation even more confusing since now there would be 3 places (4 with defaults) to derive the information. Where does it fall in terms of precedence?

And you probably won't either. :) IIRC the ISOBMFF-AV1 bindings say that the optional config OBUs should (or maybe even shall?) not be used for stills. So it's not really possible to put it in the codec config. But since the ISOBMFF-AV1 bindings don't mandate that a colr box is required, we then basically have undefined behaviour.

leo-barnes commented 1 year ago

We can of course play devil's advocate and say that the number of files out there that contain a grid of mixed matrix coefficients or full/video-range are most likely 0, and that the number of files out there that contain an ICC profile and non-601 matrix coefficients is sizeable and we should change the spec to take this into account since the behaviour was undefined.

I'm fine with that, but it then needs to actually be specified in the spec that this is the required behaviour. Which needs the following to happen:

  1. MIAF wording is changed to allow AVIF to query the bitstream for the case where we have an colr-ICC profile but no colr-NCLX.
  2. AVIF spec is changed to solve #194. Maybe AVIF can require that items that share av1C shall have identical Sequence Header OBU or something like that.
  3. AVIF spec (or potentially ISOBMFF-AV1 bindings?) is changed to indicate that if color properties are only partially specified (but not completely unspecified since the default then holds) the missing pieces should be retrieved from the bitstream.
novomesk commented 1 year ago

@leo-barnes When you have time, do you know why this animation doesn't work in Safari on my iPhone8 with iOS 16.1? animation2.zip

I saw some news that animated AVIF should be supported.

leo-barnes commented 1 year ago

@leo-barnes When you have time, do you know why this animation doesn't work in Safari on my iPhone8 with iOS 16.1? animation2.zip

I saw some news that animated AVIF should be supported.

@novomesk The WebKit folks said it's due to this: https://commits.webkit.org/253002@main

For iOS it will land in a coming update. For macOS, it is already fixed in Safari Tech Preview -- release 156.

novomesk commented 1 year ago

Thanks! So we will wait till the next update.

Xredmi commented 1 year ago

I think it would be useful to have testfiles with zero MatrixCoefficients, so that various AVIF implementations can test before release.

For example following file is rendered differently in various applications, because they perform unnecessary YUV->RGB conversion: avif-rgb-8bit.zip

Safari on iPhone (iOS 16.0): iOS16-Safari

AV1 Extension on Windows: BTW, there is some weird stripe on the right side. Microsoft-AV1_Video_Extension

It looks OK in the following apps (background color may be different but that's expected):

Opera browser: Opera

qimgv viewer: qimgv

GIMP: GIMP

Xredmi commented 1 year ago

I think it would be useful to have testfiles with zero MatrixCoefficients, so that various AVIF implementations can test before release.

For example following file is rendered differently in various applications, because they perform unnecessary YUV->RGB conversion: avif-rgb-8bit.zip

Safari on iPhone (iOS 16.0): iOS16-Safari

AV1 Extension on Windows: BTW, there is some weird stripe on the right side. Microsoft-AV1_Video_Extension

It looks OK in the following apps (background color may be different but that's expected):

Opera browser: Opera

qimgv viewer: qimgv

GIMP: GIMP