fix: correct VP8 IDCT pass order in WebP encoder (chroma cast)#22
Merged
Conversation
agno's inverse 4x4 DCT ran the row pass before the column pass. The VP8 reference (vp8_short_idct4x4llm_c / libwebp TransformOne) runs the column pass first, then the row pass (with the +4 rounding bias). The >>16 trig-term truncations are not associative across passes, so transposing them yields off-by-one reconstruction differences from any conformant decoder. agno's encoder reconstruction and its own decoder shared the wrong order, so round-tripping through agno looked correct and hid the bug. When agno's WebP is decoded by libwebp (browsers, Weblens), chroma drifts: chroma DC flows through this IDCT (there is no Y2/WHT path for chroma), so the +/-1 errors are not zero-mean on structured content and accumulate via intra prediction into a visible chroma cast (~+2 U/V on a 1024px crop, ~+6 on full frames). Luma DC goes through the order-independent WHT, so luma was barely affected -- matching the observed chroma-only signature. Swap both idct4x4 implementations (encoder transform.rs, decoder decode.rs) to column-first / row-second, matching the reference. Add a regression test pinning the pass order; a round-trip test cannot catch this because the encoder and decoder shared the deviation.
ef09327 to
22d1611
Compare
There was a problem hiding this comment.
Pull request overview
Fixes a WebP chroma color cast by correcting the VP8 inverse 4×4 DCT pass order to match the reference implementation (column pass first, row pass second with +4 bias), ensuring agno’s encoder reconstruction matches conformant decoders (e.g., libwebp).
Changes:
- Swap IDCT pass order in the encoder-side transform implementation to column-first / row-second.
- Swap IDCT pass order in the decoder implementation to the same reference order.
- Add a targeted regression test that fails for the old (row-first) order and pins outputs to the reference order for a known-differing block.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| agno/src/codec/webp/transform.rs | Fixes encoder-side IDCT pass order and adds a regression test to pin reference behavior. |
| agno/src/codec/webp/decode.rs | Fixes decoder-side IDCT pass order to match the VP8 reference implementation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
agno's WebP output had a chroma color cast (visible as a green/magenta tint) when decoded by libwebp — i.e. in browsers and the Weblens consumer. The root cause is in the VP8 inverse 4×4 DCT: it ran the row pass before the column pass, while the reference (
vp8_short_idct4x4llm_c/ libwebpTransformOne) runs the column pass first, then the row pass (with the+4rounding bias).The
>>16truncations in the trig terms make the two passes non-associative, so transposing them yields off-by-one reconstruction differences from any conformant decoder.Why it was hidden
agno's encoder reconstruction loop and agno's own decoder shared the identical wrong order, so encode→decode round-trips through agno looked near-lossless. The defect only appeared when a conformant decoder (libwebp) decoded agno's output.
Why it presented as an accumulating chroma cast
Fix
idct4x4implementations (codec/webp/transform.rsfor the encoder's reconstruction,codec/webp/decode.rsfor the decoder) to column-first / row-second, matching the reference.webp_gpu.rs) only does RGB→YUV on the GPU and reuses the shared CPU encoder, so it is fixed automatically (no separate IDCT).idct4x4_uses_vp8_reference_pass_order) pinning the pass order against the reference output for a block where the two orders differ. A round-trip test cannot catch this, because the encoder and decoder shared the deviation.Test plan
cargo test -p agno --no-default-features --features jpeg,png,webp --lib— 225/225 pass (incl. the new IDCT test)dV +6 → +0.09on a full frame,+2 → ~0on a crop; before/after on clean content shows the cast removedNote: independent of the Sony ARW2 decode fix in #21.