search

webmproject / sjpeg

SimpleJPEG: simple jpeg encoder
Apache License 2.0
70 stars 12 forks source link

Quantization tables should have wider type (probably uint16_t) #21

Closed magicgoose closed 6 years ago

magicgoose commented 6 years ago

8 bits are not enough to store many useful default quant tables (example: https://github.com/mozilla/mozjpeg/blob/master/rdswitch.c#L322-L333 by the way, this is apparently the table which is used by default in mozjpeg, they probably have good reasons for using it instead of the JPEG Annex K).

Even when using a table which is all ≤ 255, after the scaling with quality parameter the values can become bigger and I think it's not good to have to limit them. (both the standard cjpeg tool and the mozjpeg fork allow values larger than 255 in final quantization tables, it can be seen by inspecting jpeg files produced by them with djpeg -v -v example.jpg > /dev/null; as an extreme example, a file produced by cjpeg -q 1 has quantizer values as big as 6050)

If you agree with the idea of changing the quant tables type from uint8_t to uint16_t, I might try to develop a patch.

skal65535 commented 6 years ago

Hi,

thanks for the suggestion. I'm not convinced going above 255 is that useful. In the samples i've seen, the large entries above 255 are of the bottom-right corner of the quant matrices and i wonder if any coefficient ever gets dequantized using them at such low quality setting! (of course, twisted bitstreams doing this are always possible).

In any case, supporting > 255 tables is a bit more involved that what it looks: you need to change MAKE_INV_QUANT() and QUANTIZE() macros, re-write the sse2/NEON code, etc.

Btw, i noted a bug in my DQT-parsing function, fixed here: https://github.com/webmproject/sjpeg/commit/897e13af57194d7aa34764cf08ba2064cf402a56 Thanks for bringing this to my attention!

magicgoose commented 6 years ago

hmm, yes, this indeed looks like too much work. I thought it's easier.