Skip to content

Fix progressive JPEG AC refinement ZRL handling in Iris decoder - #981

Open
korale77 wants to merge 1 commit into
antirez:mainfrom
korale77:fix-progressive-jpeg-ac-refine-zrl
Open

Fix progressive JPEG AC refinement ZRL handling in Iris decoder#981
korale77 wants to merge 1 commit into
antirez:mainfrom
korale77:fix-progressive-jpeg-ac-refine-zrl

Conversation

@korale77

@korale77 korale77 commented Sep 5, 2026

Copy link
Copy Markdown

Problem

ds4_image_decode_* failed on many progressive JPEGs, including plain
cjpeg -progressive output and files produced by common tools. Symptoms were
either jpeg_load returning NULL ("could not load image") or a successful
decode with wrong pixels.

Root cause

In third_party/iris/jpeg.h, jpeg_prog_decode_ac_refine mishandled the
ZRL symbol (run=15, size=0) in successive-approximation AC refinement scans
(Ah != 0). It set run = 16 and reused the skip loop, which:

  1. skipped 16 zero-history coefficients (correct), then
  2. kept walking and consuming correction bits for every non-zero coefficient
    after the 16th zero, until it reached a 17th zero.

Per ITU T.81 G.1.2.3 (and libjpeg's decode_mcu_AC_refine), the correction
bits for those trailing non-zero coefficients belong after the next Huffman
symbol. Consuming them early desynchronises the bitstream for the rest of the
block/scan: usually the next symbol decodes as an invalid size and the whole
image fails, otherwise the coefficients come out subtly wrong.

The fix unifies the ZRL and size==1 paths: skip run zeros while refining
non-zeros on the way, write new_val (0 for ZRL) at the (run+1)-th zero, and
stop. This matches libjpeg's structure exactly. The upstream Iris repository
(antirez/iris.c, jpeg.h at HEAD) still has the bug; I will send the fix there too.

Testing

Machine: Apple M5 Max, macOS 25.5, clang, default make (Metal backend).

New regression test tests/test_image_decode (wired into make test)
decodes two committed fixtures generated with libjpeg-turbo 3.2.0 cjpeg
(tests/vision-fixtures/jpeg/generate.sh recreates them byte-identically):

Fixture Before After
prog_ac_refine_zrl_gray.jpg (24x16 gray) decodes, 64/384 bytes differ from djpeg (max 11) bit-exact with djpeg
prog_ac_refine_zrl_420.jpg (64x48 4:2:0) jpeg_load returns NULL decodes, max diff 1 vs djpeg -nosmooth

Differential check of old vs new decoder against djpeg over 25 images
(baseline, progressive 4:4:4/4:2:2/4:2:0, grayscale, restart intervals,
q50-q100, custom multi-refinement scan scripts, and the 1600x309 image that
triggered the report):

  • Old decoder returned NULL on 15 of 25 images (every default
    cjpeg -progressive 4:2:0 encode of the earth fixture, every 4:2:2/4:2:0
    restart-interval variant, and the reporting image). New decoder decodes all 25.
  • On the 8 images where both decoders succeeded and the scan has no
    refinement ZRL, old and new outputs are byte-identical (no behaviour change
    for baseline or non-affected progressive files).
  • New decoder vs djpeg: grayscale is bit-exact; colour images differ by
    at most 1/255 vs djpeg -nosmooth (Iris uses box chroma upsampling; the
    remaining delta is the pre-existing upsampler/IDCT difference, identical to
    what baseline JPEGs already show).
  • ASan+UBSan build decodes the full corpus cleanly. 240 truncated/bit-flipped
    variants of the fixtures: no crashes or hangs introduced; the two UBSan
    reports found (shift exponent in jpeg_get_bits, jpeg_zigzag[64] when a
    corrupt SOS sets Se > 63) reproduce identically on the unpatched decoder and
    are left for a separate PR.
  • Full make test passes, including the model-backed ds4_test runs against the local DeepSeek 4 Flash GGUF.
  • -Wall -Wextra -std=c99: no new warnings.

🤖 Generated with Claude Code

@korale77
korale77 force-pushed the fix-progressive-jpeg-ac-refine-zrl branch from 4371e2d to 0a0db0f Compare September 5, 2026 03:54
jpeg_prog_decode_ac_refine treated ZRL (run=15, size=0) as run=16 and
reused the skip loop, which kept consuming correction bits for non-zero
coefficients after the 16th zero-history coefficient. Those bits belong
after the next Huffman symbol (ITU T.81 G.1.2.3, libjpeg
decode_mcu_AC_refine), so the bitstream desynchronised: many progressive
JPEGs, including plain `cjpeg -progressive` output, failed to load or
decoded with wrong pixels.

Skip `run` zeros while refining non-zeros on the way, write the new value
(0 for ZRL) at the next zero, and stop, matching libjpeg's structure.

Add tests/test_image_decode with two cjpeg-generated fixtures that fail
on the old decoder (wrong pixels for grayscale, NULL for 4:2:0) and are
bit-exact with djpeg after the fix. Wire it into `make test`.
@korale77
korale77 force-pushed the fix-progressive-jpeg-ac-refine-zrl branch from 0a0db0f to 100b53d Compare September 6, 2026 04:13
@korale77 korale77 changed the title Fix progressive JPEG AC refinement ZRL handling in vendored Iris decoder Fix progressive JPEG AC refinement ZRL handling in Iris decoder Sep 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant