Skip to content

Fix lossless restart boundary handling in reconstruction - #287

Open
TidalPaladin wants to merge 1 commit into
image-rs:masterfrom
TidalPaladin:fix/lossless-restart-boundary-286
Open

Fix lossless restart boundary handling in reconstruction#287
TidalPaladin wants to merge 1 commit into
image-rs:masterfrom
TidalPaladin:fix/lossless-restart-boundary-286

Conversation

@TidalPaladin

@TidalPaladin TidalPaladin commented Feb 16, 2026

Copy link
Copy Markdown

Motivation

Lossless JPEG scans with restart markers can decode incorrect pixel values if prediction is not reset exactly at restart boundaries.

Solution

Detect restart boundaries from the absolute pixel index and reset predictor initialization at those boundaries. Keep the reconstruction logic aligned across predictor paths with shared indexing helpers.

Changes

  • Fix restart-boundary handling in lossless reconstruction.
  • Apply the same boundary behavior in both Predictor::Ra and generic predictor paths.
  • Add a regression reftest fixture (.jpg + expected .png) that exercises lossless restart-marker decoding.

Closes #286

Generated with Codex

Reset prediction at restart intervals based on pixel index and add a restart-marker regression fixture to guard decoder behavior.
@TidalPaladin
TidalPaladin marked this pull request as ready for review February 16, 2026 19:54
TidalPaladin added a commit to medcognetics/dicom-preprocessing that referenced this pull request Feb 16, 2026
Note that the git override should be removed once image-rs/jpeg-decoder#287 is available in a stable crates.io release.
TidalPaladin added a commit to medcognetics/dicom-preprocessing that referenced this pull request Feb 16, 2026
* Pin jpeg-decoder to restart-boundary fix fork

Use a crates.io patch override to pull TidalPaladin/jpeg-decoder branch fix/lossless-restart-boundary-286 and lock the resolved commit.

* Document temporary jpeg-decoder fork pin

Note that the git override should be removed once image-rs/jpeg-decoder#287 is available in a stable crates.io release.
@jasonklotzer

Copy link
Copy Markdown

We independently hit this exact bug (before finding this issue/PR) while debugging a rendering failure in dcmnorm, a DICOM toolkit that uses jpeg-decoder via dicom-transfer-syntax-registry. A real customer mammography DICOM file (Hologic tomosynthesis/C-View, transfer syntax 1.2.840.10008.1.2.4.70) failed to render correctly, showing periodic horizontal banding while decoding "succeeded" — same symptom described in #286.

That file has DRI=63872 samples, which is exactly 32 image rows (1996 × 32), predictor selection Ra (PSV=1). That's the worst case for this bug: every restart boundary falls exactly on column 0 of a row, so the Predictor::Ra fast path's "start of line, use pixel above" logic silently absorbs the restart reset instead of applying the default prediction. We confirmed the corrupted output numerically — flat background bands decreased by exactly 512 (8 in the real 10-bit sample domain) every single restart interval, a clean signature of the reconstructed baseline drifting forward across restarts instead of resetting.

We wrote an equivalent fix independently (tracking restart-marker positions and using them in reconstruction for both the Ra fast path and the generic predictor path, which has the same bug via the stale mcus_left_until_restart check) before finding this PR. The diagnosis and approach here match ours exactly. Applying this patch resolves the corruption completely on our real-world file — verified visually against the file (and against another viewer's rendering of the same study) and against dcmnorm's full test suite (74 tests, no regressions).

One thing that might be blocking merge: the cross_compile_aarch64 jobs are failing on this PR's CI run, while they pass on current master. Everything else (all build matrix jobs across stable/beta/nightly, wasm) is green. Given this change only touches src/decoder/lossless.rs and doesn't touch the NEON/aarch64-specific IDCT code, it's possible that's a transient/flaky failure rather than something this diff caused — might just need a re-run.

Given restart intervals are common in mammography JPEG Lossless (at least Hologic uses them), this affects real clinical images beyond synthetic test cases. Would be great to see this merged.

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.

JPEG Lossless restart boundaries not applied during predictor reconstruction

2 participants