AOMediaCodec / av1-avif

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

ICC Profiles interacting with CICP Enums (in AV1 Bitstreams) #84

Closed joedrago closed 3 years ago

joedrago commented 4 years ago

(I realize this is very long, but there is a lot to explain, and a lot of ambiguity. Please take your time!)

The Refactor:

As a part of a refactor I have recently done in libavif in an attempt to handle more interesting use cases with color profiles, I've created a few additional use cases which I don't have answers for (from the standard), and a few use cases before that accidentally worked are likely to break. I decided that the best thing to do would be to lay out the goals of my refactor, and then cite the ambiguities in some of the use cases. Hopefully we can come up with some concrete decisions that'll drive all future AVIF implementations.

Prior to my new refactor, libavif had exposed the colr box from the AVIF container as a mutually-exclusive choice between an ICC profile and the contents of an nclx type box (BMFF 12.1.5.2). While I believe this is technically correct (as you can only have one colr box per item (HEIF 6.5.5.1)), two things make this a bit of a false dilemma / issue:

Since ICC profiles can't signal the matrixCoefficients enum, my current implementation of libavif (pre-refactor) assumed a value of matrixCoefficients==12 (chromaticity-derived), and would attempt to harvest the color primaries out of the ICC profile itself. In my other tool (colorist), I spent a lot of time attempting to properly harvest color primaries out of ICC profiles already, so while I know it can be done (and was functional in the friendliest of cases in libavif), it is full of awful corner cases. This is especially true in an iccMAX world with MPE, where colorant tags aren't even required to be in the ICC profile at all, let alone honoring chad tags properly, and making special decisions based on the the ICC profile's version.

This puts a huge burden on AVIF implementations to not only deliver ICC profiles up to the next layer, but be able to precisely interpret them to derive the YUV coefficients. It also tightly couples AVIFs with ICC profiles to using chromaticity-derived YUV coefficients, which throws out the ability to encode Identity(0) GBR-encoded AVIFS with ICC profiles (for true lossless RGB).

I decided to fix all of these simultaneously, by leveraging:

In my refactor, an avifImage now always exposes the CICP values (all defaulted to unspecified(2)) in addition to an ICC profile chunk, optionally. I now always pass the CICP values to the AV1 encoder, even if an ICC profile is present, and upon decoding, I always prioritize CICP in an nclx colr box if it is present, otherwise I supply what i found in the AV1 OBU. This decoupling allows for ICC profiles to exist, but still allows for a user to specify any matrixCoefficients they want, which I see as the best of both worlds.

This is where the ambiguity comes in. I'll list a handful of combinations of CICP (assuming all have an ICC profile present), and I'll explain what the new refactor does and why it might be wrong/ambiguous. Please take the time to read each one and give your opinion or shed light on parts of the standard I am not familiar with!

Case: Pre-existing incorrect images where the CICP was ignored before, such as 1/13/1

If an AVIF encoder (such as cavif) has already created an AVIF with an ICC profile under the expectations that the AV1 OBU's CICP was happily ignored, they could have completely incorrect values in them. For example, an ICC profile specifying ColorSpin (rotated primaries) or P3, but the AV1 OBU signals 1/13/1, the new refactor would convert from YUV using BT709 coefficients instead of leveraging the embedded primaries in the ICC profile, as matrixCoefficients==1 is BT709.

While this is a real issue that I've already observed, I'm confident that this is way more likely to exist in test suite images only, and once we all come to consensus on how this should work, we can just correct those and move on.

Case: CICP x/x/2 (unspecified matrixCoefficients)

This is another variant of the first case. As discussed here, the MIAF standard says to assume to BT709 coefficients if matrixCoefficients is unspecified. Now that we no longer assume chroma-derived (12) for matrixCoefficients, any attached ICC profile that doesn't happen to also provide BT709 color primaries will decode differently than intended here.

Case: CICP x/x/12 (chroma derived)

If I left the ICC profile colorant harvesting code intact during the refactor, this would be the way to mimic the old behavior. However, I really think that querying the color primaries from ICC profiles (to calculate the YUV coefficients) is an implementation quagmire, and I think we shouldn't do it. The CICP standard cites for matrixCoefficients==12:

