Skip to content

Commit 569b224

Browse files
committed
fix: codec correctness fixes split out of the pixel-correctness test PR
Second-round fixes found by the pixel-correctness suite (#72), split out so that PR stays a pure test/CI change. First-round fixes (12-bit decode path, codecFactory instance cleanup, openjpeg decoder guards and BufferStream bounds) are already in #71 — this PR carries only what the test branch itself introduced, each fix together with the tests that fail without it (verified against wasm rebuilt from the fixes branch's C++ with CI's emsdk 3.1.74 image): - openjpeg: J2KEncoder throws on setup/compress failure instead of silently returning, with the encoded buffer zeroed (callers previously read back a garbage pre-sized allocation as a successful encode); frees codec/stream/image on every exit path (repeated encodes grew the wasm heap monotonically); sizes the output buffer with headroom so clamped writes surface as errors instead of truncation. Pinned by the encoder-failure-throw tests, the encode/delete heap-stability test, and both dicom-codec J2K round-trips (encode .90 and transcode .80->.90), which fail byte-exactness without this rework. - openjphjs: HTJ2KEncoder rounds bytesPerPixel UP; bitsPerSample/8 truncated to 1 for 9..15-bit samples, halving the row stride so every row after the first was read from the wrong offset (12-bit encodes corrupted). Pinned by the 12-bit encoder round-trip. - libjpeg-turbo-12bit: fail closed on multi-component input — forcing JCS_GRAYSCALE on a color 12-bit JPEG silently discards chroma and reports componentCount=1. Pinned by the multi-component rejection test; also adds the CodSpeed bench and its package script. - dicom-codec: adaptImageInfo preserves planarConfiguration (decode8Planar was unreachable; PlanarConfiguration=1 RLE silently produced interleaved output). Pinned by the planar RLE test. - little-endian/big-endian: 32-bit pixel data decodes to Uint32Array/Int32Array per pixelRepresentation with Float32Array only as the no-pixelRepresentation fallback (review feedback from wayfarer3130, matching cornerstone3D's decodeLittleEndian); 32-bit views realign to 4-byte boundaries (the old offset % 2 check threw a RangeError at offset % 4 == 2); big-endian gains 1-bit passthrough and byte-swapped 32-bit support. Same typing fix applied to dicom-codec's littleEndian getPixelData.
1 parent a53a394 commit 569b224

17 files changed

Lines changed: 453 additions & 36 deletions

File tree

packages/big-endian/src/index.js

Lines changed: 53 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,29 @@
22
function swap16(val) {
33
return ((val & 0xff) << 8) | ((val >> 8) & 0xff);
44
}
5-
5+
6+
function swap32(val) {
7+
return (
8+
((val & 0xff) << 24) |
9+
((val & 0xff00) << 8) |
10+
((val >> 8) & 0xff00) |
11+
((val >> 24) & 0xff)
12+
);
13+
}
14+
615
/**
716
* Decodes the provided pixelData and sets the `pixelData` property
817
* of the imageFrame object to the decoded representation.
9-
*
10-
* Set pixelData will be `Uint16Array` if `pixelRepresentation` is 0,
11-
* otherwise it will be an `Int16Array`
12-
*
18+
*
19+
* 16-bit and 32-bit data are byte-swapped and become unsigned
20+
* (`pixelRepresentation` 0) or signed (`pixelRepresentation` 1) integer
21+
* arrays. 32-bit data with no `pixelRepresentation` is treated as float
22+
* (e.g. FloatPixelData), mirroring the little-endian package.
23+
*
1324
* @param {object} imageFrame
14-
* @param {number} imageFrame.bitsAllocated - 16 or 8
25+
* @param {number} imageFrame.bitsAllocated - 32, 16, 8 or 1
1526
* @param {number} imageFrame.pixelRepresentation - 0 or 1
16-
* @param {*} pixelData
27+
* @param {*} pixelData
1728
*/
1829
function decode(imageFrame, pixelData) {
1930
if (imageFrame.bitsAllocated === 16) {
@@ -38,8 +49,42 @@ function decode(imageFrame, pixelData) {
3849
for (let i = 0; i < imageFrame.pixelData.length; i++) {
3950
imageFrame.pixelData[i] = swap16(imageFrame.pixelData[i]);
4051
}
41-
} else if (imageFrame.bitsAllocated === 8) {
52+
} else if (imageFrame.bitsAllocated === 8 || imageFrame.bitsAllocated === 1) {
53+
// 1-bit data must already be extracted per frame by the caller:
54+
// multi-frame 1-bit pixel data is bit-packed across frame boundaries,
55+
// so frame extraction cannot happen at this level
4256
imageFrame.pixelData = pixelData;
57+
} else if (imageFrame.bitsAllocated === 32) {
58+
let arrayBuffer = pixelData.buffer;
59+
60+
let offset = pixelData.byteOffset;
61+
const length = pixelData.length;
62+
// pixelData is typically a view into the full DICOM P10 buffer, so its
63+
// byteOffset is even (DICOM guarantees even lengths) but not necessarily
64+
// 4-byte aligned; 32-bit typed-array views require 4-byte alignment,
65+
// so copy the bytes to a fresh, aligned buffer when needed
66+
if (offset % 4) {
67+
arrayBuffer = arrayBuffer.slice(offset);
68+
offset = 0;
69+
}
70+
71+
// The swap is a pure byte permutation, so it is done through a
72+
// Uint32Array view regardless of how the result is interpreted below
73+
const swapView = new Uint32Array(arrayBuffer, offset, length / 4);
74+
for (let i = 0; i < swapView.length; i++) {
75+
swapView[i] = swap32(swapView[i]);
76+
}
77+
78+
// 32-bit PixelData is integer data (signed per pixelRepresentation);
79+
// it is only float when pixelRepresentation is absent (e.g. the
80+
// FloatPixelData element), matching cornerstone3D's decodeLittleEndian
81+
if (imageFrame.pixelRepresentation === 0) {
82+
imageFrame.pixelData = swapView;
83+
} else if (imageFrame.pixelRepresentation === 1) {
84+
imageFrame.pixelData = new Int32Array(arrayBuffer, offset, length / 4);
85+
} else {
86+
imageFrame.pixelData = new Float32Array(arrayBuffer, offset, length / 4);
87+
}
4388
}
4489

4590
return imageFrame;

packages/big-endian/test/decode.test.js

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,69 @@ describe("big-endian decode", () => {
4343
expect(Array.from(imageFrame.pixelData)).toEqual([1, 2])
4444
})
4545

46+
it("passes 1-bit pixel data through unchanged", () => {
47+
const pixelData = new Uint8Array([0b10101010])
48+
const imageFrame = { bitsAllocated: 1 }
49+
50+
decode(imageFrame, pixelData)
51+
52+
expect(imageFrame.pixelData).toBe(pixelData)
53+
})
54+
55+
it("byte-swaps 32-bit unsigned pixel data into Uint32Array", () => {
56+
const source = [1, 2, 0xdeadbeef]
57+
// Build the big-endian byte stream for those values
58+
const bigEndianBytes = new Uint8Array(source.length * 4)
59+
const view = new DataView(bigEndianBytes.buffer)
60+
source.forEach((value, i) => view.setUint32(i * 4, value, false))
61+
const imageFrame = { bitsAllocated: 32, pixelRepresentation: 0 }
62+
63+
decode(imageFrame, bigEndianBytes)
64+
65+
expect(imageFrame.pixelData).toBeInstanceOf(Uint32Array)
66+
expect(Array.from(imageFrame.pixelData)).toEqual([1, 2, 0xdeadbeef])
67+
})
68+
69+
it("byte-swaps 32-bit signed pixel data into Int32Array", () => {
70+
const source = [-1, 2, -100000]
71+
const bigEndianBytes = new Uint8Array(source.length * 4)
72+
const view = new DataView(bigEndianBytes.buffer)
73+
source.forEach((value, i) => view.setInt32(i * 4, value, false))
74+
const imageFrame = { bitsAllocated: 32, pixelRepresentation: 1 }
75+
76+
decode(imageFrame, bigEndianBytes)
77+
78+
expect(imageFrame.pixelData).toBeInstanceOf(Int32Array)
79+
expect(Array.from(imageFrame.pixelData)).toEqual([-1, 2, -100000])
80+
})
81+
82+
it("byte-swaps 32-bit pixel data into Float32Array when pixelRepresentation is absent", () => {
83+
const source = new Float32Array([1.5, -2.25, 3.75])
84+
const bigEndianBytes = new Uint8Array(source.length * 4)
85+
const view = new DataView(bigEndianBytes.buffer)
86+
source.forEach((value, i) => view.setFloat32(i * 4, value, false))
87+
const imageFrame = { bitsAllocated: 32 }
88+
89+
decode(imageFrame, bigEndianBytes)
90+
91+
expect(imageFrame.pixelData).toBeInstanceOf(Float32Array)
92+
expect(Array.from(imageFrame.pixelData)).toEqual([1.5, -2.25, 3.75])
93+
})
94+
95+
it("realigns 32-bit pixel data when byteOffset is not 4-byte aligned", () => {
96+
const source = new Float32Array([1.5, -2.25])
97+
const padded = new Uint8Array(2 + source.length * 4)
98+
const view = new DataView(padded.buffer)
99+
source.forEach((value, i) => view.setFloat32(2 + i * 4, value, false))
100+
const pixelData = new Uint8Array(padded.buffer, 2, source.length * 4)
101+
const imageFrame = { bitsAllocated: 32 }
102+
103+
decode(imageFrame, pixelData)
104+
105+
expect(imageFrame.pixelData).toBeInstanceOf(Float32Array)
106+
expect(Array.from(imageFrame.pixelData)).toEqual([1.5, -2.25])
107+
})
108+
46109
it("returns the same imageFrame object", () => {
47110
const imageFrame = { bitsAllocated: 8 }
48111
const result = decode(imageFrame, new Uint8Array([0]))

packages/dicom-codec/src/codecs/index.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,18 @@ function getCodec(transferSyntaxUID) {
9090
* @returns {ExtendedImageInfo} Adapted imageInfo to all codecs.
9191
*/
9292
function adaptImageInfo(imageInfo) {
93-
const { rows, columns, bitsAllocated, signed, samplesPerPixel, pixelRepresentation } = imageInfo;
93+
const { rows, columns, bitsAllocated, signed, samplesPerPixel, pixelRepresentation, planarConfiguration } = imageInfo;
9494

9595
return {
9696
pixelRepresentation,
9797
bitsAllocated,
9898
samplesPerPixel,
99+
// Must survive adaptation: rleLossless dispatches between interleaved
100+
// (decode8) and plane-sequential (decode8Planar) output on this flag.
101+
// It was previously dropped here, which made decode8Planar unreachable
102+
// through the public decode() API — PlanarConfiguration=1 datasets
103+
// silently produced interleaved output.
104+
planarConfiguration,
99105
rows, // Number with the image rows/height
100106
columns, // Number with the image columns/width
101107
width: columns,

packages/dicom-codec/src/codecs/littleEndian.js

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,13 +82,25 @@ function getPixelData(imageFrame, imageInfo) {
8282
} else if (bitsAllocated === 8 || bitsAllocated === 1) {
8383
result = imageFrame;
8484
} else if (bitsAllocated === 32) {
85-
// if pixel data is not aligned on even boundary, shift it
86-
if (offset % 2) {
85+
// imageFrame is typically a view into the full DICOM P10 buffer, so its
86+
// byteOffset is even (DICOM guarantees even lengths) but not necessarily
87+
// 4-byte aligned; 32-bit typed-array views require 4-byte alignment,
88+
// so copy the bytes to a fresh, aligned buffer when needed
89+
if (offset % 4) {
8790
arrayBuffer = arrayBuffer.slice(offset);
8891
offset = 0;
8992
}
9093

91-
result = new Float32Array(arrayBuffer, offset, length / 4);
94+
// 32-bit PixelData is integer data (signed per pixelRepresentation);
95+
// it is only float when pixelRepresentation is absent (e.g. the
96+
// FloatPixelData element), matching cornerstone3D's decodeLittleEndian
97+
if (pixelRepresentation === 0) {
98+
result = new Uint32Array(arrayBuffer, offset, length / 4);
99+
} else if (pixelRepresentation === 1) {
100+
result = new Int32Array(arrayBuffer, offset, length / 4);
101+
} else {
102+
result = new Float32Array(arrayBuffer, offset, length / 4);
103+
}
92104
}
93105

94106
return result;

packages/dicom-codec/test/color-and-depth.test.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,26 @@ describe.skipIf(!ALL_BUILT)("dicom-codec color and bit-depth dispatch", () => {
6161
expect(frameBytes(result.imageFrame).equals(us1)).toBe(true)
6262
})
6363

64+
it("decodes color RLE plane-sequential when planarConfiguration is 1", async () => {
65+
const rleBytes = readFileSync(
66+
resolve(packagesRoot, "dicom-codec/test/fixtures/rle/US1-color.rle")
67+
)
68+
const result = await dicomCodec.decode(
69+
new Uint8Array(rleBytes),
70+
{ rows: 480, columns: 640, bitsAllocated: 8, samplesPerPixel: 3, planarConfiguration: 1 },
71+
"1.2.840.10008.1.2.5"
72+
)
73+
const out = frameBytes(result.imageFrame)
74+
expect(out.length).toBe(us1.length)
75+
// expected layout: RRR...GGG...BBB (de-interleaved planes of US1)
76+
const frameSize = 640 * 480
77+
const planar = Buffer.alloc(us1.length)
78+
for (let s = 0; s < 3; s++) {
79+
for (let i = 0; i < frameSize; i++) planar[s * frameSize + i] = us1[i * 3 + s]
80+
}
81+
expect(out.equals(planar)).toBe(true)
82+
})
83+
6484
it("decodes an 8-bit JPEG-LS (.80) through the dispatcher losslessly", async () => {
6585
const jls = readFileSync(resolve(packagesRoot, "charls/test/fixtures/CT2-gray8.jls"))
6686
const ct2 = readFileSync(resolve(packagesRoot, "charls/test/fixtures/CT2.RAW"))

packages/dicom-codec/test/transcode-and-pixeldata.test.js

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const packagesRoot = resolve(__dirname, "../..")
88

99
const REQUIRED = [
1010
"charls/dist/charlsjs.js",
11+
"openjpeg/dist/openjpegjs.js",
1112
]
1213
const ALL_BUILT = REQUIRED.every((p) => existsSync(resolve(packagesRoot, p)))
1314

@@ -19,9 +20,10 @@ it.runIf(process.env.CI)("sibling codec dists are present in CI (transcode suite
1920
expect(ALL_BUILT, "codec dists missing — artifacts not replayed").toBe(true)
2021
})
2122

22-
describe.skipIf(!ALL_BUILT)("dicom-codec encode", () => {
23+
describe.skipIf(!ALL_BUILT)("dicom-codec transcode and encode", () => {
2324
let dicomCodec
2425
const ct1Raw = readFileSync(resolve(packagesRoot, "openjpeg/test/fixtures/raw/CT1.RAW"))
26+
const ct1Jls = readFileSync(resolve(packagesRoot, "charls/test/fixtures/CT1.JLS"))
2527
const ctImageInfo = {
2628
rows: 512,
2729
columns: 512,
@@ -36,11 +38,32 @@ describe.skipIf(!ALL_BUILT)("dicom-codec encode", () => {
3638
dicomCodec = mod.default ?? mod
3739
})
3840

41+
it("encode() to J2K Lossless (.90) round-trips byte-exact", async () => {
42+
const encoded = await dicomCodec.encode(new Uint8Array(ct1Raw), ctImageInfo, "1.2.840.10008.1.2.4.90")
43+
expect(encoded.imageFrame.byteLength).toBeGreaterThan(0)
44+
45+
const decoded = await dicomCodec.decode(encoded.imageFrame, ctImageInfo, "1.2.840.10008.1.2.4.90")
46+
expect(frameBytes(decoded.imageFrame).equals(ct1Raw)).toBe(true)
47+
})
48+
3949
it("encode() to JPEG-LS Lossless (.80) round-trips byte-exact", async () => {
4050
const encoded = await dicomCodec.encode(new Uint8Array(ct1Raw), ctImageInfo, "1.2.840.10008.1.2.4.80")
4151
const decoded = await dicomCodec.decode(encoded.imageFrame, ctImageInfo, "1.2.840.10008.1.2.4.80")
4252
expect(frameBytes(decoded.imageFrame).equals(ct1Raw)).toBe(true)
4353
})
54+
55+
it("transcode() JPEG-LS -> J2K preserves the pixels exactly (both lossless)", async () => {
56+
const transcoded = await dicomCodec.transcode(
57+
ct1Jls,
58+
ctImageInfo,
59+
"1.2.840.10008.1.2.4.80",
60+
"1.2.840.10008.1.2.4.90"
61+
)
62+
expect(transcoded.imageFrame.byteLength).toBeGreaterThan(0)
63+
64+
const decoded = await dicomCodec.decode(transcoded.imageFrame, ctImageInfo, "1.2.840.10008.1.2.4.90")
65+
expect(frameBytes(decoded.imageFrame).equals(ct1Raw)).toBe(true)
66+
})
4467
})
4568

4669
describe.skipIf(!ALL_BUILT)("dicom-codec getPixelData typed-array contract", () => {
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
// Cold vs warm decode benches for the 12-bit codec, mirroring the 8-bit
2+
// package's bench shape (see libjpeg-turbo-8bit/bench/decode.bench.js for
3+
// the cold/warm methodology notes). Without this file the 12-bit package
4+
// was invisible to CodSpeed — a toolchain bump's full bench sweep measured
5+
// nothing for it.
6+
import { bench, describe } from "vitest"
7+
import { existsSync, readFileSync } from "node:fs"
8+
import { fileURLToPath } from "node:url"
9+
import { dirname, resolve } from "node:path"
10+
11+
const __dirname = dirname(fileURLToPath(import.meta.url))
12+
const distDir = resolve(__dirname, "../dist")
13+
const fixturesDir = resolve(__dirname, "../test/fixtures")
14+
15+
const distPath = resolve(distDir, "libjpegturbo12wasm.js")
16+
const skip = !existsSync(distPath)
17+
18+
const encoded = !skip
19+
? readFileSync(resolve(fixturesDir, "jpeg/CT-512x512-12bit.jpg"))
20+
: null
21+
22+
let codec
23+
let warmDecoder
24+
25+
async function loadCodec() {
26+
const mod = await import(distPath)
27+
const factory = mod.default ?? mod
28+
// Silence wasm stdout/stderr so vitest's console interception never runs
29+
// inside a measured bench body (see openjphjs bench for details).
30+
return await factory({ print: () => {}, printErr: () => {} })
31+
}
32+
33+
function decodeOnce(decoder) {
34+
decoder.getEncodedBuffer(encoded.length).set(encoded)
35+
decoder.decode()
36+
return decoder.getDecodedBuffer()
37+
}
38+
39+
if (!skip) {
40+
codec = await loadCodec()
41+
warmDecoder = new codec.JPEGDecoder()
42+
for (let i = 0; i < 5; i++) decodeOnce(warmDecoder)
43+
}
44+
45+
describe.skipIf(skip)("libjpeg-turbo-12bit (wasm)", () => {
46+
bench("decode CT-512x512-12bit.jpg (512x512x12bit) — cold", () => {
47+
const decoder = new codec.JPEGDecoder()
48+
decodeOnce(decoder)
49+
decoder.delete()
50+
})
51+
52+
bench("decode CT-512x512-12bit.jpg (512x512x12bit) — warm", () => {
53+
decodeOnce(warmDecoder)
54+
})
55+
})

packages/libjpeg-turbo-12bit/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@
2727
"test": "vitest run",
2828
"test:ci": "yarn run test",
2929
"test:watch": "vitest",
30-
"prepublishOnly": "yarn run build"
30+
"prepublishOnly": "yarn run build",
31+
"bench": "vitest bench --run"
3132
},
3233
"author": "",
3334
"license": "ISC"

packages/libjpeg-turbo-12bit/src/JPEGDecoder.hpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include <cstdint>
77
#include <memory>
88
#include <stdexcept>
9+
#include <string>
910
#include <vector>
1011
// #include "config.h"
1112
#include "jpeglib.h"
@@ -125,6 +126,16 @@ class JPEGDecoder {
125126
jpeg_mem_src(&cinfo, encoded_.data(), encoded_.size());
126127
// Read file header, set default decompression parameters
127128
jpeg_read_header(&cinfo, TRUE);
129+
// Fail closed on multi-component images. This codec only supports
130+
// single-component (grayscale) 12-bit JPEGs; forcing JCS_GRAYSCALE on a
131+
// color image would make libjpeg silently discard the chroma channels
132+
// and report componentCount=1, corrupting color data without any error.
133+
if (cinfo.num_components != 1) {
134+
jpeg_destroy_decompress(&cinfo);
135+
throw std::runtime_error(
136+
"Unsupported 12-bit JPEG: expected 1 component (grayscale), got " +
137+
std::to_string(cinfo.num_components));
138+
}
128139
// Decode as single-component grayscale. This is a 12-bit-per-sample
129140
// codec: each output value is a 16-bit-wide JSAMPLE (holding 0..4095),
130141
// not an 8-bit RGBA quad. Previously this forced a 4-samples-per-pixel

0 commit comments

Comments
 (0)