Skip to content

fix(jpeg2000): decode subsampled HTJ2K components correctly - #5643

Open
lgritz wants to merge 4 commits into
AcademySoftwareFoundation:mainfrom
lgritz:lg-j2k-subsampling
Open

lgritz wants to merge 4 commits into
AcademySoftwareFoundation:mainfrom
lgritz:lg-j2k-subsampling

Conversation

@lgritz

@lgritz lgritz commented Nov 2, 2026 •

Copy link
Copy Markdown
Collaborator

The OpenJPH (HTJ2K) path assumed every component is full resolution, pulling one full-width line per component for each image row. Subsampled components cause it to fall out of step, and OpenJPH spins indefinitely in codestream::pull(). Found by our nightly fuzzing with corrupt headers, but valid 4:2:0 files hang the same way.

Each component is now pulled at its own reconstructed size, and each sample is replicated over the full-resolution pixels it covers (the same nearest-sample mapping as the OpenJPEG path, but honoring the image offset). The image size now comes from the image extent rather than from component 0, which is smaller when it is subsampled.

Color-transformed files stay in OpenJPH's interleaved order, which needs equal subsampling, so unequal ones are rejected. Empty components, which also hung pull(), are rejected too.

Additionally, when tiles are smaller than a component's subsampling, the component has no samples in some tiles. OpenJPH 0.24 crashes pulling such a component (0.33 copes), so reject these files in the header check.

Assisted-by: Claude Code / Claude Opus 5.5

@lgritz
lgritz force-pushed the lg-j2k-subsampling branch 2 times, most recently from 6dbae55 to b0b387c Compare November 6, 2026 06:04
@lgritz
lgritz requested a balanced review from Copilot November 7, 2026 08:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The decoder can corrupt color-transformed images and hang on zero-height components, while offset and extent calculations lack targeted regression coverage.

3 open issues
What changed in this PR

This PR updates OpenImageIO’s OpenJPH decoder to handle subsampled HTJ2K components without hanging and reconstruct them at full image resolution.

Changes:

  • Decode subsampled components in planar order and expand samples using offset-aware mapping.
  • Derive image dimensions from the image extent rather than component 0.
  • Add subsampling regression cases and document the decoder behavior.
File Description
testsuite/​htj2k/​run.py Adds valid subsampling cases and a corrupt-file hang regression.
testsuite/​htj2k/​ref/​out.txt Records expected dimensions, pixel hashes, and completion output.
src/​jpeg2000.imageio/​jpeg2000input.cpp Implements per-component decoding, sample expansion, and canvas sizing.
.agents/​skills/​oiio-input-security-audit/​references/​format-notes.md Documents the subsampling fix and OpenJPEG fallback limitations.
.agents/​skills/​oiio-input-security-audit/​references/​bug-catalog.md Records the component-size mismatch and its decoding consequences.

Review effort: Balanced

Comment thread src/jpeg2000.imageio/jpeg2000input.cpp Resolved
Comment thread src/jpeg2000.imageio/jpeg2000input.cpp Resolved
Comment thread src/jpeg2000.imageio/jpeg2000input.cpp Resolved
The OpenJPH (HTJ2K) path assumed every component is full resolution,
pulling one full-width line per component for each image row.
Subsampled components cause it to fall out of step, and OpenJPH spins
indefinitely in codestream::pull(). Found by our nightly fuzzing with
corrupt headers, but valid 4:2:0 files hang the same way.

Each component is now pulled at its own reconstructed size, and each
sample is replicated over the full-resolution pixels it covers (the
same nearest-sample mapping as the OpenJPEG path, but honoring the
image offset). The image size now comes from the image extent rather
than from component 0, which is smaller when it is subsampled.

Color-transformed files stay in OpenJPH's interleaved order, which
needs equal subsampling, so unequal ones are rejected. Empty
components, which also hung pull(), are rejected too.

Assisted-by: Claude Code / Claude Opus 5.5

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Keep OpenJPH's pull order (interleaved for color-transformed files),
reject empty components, add offset/MCT/empty-component tests.

Assisted-by: Claude Code / Claude Opus 5.5

Signed-off-by: Larry Gritz <lg@larrygritz.com>
@lgritz
lgritz force-pushed the lg-j2k-subsampling branch from b0b387c to 7a70cbf Compare November 7, 2026 13:35
@lgritz

lgritz commented Nov 7, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the copilot comments and pushed an amentment.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The column-map allocation can fail outside the scanline error handler, and the regression test can pass even when the decoder crashes.

2 open issues
3 resolved since last review

Review effort: Balanced

Comment thread src/jpeg2000.imageio/jpeg2000input.cpp Resolved
Comment thread testsuite/htj2k/run.py Resolved
Claude says:

Report column-map allocation failure as a read error, and let the
hang regression test fail if the decoder crashes.

Assisted-by: Claude Code / Claude Opus 5.5

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Claude says:

When tiles are smaller than a component's subsampling, the component
has no samples in some tiles. OpenJPH 0.24 crashes pulling such a
component (0.33 copes), so reject these files in the header check.

The hang regression test now checks the reader's error message rather
than relying on a bare exit status, so a crash fails the test.

Assisted-by: Claude Code / Claude Opus 5.5

Signed-off-by: Larry Gritz <lg@larrygritz.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The tile-layout check incorrectly rejects valid nonempty layouts with more than two tiles when the tile size is smaller than the subsampling factor.

1 open issue
2 resolved since last review

Review effort: Balanced

uint64_t last = toff + (ntiles - 1) * tsz;
return has_sample(first, std::min(ext, toff + tsz))
&& has_sample(std::max(off, last), ext)
&& (ntiles <= 2 || tsz >= ds);
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.

2 participants