Otherwise (MatrixCoefficients is equal to 12 or 13), the constants KR
and KB are computed as follows, using the chromaticity coordinates
(xR, yR), (xG, yG), (xB, yB) and (xW, yW) specified in Table 2 for the
ColourPrimaries code point for the red, green, blue and white colour
primaries, respectively

My intepretation of this is that specifying 12 for matrixCoefficients means you should look up the chromaticity based on the adjacent colorPrimaries enum, not an ICC profile. My refactor has pivoted to this strategy, and thus has thrown out all ICC primaries-harvesting code (thankfully). Unfortunately, there exists one more case which could be an issue.

Case: CICP 2/x/12 (chroma derived, unspecified color primaries)

So we've pivoted to interpreting 12 as "use the colorPrimaries enum", but that enum is signaling unspecified(2)! Going back to the CICP standard, colorPrimaries==2 is documented as:

Image characteristics are unknown or are determined by the application.

If we rely exclusively on the MIAF standard (as discussed here), we could interpret this as simply treading colorPrimaries(2) as if it were colorPrimaries(1) - BT709. This makes for easy implementation and echoes how matrixCoefficients==2 is handled.

However, as @wantehchang pointed out in our discussion, "determined by the application" could suggest that we could interpret colorPrimaries==2 with an ICC profile present as a request to harvest the primaries out of the profile, putting us back on the hook for ICC color primaries harvesting nuances.

