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

@JimBobSquarePants

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

@br3aker

@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!)

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?

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?

I wonder whether it was an issue with png optimization?

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

They have 3 different implementations for IDCT:

'slow accurate' aka float AAN with intrinsics
'fast integer' aka 16-bit integer with intrinsics
'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.

@antonfirsov

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

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?

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.

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

@br3aker

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?

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.

can you do diff command on ImageMagick with initial testorig.jpeg and generated png from ImageMagick?

I don't know how to do diffs with ImageMagick, so there should be a re-encoding of the output before I feed it to a diff tool. If I compare the BMP output with the PNG output they are identical, which excludes PNG encoder bug IMO.

Funny enough different online services produce different diff's of current reference pngs. I guess we simply can't get a confirmed result here. Question is, are you comfortable with raising percentage thresholds a bit?

Raising threshold can be an acceptable temporary solution, though I also consider it to be a tech debt. The long term thing would be to have a reference image generator (ImageMagick script? small C++ app?) that uses libjpeg-turbo with accurate IDCT settings.

Adding a little bit more fuel to the fire of my jpeg misunderstanding. This was taken directly from itu spec:

It is explicitly stated that rounding is only necessray for FDCT -> Quantization but not for Dequantization -> IDCT.

The only 'note' about dequantization is that we need to clamp dequantized values:

But I still don't understand why no-rounding approach produces worse results in the first couple of images but wins in the long run :/

I guess we don't really have a choice then but to follow the spec ☹️

I'll admit. I'm not sure why rounding would be considered beneficial since it's destructive.

I'd say we should wait a bit with this. I found a couple of bottnecks and will push fixes in smaller PRs without drastical changes like rounding.

But I still don't understand why no-rounding approach produces worse results in the first couple of images

Wait, aren't results "worse" only in compared to libjpeg and WIC runs with integer IDCT, and good when you compare to floating point PhotoShop codec run?

There's no way to determine what IDCT photoshop uses. All info I gathered is that ImageMagick has no options to set jpeg decoder/encoder to float DCT mode.

I've posted comparison results with ps/libjpeg reference and with/without rounding somewhere above:

Current master

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

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

No rounding & new IDCT

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

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

I see, missed that removal of rounding also increases difference to PS reference.

I won't exclude than that PS also uses integer algorithms, and would accumulate some generational loss the same way our current rounding codec does.

I agree that potential removal of rounding is something that we should keep on our radar instead of addressing it immediately. I can contribute by creating a reference image generator app that directly uses libjpeg and libpng but can't commit to any timeline before finishing my memory PR and a bunch of other things.

I can contribute by creating a reference image generator app that directly uses libjpeg and libpng but can't commit to any timeline before finishing my memory PR and a bunch of other things.

This would be awesome! Don't rush, memory thing is a lot more important than this. My main concern was about generational loss, removing rounding does improve performance but it's too small to spend time on it right now.

Waves Necromancer wand. 🪄

It's been 5 years since we looked at this but I'm on an issue bashing bust so....

Looking at the spec again, I do not think the extra rounding step is justified.

ISO/IEC 10918-1 explicitly defines rounding only as part of quantization during encoding:

Sq_uv = round(S_uv / Q_uv)

Where S_uv are the DCT coefficients and Q_uv is the quantization table. This produces the integer quantized coefficients that are written to the bitstream.

On the decoder side the process is simply the inverse:

R_uv = Sq_uv * Q_uv

This is dequantization. No rounding step is defined here because both Sq_uv and Q_uv are already integers.

After this the decoder performs the IDCT to reconstruct the spatial samples. The only unavoidable rounding occurs when converting the reconstructed samples to integer pixel values for the final image representation.

In other words, the spec requires rounding during quantization, not as a separate step in the decode pipeline after IDCT. The additional rounding historically present in some implementations appears to be an artifact of specific integer pipelines rather than something mandated by the standard.

Given that, I do not see a strong reason for ImageSharp to intentionally introduce additional rounding error purely to mimic legacy behavior. The decoder should simply perform dequantization and IDCT according to the spec and only round when converting the final sample values to the target pixel format.

Unless there is a concrete interoperability problem caused by removing the extra rounding, I would strongly lean towards simplifying the pipeline and removing it.

Spec conformance is not the only aspect here. I think there should be an investigation on libjpeg-turbo's decoding settings related to this behavior, and understand how they are configured in popular software and why. I would assume that they default to an integer pipeline (making the rounding behavior our codec wants to conform to), we could check what happens by opting into the floating point one.

IMO the above is a natural step, because if we want to make this change, we can't escape the work of actually producing some reference images with a more standard conform configuration of a widely adapted external decoder (which is in fact libjpeg). Otherwise we would need to define the expected output of our decoder by the decoder itself - IMO this wouldn't be wise.

Also, user expectations may vary, we should research them and potentially keep an opt-in switch into the "legacy behavior".

It might be even better to look into IJG libjpeg (or here), which is the reference implementation.

The reason libjpeg rounds is simply because it uses a fixed-point integer IDCT.

The original IJG implementation dates back to a time when floating point was either slow or unavailable on many systems, so the DCT/IDCT was implemented using scaled integers. In that model the math constantly rescales intermediate values with right shifts. A plain shift would truncate and introduce bias, so the code adds a bias before shifting, effectively rounding to nearest.

That rounding is therefore just a consequence of the integer arithmetic pipeline. It keeps the fixed-point math stable and unbiased when reducing precision between stages.

If the pipeline keeps higher precision (for example floats) until the final sample conversion, that intermediate rounding step simply is not needed.

Navigation Menu

Repository navigation

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

8 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	// 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);

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

Description

Activity

JimBobSquarePants commented on Oct 20, 2021

antonfirsov commented on Oct 20, 2021

antonfirsov commented on Oct 20, 2021

JimBobSquarePants commented on Oct 20, 2021

br3aker commented on Oct 21, 2021

br3aker commented on Oct 24, 2021

JimBobSquarePants commented on Oct 24, 2021

br3aker commented on Oct 24, 2021

JimBobSquarePants commented on Oct 24, 2021

antonfirsov commented on Oct 24, 2021

br3aker commented on Oct 25, 2021

8 remaining items

antonfirsov commented on Oct 25, 2021

br3aker commented on Oct 25, 2021

antonfirsov commented on Oct 25, 2021

br3aker commented on Oct 27, 2021

JimBobSquarePants commented on Oct 29, 2021

br3aker commented on Oct 29, 2021

antonfirsov commented on Oct 29, 2021

br3aker commented on Oct 29, 2021

antonfirsov commented on Oct 29, 2021

br3aker commented on Oct 29, 2021

JimBobSquarePants commented on Mar 7, 2026

antonfirsov commented on Mar 9, 2026

antonfirsov commented on Mar 9, 2026

JimBobSquarePants commented on Mar 12, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions