nico
commented
5 months ago Oh wait, I had a local diff applied that makes this a bit better. On trunk, the "reencode 100 times" experiment leads to a final color of (98, 150, 90), which is very off.
My local diff is not truncating to i16 in the middle of the inverse DCT, and rounding in the YCbCr->RGB conversion:
local diff
```diff diff --git a/Userland/Libraries/LibGfx/ImageFormats/JPEGLoader.cpp b/Userland/Libraries/LibGfx/ImageFormats/JPEGLoader.cpp index 32d19ae49d..144c42c0a9 100644 --- a/Userland/Libraries/LibGfx/ImageFormats/JPEGLoader.cpp +++ b/Userland/Libraries/LibGfx/ImageFormats/JPEGLoader.cpp @@ -1406,6 +1406,8 @@ static void inverse_dct(JPEGLoadingContext const& context, VectorBut that doesn't fix all the things (it's a bit better though), and it's kinda ad-hoc.
It'd be good to know what the IDCT code we have was based on (@devsh0, do you remember?), etc.
Apparently ITU T83 is about jpeg conformance testing, but it's not freely available – more ISO nonsense :/
LucasChollet
devsh0
I think the jpeg decoder might have channels off by a small amount, due to various things suggesting it:
If I make
green.pngfilled with (0, 255, 0) and then repeatedly decode and re-encode (for i in {1..100}; do Build/lagom/bin/image -o green.png green.jpg; Build/lagom/bin/image -o green.jpg green.png; done), the image's color is eventually (1, 239, 6). After that it seems to no longer change. (Edit: See comment below!) If I do the same with macOS's built-in image processor (for i in {1..200}; do sips -s format png -o green.png green.jpg; sips -s format jpeg -o green.jpg green.png; done), it only goes to (0, 255, 1). Now, jpeg compression is lossy, and maybesipshas a higher default quality and this is expected – but it seems like a pretty big error?The zip file available for download at https://displaycal.net/icc-color-management-test/ has a png, a jpg, and a tiff all containing exactly the same color profile. If I convert the PNG or the TIFF file to sRGB then the output looks like expected, but the jpeg has its colors off. (I imagine the color profile maps small differences in pixel values in the input space to big differences in the output.) And converting the jpg to a png with sips and then doing the color space conversion from that png file using
imagealso produces good output.Build/lagom/bin/icc -n sRGB --reencode-to serenity-sRGB.icc
Build/lagom/bin/image -o out-jpg.png ~/Downloads/ICC-CM-Test/ICC\ Rendering\ Intent\ Test.jpg --convert-to-color-profile serenity-sRGB.icc Build/lagom/bin/image -o out-png.png ~/Downloads/ICC-CM-Test/ICC\ Rendering\ Intent\ Test.png --convert-to-color-profile serenity-sRGB.icc
Similarly, if I convert https://littlecms.com/img/blog/check_full.jpg to sRGB, the background is too light, but if I convert it to png with a non-serenity tool first and then do the color space conversion on the PNG, it's better (not perfect: The background is now no longer too bright, but the foreground is too bright).
It's possible to strip the color profile and convert to png to see the raw pixel data before the synthetic profiles (using either
sips -d profile -s format png -o out.png in.jpg, orimage -o out.png --strip-color-profile in.jpg, and then compare the raw rgb output decoder of serenity's jpeg loader to other programs. The color values here are a bit off as well. (This could be due to us doing nearest-neighbor chroma upsampling, but all these files don't use chroma downsampling, so this doesn't apply.)