Skip to content

Update fork to upstream master (streaming fix #331), zero carried patches - #6

Open
wayfarer3130 wants to merge 470 commits into
masterfrom
update/upstream-master-2026-08-19
Open

Update fork to upstream master (streaming fix #331), zero carried patches#6
wayfarer3130 wants to merge 470 commits into
masterfrom
update/upstream-master-2026-08-19

Conversation

@wayfarer3130

@wayfarer3130 wayfarer3130 commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Brings the fork up to upstream master (6f3caf3) and drops the last two carried patches, leaving zero delta from upstream. Supersedes #5 (0.30.1) and the fork side of #3/#4.

Why master and not a release

The streaming/truncated-decode fix we contributed landed upstream as 638ccb4 "Cs3d/truncated decode graceful 0.30.1 (aous72#331)" on 2026-08-08. The newest upstream release, 0.31.0, was published 2026-07-27 — twelve days earlier. git tag --contains 638ccb4 is empty, so no tagged release carries the fix yet. master is the only target that does, and it sits just four commits past it (three dependabot codeql bumps and a warning fix).

Worth re-pinning to a tag as soon as upstream cuts one that includes aous72#331.

Patches dropped, and what replaces them

Both were in src/core/codestream/ojph_codestream_local.cpp:

Dropped Replaced by
resilient = false → true codestream::enable_resilience() — public API in src/core/openjph/ojph_codestream.h, called by openjphjs' HTJ2KDecoder before read_headers
commented-out OJPH_INFO(0x00030067, "File terminated early") ojph::set_message_level(OJPH_MSG_WARN), set by openjphjs

Patching the resilient default meant every consumer of this fork got resilient mode whether or not it asked; the API scopes it to the decoder that wants it. And set_message_level suppresses INFO generally, so it also silences the per-construction "v06 HTJ2K Decoder" banner — the same class of noise, and never worth a source patch either.

The behaviour those patches propped up is upstream now, along with tests/test_truncated_decode.cpp.

Verification

  • git diff upstream/master HEAD is empty — zero fork delta.
  • git merge-base --is-ancestor 638ccb4 HEAD passes — the streaming fix is present.
  • Future upstream bumps become fast-forwards rather than conflict resolution.

Consumed by cornerstonejs/codecs#76.

aous72 and others added 30 commits November 10, 2024 11:07
@aous72
This address codeQL
6541f2c
@aous72
Merge pull request aous72#158 from aous72/Adding_32bit_lossless
bca572f 
Adding 32bit lossless support.
@aous72
Addresses a couple of warnings.
dd011ac
@aous72
Address warnings in clang.
811f3a2
@aous72
Fixes LTO warnings, and hopefully illegal instruction.
f92e28d
@aous72
Version bump
c262935
@aous72
Merge pull request aous72#160 from aous72/addressing_lto
4c1cad5 
This address the illegal instruction issue when -flto flag is used (link time optimization) as details in this thread
https://bugzilla.redhat.com/show_bug.cgi?id=2307795
@aous72
This fixes a bug which wrongly sets implementation precision.
ca66e88
@aous72
Merge pull request aous72#161 from aous72/implementation_precision_bug
949d376 
This fixes a bug which wrongly sets implementation precision.
@aous72
Fixes a bug when a resolution has one 64bit line.
a85e756
@aous72
Merge pull request aous72#162 from aous72/bug_64bit_oneline_transform
7d8ea08 
Fixes a bug when a resolution has one 64bit line.
@aous72
Makes all colour components involved in colour transform employ the s…
4a3958f 
…ame precision.
@aous72
Fixes warning.
23633ba
@aous72
Merge pull request aous72#163 from aous72/make_all_colour_comps_use_s…
1b20dbc 
…ame_precision

Makes all colour components involved in colour transform employ the same precision (32bit or 64bit).
@palemieux
Add relative includes
50cdb9e
@palemieux
Remove export
fae7a95
@palemieux
Add missing slash
5a3d15f
@aous72
Merge pull request aous72#166 from palemieux/feature/install-fix-openexr
7a2f6ea 
Allow OpenJPH to be exported by cmake
@aous72
Cosmetic changes, moved param_qcd validation from .h to .cpp
124270e
@aous72
Merge branch 'master' of https://github.com/aous72/OpenJPH
579d7dd
@aous72
Modified NLT, NLT Validity needs work
af15dd0
@aous72
Major change to NLT, QCD, QCC to support differing multiple component…
9e675b3 
…s. Changes to API. May still be buggy (WIP).
@aous72
Improvements. Bug Fixes. Version Bump.
8d3b058
@aous72
Improvements, bug fixes.
bd914a9
@aous72
Improvements and bug fixes.
5551112
@aous72
Fixes warnings.
a81e55f
@aous72
Merge pull request aous72#169 from aous72/supporting_differing_compon…
aed95f6 
…ents

Supporting differing components in QCD
@aous72
Generic NLT code is added. Testing is needed.
7086c36
@aous72
Fixes warning on Mac.
d065c76
@aous72
Fixes compilation.
bf48100
aous72 and others added 25 commits July 2, 2026 17:48
@aous72
Removes direct access to COC segment marker. (aous72#312)
2d0a033 
* Removes direct access to COC segment marker, and adds a test.  This commit borrows from aous72#272 and aous72#309.

* Changed the default creation behaviour of COC marker segment and removed an unneeded file.
@aous72
Prevents running CI twice
9ab2e54
@wayfarer3130 @sedghi
Remove the partial stream message
18ffcdc 
(cherry picked from commit d6297b2)
@wayfarer3130 @sedghi
Fixes for error messages
0748112 
(cherry picked from commit 4417d4b)
@sedghi
Merge fork master into the 0.30.1 update
319c70f 
master holds the old (~0.21-era) fork state whose custom patches have been
re-applied on top of upstream 0.30.1 in this branch, so its content is fully
superseded. Recorded as a merge parent (strategy=ours) purely to make the PR
fast-forward-mergeable; the tree stays 0.30.1 + the carried resilient-decode
patches.
@dependabot
Bump actions/checkout from 7.0.0 to 7.0.1 (aous72#324)
3ceb289 
Bumps [actions/checkout](https://github.com/actions/checkout) from 7.0.0 to 7.0.1.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@9c091bb...3d3c42e)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-version: 7.0.1
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@aous72 @dependabot
Combined codeql action (aous72#325)
707ff7f 
* Bump github/codeql-action/analyze from 4.36.2 to 4.37.1

Bumps [github/codeql-action/analyze](https://github.com/github/codeql-action) from 4.36.2 to 4.37.1.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@8aad20d...7188fc3)

---
updated-dependencies:
- dependency-name: github/codeql-action/analyze
  dependency-version: 4.37.1
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* Bump github/codeql-action/autobuild from 4.36.2 to 4.37.1

Bumps [github/codeql-action/autobuild](https://github.com/github/codeql-action) from 4.36.2 to 4.37.1.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@8aad20d...7188fc3)

---
updated-dependencies:
- dependency-name: github/codeql-action/autobuild
  dependency-version: 4.37.1
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* Bump github/codeql-action/init from 4.36.2 to 4.37.1

Bumps [github/codeql-action/init](https://github.com/github/codeql-action) from 4.36.2 to 4.37.1.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@8aad20d...7188fc3)

---
updated-dependencies:
- dependency-name: github/codeql-action/init
  dependency-version: 4.37.1
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@aous72
Improving visual weighting code and implementation. (aous72#311)
c07f091 
* For visual weighting, this replaces vector with array, to give rid of the new/delete. A small change for bit-depth-based quantization; now it is limited to 2^-16, if not explicitly chosen.

* Fixes compilation issues

* Removed dependence on std::array.

* Fixes a warning.

* Rearranged visual weighting and qfactor settings

* Fixes warnings.

* Reverts visual weights to an older version and create a factory function for it. Also break setting SPcod into three functions.

* Removes one forgotten line

* Removes dependency on ojph_visual_weights.h. Implements everything in ojph_param.cpp & uses new qfactor weights -- qfactor tests will fail.

* Fixes a bug and warnings

* Fixes visual weight channel gains -- tests must pass

* Update Qfactor comment

* Updated set_qfactor comment

* Some code cleanup
@aous72
Version bump and deleting unneeded file
0221b31
@aous72
Merge branch 'master' of https://github.com/aous72/OpenJPH
566c091
@aous72
For ppc64le, fixes CI and addresses clang warning (aous72#327)
c68064d 
* Fixes to ppc64le

* revert some changes

* Clears clang warnings
@dependabot
Bump github/codeql-action/init from 4.37.1 to 4.37.2 (aous72#328)
ae827e0 
Bumps [github/codeql-action/init](https://github.com/github/codeql-action) from 4.37.1 to 4.37.2.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@7188fc3...e064762)

---
updated-dependencies:
- dependency-name: github/codeql-action/init
  dependency-version: 4.37.2
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@dependabot
Bump github/codeql-action/autobuild from 4.37.1 to 4.37.2 (aous72#329)
3b4f552 
Bumps [github/codeql-action/autobuild](https://github.com/github/codeql-action) from 4.37.1 to 4.37.2.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@7188fc3...e064762)

---
updated-dependencies:
- dependency-name: github/codeql-action/autobuild
  dependency-version: 4.37.2
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@dependabot
Bump github/codeql-action/analyze from 4.37.1 to 4.37.2 (aous72#330)
b44e3d8 
Bumps [github/codeql-action/analyze](https://github.com/github/codeql-action) from 4.37.1 to 4.37.2.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@7188fc3...e064762)

---
updated-dependencies:
- dependency-name: github/codeql-action/analyze
  dependency-version: 4.37.2
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@curtisc @aous72
Fix: raw_in reads every .raw file as signed, and zero-extends signed …
dfd7b07 
…16-bit samples (aous72#336)

* Fix: sign-extend 16-bit signed raw samples in raw_in::read()

Casting the signed sample to ui16 for swap_bytes_if_be() and then
widening to si32 zero-extends instead of sign-extends, corrupting
every negative 16-bit signed raw input sample on encode since 0.29.0.
Reinterpret the (possibly byte-swapped) bits as si16 before widening,
mirroring the 32-bit is_signed branch's explicit outer cast.

Fixes aous72#335

* Fix: give raw_in the requested signedness, not the option array

ojph_compress's is_signed is a si32* holding one entry per component,
but raw_in::set_img_props() takes a bool; passing the array converted
the pointer to bool, which is always true.  Every .raw file was
therefore read as if its samples were signed, whatever -signed said,
so unsigned samples with the top bit set were read as negative and
clamped away on decode.

This is why the sign extension fix in the previous commit is not
sufficient on its own: with the reader stuck in its signed branch, the
zero extension that corrupted signed samples was also what kept
unsigned 16 bit samples intact.  Correcting one without the other
moves the corruption from signed to unsigned input.

* Add .raw round trip tests for every sample width and signedness

raw_in::read() and raw_out::write() extend samples differently for
each of the four sample widths and for each signedness, and none of
those eight combinations was covered: the existing raw tests all go
through yuv_in with 8 bit unsigned samples.  Each new test encodes one
256x256 frame reversibly and requires the decoded frame to be bit
exact.  The frame ramps from the smallest to the largest sample the
component can hold, so at 16 bits the step is exactly 1 and the frame
sweeps all 65536 sample values.

Against the two preceding commits' parent these tests fail for 8 bit
unsigned, 16 bit signed and 24 bit unsigned samples.

* Fix: vsx_i64x2_extend_low/high_i32x4 return the wrong half

vec_unpackh unpacks the half of a vector holding elements 0 and 1, and
vec_unpackl the half holding elements 2 and 3; the compiler keeps that
element numbering on little endian.  vsx_i64x2_extend_low_i32x4 called
vec_unpackl and vsx_i64x2_extend_high_i32x4 called vec_unpackh, so
every si32 quad widened to si64 lines as [x2, x3, x0, x1].

The only callers are the si32 to si64 conversions in
ojph_colour_vsx.cpp (vsx_rev_convert, vsx_rev_convert_nlt_type3 and
vsx_rct_forward), all on the encode side, so a reversible encode of a
component deep enough to need si64 lines produced a codestream of the
pair-swapped image on ppc64le.  The decode side narrows with
byte-indexed vec_perm shuffles and is unaffected, which is why the
corruption survives into the output: the decoder faithfully
reconstructs the scrambled frame.

Caught by the new SimpleEncRev53Raw32* round-trip tests, whose 32-bit
samples are the first in the suite deep enough to widen: on ppc64le
sample 0 decoded to sample 2's value (+131074, exactly two steps of
the test ramp) and the mismatch pattern over each quad was
+2, +2, -2, -2 steps.  Verified under qemu-ppc64le (ubuntu 22.04 g++):
with this change the VSX and OJPH_DISABLE_SIMD builds produce
byte-identical codestreams and all four cross encode/decode pairings
round-trip bit-exact; before it, a VSX encode was corrupt under either
decoder while a generic encode decoded cleanly under both.

* Update ojph_img_io.cpp

A small touch to be safe.

---------

Co-authored-by: Aous Naman <aous@unsw.edu.au>
@dependabot
Bump github/codeql-action/init from 4.37.2 to 4.37.3 (aous72#334)
e1edd22 
Bumps [github/codeql-action/init](https://github.com/github/codeql-action) from 4.37.2 to 4.37.3.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@e064762...e4fba86)

---
updated-dependencies:
- dependency-name: github/codeql-action/init
  dependency-version: 4.37.3
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@dependabot
Bump github/codeql-action/analyze from 4.37.2 to 4.37.3 (aous72#333)
66af5f2 
Bumps [github/codeql-action/analyze](https://github.com/github/codeql-action) from 4.37.2 to 4.37.3.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@e064762...e4fba86)

---
updated-dependencies:
- dependency-name: github/codeql-action/analyze
  dependency-version: 4.37.3
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@dependabot
Bump github/codeql-action/autobuild from 4.37.2 to 4.37.3 (aous72#332)
3c13474 
Bumps [github/codeql-action/autobuild](https://github.com/github/codeql-action) from 4.37.2 to 4.37.3.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@e064762...e4fba86)

---
updated-dependencies:
- dependency-name: github/codeql-action/autobuild
  dependency-version: 4.37.3
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@wayfarer3130 @sedghi
Cs3d/truncated decode graceful 0.30.1 (aous72#331)
638ccb4 
* Change to debug build for a while

* Fixes for error messages

* Remove the partial stream message

* Remove the partial stream message

(cherry picked from commit d6297b2)

* Fixes for error messages

(cherry picked from commit 4417d4b)

* fix: handle truncated tile header decode errors gracefully

(cherry picked from commit 4d5eab5)

* fix: keep the const char* handler when widening tile-header catches

The truncation paths in ojph_bitbuffer_read.h and ojph_precinct.cpp throw
string literals, so replacing catch (const char*) outright would route the
most common truncated-stream errors into catch (...) and lose their
messages. Keep it and add std::exception / ... alongside.

* Fixes the decode to allow partial decodes

* Add test and resolve PR comments

---------

Co-authored-by: Alireza <ar.sedghi@gmail.com>
@dependabot
Bump github/codeql-action/init from 4.37.3 to 4.37.4 (aous72#339)
e1307d3 
Bumps [github/codeql-action/init](https://github.com/github/codeql-action) from 4.37.3 to 4.37.4.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@e4fba86...f205ea1)

---
updated-dependencies:
- dependency-name: github/codeql-action/init
  dependency-version: 4.37.4
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@dependabot
Bump github/codeql-action/analyze from 4.37.3 to 4.37.4 (aous72#338)
bfa22eb 
Bumps [github/codeql-action/analyze](https://github.com/github/codeql-action) from 4.37.3 to 4.37.4.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@e4fba86...f205ea1)

---
updated-dependencies:
- dependency-name: github/codeql-action/analyze
  dependency-version: 4.37.4
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@dependabot
Bump github/codeql-action/autobuild from 4.37.3 to 4.37.4 (aous72#337)
7fff736 
Bumps [github/codeql-action/autobuild](https://github.com/github/codeql-action) from 4.37.3 to 4.37.4.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@e4fba86...f205ea1)

---
updated-dependencies:
- dependency-name: github/codeql-action/autobuild
  dependency-version: 4.37.4
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@aous72
Warning fix (aous72#342)
6f3caf3
@wayfarer3130
Merge remote-tracking branch 'upstream/master' into update/upstream-m…
520c4d4 
…aster-2026-08-19
@wayfarer3130
Drop the last two carried patches in favour of upstream APIs
4a68609 
Both local edits to ojph_codestream_local.cpp are now redundant, so the fork
tracks upstream with zero delta and future bumps are fast-forwards.

  resilient = true
      Replaced by codestream::enable_resilience(), public API in
      src/core/openjph/ojph_codestream.h and called by openjphjs'
      HTJ2KDecoder before read_headers. Patching the default meant every
      consumer of this fork got resilient mode whether or not they asked;
      the API scopes it to the decoder that wants it.

  commented-out OJPH_INFO "File terminated early"
      Replaced by ojph::set_message_level(OJPH_MSG_WARN), which openjphjs
      sets so INFO is suppressed and WARN/ERROR still surface. That also
      silences the per-construction "v06 HTJ2K Decoder" banner, which is
      the same class of noise and was never worth a source patch.

The truncated-decode behaviour these were propping up is upstream as of
638ccb4 "Cs3d/truncated decode graceful 0.30.1 (aous72#331)", which also brings
tests/test_truncated_decode.cpp.
@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fb5c5f25-5541-497b-af4c-b38d7aa78d7d

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

wayfarer3130 added a commit to cornerstonejs/codecs that referenced this pull request Aug 19, 2026
@wayfarer3130
build(openjphjs): track upstream OpenJPH master for the streaming fix
18fe9aa 
Moves extern/openjph from 0.30.1 + carried patches to cornerstonejs/OpenJPH#6,
which merges upstream master (6f3caf3) with ZERO fork delta.

Why master rather than a release: the streaming/truncated-decode fix landed
upstream as 638ccb4 "Cs3d/truncated decode graceful 0.30.1 (aous72#331)" on
2026-08-08, and the newest upstream release 0.31.0 was published 2026-07-27 --
twelve days earlier. `git tag --contains 638ccb4` is empty, so no tagged release
carries it yet. master is four commits past the fix (three dependabot codeql
bumps and a warning fix). Re-pin to a tag once upstream cuts one with #331.

Both patches the fork used to carry are gone, replaced by public API:

  resilient = true
      codestream::enable_resilience() -- already called by HTJ2KDecoder on main
      (line 270), so this patch was redundant before this PR.

  commented-out OJPH_INFO "File terminated early"
      ojph::set_message_level(OJPH_MSG_WARN), set here in jslib.cpp.

The message level is worth its own note. OpenJPH INFO goes to stdout, which
emscripten forwards to console.log, and there were two sources of per-decode
noise: HTJ2KDecoder's constructor banner ("v06 HTJ2K Decoder") on every
construction, and "File terminated early" on every resilient decode of a
truncated stream -- which with streaming support is the normal case. Raising the
threshold to WARN drops both and keeps warnings and errors, so it replaces a
source patch with a supported call and makes future upstream bumps
fast-forwards.

Verified: OJPH_DISABLE_SIMD still exists upstream (this branch's FORCE OFF is
still correct), and every header HTJ2KDecoder.hpp includes is present under
src/core/openjph on master.
This was referenced Aug 19, 2026
Merged
Closed
wayfarer3130 added a commit to cornerstonejs/codecs that referenced this pull request Aug 27, 2026
@sedghi @wayfarer3130 @claude
chore(openjphjs): upstream OpenJPH master + partial HTJ2K decode (sup…
9548a57 
…ersedes #68) (#76)

codspeed regression is due to parallelization not being consistent in the simulation, not due to code speed changes

* chore(openjphjs): bump extern/openjph submodule to upstream 0.30.1

Points the openjph submodule at cornerstonejs/OpenJPH#5, which rebases the
fork onto upstream OpenJPH 0.30.1 (was ~22 months behind) and re-applies our
custom patches. Net cornerstonejs delta from stock 0.30.1 is 3 lines in
ojph_codestream_local.cpp: resilient=true (tolerate truncated streams) +
suppressed 'File terminated early' log. Dropped the cosmetic SIZ-marker
message renames (conflicted with upstream's swap_byte rename) and the
temporary debug-build toggle.

CI is the first real build/validation of this bump (not built locally).
After OpenJPH#5 merges, re-point this submodule at the merge commit.

* fix(openjphjs): add OpenJPH 0.30.1 header dirs to the wasm target include path

0.30.1 relocated its public headers under src/core/openjph (+ src/core/shared);
our glue's bare <ojph_arch.h> include no longer resolved and the openjphjs wasm
build failed with 'ojph_arch.h file not found'. Add both 0.30.1 header roots to
the openjphjs target's include path.

* fix(openjphjs): link the OpenJPH 0.30.1 'openjph' target (was renamed from openjphsimd)

0.30.1 builds a single architecture-agnostic 'openjph' library; the old
'openjphsimd' target no longer exists, so wasm-ld failed with
'unable to find library -lopenjphsimd'. Link 'openjph', matching upstream's
own wasm wrapper (subprojects/js).

* fix(openjphjs): keep WASM SIMD enabled for OpenJPH 0.30.1

0.30.1 deprecated OJPH_DISABLE_INTEL_SIMD and bridges it onto the new
OJPH_DISABLE_SIMD; our old 'OJPH_DISABLE_INTEL_SIMD=ON' therefore disabled ALL
SIMD (OJPH_ENABLE_WASM_SIMD=OFF), shipping a scalar wasm ~2x slower on decode/
encode. Stop setting the deprecated option and force OJPH_DISABLE_SIMD=OFF so
0.30.1's Emscripten path builds the WASM SIMD kernels (-msimd128).

* fix(openjphjs): build the wasm in Release, not Debug

build.sh forced CMAKE_BUILD_TYPE=Debug, so the shipped openjph wasm was built
-O0 with unoptimized SIMD intrinsics — the reason decode/encode benched far
slower (SIMD-on was even slower than scalar under -O0) and the binary was
oversized. Release (-O3) is the correct artifact and is what makes the 0.30.1
SIMD kernels fast and the wasm small.

* ci: squash in the pnpm migration from #87

Single squashed commit of ci/pnpm-trusted-publishing (#87), on the assumption
that #87 lands on main before this PR. Purpose is measurement: the pnpm
migration shifts CodSpeed's baseline on its own, and the HTJ2K work in this
branch shifts it again, so carrying both here lets one report show the combined
effect instead of attributing the sum to whichever merges second.

Expect this commit to become a no-op the moment #87 merges -- it should then
either drop out of the diff or merge cleanly against itself. It is NOT a second
copy of that work to review; review it in #87.

Merged with no conflicts. Two things worth noting about the overlap:

  - The submodule gitlink stayed at this branch's 0748112b rather than taking
    #87's e01c7b7, because #87 only reverted its own accidental bump back to
    the value main already had. Updated separately in the next commit.

  - dicom-codec/src/codecs/codecFactory.js is touched by both branches and did
    not conflict: #87 changes initialize() (routing emscripten's print through
    the logger) while this branch's carried work from #68 changes decode()
    (decoder reuse). They are independent edits to the same file.

Includes the pnpm.overrides pinning esbuild/rollup/webpack/terser to the
versions yarn.lock resolved, so build output does not drift across the
migration -- relevant here because this PR is measured against those baselines.

* build(openjphjs): track upstream OpenJPH master for the streaming fix

Moves extern/openjph from 0.30.1 + carried patches to cornerstonejs/OpenJPH#6,
which merges upstream master (6f3caf3) with ZERO fork delta.

Why master rather than a release: the streaming/truncated-decode fix landed
upstream as 638ccb4 "Cs3d/truncated decode graceful 0.30.1 (aous72#331)" on
2026-08-08, and the newest upstream release 0.31.0 was published 2026-07-27 --
twelve days earlier. `git tag --contains 638ccb4` is empty, so no tagged release
carries it yet. master is four commits past the fix (three dependabot codeql
bumps and a warning fix). Re-pin to a tag once upstream cuts one with #331.

Both patches the fork used to carry are gone, replaced by public API:

  resilient = true
      codestream::enable_resilience() -- already called by HTJ2KDecoder on main
      (line 270), so this patch was redundant before this PR.

  commented-out OJPH_INFO "File terminated early"
      ojph::set_message_level(OJPH_MSG_WARN), set here in jslib.cpp.

The message level is worth its own note. OpenJPH INFO goes to stdout, which
emscripten forwards to console.log, and there were two sources of per-decode
noise: HTJ2KDecoder's constructor banner ("v06 HTJ2K Decoder") on every
construction, and "File terminated early" on every resilient decode of a
truncated stream -- which with streaming support is the normal case. Raising the
threshold to WARN drops both and keeps warnings and errors, so it replaces a
source patch with a supported call and makes future upstream bumps
fast-forwards.

Verified: OJPH_DISABLE_SIMD still exists upstream (this branch's FORCE OFF is
still correct), and every header HTJ2KDecoder.hpp includes is present under
src/core/openjph on master.

* fix(openjphjs): decode truncated HTJ2K streams without a known full length

Carried from #68, which this PR supersedes. Previously the decoder could handle
a partial HTJ2K stream only if the caller already knew the full length; now a
truncated buffer decodes as far as its data allows.

readHeader, decode and decodeSubResolution wrap their codestream work in
try/catch and report instead of propagating, so resilient mode's throw at the
end of the available data yields a partial image rather than a failed decode.
frameInfo_ keeps whatever the header established, so dimensions survive.

Two deliberate changes from #68's version:

  - The diagnostics are OJPH_WARN, not OJPH_INFO. jslib.cpp raises OpenJPH's
    threshold to WARN to kill the per-construction banner, so INFO here would be
    dropped exactly when a decode failed.

  - DISABLE_EXCEPTION_CATCHING flips 1 -> 0 (double negative: catching ENABLED).
    This is required, not stylistic: with catching disabled emscripten compiles
    the handlers out and the throw terminates the module instead of being
    caught. It costs wasm size, so dist-size may need re-baselining.

test/truncated.test.js covers truncated and lossy decodes, and decoder reuse
across 500 decodes. NOTE: its performance assertions are wall-clock
(reused-faster-than-fresh, and a min/max ratio across milestones), so they are
inherently softer than the pixel-exactness tests and may prove flaky on shared
CI runners. Worth watching, and worth converting to a looser bound or dropping
if they turn noisy.

The core-side work is upstream as of aous72#331, so this is only the emscripten
wrapper plus tests -- the corresponding fork patches are gone.

* perf(dicom-codec): reuse the HTJ2K decoder instead of one per frame

Carried from #68. codecFactory.decode gains an opt-in reuseDecoder option: the
decoder is held on codecConfig (the per-codec singleton the wrapper modules
already share) and not deleted after each call. htj2k.js opts in; every other
codec keeps the construct-and-delete behaviour.

This is very likely the bulk of #68's measured 3.5x speedup on the dicom-codec
dispatch bench for HTJ2K -- CodSpeed reported 141.4ms -> 40.6ms there, and 40.6ms
is about what openjphjs' own direct decode benches cost, i.e. reuse closes the
gap between dispatching through this factory and calling the codec directly.
Constructing a wasm decoder per frame allocates heap and registers embind
bindings each time; for openjph it also ran the constructor banner through the
console on every frame.

Opt-in rather than default on purpose: a decoder that carries state between
decodes, or whose retained buffers grow without bound, must not enable it.
openjphjs' reuse test covers the consequence that matters for HTJ2K -- 500
successive decodes on one instance without progressive slowdown.

Independent of #87's change to the same file: that one routes emscripten's print
through the logger in initialize(); this one changes decode(). They merged with
no conflict.

* test(openjphjs): make the decoder-reuse perf assertion measure something real

Built openjphjs locally via tools/docker/build.sh and ran the suite, which is
how this surfaced: "reused decoder is faster than instantiate+decode+destroy per
frame" FAILED locally (3.34 ms vs 2.72 ms) while passing CI by 5% (2.38 vs 2.50).

The original had a structural flaw, not bad luck. It took ONE sample per path
with no warmup, and measured the reused path FIRST -- so V8's JIT warmup was
charged to exactly the side the assertion expects to win. Construction costs well
under a millisecond against a ~2.5 ms decode, so a single cold sample measures
warmup rather than the difference under test.

Fixed the measurement: warm both paths, then compare medians of 25 iterations.
That removed the order bias but showed the assertion itself is not sound at this
granularity -- warmed, construct+decode+destroy costs about the same as decode
alone (~1.6 ms each), so the medians sit inside each other's noise. Eight
observed runs produced two failures on unchanged code.

So the assertion is now a bound in the useful direction: reuse must not be
materially SLOWER (the real risk, e.g. retained state degrading each decode)
rather than provably faster. The medians are still logged.

This does not weaken the perf claim, it relocates it to the tool that can
actually measure it. CodSpeed on this branch reports the dispatch bench
141.5 ms -> 24.1 ms and instantiate+destroy HTJ2KDecoder x50 2315 us -> 458 us,
because Simulation counts instructions where wall-clock at ~3% of a decode
cannot resolve it. The 500-decode stability test is untouched and still guards
the thing that matters for reuse: no progressive slowdown from retained buffers.

Verified: 5 consecutive local runs stable, full openjphjs suite 30 passed.

* fix(htj2k): own the decoded buffer and surface swallowed decode failures

Addresses the review findings on #76, all of which stem from the same two
changes in this PR: reusing one HTJ2K decoder across a series, and swallowing
OpenJPH's exceptions so a partial codestream degrades to a partial image.

Buffer ownership (the critical one). getDecodedBuffer()/getEncodedBuffer()
return an emscripten typed_memory_view -- a live window onto the wasm heap
owned by the codec instance. Returning it as `imageFrame` was wrong three ways:
delete() frees the memory it points at, the next decode on a reused instance
overwrites it, and heap growth detaches it outright. Frames 1..n of a series
all showed frame n. copyFromWasm() now copies on both the decode and encode
paths -- unconditionally, because the non-reuse path was already handing back
memory delete() had just freed. Measured side effect: the raw view's .buffer is
the whole 50 MB heap, so callers passing imageFrame.buffer to a worker were
transferring the heap rather than the frame.

Failure reporting. decode()/readHeader()/decodeSubResolution() returned
normally after swallowing an exception, so codecFactory reported success and
the OJPH_WARN went to a logger that is silent unless setVerbose. HTJ2KDecoder
now exposes getIsHeaderValid()/getLastErrorMessage(), reset per call, and
codecFactory throws on an invalid header while flagging processInfo.partial
otherwise. Verified: before this, garbage input on a reused decoder resolved
successfully with the previous slice's pixels under the new frame's metadata.

Stale pixels. decode_ used resize(), which only value-initialises NEW elements,
so anything the decoder did not write kept the previous frame's pixels. Now
assign(size, 0), which zero-fills without giving up the capacity reuse depends
on. readHeader_ likewise resets every header-derived field before parsing, and
the previously uninitialised members (numDecompositions_, numLayers_, ...) get
initialisers -- a failed parse on a fresh decoder was doing arithmetic on heap
garbage.

Two comments in this PR claimed things that turned out to be false, corrected
in place. Truncation does NOT throw into decode()'s catch: swept CT1.j2c at
every length from 60 bytes up plus 875 single-byte corruptions and not one
input aborts mid-decode -- resilient mode absorbs a short codestream as zero
coefficients and reports success. The reachable stale-buffer window is
restrict_input_resolution() throwing for a decomposition level the codestream
does not carry, which is what the new regression test uses (it fails against
resize() with 128 stale bytes; a truncation-based test passes either way and
tests nothing).

Also:
- unset(OJPH_DISABLE_INTEL_SIMD CACHE): deleting the option() line does not
  remove it from an existing CMakeCache.txt, and upstream's `DEFINED` bridge
  shadows the forced OJPH_DISABLE_SIMD=OFF, so incremental local builds kept
  shipping the scalar wasm this PR's own comment warns about.
- releaseDecoder()/htj2k.release()/dicomCodec.release(): a reused decoder held
  its largest frame's buffers for the module's lifetime with no way to free.
- dist-size baseline for openjphjs: stale at 2241 KiB against a 293 KiB
  artifact, because THIS PR switched the build Debug -> Release. The gate only
  fails on growth, so it would have tolerated a 7.7x regression.

Rebuilt with tools/docker/build.sh (emsdk 3.1.74, SIMD confirmed intact);
openjphjs 34 passed, dicom-codec 41 passed, other codec suites unaffected.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Bill Wallace <wayfarer3130@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.