allow changing predictor used in delta palette #627
Conversation
jyrkialakuijala
left a comment
There was a problem hiding this comment.
Nice!
I'll look into why weighted predictor is so bad. My original hypothesis (and why I designed Average4) was that dithering messes up the directionality assumptions.
|
In general we could use longer codewords in benchmark codec specification for palette (than lp, p and P). Only about 5 people in the world are using this I think and having more readability/clarity would help them and the rest of the people. Something like jxl:lp:palette_size=7:delta_palette_size=11:delta_palette_predictor=4:palette_index_predictor=1 would not be excessive to me. This does not need to be addressed in this MR. |
Yes, I also suspect that the self-correcting nature of WP interacts poorly with a delta approach that often ends up alternating between overshooting and undershooting, but mostly I think the main issue is that each channel will have its own WP state and evolves subpredictor weights independently. I suspect the predictor feedback loop can diverge chaotically and becomes hard to 'tame' when effectively each channel potentially uses very different predictors. |
Once the encode api is more mature, and cjxl/benchmark_xl can use the api, I think we should revise all encode options that are currently not shown in |
Flyby fix to reinstate libjxl#627 after being reverted in libjxl#3420
* Allow Squeeze predictors and fix faster decoding * Allow LZ77 for faster decoding * Fix syntax error Wouldn't build. * Use smallest group size, remove LZ77 only * Allow changing Delta Palette predictor * Allow changing Delta Palette predictor Flyby fix to reinstate #627 after being reverted in #3420 * Fixing setting smaller group size by default * Tweaked some settings + comments * More Tweaks * Revert Delta fix Code order was changed making it infeasible * Revert Delta fix Code order was changed making it infeasible * Increase density of Progressive Lossless * bug fix * Tweaks 2.0 + better lossless progressive * Update ANS with Tweaks 2.0 * Local MA trees for progressive lossless * Attempt to fix progressive YCoCg * Allow overriding RCT and Predictor * Underscores are important * Fix Lossy/Delta Palette * Round progressive lossless faster decoding * Try all predictors at effort 10 for progressive lossless * Syntax and add comments * Shifting this function to enc_modular.cc * There we go * Making the compiler happy * ugh * I forgot an additional check * Fix VarDCT density regression * Code cleanup and more fixes * Faster VarDCT Encoding Speeds * oops silly me :P * Fix ordering of progressive FD1 check * Typos * Typos * Typos * yippie! * Forgot something :( * Fix merge conflict * Try to fix progressive lossless * Revert progressive lossless buffering * Brackets are important. * Fix small image density regression * I can't believe this little thing was causing me soo much pain * Revert failed fix Also trigger checks to run * Finally, it should pass all tests now * Last one hopefully * Comments --------- Co-authored-by: Galaxy4594 <164440799+Galaxy4594@users.noreply.github.com> Co-authored-by: Jon Sneyers <jon@cloudinary.com>
Currently, the predictor used for delta palette (
cjxl --lossy-palette) is hardcoded toAverage4(which is the one that works best). This changes that to use that one by default, but allows changing it by setting-P. The predictor used to encode the indices themselves is now hardcoded toZero, which is the one that should work best.Before: (the predictor choice affects how the indices are encoded, not the predictor used in delta-palette)
After: (the predictor choice determines what is used in delta-palette)
As expected, the other predictors are worse than Average4, and in particular the Weighted Predictor (P6) works very poorly with the current encoder for delta-palette - there might be an encoder bug there, or maybe that predictor just leads to bad error accumulation - the images look bad but not completely buggy.
Also gets rid of a specialized codepath for the Gradient predictor in delta-palette, which was unreachable before (since that predictor was never used), and is not worth specializing for (since Gradient is not a good predictor for delta-palette; something that averages values works better).
Finally a test is added for the case of the Weighted predictor, which has a separate code path that wasn't covered by tests yet. Note that the BA distance is quite high in this case, so it might be worth investigating if there's an encoder bug in that case.