diff --git a/README.md b/README.md index 66b55c45..a411e0fe 100644 --- a/README.md +++ b/README.md @@ -64,7 +64,7 @@ Transfer Syntax is the language used in DICOM to describe the DICOM file format - \*\*\* - Unlike all other DICOM transfer syntaxes, the deflate transfer syntaxes compress the whole of the DICOM data (tags, lengths, VR etc.) rather than just the pixel data - this is done using the standard “deflate” mechanism as used in gzip etc.) It is therefore most suitable for non-pixel objects such as structured reports, presentation states etc. - 5: [JS Decoder](https://github.com/cornerstonejs/cornerstoneWADOImageLoader/blob/4bfa04759412d58647cc5d6bd0204aa37e4542e3/src/shared/decoders/decodeRLE.js) -- 57 & 70: [JS Decoder](https://github.com/cornerstonejs/cornerstoneWADOImageLoader/blob/4bfa04759412d58647cc5d6bd0204aa37e4542e3/codecs/jpegLossless.js) +- 57 & 70: [JS Decoder](https://github.com/cornerstonejs/JPEGLosslessDecoderJS) — used as the published [`@cornerstonejs/jpeg-lossless-decoder-js`](https://www.npmjs.com/package/@cornerstonejs/jpeg-lossless-decoder-js), a fork that carries the end-of-scan fix the unscoped `jpeg-lossless-decoder-js` still lacks - 1.2 & 2.1 & 99: [JS Decoder](https://github.com/cornerstonejs/cornerstoneWADOImageLoader/blob/4bfa04759412d58647cc5d6bd0204aa37e4542e3/src/shared/decoders/decodeLittleEndian.js) - 2.2: [JS Decoder](https://github.com/cornerstonejs/cornerstoneWADOImageLoader/blob/4bfa04759412d58647cc5d6bd0204aa37e4542e3/src/shared/decoders/decodeBigEndian.js) diff --git a/packages/dicom-codec/package.json b/packages/dicom-codec/package.json index 23268e03..c8de5c84 100644 --- a/packages/dicom-codec/package.json +++ b/packages/dicom-codec/package.json @@ -39,7 +39,7 @@ "@cornerstonejs/codec-little-endian": "^0.0.9", "@cornerstonejs/codec-openjpeg": "^1.3.6", "@cornerstonejs/codec-openjph": "^2.4.11", - "browser-or-node": "^2.0.0", - "jpeg-lossless-decoder-js": "^2.0.4" + "@cornerstonejs/jpeg-lossless-decoder-js": "^2.2.0", + "browser-or-node": "^2.0.0" } } diff --git a/packages/dicom-codec/src/codecs/jpegLossless.js b/packages/dicom-codec/src/codecs/jpegLossless.js index 31a14604..d69d7c93 100644 --- a/packages/dicom-codec/src/codecs/jpegLossless.js +++ b/packages/dicom-codec/src/codecs/jpegLossless.js @@ -1,4 +1,7 @@ -const codecModule = require("jpeg-lossless-decoder-js"); +// @cornerstonejs/jpeg-lossless-decoder-js, not the unscoped +// jpeg-lossless-decoder-js: the published 2.1.2 of that package loses the last +// sample of any frame whose final Huffman code ends on a byte boundary. +const codecModule = require("@cornerstonejs/jpeg-lossless-decoder-js"); const codecFactory = require("./codecFactory"); /** diff --git a/packages/dicom-codec/test/integration.test.js b/packages/dicom-codec/test/integration.test.js index c1f95873..89c69f97 100644 --- a/packages/dicom-codec/test/integration.test.js +++ b/packages/dicom-codec/test/integration.test.js @@ -218,8 +218,8 @@ describe.skipIf(!ALL_BUILT)("dicom-codec integration", () => { describe("JPEG Lossless (1.2.840.10008.1.2.4.57 / .70)", () => { // These go through dicom-codec's internal jpegLosslessCodec - // (jpeg-lossless-decoder-js, pure JS — no separate wasm package). Both - // fixtures encode the same 512x512x16 CT slice; the reference + // (@cornerstonejs/jpeg-lossless-decoder-js, pure JS — no wasm package). + // Both fixtures encode the same 512x512x16 CT slice; the reference // fixtures/raw/CT-512x512.raw was cross-validated three ways: the RLE // decoder, the Process 14 path of jpeg-lossless-decoder-js and DCMTK's // dcmdjpeg all produce these exact bytes. @@ -258,33 +258,21 @@ describe.skipIf(!ALL_BUILT)("dicom-codec integration", () => { expect(frameBytes(result.imageFrame).equals(ctRaw)).toBe(true) }) - // KNOWN UPSTREAM BUG (jpeg-lossless-decoder-js): the SV1 path decodes - // the final pixel of this fixture as 0 instead of -2000. DCMTK's - // dcmdjpeg confirms the fixture itself is correct (its decode matches - // CT-512x512.raw exactly, last pixel included), so the defect is in the - // JS decoder. The test below pins today's behavior: every sample except - // the last matches the reference. When the upstream bug is fixed, the - // paired `it.fails` test starts passing and vitest will flag it — then - // fold these two tests into a single exact comparison. - it("decodes Process 14 SV1 through the dispatcher (.70) — all but the last pixel match", async () => { + // This fixture is the regression case for the forked decoder: its scan + // ends in a long run of the image minimum whose zero-difference codes tile + // the final byte exactly, and published jpeg-lossless-decoder-js 2.1.2 + // decoded that last sample as 0 instead of -2000 (it read the 0xFF + // introducing EOI as entropy coded data and stopped one sample early). + // DCMTK's dcmdjpeg always decoded this fixture to CT-512x512.raw exactly, + // last pixel included. An exact comparison here fails on any decoder + // without the fix. + it("decodes Process 14 SV1 through the dispatcher (.70) to the exact reference pixels", async () => { const result = await dicomCodec.decode( jpllProcess14Sv1, ctImageInfo, "1.2.840.10008.1.2.4.70" ) expect(result.imageFrame.byteLength).toBe(ctRaw.length) - - const actual = frameBytes(result.imageFrame) - // Everything up to the final 16-bit sample must match exactly. - expect(actual.subarray(0, ctRaw.length - 2).equals(ctRaw.subarray(0, ctRaw.length - 2))).toBe(true) - }) - - it.fails("decodes Process 14 SV1 (.70) to the exact reference pixels (known upstream last-pixel bug)", async () => { - const result = await dicomCodec.decode( - jpllProcess14Sv1, - ctImageInfo, - "1.2.840.10008.1.2.4.70" - ) expect(frameBytes(result.imageFrame).equals(ctRaw)).toBe(true) }) }) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 7004b97a..ca482534 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -85,12 +85,12 @@ importers: '@cornerstonejs/codec-openjph': specifier: ^2.4.11 version: link:../openjphjs + '@cornerstonejs/jpeg-lossless-decoder-js': + specifier: ^2.2.0 + version: 2.2.0 browser-or-node: specifier: ^2.0.0 version: 2.1.1 - jpeg-lossless-decoder-js: - specifier: ^2.0.4 - version: 2.1.2 devDependencies: clean-webpack-plugin: specifier: ^4.0.0 @@ -684,6 +684,9 @@ packages: vite: ^4.2.0 || ^5.0.0 || ^6.0.0 || ^7.0.0 vitest: ^3.2 || ^4 + '@cornerstonejs/jpeg-lossless-decoder-js@2.2.0': + resolution: {integrity: sha512-41y2/PDEsH8FGr6zZZ58TFSak8DLtEEInCuIGJb/sQ3aAPHx7dmZZEtyPamRS6eW3jp9hWiA59KziPiyXSSXSg==} + '@discoveryjs/json-ext@0.5.7': resolution: {integrity: sha512-dBVuXR082gk3jsFp7Rd/JI4kytwGHecnCoTtXFb7DB6CNHp4rg5k1bhg0nWdLGLnOV71lmDzGQaLMy8iPLY0pw==} engines: {node: '>=10.0.0'} @@ -1127,12 +1130,6 @@ packages: os: [linux] libc: [glibc] - '@rollup/rollup-linux-x64-gnu@4.13.0': - resolution: {integrity: sha512-yUD/8wMffnTKuiIsl6xU+4IA8UNhQ/f1sAnQebmE/lyQ8abjsVyDkyRkWop0kdMhKMprpNIhPmYlCxgHrPoXoA==} - cpu: [x64] - os: [linux] - libc: [glibc] - '@rollup/rollup-linux-x64-gnu@4.62.2': resolution: {integrity: sha512-5BqxR/pshjey51iliyzTD5Xi3EN0aLmQ2lZ3lvefVV9c82BvrLo2/6OT55iifpWBufs6kdwWbuOKS841DrmK9A==} cpu: [x64] @@ -2120,9 +2117,6 @@ packages: resolution: {integrity: sha512-7vuh85V5cdDofPyxn58nrPjBktZo0u9x1g8WtjQol+jZDaE+fhN+cIvTj11GndBnMnyfrUOG1sZQxCdjKh+DKg==} engines: {node: '>= 10.13.0'} - jpeg-lossless-decoder-js@2.1.2: - resolution: {integrity: sha512-fYf/plymnuKwVw+s8gJ8O/BPw8y8OLaN11MBvEcT05kuy3m45o7BEC4J0JbxMNUYyO+MdK/I/jq0q1gkVYZm2Q==} - js-tokens@10.0.0: resolution: {integrity: sha512-lM/UBzQmfJRo9ABXbPWemivdCW8V2G8FHaHdypQaIy523snUjog0W71ayWXTjiR+ixeMyVHN2XcpnTd/liPg/Q==} @@ -3633,6 +3627,8 @@ snapshots: - debug - supports-color + '@cornerstonejs/jpeg-lossless-decoder-js@2.2.0': {} + '@discoveryjs/json-ext@0.5.7': {} '@esbuild/aix-ppc64@0.28.1': @@ -3880,9 +3876,6 @@ snapshots: '@rollup/rollup-linux-s390x-gnu@4.62.2': optional: true - '@rollup/rollup-linux-x64-gnu@4.13.0': - optional: true - '@rollup/rollup-linux-x64-gnu@4.62.2': optional: true @@ -5078,10 +5071,6 @@ snapshots: merge-stream: 2.0.0 supports-color: 8.1.1 - jpeg-lossless-decoder-js@2.1.2: - optionalDependencies: - '@rollup/rollup-linux-x64-gnu': 4.13.0 - js-tokens@10.0.0: {} js-tokens@4.0.0: {} diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index d76285a3..0c386e75 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -62,3 +62,20 @@ frozenLockfile: true # local decision. allowBuilds: esbuild: true + +# pnpm refuses a dependency version that the registry published very recently, +# which is a supply-chain measure: it buys time for a compromised release to be +# found and unpublished before this repo installs it. That measure is aimed at +# third-party code, and it cannot judge our own. +# +# @cornerstonejs/jpeg-lossless-decoder-js is published by this organisation from +# cornerstonejs/JPEGLosslessDecoderJS, and dicom-codec needs it for transfer +# syntaxes 1.2.840.10008.1.2.4.57 and .70. Waiting out the age window on our own +# release would block a decode fix behind a delay that protects nothing here. +# +# The entry names the package and no version on purpose. pnpm writes a +# version-pinned entry when it adds one itself, and that entry goes stale at +# every release of the package, which turns a routine version bump into an +# install failure. Keep this list to packages that this organisation publishes. +minimumReleaseAgeExclude: + - '@cornerstonejs/jpeg-lossless-decoder-js' diff --git a/tools/fixture-verification/README.md b/tools/fixture-verification/README.md index f7e6e47a..c180a5e4 100644 --- a/tools/fixture-verification/README.md +++ b/tools/fixture-verification/README.md @@ -3,9 +3,9 @@ Independent, from-scratch decoders used to verify that the `.raw`/`.RAW` pixel references committed under `packages/*/test/fixtures` are correct. They share **no code** with the codecs under test (the emscripten wasm -builds, `jpeg-lossless-decoder-js`, or dicom-codec's `rleLossless.js`), so -byte-exact agreement means two independent implementations produce the -same pixels from the same codestream. +builds, `@cornerstonejs/jpeg-lossless-decoder-js`, or dicom-codec's +`rleLossless.js`), so byte-exact agreement means two independent +implementations produce the same pixels from the same codestream. ``` node tools/fixture-verification/run-all.js @@ -98,11 +98,17 @@ context, with CharLS and pylibjpeg agreeing with each other. The bug is in remain valid (independently confirmed by DCMTK/RLE cross-checks). Fixing the 12-bit run-interruption path here is a TODO. -## Bug found by this verification +## Bug found by this verification (now fixed) -`jpeg-lossless-decoder-js` (used by dicom-codec for transfer syntaxes -.57/.70) decodes the final pixel of the SV1 fixture as 0 instead of -2000. -Four independent decoders agree the fixture is correct: the from-scratch -`jpll.js` here, DCMTK's `dcmdjpeg`, the RLE decode of the same slice, and -the library's own Process-14 path. See the pinned `it.fails` test in -`packages/dicom-codec/test/integration.test.js`. +Published `jpeg-lossless-decoder-js` 2.1.2 (used by dicom-codec for transfer +syntaxes .57/.70) decoded the final pixel of the SV1 fixture as 0 instead of +-2000. Four independent decoders agreed the fixture was correct: the +from-scratch `jpll.js` here, DCMTK's `dcmdjpeg`, the RLE decode of the same +slice, and the library's own Process-14 path. + +The cause was an off-by-one in the end-of-scan guards, which read the 0xFF +introducing EOI as entropy coded data whenever the last Huffman code ended +exactly on a byte boundary. dicom-codec now depends on +`@cornerstonejs/jpeg-lossless-decoder-js`, a fork of the library that carries +the fix, and `packages/dicom-codec/test/integration.test.js` compares both +fixtures byte-for-byte.