Post IDCT rounding needs to go - it induces noise & generational loss and takes extra time to compute

[br3aker]

New AAN IDCT implementation which I'm working on as part of jpeg decoder optimization/refactoring provides a faster and more accurate IDCT calculation.

Current conversion pipeline has this "conform to libjpeg" step:

ImageSharp/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBlockPostProcessor.cs

Lines 76 to 79 in 07d50d0

	// To conform better to libjpeg we actually NEED TO loose precision here.
	// This is because they store blocks as Int16 between all the operations.
	// To be "more accurate", we need to emulate this by rounding!
	block.NormalizeColorsAndRoundInPlace(maximumValue);

This does a huge quality hit (well, not really 'huge' if we are not talking about a couple of re-encodings) with noise. This is especially noticeable for so called 'generational loss'. Below there's a comparison of rounding vs no-rounding jpeg re-encoding without any change to the original image - no quality or subsampling changed.

Original image

5 generations

*Bottom row is a difference between original image and re-encoded one

25 generations

---

Of course users won't re-encode their images 5 or even 25 times but this example shows the uselessness of IDCT rounding. Right now ImageSharp has a slower but more accurate IDCT which should be a 'feature', not a 'downside' which must be hidden. And on top of that it takes extra time to compute so it's a win-win :P

Issues like #245 can't be solved simply because JFIF is a lossy image container - 1/255 pixel miss is an expected trade-off.

Update
Same applies to downscaling decoding from #2076.

No-rounding code provides slightly better PSNR results but linked PR would provide IDCT algorithms with rounding before this issue is resolved.

@JimBobSquarePants @antonfirsov

[JimBobSquarePants]

@br3aker Absolutely agree. I wasn't aware how detrimental the rounding was to quality. Feel free to remove it as part of your optimization (which I'm very existed about!)

[antonfirsov]

I'm generally fine sacrificing some of our libjpeg conformance in favor of performance and actual quality, but I would prefer to see some quantitative data to understand the impact better.

Our reference PNG-s were originally created by resaving libjpeg-turbo output, and we should keep doing that. The questions we should ask: What is the average change in decoder """"accuracy"""" in percents (master VS your branch)? Does any of our decoder tests fail with the current tolerance settings after the change?

[antonfirsov]

Related:

I just noticed #1694 (comment) and 2e5b0ad. As far as I remember, I used ImageMagick to generate the output, which uses libjpeg-turbo internally. Does this mean that the bug is actually not in our decoder but in libjpeg-turbo? Does photoshop maintain it's own codec?

[JimBobSquarePants]

I wonder whether it was an issue with png optimization?

[br3aker]

Does this mean that the bug is actually not in our decoder but in libjpeg-turbo?

They have 3 different implementations for IDCT:

1. 'slow accurate' aka float AAN with intrinsics
2. 'fast integer' aka 16-bit integer with intrinsics
3. 'accurate integer' aka 16-bit integer with intrinsics with more accurate but slower algorithm

16 bit integer is a quality tradeoff for performance, if you didn't specify accurate float IDCT switch manually then ImageMagick should had generated inaccurate results.

Oh and that test image had bad edges between colors, I guess it was saved via 4:2:0 subsampling which led to averaged colors.

Does photoshop maintain it's own codec?

Most likely yes but we can only guess.

Does any of our decoder tests fail with the current tolerance settings after the change?

No

I would prefer to see some quantitative data to understand the impact better

Current pixel comparison tests (master -> no rounding branch):

badeof.jpg                0,3756% -> 0,4222%
Calliphora.jpg            0,0000% -> 0,1084%
cmyk.jpg                  0,0000% -> 0,0167%
jpeg400jfif               0,0000% -> 0,0001%
jpeg420small.jpg          0,2863% -> 0,3807%
jpeg444.jpg               0% -> 0,0511%
MultiScanBaselineCMYK.jpg 0% -> 0,0084%
testorig.jpg              0,3754% -> 0,4219%

I would actually try to dig into it more because there's clearly some extra noise with no-rounding mode, maybe it's even caused by encoder rounding errors. Problem with current rounding master implementation is that it produces a bit less noise but leaves some contrast stains which are then multiplied during multiple re-encodings.

Compare to more distributed noise from no-rounding mode:

Which stays the same during further encoding generations.

*Difference shown above is normalized, those are not spottable by human eye in normal conditions.

[br3aker]

I found a problem in current tests - spoiled 'reference' PNG images.

Here are some difference examples for original jpeg and reference png:

Those are not normalized so you can see that difference is very much human-eye spottable. Can I have a permissions to overwrite all of jpeg reference images with photoshop lossless png save? @antonfirsov

[JimBobSquarePants]

There's a strong chance those images were spoilt by bad optimization. Since we don't store the images in the repository directly anymore I have no issue updating the output references if you have found issues with them.

That's assuming these are encoding references yeah?

[br3aker]

That's assuming these are encoding references yeah?

Nope, decoder. I didn't check all of them but every image I did check has lossy compression artifacts.

[JimBobSquarePants]

Ah sorry, I misread. So those are the references we generate by decoding jpeg and saving as png. Sure go ahead then

[antonfirsov]

There's a strong chance those images were spoilt by bad optimization.

If this is the case, it should be a bug in optipng, which is very unlikely IMO, since I haven't seen an issue in any other case. I think what we see is likely a difference between proprietary PhotoShop codec being accurate VS libjpeg-turbo on default (fast integer?) settings.

@br3aker can you check for some example image if there is a difference between resaving with photoshop VS resaving with chrome? The latter definitely uses libjpeg-turbo with defaults.

Unless it causes unreasonable pain finishing the optimization work, I would prefer to create reference images with an open tool any contributor has access to if needed instead of PhotoShop. Maybe we can utilize libjpeg-turbo with "slow accurate" IDCT? @dlemstra is there a way to achive this with ImageMagick?

[br3aker]

I have a strong feeling that those PNG's were saved with lossy compressions before entering optipng. You can see the difference without any amplifying. Look for these images for example:

.\ImageSharp\tests\Images\Input\Jpg\baseline\testorig.jpg
.\ImageSharp\tests\Images\External\ReferenceOutput\JpegDecoderTests\DecodeBaselineJpeg_testorig.png

They have quite a difference, resaving initial jpeg to png via photoshop yield no difference.

can you check for some example image if there is a difference between resaving with photoshop VS resaving with chrome? The latter definitely uses libjpeg-turbo with defaults.

But reference png(s) are spoiled, not jpeg(s). Chrome/PS would produce different results due to internal quality settings. But we want reference pixels for testing which is done via lossless png images which have a diff compared to initial jpegs.

Photoshop lossless png matters even on current master. Test results for comparing pixels after jpeg decoding:

Current png reference
*** Jpg/baseline/testorig.jpg ***
Difference: 0,3754%

PS jpeg resave to lossless png
*** Jpg/baseline/testorig.jpg ***
Difference: 0,1936%

Test results on my branch with new IDCT and no rounding:

Original png reference
*** Jpg/baseline/testorig.jpg ***
Difference: 0,4219%

PS jpeg resave to lossless png
*** Jpg/baseline/testorig.jpg ***
Difference: 0,2683%

Unless it causes unreasonable pain finishing the optimization work

It does, I didn't check all the tests with comparison and some of them actually fail without rounding. So we have three options: leave rounding in place (I would prefer not to), make test pass difference threshold higher or re-encode reference images.