Summary of Questions (assuming you've read all above thoroughly):

My biased opinions:

I think the ICC profile should just be a black box (binary blob) to libavif, and all YUV conversion it offers should be signaled entirely by the CICP values either in the nclx box or the OBU (as a fallback). As long as matrixCoefficients isn't 12 or 13 (chroma-derived), libavif is happily ignorant of all other CICP and ICC data in the first place. If matrixCoefficient is set to chroma-derived, it can simply switch on the colorPrimaries value, falling back to BT709(1) if it is unspecified. This matches how the matrixCoefficients fallback works, and is easy to implement.

None of this strategy interferes with any actual ICC profile usage, as the ICC profile doesn't get involved until we've converted back to RGB/full anyway, in which libavif is out of the picture anyway. I think this is the right way to go moving forward, but I'm looking to the standards authors and experts in the field on how we should interpret these values, and which values we should ignore/assume when specific data (such as ICC profiles) are present.

Thanks for the read, I'm looking forward to the discussion.

joedrago commented 4 years ago

Tagging people here that I hope will participate in this discussion:

@wantehchang @cconcolato @aklemets @dalecurtis @bnason-nf @ledyba-z

paperboyo commented 4 years ago

Terribly sorry to be commenting, as my understanding of both the science and the tech behind all this in non-existent, I’m merely someone who, as a user only, have some experiences trying to get correct colour from older file formats, mostly using consumer apps, but also, occasionally, some libraries or command line binaries directly. Apologies again, and please ignore, especially when it seem to suggest dangers of decisions already made in this or related standards.

I see several layers of signalling correct colour of image data:

  1. external specs (like when the CSS specifies sRGB a default color space for the web)
  2. image specs (like when SVG is defined as being sRGB when none other is specified)
  3. image metadata indirect (like sRGB PNG chunk or DCF Exif)
  4. image metadata direct (like gAMA, cHRM PNG chunks)
  5. image metadata blob (ICC profile embedded)

My experience is that all these ways, are rarely, if ever, in sync. And support for them is patchy at best, even among the best players and after years of standards being set in stone. In theory, everybody observes pts. 1 & 2 while looking at any extra information in later points and has a clear guidance on assuming missing and reconciling conflicting information. In theory, they are all always taken into consideration by an app and kept in sync when user overrides colour. In theory, some (pt. 4) were invented for simpler apps, but in practice they aren’t even observed by the most advanced ones, let alone information there updates to be kept in sync.

In my mind, pts. 3–4 are there only to save on image file weight related to embedding 5 (because being able to correctly act upon info in 3 & 4 requires having tech to deal with 5 present, anyway). In my mind, pts. 1–2 are there only to provide guidance on consistently dealing with a lack of any 3–5 information. If a default color space for the web wouldn’t incur a 2,6KB extra weight per image, pt. 3 wouldn’t even exist, one might half-joke.

What’s the point of me littering this discussion? It’s seems to me AVIF expanded greatly on pt. 4. color_primaries, transfer_characteristics and matrix_coefficients all provide a way of indirectly specifying a much wider set of standard profiles (sort of like an expanded pt. 3). But all this puts a greater burden on apps to keep information there consistent, in sync and to reconcile it all with information from other levels.

As most standard ICC profiles (eg the ones descibable by AVIF’s pt. 4) are synthetic and tiny, there isn’t much to be gained in terms of file size by specifying them via (a set of) metadata values versus explicitly embedding an ICC blob of those standard profiles.

In a way, an extremely simplified approach of just allowing pt. 2 and pt. 5 (specify a set of default profiles for a set of different colour models when no ICC is embedded and just giving ability of embedding ICC) without giving a possibility of muddying the waters with pts 3 and 4 at all, seems to present less possibility of confusion, expects less intelligence from implementers and lessens the burden on apps when it comes to keeping all levels correct when modifying files. It also makes it less possible for inexperienced users of libraries that do not guide your hand as much as consumer-level apps to produce image files with mismatched, crazy metadata (as, IIUC, in AVIF you realise pt. 3 by a specific, constant sets of values for pt. 4).

While I’m all-excited about this new car to have all the bells and whistles and new obscure features, I worry it will be hard to service. And – hard to drive.

Having written all this, I’m even less sure it is of any use, than when I started. Also, I definitely missed something obvious… Sorry again and sorry for the length. I am excited to read what proper people will have to say on the subject!

novomesk commented 4 years ago

I slightly disagree with Joe's opinion - I was hoping for deeper interaction with ICC during YUV conversion process. However I understand Joe's reasons and some of the advantages his implementation brings. I don't insist on my opinion. The more important is to clarify and to specify these things so everyone has compatible implementation.

joedrago commented 4 years ago

After re-reading my own post, I decided I should summarize how I think decoding and interpretation "should work" as a small numbered list of instructions, so we can discuss each part more easily:

  1. avifImage CICP (colorPrimaries/transferCharacteristics/matrixCoefficients) all initialized to Unspecified(2) before decode, ICC profile chunk initialized to "absent"
  2. If colr box is type nclx, set CICP from its contents
  3. If colr box is type prof, set ICC profile chunk
  4. If CICP was not set, set CICP from AV1 bitstream on image decode
  5. Convert YUV->RGB using matrixCoefficients: 5a. If matrixCoefficients is Unspecified(2), fall back to BT709(1) as MIAF states 5b. If matrixCoefficients is ChromaDerived(12), use the value of colorPrimaries to choose coefficients 5c. If colorPrimaries is Unspecified(2), fall back to BT709(1) as MIAF states
  6. Determine color profile for RGB pixels: 6a. If ICC profile is present, honor it, and ignore CICP colorPrimaries/transferCharacteristics 6b. If ICC profile is absent, Use colorPrimaries/transferCharacteristics to determine color profile, each falling back to 1/13 (SRGB) respectively if either are Unspecified(2), as MIAF states

This is exactly how my refactor works, and hopefully presents a more concrete context for my questions in my original post. Step 6 would (of course) be up to the user of libavif to perform/honor.

The questions in my original post concern steps 5 and 6. Prior to my refactor, step 5 began with "if ICC profile is present, assume matrixCoefficients==12 and try to harvest the primaries from the ICC profile." Alternatively, other places where such an ICC profile query could be made could be if 5b honors an ICC profile before falling back to the colorPrimaries CICP, or (conversely) if colorPrimaries==2, fall back to the ICC profile before falling back to BT709(1).

Again, I'd prefer to not require AVIF decoder implementers to need to parse ICC profiles to do YUV to RGB conversion, but I am curious what the AVIF standard authors have to say about it. I hope these steps illustrate the new path I'm recommending, and give us something to point at in the upcoming discussion.

ledyba-z commented 4 years ago

I really would like to join the discussion, but I am currently working on another task... So please wait a little, sorry. I will post my opinion soon (until tomorrow or day after tomorrow).

wantehchang commented 4 years ago

Joe: Thank you very much for the summary.

In Steps 5a, 5c, 6b you suggested how to resolve Unspecified(2) "as MIAF states". Your suggestion is actually an extension of what MIAF states, which I reproduced here (from Cyril's comment in issue #83):

So the MIAF updated text would be:

If a coded image has no associated colour property, the default property is defined as having colour_type equal to 'nclx' with properties as follows:

– For YCbCr encoding, the signal should be assumed to have a colour representation with colour_primaries equal to 1, transfer_characteristics equal to 13, matrix_coefficients equal to 1, and full_range_flag equal to 1.

Note that MIAF only describes the case when the colour property is absent. MIAF does not describe what we should do if a coded image has an associated colour property of colour_type 'nclx' that contains colour_primaries, transfer_characteristics, or matrix_coefficients with a value of Unspecified(2).

It is reasonable to extend the quoted MIAF text to apply to a partially unspecified colour property of colour_type 'nclx', as you suggested. But we should agree on that.

Your proposed extension of the quoted MIAF text seems to be the only natural option in Steps 5a and 6b. In Step 5c, there is another option if an ICC profile is present: we may be able to obtain the colour primaries from the ICC profile.

In general, Steps 5b and 5c somewhat contradict Step 6a. Step 6a says if an ICC profile is present, then ignore CICP colorPrimaries. But Step 5b says if CICP matrixCoefficients is ChromaDerived(12), then use CICP colorPrimaries, whether an ICC profile is present or not. We can say this apparent contradiction is not real because Steps 5b/5c and Step 6a apply to different processes, but I wonder if we can eliminate this apparent contradiction by imposing more constaints, for example, we can say if the AV1 bitstream has matrix_coefficients=ChromaDerived(12), then the AVIF file must not have an ICC profile.

joedrago commented 4 years ago

We can say this apparent contradiction is not real because Steps 5b/5c and Step 6a apply to different processes

I definitely think that the YUV conversion process should be considered independent (as much as possible) to the signaling of the color profile of the resultant RGB pixels, and this refactor is attempting to decouple them as such. If we're trying to offer people the same functionality as other common image libraries, we should try to distinguish between what an AVIF library enduser actually wants to give/receive vs what is an "implementation detail". We should lean hard into the notion of supplying full-ranged RGB(A) pixels bidirectionally, along with an optional ICC profile and "fallback" color profile signaling, the way (say) libpng does with its iCCP and gAMA chunks. Most people are happily ignorant to the fact that JPEG stores in YUV, because the common libraries trade in RGB.

I think we can do one step better than that by offering people the ability to tune matrixCoefficients instead of hiding it. However, with sensible defaults, that is just an additional encoder/decoder dial to tune. Hopefully, most people will just decode an AVIF, get the interleaved RGB(A) data from it (based on this conversation's ultimate decisions), and then just honor the signaled profile, ignoring the YUV step and any dials associated with it from that point on.

I wonder if we can eliminate this apparent contradiction by imposing more constaints, for example, we can say if the AV1 bitstream has matrix_coefficients=ChromaDerived(12), then the AVIF file must not have an ICC profile.

I think this would be a perfectly reasonable restriction, if we go down this path. I'm on board most any strategy that avoids harvesting color primaries from ICC profiles.

aklemets commented 4 years ago

Microsoft's implementation is aligned with Joe described. I currently don't parse the ICC profile in AVIF stack, and I don't want to have to add any ICC parsing code to it.

If the 'colr' box is present and contains 'nclx' information, then that takes precedence and I will use that information. If colr+nclx is not present, then I use the matrix coefficients, etc., that are obtained from the AV1 bit stream itself.

If 'nclx' is present but some fields are set to "Unspecified", then I pick some default values, I believe, in accordance with the MIAF spec.

If the file contains an ICC profile, then the ICC profile is exposed to the app through the Windows Imaging API along with the decoded pixels, that are now in RGB format. An app that uses the app can choose to use ICC profile to decide how to render the RGB pixels.

joedrago commented 4 years ago

That is fantastic news!

I still want to hear from @cconcolato and @ledyba-z for sure, but I hope once we've pinned down exactly how we want to signal (and restrict?) these values, we can get some kind of official addendum added for this.

(Side note: Is MS tracking 444 and matrixCoefficients==0 support? Both are important/useful.)

aklemets commented 4 years ago

This is what I am doing right now: If the 'colr' box is missing or does not contain the 'nclx' info, then I always try get the CICP settings from the image bit stream. Only if I cannot find CICP settings, then I apply the defaults as specified in MIAF. My reasoning for doing it this way is that the CICP info in the bit stream must be closer to the truth than some default values picked by the MIAF spec. Does this match what you were thinking?

For AV1-encoded images, I am assuming the encoding is YCbCr, so if I have to use the MIAF defaults, then the matrixCoefficients value will be 1 (BT.709). I don't think I have to worry about matrixCoefficients ever being 0 for an AVIF image, unless the author explicitly specified that value for some odd reason. But you said matrixCoefficients==0 can happen, so I must have missed something. Can you explain when it would happen?

About 444, yes, we want to support 444 but last time I checked there was still a bug in our decoder that caused the colors of 444 images to be rendered incorrectly.

joedrago commented 4 years ago

I'm doing the same "get CICP from AV1 bitstream, let nclx (if present) override it" as you mention, but my intention is to always properly honor matrixCoefficients, at any value. If it is 1, use BT709 coefficients, if it is 9, use BT2020 coefficients, etc. I don't have comprehensive support personally yet (YCgCo, BT2020CL, a few others), but I support 0 (identity) for sure. It is easy to implement and it allows for losslessly packing RGB without increasing the bit depth (unless you have another method). Part of this refactor was to allow matrixCoefficients to be "any" legal value independent of the presence of an ICC profile.

I don't foresee most of the matrixCoefficients values being used, but I can definitely see 0, 1, 6, 9, and 12 all getting some use.

ledyba-z commented 4 years ago

I wish I had attended this interesting discussion sooner!

I generally agree with Joe's algorithm (https://github.com/AOMediaCodec/av1-avif/issues/84#issuecomment-626480194), but when the matrixCoefficients is 12(chroma derived), I think it's better to take the ColorPrimaries from ICC profiles, to obtain smaller encoded images.

IIRC, ancient cavif didn't support ICC profiles at all and converted images with ColorPrimaries = BT709, TransferCharacteristics = BT709 and MatrixCoefficients = **BT2020** (yeah, we were beginners...).

Of course, we can encode or decode correctly with those settings without a doubt. However, I observed that encoded AVIF files will become smaller (about 2~3%?) when we encode with MatrixCoefficients = BT709, which is the same value as ColorPrimaries.

This seems natural, since the video encoder achieves highly encoding ratio by separating the color difference signal and the brightness signal by using the right matrix.

And to properly separate the color difference signal and the brightness signal, I think we have to calculate the MatrixCoefficients from ColorPrimaries.

(As I wrote in README in cavif, our main interest is compression rate.)

I am not so familiar about color space or ICC profile, etc., so if anyone is familiar with it, please let me know...

ledyba-z commented 4 years ago

Case: Pre-existing incorrect images where the CICP was ignored before, such as 1/13/1 While this is a real issue that I've already observed, I'm confident that this is way more likely to exist in test suite images only, and once we all come to consensus on how this should work, we can just correct those and move on.

I agree. It is okay to change the standard yet. It's not too late even from now.

joedrago commented 4 years ago

I don't have any measurements myself, but I believe you that matching the color primaries to the actual content buys you some efficiency. However, in terms of priority/tradeoffs, I think your implementation of ChromaDerived(12) could be simulated closely enough in real-world situations (as a cavif feature) that you could have your gains and we could avoid requiring people to parse ICC profiles to decode AVIF.

There is enough variety of primaries in the colorPrimaries CICP enum that cavif could pretty easily walk them once during an encode, and find the enum which has the smallest error term (distance to associated primaries) to your ICC-harvested primaries. You'd then signal 12 for your matrixCoefficients value (as you are), but would signal the "closest" value you found for colorPrimaries. For example, if your ICC profile was DCI-P3, you'd find that the error term for AVIF_COLOR_PRIMARIES_SMPTE432 would be near-zero, so you'd signal 12/x/12. I feel like this would track very closely to the gains you're seeing in all but the most contrived ICC profiles (such as test profiles like ColorSpin, which we shouldn't be optimizing for at all). You could even go one step further, and if you find that the best match is something directly referenced in matrixCoefficients (like 1), you switch matrixCoefficients away from 12 and use the "simpler" choice (1/x/1, 9/x/9, etc).

In summary, I believe you that there are wins with corresponding coefficients, but there might be an easy compromise here that saves AVIF implementers from the woes of ICC parsing on every decode.

(Edit: If it wasn't clear in my explanation for the compromise idea, you would then encode using those "closest" coefficients, not the actual coefficients, so any decoder would get the correct RGB values.)

ledyba-z commented 4 years ago

@joedrago thank you for response!

I found that mp4 movie and ffmpeg also supports ICC profile: FFmpeg now supports ICC Profiles in MP4

In other words, there should be the same problem also in AV1 video (maybe also in H265, VP9 or H264?). Could you give me some time to survey about it?

(Or if you know about ICC profile in AV1 video, please tell me)

wantehchang commented 4 years ago

(Or if you know about ICC profile in AV1 video, please tell me)

Ryo: You can take a look at the AV1 ISO Base Media File Format Binding Specification. It may contain the answer to your question.

ledyba-z commented 4 years ago

@wantehchang

The sample entry SHOULD contain a ‘colr’ box with a colour_type set to ‘nclx’. (2.3)

....Oh! ....Thanks!

ledyba-z commented 4 years ago

I found that WebP also supports ICC Profiles, but it always uses BT609's matrix.

https://github.com/webmproject/libwebp/blob/de08d72741c696f8f2988f2db3f02c773e0fb6f5/src/dsp/yuv.h#L79

I will also survey about JPEG XR (it also supports ICC Profile) tomorrow (It's 12 o'clock in Japan...).

ledyba-z commented 4 years ago

I agree that Joe's plan is simpler and easy to implement and it's enough in the vast majority of cases. (A photographer or designer who uses Adobe RGB might be faced with this problem.)

However, if color information is stored in the ICC Profile, I still feel it is natural to utilize it as possible, not throwing away.

@joedrago I have a question.

This is especially true in an iccMAX world with MPE, where colorant tags aren't even required to be in the ICC profile at all, let alone honoring chad tags properly, and making special decisions based on the the ICC profile's version.

Is it impossible to calculate ColorPrimaries in these cases (or we have to add additional assumptions to calculate)? (I have read some doc. According to them, we can define a color as a spectrum, not a point in xyY color space. And also, D50 is not expected in PCS any more...)

joedrago commented 4 years ago

I am certainly not advocating throwing it away (ever), simply not considering it for the conversion from RGB. The fact that we store in YUV can almost be considered an implementation detail of AVIF, in which coefficients we use is a minor decision as long as we signal which we used.

You can take an Adobe RGB image and convert it to YUV with any YUV coefficients, and as long as you use those same coefficients when you convert back, you will have only drifted one code point in each channel at most. In fact, if you signal Identity(0), the conversion won't drift at all! Once it is back in RGB, the project using libavif would then honor the attached ICC profile in their graphics engine. This allows any AVIF decoding library to not be responsible for understanding ICC; the actual app should still use it.

My suggestion earlier was to try to find the closest color primaries offered in the CICP table to the primaries that cavif might have discovered in the ICC profile, if you are really trying to find the closest match. I don't think this is going to give significant gains, but if you are measuring a 2 to 3% efficiency lift, I think that is an easy compromise.

Summary: I am not advocating throwing away the ICC profile. I am providing a solution that doesn't require AVIF decoders to read them; only pass them through.

joedrago commented 4 years ago

Just as a followup to my previous comment with actual testing, I ran the following commands using the master branch of libavif (which has all of my CICP refactor changes):

avifenc --min 0 --max 0 --cicp 2/2/0 Shoes-AdobeRGB.png shoes0.avif
avifenc --min 0 --max 0 --cicp 2/2/1 Shoes-AdobeRGB.png shoes1.avif
avifenc --min 0 --max 0 --cicp 2/2/9 Shoes-AdobeRGB.png shoes9.avif
avifdec shoes0.avif shoes0.png
avifdec shoes1.avif shoes1.png
avifdec shoes9.avif shoes9.png

I started with a PNG version of Shoes-AdobeRGB.jpg found here (and attached):

https://webkit.org/blog-files/color-gamut/

Shoes-AdobeRGB

If I compare either shoes1.png or shoes9.png to the original, I get a code point drift of 1:

image

image

And if I compare the original to shoes0.png, it is lossless:

image

shoes1.avif and shoes9.avif are only ~300 bytes different in size, and shoes0.avif is about 35% bigger, but lossless. All 3 of these AVIFs are carrying Adobe RGB's ICC profile, and none of them were created by parsing the ICC profile. This is the strategy I hope we use (and add to the standard).

ledyba-z commented 4 years ago

@joedrago

I am certainly not advocating throwing it away (ever), simply not considering it for the conversion from RGB.

Sorry, you are right. I apologize for my inaccurate wording.

And also, I reconsidered that when the ICC profile is attached to an AVIF file, color is considered more important, thus 300bytes or ~2% is acceptable.

Thank you very much for your experiment!

ledyba-z commented 4 years ago

However, I think there is a point left to discuss with this issue.

How should we treat monochrome mode?

As @joedrago discssed, when we convert YUV ⇔ RGB, we can use any matrix to convert (because they have invert matrices). However, in monochrome mode, U or V will be ignored, thus we can't restore RGB using invert matrices. In other words, a graphics engine receives different results depending on the matrixCoefficients, unlike the YUV ⇔ RGB.

Please tell me your idea!

My idea

Write in standard that "RGB color will be converted by CICP matrices and U and V will be ignored".

Also write "If you want to use ICC profile in AVIF file with monochrome mode, you have to convert it to monochrome images before encoding." in the standard (with more formal English!).

joedrago commented 4 years ago

I currently have monochrome implemented in a branch, and I am waiting for rav1e 0.4 to officially be released before committing it. My code doesn't behave any differently than if there is color, it simply populates/honors Y when it is all done and leaves UV alone. I still honor the matrix coefficients the same.

joedrago commented 4 years ago

Alright, after the meeting just now, it sounds like we have general agreement on these two things:

The only thing that is a lingering question are the proper "fallback" values (if any) when CICP is Unspecified(2). It sounds like Microsoft matches my first-pass implementation of falling back to 1/13/1 (respectively) if any individual value is 2, and we want to add some wording to the standard "strongly recommending" that Unspecified(2) is never used for matrixCoefficients. If we do this, the fallback is almost moot.

I think people agree on CP=1 and TC=13 for those fallbacks. MC could fallback to BT709(1) like the MIAF standard, or it could fallback to BT601(5) like JPEG. I would accept either as the official fallback; I'd just like us to pin something down.

I believe this the the ongoing discussion for this last bit:

https://github.com/MPEGGroup/MIAF/issues/2

novomesk commented 4 years ago

I estimate that the fallback will be MatrixCoefficients = 1

wantehchang commented 4 years ago

Joe: Thank you for leading the discussion yesterday. I propose that we refine the second bullet point in your summary as follows:

Also, should the second bullet point also cover matrixCoefficients==13 because it is also chromaticity-derived?

tdaede commented 4 years ago

I updated Firefox to handle the MIAF defaults here: https://bugzilla.mozilla.org/show_bug.cgi?id=1657346 (not including ICC support yet)

novomesk commented 4 years ago

On one hand I am sad that unspecified is handled in different ways in various implementations. But at least it will learn people to avoid unspecified when saving new files, because the rendering results are unpredictable. I would like that AVIF standard document gives recommendation how to handle unspecified values.

tdaede commented 4 years ago

At least in MIAF we have unambiguous guidance now, I think? For general AV1 streams, we still don't.

cconcolato commented 3 years ago

Given the recent changes in HEIF and MIAF, I propose to close this issue with no action in AVIF.

wantehchang commented 3 years ago

Given the recent changes in HEIF and MIAF, I propose to close this issue with no action in AVIF.

I agree with addressing this issue in HEIF or MIAF, not in AVIF.

Joe: Before we close this issue, it would be good to update your proposed algorithm to reflect the latest amendment to HEIF or MIAF . Thanks!

joedrago commented 3 years ago

I was waiting to commit changes to libavif until it ended up "officially" in MIAF, but I suppose it would be harmless to add sooner than that.

cconcolato commented 3 years ago

Discussed in the group and the final algorithm will be described in libavif