modular: wrap residual subtraction for lossless fp32 predictors - #4948
modular: wrap residual subtraction for lossless fp32 predictors#4948Alb3e3 wants to merge 3 commits into
Conversation
|
Follow-up for #3511 is pushed in 0607064. This keeps the earlier predictor fix intact and narrows the palette regression fix to the exact safe case: allow the lossless single-channel palette path for 32-bit inputs again, while leaving the 32-bit bailout in place for the wider implicit/multi-channel palette code that 27afa5e was protecting. Focused proof:
Commands rerun locally after the push:
|
|
I assume the FWIW, I wonder if it might make more sense to revert commit 9f72fea, 87bee19 and 27afa5e first and then apply your fixes on top of that. That might make the changes easier to review. |
|
Yes, good catch — removed in On the restructure: I'd rather not fold the reverts into this PR. 9f72fea, 87bee19 and 27afa5e each fixed a real problem, and a revert-then-reapply would put the tree through a state where those are unfixed — bad for bisecting, and it makes the diff look like "undo three fixes" rather than "here is what was wrong with them". What's here now is three commits that read in order:
That said, it's your call and I don't feel strongly. If you and the other reviewers would rather see revert-first, say the word and I'll restructure it. Verified on this branch after the removal:
I'll rebase onto current main as well since the branch is showing behind. |
The residual subtraction in enc_encoding.cc no longer needs an overflow check, so SubOverflow has no callers left. Removing it also drops the MSVC-only <intrin.h> include it required, along with the _M_AMD64 / _M_IX86 guard added in libjxl#4877 for the MSVC ARM64 build.
5c0503b to
ed3fb70
Compare
Summary
Root cause
Lossless float32 input is encoded through modular int32 channels that carry the raw IEEE-754 bit patterns. For mixed-sign samples, predictor guesses can differ from the source value by more than the signed int32 range while still being valid modulo-2^32 residuals.
The fast encoder paths were using
SubOverflow(...)and returningResidual overflowwhen that happened, even though the decoder and slower modular paths operate on wrapped 32-bit residuals. That made valid lossless fp32 inputs fail withJXL_ENC_ERR_GENERIC.Verification
EncodeTest.LosslessFloatMixedSignValuesat efforts 1, 4, and 7./tests/encode_test --gtest_filter=EncodeTest.LosslessFloatMixedSignValues./tests/encode_test --gtest_filter=EncodeTest.FrameEncodingTest:EncodeTest.LosslessFloatMixedSignValuesAI disclosure
Prepared with AI assistance, then manually reviewed and verified locally.
Fixes #4902.