Default to chroma=444/matrix_coefficients=0 in lossless mode

[kleisauke]

Resolves: #62.

[bigcat88]

And if encoding goes in YCbCr mode, why should be matrix_coefficients set to 0 automatically?
The same for L(grayscale) and LA(grayscale with alpha) modes.

[kleisauke]

And if encoding goes in YCbCr mode, why should be matrix_coefficients set to 0 automatically?
The same for L(greyscale) and LA(greyscale with alpha) modes.

I'm not entirely familiar with these pixel formats. The latest revision of this PR makes matrix_coefficients=0 default in lossless mode instead of forcing it, allowing users to still use other matrix coefficients.

For reference, avifenc fails when chroma is not 4:4:4 or the matrix coefficients is not identity (x/y/0) in lossless mode.
https://github.com/AOMediaCodec/libavif/blob/484984aa0365b71d211a7f6612f30787a1c88230/apps/avifenc.c#L1949-L1956
https://github.com/AOMediaCodec/libavif/blob/484984aa0365b71d211a7f6612f30787a1c88230/apps/avifenc.c#L1971-L1986
https://github.com/AOMediaCodec/libavif/wiki/CICP#common-cicp-triples

[bigcat88]

matrix_coefficients=0 should not be default for lossless mode, as not all colour spaces require it, it is only for RGB(A).
With the subsampling chroma is the same situation, if image was already in 4:2:0 or 4:2:2 for example, you do not want to set chroma=444 when encoding it in lossless mode.

[farindk]

We have to differentiate two things here. From the codec side of view (h265 or av1), the codec can be switched to lossless no matter what input color space. I.e. we could have YCbCr 4:2:0 and encode that lossless from the view of the codec. That's what is switched on with the lossless flag(s) in heif-enc. However, that is not necessarily lossless from the user's point of view because there can be color conversions and chroma changes. Hence the need for matrix_coefficients=0 and chroma-4:4:4.

This PR is mixing the two concepts. I would like to avoid this. However, I'm fine to redefine the parameters such that "-p lossless=true" is the codec setting, but "--lossless" is not a mere shortcut for that, but also sets chroma and matrix as noted at the bottom of the help:

Note: to get lossless encoding, you need this set of options:
  -L                       switch encoder to lossless mode
  -p chroma=444            switch off chroma subsampling
  --matrix_coefficients=0  encode in RGB color-space

However, that could be handled in the heif-enc without introducing changes to the library behaviour.

[farindk]

I've implemented this differently in heif-enc in this commit: ee69907
Do you think that is ok?
I'm still a bit worried as this forces also YCbCr input to be encoded in RGB. Maybe we would have to check the input color space and set the matrix coefficients accordingly.

[farindk]

I've improved this further in 94c3b21. When the input is YCbCr, -L automatically sets the output NCLX to the input and chooses the matching chroma format such that the encoding is also lossless for all YCbCr formats.

[bigcat88]

I've improved this further in 94c3b21. When the input is YCbCr, -L automatically sets the output NCLX to the input and chooses the matching chroma format such that the encoding is also lossless for all YCbCr formats.

wow, that's really cool

[kleisauke]

Thanks for the explanation! Indeed, this logic should probably not be included at library level. I'll abandon this PR in favor of the commits mentioned above. 👍

libvips always encodes in heif_colorspace_RGB, so I'll investigate if we could set matrix_coefficients=0 by default in lossless mode. This would help to resolve this FIXME:
https://github.com/libvips/libvips/blob/c37b9750fdd44c97461f32da7a4b17b628f6f0e8/test/test-suite/test_foreign.py#L1416-L1418

Note that heif-enc has a similiar issue with commit 94c3b21 when using -L, it looks like nclx_matrix_coefficients = 0 has no effect.

libheif/examples/heif_enc.cc

Line 797 in 94c3b21

nclx_matrix_coefficients = 0;

Since it's conditionally excluded here:

libheif/examples/heif_enc.cc

Lines 845 to 846 in 94c3b21

	if (!lossless) {
	error = heif_nclx_color_profile_set_matrix_coefficients(nclx, nclx_matrix_coefficients);

Reproducer:

# heif-enc
$ curl -LO https://wsrv.nl/transparency_demo.png
$ heif-enc -L transparency_demo.png -o x.avif
$ heif-convert x.avif output.png
File contains 1 image
Written to output.png
$ vips subtract transparency_demo.png output.png x.v
$ vips abs x.v x2.v
$ vips max x2.v
1.000000

# vips
# subsample_mode=off is not needed after PR https://github.com/libvips/libvips/pull/3760
$ vips copy transparency_demo.png x.avif[lossless,subsample_mode=off]
$ vips copy x.avif output.png
$ vips subtract transparency_demo.png output.png x.v
$ vips abs x.v x2.v
$ vips max x2.v
1.000000

[farindk]

@kleisauke Yes, the bug for RGB input is a regression from the second commit. Thanks for reporting this. Should be fixed now.