diff --git a/packages/big-endian/src/index.js b/packages/big-endian/src/index.js index df60a6a0..68732350 100644 --- a/packages/big-endian/src/index.js +++ b/packages/big-endian/src/index.js @@ -2,18 +2,29 @@ function swap16(val) { return ((val & 0xff) << 8) | ((val >> 8) & 0xff); } - + +function swap32(val) { + return ( + ((val & 0xff) << 24) | + ((val & 0xff00) << 8) | + ((val >> 8) & 0xff00) | + ((val >> 24) & 0xff) + ); +} + /** * Decodes the provided pixelData and sets the `pixelData` property * of the imageFrame object to the decoded representation. - * - * Set pixelData will be `Uint16Array` if `pixelRepresentation` is 0, - * otherwise it will be an `Int16Array` - * + * + * 16-bit and 32-bit data are byte-swapped and become unsigned + * (`pixelRepresentation` 0) or signed (`pixelRepresentation` 1) integer + * arrays. 32-bit data with no `pixelRepresentation` is treated as float + * (e.g. FloatPixelData), mirroring the little-endian package. + * * @param {object} imageFrame - * @param {number} imageFrame.bitsAllocated - 16 or 8 + * @param {number} imageFrame.bitsAllocated - 32, 16, 8 or 1 * @param {number} imageFrame.pixelRepresentation - 0 or 1 - * @param {*} pixelData + * @param {*} pixelData */ function decode(imageFrame, pixelData) { if (imageFrame.bitsAllocated === 16) { @@ -22,10 +33,16 @@ function decode(imageFrame, pixelData) { let offset = pixelData.byteOffset; const length = pixelData.length; // if pixel data is not aligned on even boundary, shift it so we can create the 16 bit array - // buffers on it - + // buffers on it. + // + // The end bound is not optional. slice(offset) copies through the end of + // the BACKING buffer, and pixelData is typically a single frame's view into + // a whole multi-frame P10 buffer — so the one-argument form allocates and + // copies the entire rest of the file to realign one frame (measured: 67 MB + // for a 1 MB frame in a 64 MB buffer). The returned view's length hides it, + // because it is correct either way. if (offset % 2) { - arrayBuffer = arrayBuffer.slice(offset); + arrayBuffer = arrayBuffer.slice(offset, offset + pixelData.byteLength); offset = 0; } @@ -38,8 +55,54 @@ function decode(imageFrame, pixelData) { for (let i = 0; i < imageFrame.pixelData.length; i++) { imageFrame.pixelData[i] = swap16(imageFrame.pixelData[i]); } - } else if (imageFrame.bitsAllocated === 8) { + } else if (imageFrame.bitsAllocated === 8 || imageFrame.bitsAllocated === 1) { + // 1-bit data must already be extracted per frame by the caller: + // multi-frame 1-bit pixel data is bit-packed across frame boundaries, + // so frame extraction cannot happen at this level. + // + // No word swap is applied, and that is a deliberate limitation rather than + // an oversight. 1-bit PixelData is a bit-packed byte stream (first sample + // in the least significant bit of the first byte, PS3.5 8.1.1), and whether + // Big Endian transposes each byte pair of it depends on the VR the sender + // chose: PS3.5 2016b A.3.2 requires OW only when Bits Allocated is greater + // than 8, so at 1 bit either OB (no swap, byte order irrelevant) or OW + // (each 2-byte word swapped, so pixels 0..7 and 8..15 arrive transposed) is + // conformant. bitsAllocated cannot tell those apart. Passing the bytes + // through unchanged is correct for OB; a caller that knows its dataset used + // OW must swap the words itself before calling this. imageFrame.pixelData = pixelData; + } else if (imageFrame.bitsAllocated === 32) { + let arrayBuffer = pixelData.buffer; + + let offset = pixelData.byteOffset; + const length = pixelData.length; + // pixelData is typically a view into the full DICOM P10 buffer, so its + // byteOffset is even (DICOM guarantees even lengths) but not necessarily + // 4-byte aligned; 32-bit typed-array views require 4-byte alignment, + // so copy the bytes to a fresh, aligned buffer when needed — bounded to + // this frame, for the reason given on the 16-bit branch above + if (offset % 4) { + arrayBuffer = arrayBuffer.slice(offset, offset + pixelData.byteLength); + offset = 0; + } + + // The swap is a pure byte permutation, so it is done through a + // Uint32Array view regardless of how the result is interpreted below + const swapView = new Uint32Array(arrayBuffer, offset, length / 4); + for (let i = 0; i < swapView.length; i++) { + swapView[i] = swap32(swapView[i]); + } + + // 32-bit PixelData is integer data (signed per pixelRepresentation); + // it is only float when pixelRepresentation is absent (e.g. the + // FloatPixelData element), matching cornerstone3D's decodeLittleEndian + if (imageFrame.pixelRepresentation === 0) { + imageFrame.pixelData = swapView; + } else if (imageFrame.pixelRepresentation === 1) { + imageFrame.pixelData = new Int32Array(arrayBuffer, offset, length / 4); + } else { + imageFrame.pixelData = new Float32Array(arrayBuffer, offset, length / 4); + } } return imageFrame; diff --git a/packages/big-endian/test/decode.test.js b/packages/big-endian/test/decode.test.js index e0391056..9aa3a2b7 100644 --- a/packages/big-endian/test/decode.test.js +++ b/packages/big-endian/test/decode.test.js @@ -43,6 +43,69 @@ describe("big-endian decode", () => { expect(Array.from(imageFrame.pixelData)).toEqual([1, 2]) }) + it("passes 1-bit pixel data through unchanged", () => { + const pixelData = new Uint8Array([0b10101010]) + const imageFrame = { bitsAllocated: 1 } + + decode(imageFrame, pixelData) + + expect(imageFrame.pixelData).toBe(pixelData) + }) + + it("byte-swaps 32-bit unsigned pixel data into Uint32Array", () => { + const source = [1, 2, 0xdeadbeef] + // Build the big-endian byte stream for those values + const bigEndianBytes = new Uint8Array(source.length * 4) + const view = new DataView(bigEndianBytes.buffer) + source.forEach((value, i) => view.setUint32(i * 4, value, false)) + const imageFrame = { bitsAllocated: 32, pixelRepresentation: 0 } + + decode(imageFrame, bigEndianBytes) + + expect(imageFrame.pixelData).toBeInstanceOf(Uint32Array) + expect(Array.from(imageFrame.pixelData)).toEqual([1, 2, 0xdeadbeef]) + }) + + it("byte-swaps 32-bit signed pixel data into Int32Array", () => { + const source = [-1, 2, -100000] + const bigEndianBytes = new Uint8Array(source.length * 4) + const view = new DataView(bigEndianBytes.buffer) + source.forEach((value, i) => view.setInt32(i * 4, value, false)) + const imageFrame = { bitsAllocated: 32, pixelRepresentation: 1 } + + decode(imageFrame, bigEndianBytes) + + expect(imageFrame.pixelData).toBeInstanceOf(Int32Array) + expect(Array.from(imageFrame.pixelData)).toEqual([-1, 2, -100000]) + }) + + it("byte-swaps 32-bit pixel data into Float32Array when pixelRepresentation is absent", () => { + const source = new Float32Array([1.5, -2.25, 3.75]) + const bigEndianBytes = new Uint8Array(source.length * 4) + const view = new DataView(bigEndianBytes.buffer) + source.forEach((value, i) => view.setFloat32(i * 4, value, false)) + const imageFrame = { bitsAllocated: 32 } + + decode(imageFrame, bigEndianBytes) + + expect(imageFrame.pixelData).toBeInstanceOf(Float32Array) + expect(Array.from(imageFrame.pixelData)).toEqual([1.5, -2.25, 3.75]) + }) + + it("realigns 32-bit pixel data when byteOffset is not 4-byte aligned", () => { + const source = new Float32Array([1.5, -2.25]) + const padded = new Uint8Array(2 + source.length * 4) + const view = new DataView(padded.buffer) + source.forEach((value, i) => view.setFloat32(2 + i * 4, value, false)) + const pixelData = new Uint8Array(padded.buffer, 2, source.length * 4) + const imageFrame = { bitsAllocated: 32 } + + decode(imageFrame, pixelData) + + expect(imageFrame.pixelData).toBeInstanceOf(Float32Array) + expect(Array.from(imageFrame.pixelData)).toEqual([1.5, -2.25]) + }) + it("returns the same imageFrame object", () => { const imageFrame = { bitsAllocated: 8 } const result = decode(imageFrame, new Uint8Array([0])) diff --git a/packages/dicom-codec/package.json b/packages/dicom-codec/package.json index db1d067e..95f5cb66 100644 --- a/packages/dicom-codec/package.json +++ b/packages/dicom-codec/package.json @@ -33,6 +33,7 @@ "dependencies": { "@cornerstonejs/codec-big-endian": "^0.1.2", "@cornerstonejs/codec-charls": "^1.2.6", + "@cornerstonejs/codec-libjpeg-turbo-12bit": "^0.4.4", "@cornerstonejs/codec-libjpeg-turbo-8bit": "^1.2.5", "@cornerstonejs/codec-libjxl": "^1.1.0", "@cornerstonejs/codec-little-endian": "^0.0.8", diff --git a/packages/dicom-codec/src/codecs/bigEndian.js b/packages/dicom-codec/src/codecs/bigEndian.js index c6f3d23d..162b8f42 100644 --- a/packages/dicom-codec/src/codecs/bigEndian.js +++ b/packages/dicom-codec/src/codecs/bigEndian.js @@ -45,18 +45,24 @@ async function encode(imageFrame, imageInfo, options = {}) { function getPixelData(imageFrame, imageInfo) { let result; + let arrayBuffer = imageFrame.buffer; + let offset = imageFrame.byteOffset; + const length = imageFrame.length; + const { bitsAllocated, pixelRepresentation } = imageInfo; if (bitsAllocated === 16) { - let arrayBuffer = imageFrame.buffer; - - let offset = imageFrame.byteOffset; - const length = imageFrame.length; // if pixel data is not aligned on even boundary, shift it so we can create the 16 bit array - // buffers on it - + // buffers on it. + // + // The end bound is not optional. slice(offset) copies through the end of + // the BACKING buffer, and imageFrame is typically a single frame's view + // into a whole multi-frame P10 buffer — so the one-argument form allocates + // and copies the entire rest of the file to realign one frame (measured: + // 67 MB for a 1 MB frame in a 64 MB buffer). The returned view's length + // hides it, because it is correct either way. if (offset % 2) { - arrayBuffer = arrayBuffer.slice(offset); + arrayBuffer = arrayBuffer.slice(offset, offset + imageFrame.byteLength); offset = 0; } @@ -69,8 +75,40 @@ function getPixelData(imageFrame, imageInfo) { for (let i = 0; i < result.length; i++) { result[i] = swap16(result[i]); } - } else if (bitsAllocated === 8) { + } else if (bitsAllocated === 8 || bitsAllocated === 1) { + // Both are byte streams as far as this function is concerned, so there is + // nothing to swap: 8-bit samples are one byte each, and 1-bit PixelData is + // bit-packed eight samples to a byte and stays packed here (see the + // big-endian package's decode for why no word swap is applied). result = imageFrame; + } else if (bitsAllocated === 32) { + // imageFrame is typically a view into the full DICOM P10 buffer, so its + // byteOffset is even (DICOM guarantees even lengths) but not necessarily + // 4-byte aligned; 32-bit typed-array views require 4-byte alignment, + // so copy the bytes to a fresh, aligned buffer when needed — bounded to + // this frame, for the reason given on the 16-bit branch above + if (offset % 4) { + arrayBuffer = arrayBuffer.slice(offset, offset + imageFrame.byteLength); + offset = 0; + } + + // The swap is a pure byte permutation, so it is done through a + // Uint32Array view regardless of how the result is interpreted below + const swapView = new Uint32Array(arrayBuffer, offset, length / 4); + for (let i = 0; i < swapView.length; i++) { + swapView[i] = swap32(swapView[i]); + } + + // 32-bit PixelData is integer data (signed per pixelRepresentation); + // it is only float when pixelRepresentation is absent (e.g. the + // FloatPixelData element), matching cornerstone3D's decodeLittleEndian + if (pixelRepresentation === 0) { + result = swapView; + } else if (pixelRepresentation === 1) { + result = new Int32Array(arrayBuffer, offset, length / 4); + } else { + result = new Float32Array(arrayBuffer, offset, length / 4); + } } return result; @@ -81,6 +119,15 @@ function swap16(val) { return ((val & 0xff) << 8) | ((val >> 8) & 0xff); } +function swap32(val) { + return ( + ((val & 0xff) << 24) | + ((val & 0xff00) << 8) | + ((val >> 8) & 0xff00) | + ((val >> 24) & 0xff) + ); +} + exports.decode = decode; exports.encode = encode; exports.getPixelData = getPixelData; diff --git a/packages/dicom-codec/src/codecs/codecFactory.js b/packages/dicom-codec/src/codecs/codecFactory.js index b17b4f6e..ba3f2955 100644 --- a/packages/dicom-codec/src/codecs/codecFactory.js +++ b/packages/dicom-codec/src/codecs/codecFactory.js @@ -257,6 +257,86 @@ function copyFromWasm(typedArray) { return getImageFrame(typedArray).slice(); } +/** + * Expands bit-packed 1-bit PixelData to one byte per sample. + * + * DICOM packs BitsAllocated=1 PixelData eight samples to a byte, first sample + * in the least significant bit (PS3.5 8.1.1). The wasm encoders do not take + * that layout: every one of them sizes its input buffer at + * `(bitsPerSample + 7) / 8` bytes per sample, which is one byte per sample for + * any depth up to 8, and reads one sample per byte. + * + * @param {TypedArray} packed bit-packed samples, LSB of byte 0 first. + * @param {number} sampleCount number of samples to expand. + * @returns {Uint8Array} sampleCount bytes, each 0 or 1. + */ +function unpackBits(packed, sampleCount) { + const packedBytes = Math.ceil(sampleCount / 8); + if (packed.length < packedBytes) { + throw new Error( + "Bit-packed frame is too short: " + + packed.length + + " bytes for " + + sampleCount + + " samples (need " + + packedBytes + + ")" + ); + } + + const unpacked = new Uint8Array(sampleCount); + for (let i = 0; i < sampleCount; i++) { + unpacked[i] = (packed[i >> 3] >> (i & 7)) & 1; + } + + return unpacked; +} + +/** + * Returns the frame in the layout the encoder's input buffer expects. + * + * Only 1-bit frames need anything done: for every other depth the caller's + * frame already has one element per sample. A bit-packed 1-bit frame copied in + * verbatim would fill an eighth of the buffer with bytes that each hold eight + * unrelated pixels and leave the remaining seven eighths zero — an encode that + * succeeds and produces a garbage image, which is what this prevents. + * + * A 1-bit frame that is ALREADY one byte per sample is passed through: that is + * what the decoders emit for BitsAllocated=1 (they write one clamped byte per + * sample at every depth up to 8), so transcoding a 1-bit image must not try to + * unpack an unpacked frame. The two cases cannot be confused — packed is + * ceil(n/8) elements and unpacked is n, equal only for a single-sample image. + * + * @param {TypedArray} imageFrame current image frame pixels. + * @param {ExtendedImageInfo} imageInfo current image info object. + * @param {number} bufferLength length of the encoder's input buffer, which for + * 1-bit data is exactly the sample count. + * @returns {TypedArray} frame to copy into the encoder's input buffer. + */ +function toEncoderLayout(imageFrame, imageInfo, bufferLength) { + if (imageInfo.bitsPerSample !== 1) { + return imageFrame; + } + + // Packed 1-bit data is a byte stream whatever view it arrives in — a caller + // holding PixelData as a Uint16Array has the same bytes in the same order — + // so read it as bytes rather than as elements of whatever width. + const bytes = + imageFrame.BYTES_PER_ELEMENT > 1 + ? new Uint8Array( + imageFrame.buffer, + imageFrame.byteOffset, + imageFrame.byteLength + ) + : imageFrame; + + if (bytes.length >= bufferLength) { + return imageFrame; + } + + return unpackBits(bytes, bufferLength); +} + /** * Encode imageFrame using Encoder from the given local param. * @@ -272,43 +352,47 @@ function copyFromWasm(typedArray) { function encode(context, codecConfig, imageFrame, imageInfo, options = {}) { const { iterations = 1 } = options; const encoderInstance = new codecConfig.Encoder(); - const decodedTypedArray = encoderInstance.getDecodedBuffer(imageInfo); - decodedTypedArray.set(imageFrame); - - const { beforeEncode = () => {} } = options; - - beforeEncode(encoderInstance, codecConfig); - - context.timer.init("To encode length: " + imageFrame.length); - for (let i = 0; i < iterations; i++) { - encoderInstance.encode(); - } + try { + const decodedTypedArray = encoderInstance.getDecodedBuffer(imageInfo); + decodedTypedArray.set( + toEncoderLayout(imageFrame, imageInfo, decodedTypedArray.length) + ); - context.timer.end(); + const { beforeEncode = () => {} } = options; - const encodedTypedArray = encoderInstance.getEncodedBuffer(); - context.logger.log("Encoded length:" + encodedTypedArray.length); - context.logger.log( - "Encoded is a Typed array of: " + encodedTypedArray.constructor.name - ); + beforeEncode(encoderInstance, codecConfig); - // Copy BEFORE delete(): see copyFromWasm. delete() frees the vector this view - // points into, so returning the view alone hands the caller memory the wasm - // allocator may reissue at any time. - const encodedCopy = copyFromWasm(encodedTypedArray); + context.timer.init("To encode length: " + imageFrame.length); + for (let i = 0; i < iterations; i++) { + encoderInstance.encode(); + } - // cleanup allocated memory - encoderInstance.delete(); + context.timer.end(); - const processInfo = { - duration: context.timer.getDuration(), - }; + const encodedTypedArray = encoderInstance.getEncodedBuffer(); + context.logger.log("Encoded length:" + encodedTypedArray.length); + context.logger.log( + "Encoded is a Typed array of: " + encodedTypedArray.constructor.name + ); - return { - imageFrame: encodedCopy, - imageInfo: getTargetImageInfo(imageInfo, imageInfo), - processInfo, - }; + // Copy BEFORE delete(): see copyFromWasm. delete() frees the vector this + // view points into, so returning the view alone hands the caller memory the + // wasm allocator may reissue at any time. + const encodedCopy = copyFromWasm(encodedTypedArray); + + const processInfo = { + duration: context.timer.getDuration(), + }; + + return { + imageFrame: encodedCopy, + imageInfo: getTargetImageInfo(imageInfo, imageInfo), + processInfo, + }; + } finally { + // cleanup allocated memory + encoderInstance.delete(); + } } /** @@ -358,75 +442,83 @@ function decode(context, codecConfig, imageFrame, imageInfo, options = {}) { decoderInstance = new codecConfig.Decoder(); } - const { length } = imageFrame; - // get pointer to the source/encoded bit stream buffer in WASM memory - // that can hold the encoded bitstream - const encodedTypedArray = decoderInstance.getEncodedBuffer(length); - - // copy the encoded bitstream into WASM memory buffer - encodedTypedArray.set(imageFrame); - context.timer.init("To decode length: " + length); - // decode it - decoderInstance.decode(); - context.timer.end(); - - const decodedTypedArray = decoderInstance.getDecodedBuffer(); - - context.logger.log("Decoded length:" + decodedTypedArray.length); - context.logger.log( - "Decoded is a Typed array of: " + decodedTypedArray.constructor.name - ); - - // get information about the decoded image - const decodedImageInfo = decoderInstance.getFrameInfo(); - - // Copy out of WASM memory before anything can invalidate the view — the - // delete() below, or the next decode on a reused instance. See copyFromWasm. - const decodedCopy = copyFromWasm(decodedTypedArray); - - const decodeStatus = getDecodeStatus(decoderInstance); - - // cleanup allocated memory — except when reusing, where the whole point is - // that this instance survives to the next call. openjphjs' decoder-reuse - // test covers the consequence that matters: retained buffers must not make - // successive decodes progressively slower. - if (!reuseDecoder) { - decoderInstance.delete(); - } + try { + const { length } = imageFrame; + // get pointer to the source/encoded bit stream buffer in WASM memory + // that can hold the encoded bitstream + const encodedTypedArray = decoderInstance.getEncodedBuffer(length); + + // copy the encoded bitstream into WASM memory buffer + encodedTypedArray.set(imageFrame); + context.timer.init("To decode length: " + length); + // decode it + decoderInstance.decode(); + context.timer.end(); + + const decodedTypedArray = decoderInstance.getDecodedBuffer(); + + context.logger.log("Decoded length:" + decodedTypedArray.length); + context.logger.log( + "Decoded is a Typed array of: " + decodedTypedArray.constructor.name + ); - if (decodeStatus.failed && !decodeStatus.headerValid) { - // Nothing usable came back: the codec could not parse the header, so the - // dimensions and the buffer are both meaningless. Decoders that swallow - // this (openjph, so that truncated streams can degrade gracefully) would - // otherwise have this function report success on an empty or wrongly sized - // frame — and with a reused decoder, report it under the previous frame's - // pixels. Throwing here is the pre-reuse behaviour for a stream that - // genuinely cannot be decoded. - throw new Error("Decode failed: " + decodeStatus.message); - } + // get information about the decoded image + const decodedImageInfo = decoderInstance.getFrameInfo(); + + // Copy out of WASM memory before anything can invalidate the view — the + // delete() below, or the next decode on a reused instance. See + // copyFromWasm. + const decodedCopy = copyFromWasm(decodedTypedArray); + + const decodeStatus = getDecodeStatus(decoderInstance); + + if (decodeStatus.failed && !decodeStatus.headerValid) { + // Nothing usable came back: the codec could not parse the header, so the + // dimensions and the buffer are both meaningless. Decoders that swallow + // this (openjph, so that truncated streams can degrade gracefully) would + // otherwise have this function report success on an empty or wrongly + // sized frame — and with a reused decoder, report it under the previous + // frame's pixels. Throwing here is the pre-reuse behaviour for a stream + // that genuinely cannot be decoded. + throw new Error("Decode failed: " + decodeStatus.message); + } - const processInfo = { - duration: context.timer.getDuration(), - }; + const processInfo = { + duration: context.timer.getDuration(), + }; + + if (decodeStatus.failed) { + // Header parsed but the decode did not finish: a correctly sized frame + // whose undecoded region is zero-filled. Not the truncation case — + // openjph absorbs a short codestream as zero coefficients and calls that + // a success (measured across every truncation length of a 185 KB + // fixture), so what lands here is a codestream whose markers parse but + // whose parameters the decoder rejects. Reported rather than thrown, + // because the frame that came back is real as far as it goes; flagged, + // because it is not the whole image. + processInfo.partial = true; + processInfo.partialReason = decodeStatus.message; + context.logger.log("Partial decode: " + decodeStatus.message); + } - if (decodeStatus.failed) { - // Header parsed but the decode did not finish: a correctly sized frame - // whose undecoded region is zero-filled. Not the truncation case — openjph - // absorbs a short codestream as zero coefficients and calls that a success - // (measured across every truncation length of a 185 KB fixture), so what - // lands here is a codestream whose markers parse but whose parameters the - // decoder rejects. Reported rather than thrown, because the frame that came - // back is real as far as it goes; flagged, because it is not the whole image. - processInfo.partial = true; - processInfo.partialReason = decodeStatus.message; - context.logger.log("Partial decode: " + decodeStatus.message); + return { + imageFrame: decodedCopy, + imageInfo: getTargetImageInfo(imageInfo, decodedImageInfo), + processInfo, + }; + } finally { + // Cleanup runs on the throw paths too, so a failed decode cannot leak a + // single-use instance — that is what this finally is for. + // + // Except when reusing: the whole point is that the instance survives to + // the next call, and release() is what frees it. Deleting here would hand + // the next decode a dead handle. openjphjs' decoder-reuse test covers the + // consequence that matters: retained buffers must not make successive + // decodes progressively slower. + if (!reuseDecoder) { + decoderInstance.delete(); + } } - - return { - imageFrame: decodedCopy, - imageInfo: getTargetImageInfo(imageInfo, decodedImageInfo), - processInfo, - }; } /** @@ -485,3 +577,5 @@ exports.initialize = initialize; exports.getPixelData = getPixelData; exports.getTargetImageInfo = getTargetImageInfo; exports.releaseDecoder = releaseDecoder; +exports.unpackBits = unpackBits; +exports.toEncoderLayout = toEncoderLayout; diff --git a/packages/dicom-codec/src/codecs/index.js b/packages/dicom-codec/src/codecs/index.js index fb775649..c3930a11 100644 --- a/packages/dicom-codec/src/codecs/index.js +++ b/packages/dicom-codec/src/codecs/index.js @@ -108,12 +108,18 @@ function getCodec(transferSyntaxUID) { * @returns {ExtendedImageInfo} Adapted imageInfo to all codecs. */ function adaptImageInfo(imageInfo) { - const { rows, columns, bitsAllocated, signed, samplesPerPixel, pixelRepresentation } = imageInfo; + const { rows, columns, bitsAllocated, signed, samplesPerPixel, pixelRepresentation, planarConfiguration } = imageInfo; return { pixelRepresentation, bitsAllocated, samplesPerPixel, + // Must survive adaptation: rleLossless dispatches between interleaved + // (decode8) and plane-sequential (decode8Planar) output on this flag. + // It was previously dropped here, which made decode8Planar unreachable + // through the public decode() API — PlanarConfiguration=1 datasets + // silently produced interleaved output. + planarConfiguration, rows, // Number with the image rows/height columns, // Number with the image columns/width width: columns, diff --git a/packages/dicom-codec/src/codecs/libjpegTurbo12bit.js b/packages/dicom-codec/src/codecs/libjpegTurbo12bit.js index ef26caa6..e99b96db 100644 --- a/packages/dicom-codec/src/codecs/libjpegTurbo12bit.js +++ b/packages/dicom-codec/src/codecs/libjpegTurbo12bit.js @@ -1,21 +1,41 @@ +const codecModule = require("@cornerstonejs/codec-libjpeg-turbo-12bit"); +const codecWasmModule = require("@cornerstonejs/codec-libjpeg-turbo-12bit/wasmjs"); +const codecFactory = require("./codecFactory"); + /** * @type {CodecWrapper} */ const codecWrapper = { - // assign it and prevent initialization codec: undefined, Decoder: undefined, Encoder: undefined, - decoderName: "codec libjpeg turbo 12bit", - encoderName: "codec libjpeg turbo 12bit", + encoderName: "JPEGEncoder", + decoderName: "JPEGDecoder", }; +/** + * Decode imageFrame using libjpegTurbo 12bit decoder. + * + * @param {TypedArray} imageFrame to decode. + * @param {ExtendedImageInfo} imageInfo image info options. + * @returns Object containing decoded image frame and imageInfo (current) data. + */ async function decode(imageFrame, imageInfo) { - throw Error("Decoder not found for codec:" + codecWrapper.encoderName); + return codecFactory.runProcess( + codecWrapper, + codecModule, + codecWasmModule, + codecWrapper.decoderName, + (context) => { + return codecFactory.decode(context, codecWrapper, imageFrame, imageInfo); + } + ); } /** - * <> Encode imageFrame to libjpegTurbo 12bits format. + * <> The libjpeg-turbo 12bit build does not expose an + * encoder (see src/jslib.cpp — the JPEGEncoder bindings are disabled), so + * encoding is not supported for this codec. * * @param {TypedArray} imageFrame to encode. * @param {ExtendedImageInfo} imageInfo image info options. @@ -23,13 +43,11 @@ async function decode(imageFrame, imageInfo) { * @returns Object containing encoded image frame and imageInfo (current) data */ async function encode(imageFrame, imageInfo, options = {}) { - throw Error("Encoder not found for codec:" + codecWrapper.encoderName); + throw Error("Encoder not supported for codec: libjpeg-turbo 12bit"); } function getPixelData(imageFrame, imageInfo) { - throw Error( - "GetPixel not found or not applied for codec:" + codecWrapper.encoderName - ); + return codecFactory.getPixelData(imageFrame, imageInfo); } exports.decode = decode; diff --git a/packages/dicom-codec/src/codecs/littleEndian.js b/packages/dicom-codec/src/codecs/littleEndian.js index bed2e05f..86018ab9 100644 --- a/packages/dicom-codec/src/codecs/littleEndian.js +++ b/packages/dicom-codec/src/codecs/littleEndian.js @@ -68,9 +68,16 @@ function getPixelData(imageFrame, imageInfo) { if (bitsAllocated === 16) { // if pixel data is not aligned on even boundary, shift it so we can create the 16 bit array - // buffers on it + // buffers on it. + // + // The end bound is not optional. slice(offset) copies through the end of + // the BACKING buffer, and imageFrame is typically a single frame's view + // into a whole multi-frame P10 buffer — so the one-argument form allocates + // and copies the entire rest of the file to realign one frame (measured: + // 67 MB for a 1 MB frame in a 64 MB buffer). The returned view's length + // hides it, because it is correct either way. if (offset % 2) { - arrayBuffer = arrayBuffer.slice(offset); + arrayBuffer = arrayBuffer.slice(offset, offset + imageFrame.byteLength); offset = 0; } @@ -82,13 +89,26 @@ function getPixelData(imageFrame, imageInfo) { } else if (bitsAllocated === 8 || bitsAllocated === 1) { result = imageFrame; } else if (bitsAllocated === 32) { - // if pixel data is not aligned on even boundary, shift it - if (offset % 2) { - arrayBuffer = arrayBuffer.slice(offset); + // imageFrame is typically a view into the full DICOM P10 buffer, so its + // byteOffset is even (DICOM guarantees even lengths) but not necessarily + // 4-byte aligned; 32-bit typed-array views require 4-byte alignment, + // so copy the bytes to a fresh, aligned buffer when needed — bounded to + // this frame, for the reason given on the 16-bit branch above + if (offset % 4) { + arrayBuffer = arrayBuffer.slice(offset, offset + imageFrame.byteLength); offset = 0; } - result = new Float32Array(arrayBuffer, offset, length / 4); + // 32-bit PixelData is integer data (signed per pixelRepresentation); + // it is only float when pixelRepresentation is absent (e.g. the + // FloatPixelData element), matching cornerstone3D's decodeLittleEndian + if (pixelRepresentation === 0) { + result = new Uint32Array(arrayBuffer, offset, length / 4); + } else if (pixelRepresentation === 1) { + result = new Int32Array(arrayBuffer, offset, length / 4); + } else { + result = new Float32Array(arrayBuffer, offset, length / 4); + } } return result; diff --git a/packages/dicom-codec/test/color-and-depth.test.js b/packages/dicom-codec/test/color-and-depth.test.js index 00d9c0d5..fa20f67f 100644 --- a/packages/dicom-codec/test/color-and-depth.test.js +++ b/packages/dicom-codec/test/color-and-depth.test.js @@ -61,6 +61,26 @@ describe.skipIf(!ALL_BUILT)("dicom-codec color and bit-depth dispatch", () => { expect(frameBytes(result.imageFrame).equals(us1)).toBe(true) }) + it("decodes color RLE plane-sequential when planarConfiguration is 1", async () => { + const rleBytes = readFileSync( + resolve(packagesRoot, "dicom-codec/test/fixtures/rle/US1-color.rle") + ) + const result = await dicomCodec.decode( + new Uint8Array(rleBytes), + { rows: 480, columns: 640, bitsAllocated: 8, samplesPerPixel: 3, planarConfiguration: 1 }, + "1.2.840.10008.1.2.5" + ) + const out = frameBytes(result.imageFrame) + expect(out.length).toBe(us1.length) + // expected layout: RRR...GGG...BBB (de-interleaved planes of US1) + const frameSize = 640 * 480 + const planar = Buffer.alloc(us1.length) + for (let s = 0; s < 3; s++) { + for (let i = 0; i < frameSize; i++) planar[s * frameSize + i] = us1[i * 3 + s] + } + expect(out.equals(planar)).toBe(true) + }) + it("decodes an 8-bit JPEG-LS (.80) through the dispatcher losslessly", async () => { const jls = readFileSync(resolve(packagesRoot, "charls/test/fixtures/CT2-gray8.jls")) const ct2 = readFileSync(resolve(packagesRoot, "charls/test/fixtures/CT2.RAW")) diff --git a/packages/dicom-codec/test/dispatch.test.js b/packages/dicom-codec/test/dispatch.test.js index 3d587988..252b324d 100644 --- a/packages/dicom-codec/test/dispatch.test.js +++ b/packages/dicom-codec/test/dispatch.test.js @@ -42,6 +42,47 @@ const SUPPORTED_UIDS = [ "1.2.840.10008.1.2.5", ] +describe("codecFactory instance cleanup", () => { + it("frees the decoder instance even when decode() throws", () => { + const codecFactory = require("../src/codecs/codecFactory") + + let deleted = false + + class FakeDecoder { + getEncodedBuffer() { + return { set: () => {} } + } + + decode() { + throw new Error("boom") + } + + delete() { + deleted = true + } + } + + const codecConfig = { Decoder: FakeDecoder } + const context = { + timer: { + init: () => {}, + end: () => {}, + getDuration: () => 0, + }, + logger: { + log: () => {}, + }, + } + const imageFrame = new Uint8Array([1, 2, 3]) + const imageInfo = {} + + expect(() => + codecFactory.decode(context, codecConfig, imageFrame, imageInfo) + ).toThrow("boom") + expect(deleted).toBe(true) + }) +}) + // In CI a missing sibling dist means the build/artifact pipeline broke; fail // loudly instead of letting describe.skipIf() silently skip the whole suite. it.runIf(process.env.CI)("required sibling builds are present in CI", () => { diff --git a/packages/dicom-codec/test/integration.test.js b/packages/dicom-codec/test/integration.test.js index 8f21b363..c1f95873 100644 --- a/packages/dicom-codec/test/integration.test.js +++ b/packages/dicom-codec/test/integration.test.js @@ -9,6 +9,9 @@ const packagesRoot = resolve(__dirname, "../..") const LIBJPEG_8BIT_BUILT = existsSync( resolve(packagesRoot, "libjpeg-turbo-8bit/dist/libjpegturbojs.js") ) +const LIBJPEG_12BIT_BUILT = existsSync( + resolve(packagesRoot, "libjpeg-turbo-12bit/dist/libjpegturbo12js.js") +) const CHARLS_BUILT = existsSync( resolve(packagesRoot, "charls/dist/charlsjs.js") ) @@ -20,7 +23,11 @@ const OPENJPH_BUILT = existsSync( ) const ALL_BUILT = - LIBJPEG_8BIT_BUILT && CHARLS_BUILT && OPENJPEG_BUILT && OPENJPH_BUILT + LIBJPEG_8BIT_BUILT && + LIBJPEG_12BIT_BUILT && + CHARLS_BUILT && + OPENJPEG_BUILT && + OPENJPH_BUILT // Byte view over a decoded imageFrame regardless of its typed-array flavor // (Uint8Array, Uint16Array, Int16Array, ...). @@ -81,6 +88,41 @@ describe.skipIf(!ALL_BUILT)("dicom-codec integration", () => { }) }) + describe("JPEG Baseline 12-bit (1.2.840.10008.1.2.4.51)", () => { + const jpeg12BitBytes = readFileSync( + resolve( + packagesRoot, + "libjpeg-turbo-12bit/test/fixtures/jpeg/CT-512x512-12bit.jpg" + ) + ) + const jpeg12BitRaw = readFileSync( + resolve(packagesRoot, "libjpeg-turbo-12bit/test/fixtures/raw/CT-512x512-12bit.raw") + ) + + it("decodes through the dispatcher to the exact reference pixels", async () => { + const imageInfo = { + rows: 512, + columns: 512, + bitsAllocated: 16, + samplesPerPixel: 1, + pixelRepresentation: 0, + signed: false, + } + + const result = await dicomCodec.decode( + jpeg12BitBytes, + imageInfo, + "1.2.840.10008.1.2.4.51" + ) + + expect(result.imageFrame.byteLength).toBe(512 * 512 * 2) + expect(frameBytes(result.imageFrame).equals(jpeg12BitRaw)).toBe(true) + expect(result.imageInfo.width).toBe(512) + expect(result.imageInfo.height).toBe(512) + expect(typeof result.processInfo.duration).toBe("number") + }) + }) + describe("JPEG-LS Lossless (1.2.840.10008.1.2.4.80)", () => { const jlsBytes = readFileSync( resolve(packagesRoot, "charls/test/fixtures/CT1.JLS") diff --git a/packages/dicom-codec/test/one-bit.test.js b/packages/dicom-codec/test/one-bit.test.js new file mode 100644 index 00000000..4af2a449 --- /dev/null +++ b/packages/dicom-codec/test/one-bit.test.js @@ -0,0 +1,223 @@ +import { beforeAll, describe, expect, it } from "vitest" +import { existsSync, readFileSync } from "node:fs" +import { fileURLToPath } from "node:url" +import { dirname, resolve } from "node:path" +import codecFactory from "../src/codecs/codecFactory.js" +import { + bilevelFromCT2, + packBitsLsbFirst, +} from "../../../tools/fixture-verification/gen/derive.mjs" + +const __dirname = dirname(fileURLToPath(import.meta.url)) +const packagesRoot = resolve(__dirname, "../..") + +const REQUIRED = ["openjphjs/dist/openjphjs.js", "openjpeg/dist/openjpegjs.js"] +const ALL_BUILT = REQUIRED.every((p) => existsSync(resolve(packagesRoot, p))) + +function frameBytes(imageFrame) { + return Buffer.from(imageFrame.buffer, imageFrame.byteOffset ?? 0, imageFrame.byteLength) +} + +it.runIf(process.env.CI)("sibling codec dists are present in CI (1-bit suite)", () => { + expect(ALL_BUILT, "codec dists missing — artifacts not replayed").toBe(true) +}) + +// BitsAllocated=1 is the one depth where the caller's frame and the encoder's +// input buffer are not the same shape. DICOM packs the samples eight to a byte +// (first sample in the least significant bit, PS3.5 8.1.1); every wasm encoder +// sizes its input at (bitsPerSample + 7) / 8 == 1 byte per sample and reads one +// sample per byte. codecFactory bridges the two — these pin that it does. +describe("codecFactory 1-bit sample layout", () => { + it("unpacks bit-packed samples least-significant-bit first", () => { + // 0x01 -> sample 0 set; 0x80 -> sample 15 set. LSB-first, so the low bit of + // the first byte is pixel 0, not pixel 7. + const packed = Uint8Array.from([0b00000001, 0b10000000]) + expect(Array.from(codecFactory.unpackBits(packed, 16))).toEqual([ + 1, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 1, + ]) + }) + + it("unpacks a trailing partial byte without reading past the sample count", () => { + const packed = Uint8Array.from([0b11111111]) + expect(Array.from(codecFactory.unpackBits(packed, 3))).toEqual([1, 1, 1]) + }) + + it("round-trips against the packer used to build the fixtures", () => { + const samples = Uint8Array.from({ length: 1000 }, (_, i) => (i % 7 === 0 ? 1 : 0)) + const packed = packBitsLsbFirst(samples) + expect(packed.length).toBe(125) + expect(Array.from(codecFactory.unpackBits(packed, samples.length))).toEqual( + Array.from(samples) + ) + }) + + it("throws rather than silently zero-filling when the packed frame is short", () => { + expect(() => codecFactory.unpackBits(new Uint8Array(3), 64)).toThrow( + /Bit-packed frame is too short: 3 bytes for 64 samples \(need 8\)/ + ) + }) + + it("expands a packed 1-bit frame to fill the encoder buffer", () => { + const samples = Uint8Array.from({ length: 64 }, (_, i) => i & 1) + const packed = packBitsLsbFirst(samples) + const layout = codecFactory.toEncoderLayout(packed, { bitsPerSample: 1 }, 64) + expect(layout.length).toBe(64) + expect(Array.from(layout)).toEqual(Array.from(samples)) + }) + + it("passes an already-unpacked 1-bit frame through, so transcoding does not unpack twice", () => { + // What the decoders emit for BitsAllocated=1: one clamped byte per sample, + // exactly as for every other depth up to 8. + const samples = Uint8Array.from({ length: 64 }, (_, i) => i & 1) + expect(codecFactory.toEncoderLayout(samples, { bitsPerSample: 1 }, 64)).toBe(samples) + }) + + it("reads a packed frame handed over as a wider view as bytes", () => { + // A caller holding PixelData as a Uint16Array has the same packed bytes in + // the same order; counting its elements instead would see half the data. + const samples = Uint8Array.from({ length: 64 }, (_, i) => i & 1) + const packed = packBitsLsbFirst(samples) + const asWords = new Uint16Array(packed.buffer, packed.byteOffset, packed.byteLength / 2) + expect(asWords.length).toBe(4) + const layout = codecFactory.toEncoderLayout(asWords, { bitsPerSample: 1 }, 64) + expect(Array.from(layout)).toEqual(Array.from(samples)) + }) + + it("leaves frames at every other depth alone", () => { + const frame = new Uint16Array(64) + expect(codecFactory.toEncoderLayout(frame, { bitsPerSample: 16 }, 128)).toBe(frame) + const bytes = new Uint8Array(64) + expect(codecFactory.toEncoderLayout(bytes, { bitsPerSample: 8 }, 64)).toBe(bytes) + }) +}) + +describe.skipIf(!ALL_BUILT)("dicom-codec 1-bit encode/decode", () => { + let dicomCodec + // A CT silhouette rather than noise or a pattern: long runs broken by an + // irregular boundary, so a stride or bit-order mistake shows up instead of + // being masked by uniform content. + const ct2 = readFileSync(resolve(packagesRoot, "charls/test/fixtures/CT2.RAW")) + const samples = bilevelFromCT2(ct2) + const packed = packBitsLsbFirst(samples) + const imageInfo = { + rows: 512, + columns: 512, + bitsAllocated: 1, + samplesPerPixel: 1, + pixelRepresentation: 0, + signed: false, + } + + beforeAll(async () => { + const mod = await import("../src/index.js") + dicomCodec = mod.default ?? mod + }) + + it("derives a packed frame an eighth the size of the sample count", () => { + expect(samples.length).toBe(512 * 512) + expect(packed.length).toBe((512 * 512) / 8) + }) + + // The two codecs whose formats carry a 1-bit component. JPEG-LS is not in the + // list on purpose: CharLS rejects bit depths below 2 outright. + it.each([ + { name: "HTJ2K Lossless", uid: "1.2.840.10008.1.2.4.201" }, + { name: "JPEG 2000 Lossless", uid: "1.2.840.10008.1.2.4.90" }, + ])("encodes a bit-packed frame to $name ($uid) and decodes it back losslessly", async ({ uid }) => { + // Without the unpack in codecFactory.encode this passes bit-packed bytes to + // an encoder expecting one sample per byte: 1/8 of the buffer holds bytes + // that each carry eight unrelated pixels and 7/8 stays zero. The encode + // still succeeds, which is what makes it worth a test. + const encoded = await dicomCodec.encode(packed, imageInfo, uid) + expect(encoded.imageFrame.length).toBeGreaterThan(0) + + const decoded = await dicomCodec.decode(encoded.imageFrame, imageInfo, uid) + expect(decoded.processInfo.partial).toBeUndefined() + expect(decoded.imageInfo.bitsPerSample).toBe(1) + expect(decoded.imageInfo.rows).toBe(512) + expect(decoded.imageInfo.columns).toBe(512) + + // One byte per sample on the way out, values 0/1 — not repacked. + const out = frameBytes(decoded.imageFrame) + expect(out.length).toBe(512 * 512) + expect(out.equals(Buffer.from(samples.buffer, 0, samples.byteLength))).toBe(true) + }) + + it("transcodes 1-bit native little endian to HTJ2K and back", async () => { + const transcoded = await dicomCodec.transcode( + packed, + imageInfo, + "1.2.840.10008.1.2.1", + "1.2.840.10008.1.2.4.201" + ) + const decoded = await dicomCodec.decode( + transcoded.imageFrame, + imageInfo, + "1.2.840.10008.1.2.4.201" + ) + expect(frameBytes(decoded.imageFrame).equals(Buffer.from(samples.buffer, 0, samples.byteLength))).toBe(true) + }) + + it("re-encodes an already-decoded 1-bit frame without unpacking it again", async () => { + // The decoders hand back one byte per sample, so encode() sees a frame that + // is already in the encoder's layout. Unpacking that would read 8x past the + // end; passing it through has to round-trip unchanged. + const first = await dicomCodec.encode(packed, imageInfo, "1.2.840.10008.1.2.4.201") + const decoded = await dicomCodec.decode(first.imageFrame, imageInfo, "1.2.840.10008.1.2.4.201") + const second = await dicomCodec.encode( + decoded.imageFrame, + decoded.imageInfo, + "1.2.840.10008.1.2.4.201" + ) + const again = await dicomCodec.decode(second.imageFrame, imageInfo, "1.2.840.10008.1.2.4.201") + expect(frameBytes(again.imageFrame).equals(Buffer.from(samples.buffer, 0, samples.byteLength))).toBe(true) + }) + + it("getPixelData returns the packed bytes unchanged for native transfer syntaxes", () => { + // 1-bit PixelData stays bit-packed through the native codecs: unpacking is + // the renderer's job, and multi-frame 1-bit data is packed across frame + // boundaries so it cannot be split here either. + for (const uid of ["1.2.840.10008.1.2.1", "1.2.840.10008.1.2.2"]) { + const pixelData = dicomCodec.getPixelData(packed, imageInfo, uid) + expect(pixelData, uid).toBe(packed) + } + }) +}) + +// The big-endian codec's getPixelData handled only 8 and 16 bit, returning +// undefined for the 1- and 32-bit datasets its own decode() accepts. +describe("bigEndian getPixelData depth coverage", () => { + let dicomCodec + const BIG_ENDIAN = "1.2.840.10008.1.2.2" + + beforeAll(async () => { + const mod = await import("../src/index.js") + dicomCodec = mod.default ?? mod + }) + + it("returns the frame itself for bitsAllocated 1", () => { + const frame = Uint8Array.from([0b10101010, 0b01010101]) + const imageInfo = { rows: 4, columns: 4, bitsAllocated: 1, samplesPerPixel: 1, pixelRepresentation: 0 } + expect(dicomCodec.getPixelData(frame, imageInfo, BIG_ENDIAN)).toBe(frame) + }) + + it.each([ + { pixelRepresentation: 0, ctor: "Uint32Array" }, + { pixelRepresentation: 1, ctor: "Int32Array" }, + { pixelRepresentation: undefined, ctor: "Float32Array" }, + ])( + "byte-swaps bitsAllocated 32 into $ctor for pixelRepresentation=$pixelRepresentation", + ({ pixelRepresentation, ctor }) => { + // Bytes 01 02 03 04 are the big-endian encoding of 0x01020304, and the + // typed arrays below read little endian, so the swap has to leave + // 0x01020304 in the word. + const frame = Uint8Array.from([0x01, 0x02, 0x03, 0x04]) + const imageInfo = { rows: 1, columns: 1, bitsAllocated: 32, samplesPerPixel: 1, pixelRepresentation } + const pixelData = dicomCodec.getPixelData(frame, imageInfo, BIG_ENDIAN) + expect(pixelData.constructor.name).toBe(ctor) + expect(pixelData.length).toBe(1) + expect(new Uint32Array(pixelData.buffer, pixelData.byteOffset, 1)[0]).toBe(0x01020304) + } + ) +}) diff --git a/packages/dicom-codec/test/transcode-and-pixeldata.test.js b/packages/dicom-codec/test/transcode-and-pixeldata.test.js index 3dd343df..f64ae521 100644 --- a/packages/dicom-codec/test/transcode-and-pixeldata.test.js +++ b/packages/dicom-codec/test/transcode-and-pixeldata.test.js @@ -21,7 +21,7 @@ it.runIf(process.env.CI)("sibling codec dists are present in CI (transcode suite expect(ALL_BUILT, "codec dists missing — artifacts not replayed").toBe(true) }) -describe.skipIf(!ALL_BUILT)("dicom-codec encode", () => { +describe.skipIf(!ALL_BUILT)("dicom-codec transcode and encode", () => { let dicomCodec const ct1Raw = readFileSync(resolve(packagesRoot, "openjpeg/test/fixtures/raw/CT1.RAW")) const ctImageInfo = { diff --git a/packages/libjpeg-turbo-12bit/bench/decode.bench.js b/packages/libjpeg-turbo-12bit/bench/decode.bench.js new file mode 100644 index 00000000..fe7cc5e7 --- /dev/null +++ b/packages/libjpeg-turbo-12bit/bench/decode.bench.js @@ -0,0 +1,55 @@ +// Cold vs warm decode benches for the 12-bit codec, mirroring the 8-bit +// package's bench shape (see libjpeg-turbo-8bit/bench/decode.bench.js for +// the cold/warm methodology notes). Without this file the 12-bit package +// was invisible to CodSpeed — a toolchain bump's full bench sweep measured +// nothing for it. +import { bench, describe } from "vitest" +import { existsSync, readFileSync } from "node:fs" +import { fileURLToPath } from "node:url" +import { dirname, resolve } from "node:path" + +const __dirname = dirname(fileURLToPath(import.meta.url)) +const distDir = resolve(__dirname, "../dist") +const fixturesDir = resolve(__dirname, "../test/fixtures") + +const distPath = resolve(distDir, "libjpegturbo12wasm.js") +const skip = !existsSync(distPath) + +const encoded = !skip + ? readFileSync(resolve(fixturesDir, "jpeg/CT-512x512-12bit.jpg")) + : null + +let codec +let warmDecoder + +async function loadCodec() { + const mod = await import(distPath) + const factory = mod.default ?? mod + // Silence wasm stdout/stderr so vitest's console interception never runs + // inside a measured bench body (see openjphjs bench for details). + return await factory({ print: () => {}, printErr: () => {} }) +} + +function decodeOnce(decoder) { + decoder.getEncodedBuffer(encoded.length).set(encoded) + decoder.decode() + return decoder.getDecodedBuffer() +} + +if (!skip) { + codec = await loadCodec() + warmDecoder = new codec.JPEGDecoder() + for (let i = 0; i < 5; i++) decodeOnce(warmDecoder) +} + +describe.skipIf(skip)("libjpeg-turbo-12bit (wasm)", () => { + bench("decode CT-512x512-12bit.jpg (512x512x12bit) — cold", () => { + const decoder = new codec.JPEGDecoder() + decodeOnce(decoder) + decoder.delete() + }) + + bench("decode CT-512x512-12bit.jpg (512x512x12bit) — warm", () => { + decodeOnce(warmDecoder) + }) +}) diff --git a/packages/libjpeg-turbo-12bit/package.json b/packages/libjpeg-turbo-12bit/package.json index c6343f8f..80d82749 100644 --- a/packages/libjpeg-turbo-12bit/package.json +++ b/packages/libjpeg-turbo-12bit/package.json @@ -2,7 +2,11 @@ "name": "@cornerstonejs/codec-libjpeg-turbo-12bit", "version": "0.4.4", "description": "JS/WASM Build of [libjpeg-turbo](https://github.com/libjpeg-turbo) WITH12BIT=ON", - "main": "dist/libjpeg-turbojs.js", + "main": "dist/libjpegturbo12js.js", + "exports": { + ".": "./dist/libjpegturbo12js.js", + "./wasmjs": "./dist/libjpegturbo12wasm.js" + }, "publishConfig": { "access": "public" }, diff --git a/packages/libjpeg-turbo-12bit/src/JPEGDecoder.hpp b/packages/libjpeg-turbo-12bit/src/JPEGDecoder.hpp index 7a15c920..7687a528 100644 --- a/packages/libjpeg-turbo-12bit/src/JPEGDecoder.hpp +++ b/packages/libjpeg-turbo-12bit/src/JPEGDecoder.hpp @@ -3,7 +3,10 @@ #pragma once +#include #include +#include +#include #include // #include "config.h" #include "jpeglib.h" @@ -14,6 +17,7 @@ using namespace std; #include thread_local const emscripten::val Uint8ClampedArray = emscripten::val::global("Uint8ClampedArray"); +thread_local const emscripten::val Uint16Array = emscripten::val::global("Uint16Array"); #endif @@ -54,13 +58,16 @@ class JPEGDecoder { /// holds the decoded pixel data /// emscripten::val getDecodedBuffer() { - // Create a JavaScript-friendly result from the memory view - // instead of relying on the consumer to detach it from WASM memory - // See https://web.dev/webassembly-memory-debugging/ - emscripten::val js_result = Uint8ClampedArray.new_(emscripten::typed_memory_view( + // decoded_ holds one 12-bit grayscale sample per pixel in an int16_t + // (values 0..4095). Copy it into a JS-owned Uint16Array so the result is + // detached from WASM memory (see https://web.dev/webassembly-memory-debugging/). + // NOTE: must be a 16-bit typed array — wrapping in Uint8ClampedArray would + // run every sample through ToUint8Clamp and flatten anything above 255, + // destroying the 12-bit output. + emscripten::val js_result = Uint16Array.new_(emscripten::typed_memory_view( decoded_.size(), decoded_.data() )); - + return js_result; } #else @@ -116,31 +123,75 @@ class JPEGDecoder { cinfo.err = jpeg_std_error(&jerr); jpeg_create_decompress(&cinfo); + // The explicit jpeg_destroy_decompress calls on the throw paths below + // covered every check but not decoded_.resize(), which can throw + // std::bad_alloc for a large frame and leaked the whole decompress object + // and its memory pools. A destructor covers that, and cannot be forgotten + // when another early exit is added, so the explicit calls are gone and + // this is now the single point of release. + struct DecompressGuard { + jpeg_decompress_struct& info; + ~DecompressGuard() { jpeg_destroy_decompress(&info); } + } guard{cinfo}; + jpeg_mem_src(&cinfo, encoded_.data(), encoded_.size()); // Read file header, set default decompression parameters jpeg_read_header(&cinfo, TRUE); - // Force RGBA decoding, even for grayscale images - cinfo.out_color_space = JCS_EXT_RGBA; + // Fail closed on multi-component images. This codec only supports + // single-component (grayscale) 12-bit JPEGs; forcing JCS_GRAYSCALE on a + // color image would make libjpeg silently discard the chroma channels + // and report componentCount=1, corrupting color data without any error. + if (cinfo.num_components != 1) { + throw std::runtime_error( + "Unsupported 12-bit JPEG: expected 1 component (grayscale), got " + + std::to_string(cinfo.num_components)); + } + // Decode as single-component grayscale. This is a 12-bit-per-sample + // codec: each output value is a 16-bit-wide JSAMPLE (holding 0..4095), + // not an 8-bit RGBA quad. Previously this forced a 4-samples-per-pixel + // RGBA colorspace while the output buffer below was sized for 1 + // sample/pixel, causing libjpeg to write ~2x past the end of the + // allocated buffer (heap overflow). + cinfo.out_color_space = JCS_GRAYSCALE; jpeg_start_decompress(&cinfo); frameInfo_.width = cinfo.output_width; frameInfo_.height = cinfo.output_height; - frameInfo_.bitsPerSample = 8; - frameInfo_.componentCount = 1; //inColorspace == 2 ? 1 : 3; - - // Prepare output buffer - // int pixelFormat = (frameInfo_.componentCount == 1) ? TJPF_GRAY : TJPF_RGB; + frameInfo_.bitsPerSample = 12; + frameInfo_.componentCount = 1; + + // Prepare output buffer. One JSAMPLE (short, holding 0..4095) per pixel + // since output is single-component grayscale. + const int pixelFormat = 1; + + // Compute the output size (in samples) using a checked 64-bit multiply + // capped at 512 MiB so a malformed/adversarial header cannot overflow + // the size computation or force an unbounded allocation. + constexpr uint64_t kMaxOutputSamples = 512ull * 1024ull * 1024ull; // 512 MiB worth of samples + const uint64_t width64 = static_cast(cinfo.output_width); + const uint64_t height64 = static_cast(cinfo.output_height); + const uint64_t pixelFormat64 = static_cast(pixelFormat); + + if (width64 == 0 || height64 == 0) { + throw std::runtime_error("Invalid JPEG dimensions (zero width or height)"); + } - // const size_t destinationSize = frameInfo_.width * frameInfo_.height * tjPixelSize[pixelFormat]; - int pixelFormat = 1; - size_t output_size = cinfo.output_width * cinfo.output_height * pixelFormat; + uint64_t output_size64 = width64 * height64; + if (output_size64 / width64 != height64) { + // width * height overflowed + throw std::runtime_error("Overflow computing decoded buffer size"); + } + output_size64 *= pixelFormat64; + if (output_size64 == 0 || output_size64 > kMaxOutputSamples) { + throw std::runtime_error("Decoded buffer size exceeds allowed maximum or is invalid"); + } - // std::vector output_buffer(output_size); + const size_t output_size = static_cast(output_size64); decoded_.resize(output_size); - auto stride = cinfo.output_width * pixelFormat; + const size_t stride = static_cast(cinfo.output_width) * static_cast(pixelFormat); // Process data while (cinfo.output_scanline < cinfo.output_height) { @@ -148,13 +199,8 @@ class JPEGDecoder { (void)jpeg_read_scanlines(&cinfo, &output_data, 1); } jpeg_finish_decompress(&cinfo); - - // Step 7: release JPEG compression object - - // auto data = Uint8ClampedArray.new_(typed_memory_view(output_size, &output_buffer[0])); - - // This is an important step since it will release a good deal of memory. - jpeg_destroy_decompress(&cinfo); + // DecompressGuard releases the decompress object -- a good deal of memory + // -- as this scope unwinds. } /// diff --git a/packages/libjpeg-turbo-12bit/test/decode.test.js b/packages/libjpeg-turbo-12bit/test/decode.test.js new file mode 100644 index 00000000..74a51f3b --- /dev/null +++ b/packages/libjpeg-turbo-12bit/test/decode.test.js @@ -0,0 +1,150 @@ +import { beforeAll, describe, expect, it } from "vitest" +import { existsSync, readFileSync } from "node:fs" +import { fileURLToPath } from "node:url" +import { dirname, resolve } from "node:path" + +const __dirname = dirname(fileURLToPath(import.meta.url)) +const fixturesDir = resolve(__dirname, "fixtures") + +// Genuine 12-bit baseline JPEG fixture (SOF marker 0xC1, precision=12, +// 512x512, 1 component — verified by inspecting the JPEG SOF segment). +const ct12bit = readFileSync(resolve(fixturesDir, "jpeg/CT-512x512-12bit.jpg")) +// Decoded reference for the fixture above: little-endian Uint16 samples, +// verified bit-identical against DCMTK's dcmdjpeg (independent reference +// decoder) and identical across the asm.js and wasm build variants. +const ct12bitRaw = readFileSync(resolve(fixturesDir, "raw/CT-512x512-12bit.raw")) + +async function loadModule(modulePath) { + const mod = await import(modulePath) + const factory = mod.default ?? mod + return await factory() +} + +const buildVariants = [ + { name: "asm.js (libjpegturbo12js)", path: "../dist/libjpegturbo12js.js" }, + { name: "wasm (libjpegturbo12wasm)", path: "../dist/libjpegturbo12wasm.js" }, +] + +describe.each(buildVariants)("libjpeg-turbo-12bit decode — $name", ({ path }) => { + const isBuilt = existsSync(resolve(__dirname, path)) + let codec + + beforeAll(async () => { + if (isBuilt) { + codec = await loadModule(path) + } + }) + + // In CI a missing dist means the build/artifact pipeline broke; fail loudly + // instead of letting every skipIf() below silently skip the suite. + it.runIf(process.env.CI)("dist is present in CI", () => { + expect(isBuilt, `${path} missing — build artifact was not replayed`).toBe(true) + }) + + it.skipIf(!isBuilt)( + "decodes the CT-512x512 12-bit fixture and reports correct dimensions/format", + () => { + const decoder = new codec.JPEGDecoder() + const encodedBuffer = decoder.getEncodedBuffer(ct12bit.length) + encodedBuffer.set(ct12bit) + + decoder.decode() + + const frameInfo = decoder.getFrameInfo() + expect(frameInfo.width).toBe(512) + expect(frameInfo.height).toBe(512) + expect(frameInfo.bitsPerSample).toBe(12) + expect(frameInfo.componentCount).toBe(1) + + const decoded = decoder.getDecodedBuffer() + // One 16-bit-wide sample per pixel (grayscale, 1 component/pixel). + expect(decoded.length).toBe(512 * 512) + + decoder.delete() + } + ) + + it.skipIf(!isBuilt)("decodes the CT-512x512 12-bit fixture and matches the RAW reference", () => { + const decoder = new codec.JPEGDecoder() + const encodedBuffer = decoder.getEncodedBuffer(ct12bit.length) + encodedBuffer.set(ct12bit) + + decoder.decode() + + const decoded = decoder.getDecodedBuffer() + // getDecodedBuffer() returns a Uint16Array (one entry per sample); + // compare its underlying bytes against the little-endian RAW reference. + const decodedBytes = Buffer.from( + decoded.buffer, + decoded.byteOffset, + decoded.byteLength + ) + expect(decodedBytes.length).toBe(ct12bitRaw.length) + expect(decodedBytes.equals(ct12bitRaw)).toBe(true) + + decoder.delete() + }) + + it.skipIf(!isBuilt)("rejects multi-component (color) 12-bit JPEGs instead of silently dropping chroma", () => { + // Splice the grayscale fixture into a syntactically valid 3-component + // JPEG: rewrite SOF1 (FFC1) from 1 to 3 components and SOS (FFDA) from + // 1 to 3 selectors. The decoder must fail closed on the header — a + // forced JCS_GRAYSCALE decode would silently discard the chroma + // channels — so the entropy data never being read is fine. + const findMarker = (buf, marker) => { + for (let i = 2; i < buf.length - 1; i++) { + if (buf[i] === 0xff && buf[i + 1] === marker) return i + } + throw new Error("marker not found") + } + const sofAt = findMarker(ct12bit, 0xc1) + const sosAt = findMarker(ct12bit, 0xda) + + const before = ct12bit.subarray(0, sofAt) + const sofBody = ct12bit.subarray(sofAt + 4, sofAt + 4 + 5) // P(1), Y(2), X(2) + const between = ct12bit.subarray(sofAt + 2 + 11, sosAt) // after 1-comp SOF + const after = ct12bit.subarray(sosAt + 2 + 8) // after 1-comp SOS: entropy data + + const sof3 = Buffer.concat([ + Buffer.from([0xff, 0xc1, 0x00, 17]), + sofBody, + Buffer.from([3]), // Nf = 3 + Buffer.from([1, 0x11, 0, 2, 0x11, 0, 3, 0x11, 0]), + ]) + const sos3 = Buffer.concat([ + Buffer.from([0xff, 0xda, 0x00, 12, 3]), // Ns = 3 + Buffer.from([1, 0x00, 2, 0x00, 3, 0x00]), + ct12bit.subarray(sosAt + 7, sosAt + 10), // Ss, Se, Ah/Al + ]) + const colorJpeg = Buffer.concat([before, sof3, between, sos3, after]) + + const decoder = new codec.JPEGDecoder() + decoder.getEncodedBuffer(colorJpeg.length).set(colorJpeg) + + expect(() => decoder.decode()).toThrow() + + decoder.delete() + }) + + it.skipIf(!isBuilt)("handles truncated input without crashing", () => { + // libjpeg treats a premature end-of-file as a recoverable warning (it + // fills the missing scanlines rather than aborting), so decode() may + // return normally instead of throwing. The meaningful guarantee here is + // that truncated input is handled gracefully — it either throws or + // returns, but never corrupts the process. + const truncated = ct12bit.subarray(0, Math.floor(ct12bit.length / 2)) + const decoder = new codec.JPEGDecoder() + const encodedBuffer = decoder.getEncodedBuffer(truncated.length) + encodedBuffer.set(truncated) + + expect(() => { + try { + decoder.decode() + } catch (e) { + // throwing is an acceptable outcome for malformed input + } + }).not.toThrow() + + decoder.delete() + }) +}) diff --git a/packages/libjpeg-turbo-12bit/vitest.config.mjs b/packages/libjpeg-turbo-12bit/vitest.config.mjs new file mode 100644 index 00000000..1622965d --- /dev/null +++ b/packages/libjpeg-turbo-12bit/vitest.config.mjs @@ -0,0 +1,26 @@ +import { defineConfig } from "vitest/config" +import codspeedPlugin from "@codspeed/vitest-plugin" + +export default defineConfig({ + plugins: [codspeedPlugin()], + test: { + // Under the CodSpeed simulation instrument the entire process runs ~60x + // slower under valgrind while vitest's hard-coded 60s worker-RPC timer + // counts real seconds, so large bench suites structurally hit "Timeout + // calling onTaskUpdate" AFTER their benches complete and upload. Ignore + // that exit-code noise in simulation only; walltime and test runs stay + // strict. + // (CODSPEED_ENV is set whenever the CodSpeed runner is active; the + // mode string is "instrumentation" on older runners and "simulation" + // on newer ones, so match anything except walltime.) + dangerouslyIgnoreUnhandledErrors: + process.env.CODSPEED_ENV !== undefined && + process.env.CODSPEED_RUNNER_MODE !== "walltime", + name: "libjpeg-turbo-12bit", + include: ["test/**/*.test.js"], + benchmark: { + include: ["bench/**/*.bench.{js,mjs}"], + }, + testTimeout: 30000, + }, +}) diff --git a/packages/libjpeg-turbo-8bit/src/JPEGDecoder.hpp b/packages/libjpeg-turbo-8bit/src/JPEGDecoder.hpp index b3a92af4..b2280232 100644 --- a/packages/libjpeg-turbo-8bit/src/JPEGDecoder.hpp +++ b/packages/libjpeg-turbo-8bit/src/JPEGDecoder.hpp @@ -3,7 +3,9 @@ #pragma once +#include #include +#include #include #include @@ -16,6 +18,22 @@ thread_local const emscripten::val Uint8ClampedArray = emscripten::val::global(" #include "FrameInfo.hpp" +/// +/// Computes width * height * components * bytesPerPixel while guarding +/// against 32-bit size_t overflow on the wasm32 target and rejecting +/// unreasonably large decoded buffer sizes. +/// +static inline size_t checkedDecodedSize(uint64_t width, uint64_t height, uint64_t components, uint64_t bytesPerPixel) { + const uint64_t kMaxBytes = 512ull * 1024ull * 1024ull; // 512 MiB + uint64_t total = width * height; + total *= components; + total *= bytesPerPixel; + if (total == 0 || total > kMaxBytes) { + throw std::runtime_error("decoded frame size out of range"); + } + return static_cast(total); +} + /// /// JavaScript API for decoding JPEG bistreams with libjpeg-turbo /// @@ -102,24 +120,30 @@ class JPEGDecoder { if ((tjInstance = tjInitDecompress()) == NULL) { throw("initializing decompressor\n"); } - + + // tjInstance has no destructor, and two things below leave this function + // without reaching a tjDestroy call: checkedDecodedSize throws for a + // malformed header, and decoded_.resize() can throw std::bad_alloc for a + // large frame. Both leaked the decompressor. Tying it to the scope covers + // those and the explicit paths alike. + struct HandleGuard { + tjhandle& handle; + ~HandleGuard() { if (handle) tjDestroy(handle); } + } guard{tjInstance}; + if(readHeader_i(tjInstance)) { - tjDestroy(tjInstance); throw("error reading header\n"); } int pixelFormat = (frameInfo_.componentCount == 1) ? TJPF_GRAY : TJPF_RGB; - const size_t destinationSize = frameInfo_.width * frameInfo_.height * tjPixelSize[pixelFormat]; + const size_t destinationSize = checkedDecodedSize(frameInfo_.width, frameInfo_.height, 1, tjPixelSize[pixelFormat]); decoded_.resize(destinationSize); - if (tjDecompress2(tjInstance, encoded_.data(), encoded_.size(), decoded_.data(), + if (tjDecompress2(tjInstance, encoded_.data(), encoded_.size(), decoded_.data(), frameInfo_.width, 0, frameInfo_.height, pixelFormat, 0) < 0) { - tjDestroy(tjInstance); throw("~~decompressing JPEG image\n"); } - - tjDestroy(tjInstance); } /// diff --git a/packages/libjpeg-turbo-8bit/src/JPEGEncoder.hpp b/packages/libjpeg-turbo-8bit/src/JPEGEncoder.hpp index 251ca12e..922de36a 100644 --- a/packages/libjpeg-turbo-8bit/src/JPEGEncoder.hpp +++ b/packages/libjpeg-turbo-8bit/src/JPEGEncoder.hpp @@ -117,6 +117,15 @@ class JPEGEncoder { throw("initializing compressor"); } + // The tjCompress2 failure below threw without destroying the compressor, + // so every failed encode leaked one; encoded_.resize() after it can throw + // std::bad_alloc for the same result. tjInstance has no destructor of its + // own, so tie it to the scope. + struct HandleGuard { + tjhandle& handle; + ~HandleGuard() { if (handle) tjDestroy(handle); } + } guard{tjInstance}; + int pixelFormat = frameInfo_.componentCount == 1 ? TJPF_GRAY : TJPF_RGB; int outSubsamp = frameInfo_.componentCount == 1 ? TJSAMP_GRAY : subSampling_; int flags = 0; @@ -137,8 +146,7 @@ class JPEGEncoder { } encoded_.resize(jpegSize); - - tjDestroy(tjInstance); tjInstance = NULL; + // HandleGuard destroys tjInstance as this scope unwinds. } private: diff --git a/packages/little-endian/src/index.js b/packages/little-endian/src/index.js index bf99e7cf..fd1b0898 100644 --- a/packages/little-endian/src/index.js +++ b/packages/little-endian/src/index.js @@ -1,12 +1,16 @@ /** * Decodes the provided pixelData and sets the `pixelData` property * of the imageFrame object to the decoded representation. - * - * + * + * 16-bit and 32-bit data become unsigned (`pixelRepresentation` 0) or + * signed (`pixelRepresentation` 1) integer arrays. 32-bit data with no + * `pixelRepresentation` is treated as float (e.g. FloatPixelData), + * matching cornerstone3D's decodeLittleEndian. + * * @param {object} imageFrame - * @param {number} imageFrame.bitsAllocated - 32 or 16 or 8 + * @param {number} imageFrame.bitsAllocated - 32, 16, 8 or 1 * @param {number} imageFrame.pixelRepresentation - 0 or 1 - * @param {*} pixelData + * @param {*} pixelData */ function decode(imageFrame, pixelData) { let arrayBuffer = pixelData.buffer; @@ -16,9 +20,16 @@ function decode(imageFrame, pixelData) { if (imageFrame.bitsAllocated === 16) { // if pixel data is not aligned on even boundary, shift it so we can create the 16 bit array - // buffers on it + // buffers on it. + // + // The end bound is not optional. slice(offset) copies through the end of + // the BACKING buffer, and pixelData is typically a single frame's view into + // a whole multi-frame P10 buffer — so the one-argument form allocates and + // copies the entire rest of the file to realign one frame (measured: 67 MB + // for a 1 MB frame in a 64 MB buffer). The returned view's length hides it, + // because it is correct either way. if (offset % 2) { - arrayBuffer = arrayBuffer.slice(offset); + arrayBuffer = arrayBuffer.slice(offset, offset + pixelData.byteLength); offset = 0; } @@ -28,15 +39,31 @@ function decode(imageFrame, pixelData) { imageFrame.pixelData = new Int16Array(arrayBuffer, offset, length / 2); } } else if (imageFrame.bitsAllocated === 8 || imageFrame.bitsAllocated === 1) { + // 1-bit data must already be extracted per frame by the caller: + // multi-frame 1-bit pixel data is bit-packed across frame boundaries, + // so frame extraction cannot happen at this level imageFrame.pixelData = pixelData; } else if (imageFrame.bitsAllocated === 32) { - // if pixel data is not aligned on even boundary, shift it - if (offset % 2) { - arrayBuffer = arrayBuffer.slice(offset); + // pixelData is typically a view into the full DICOM P10 buffer, so its + // byteOffset is even (DICOM guarantees even lengths) but not necessarily + // 4-byte aligned; 32-bit typed-array views require 4-byte alignment, + // so copy the bytes to a fresh, aligned buffer when needed — bounded to + // this frame, for the reason given on the 16-bit branch above + if (offset % 4) { + arrayBuffer = arrayBuffer.slice(offset, offset + pixelData.byteLength); offset = 0; } - imageFrame.pixelData = new Float32Array(arrayBuffer, offset, length / 4); + // 32-bit PixelData is integer data (signed per pixelRepresentation); + // it is only float when pixelRepresentation is absent (e.g. the + // FloatPixelData element), matching cornerstone3D's decodeLittleEndian + if (imageFrame.pixelRepresentation === 0) { + imageFrame.pixelData = new Uint32Array(arrayBuffer, offset, length / 4); + } else if (imageFrame.pixelRepresentation === 1) { + imageFrame.pixelData = new Int32Array(arrayBuffer, offset, length / 4); + } else { + imageFrame.pixelData = new Float32Array(arrayBuffer, offset, length / 4); + } } return imageFrame; diff --git a/packages/little-endian/test/decode.test.js b/packages/little-endian/test/decode.test.js index 6e6b2d5c..e83a58bf 100644 --- a/packages/little-endian/test/decode.test.js +++ b/packages/little-endian/test/decode.test.js @@ -41,7 +41,29 @@ describe("little-endian decode", () => { expect(imageFrame.pixelData).toBe(pixelData) }) - it("decodes 32-bit pixel data into Float32Array", () => { + it("decodes 32-bit unsigned pixel data into Uint32Array", () => { + const source = new Uint32Array([1, 2, 0xffffffff]) + const pixelData = new Uint8Array(source.buffer) + const imageFrame = { bitsAllocated: 32, pixelRepresentation: 0 } + + decode(imageFrame, pixelData) + + expect(imageFrame.pixelData).toBeInstanceOf(Uint32Array) + expect(Array.from(imageFrame.pixelData)).toEqual([1, 2, 0xffffffff]) + }) + + it("decodes 32-bit signed pixel data into Int32Array", () => { + const source = new Int32Array([-1, 2, -100000]) + const pixelData = new Uint8Array(source.buffer) + const imageFrame = { bitsAllocated: 32, pixelRepresentation: 1 } + + decode(imageFrame, pixelData) + + expect(imageFrame.pixelData).toBeInstanceOf(Int32Array) + expect(Array.from(imageFrame.pixelData)).toEqual([-1, 2, -100000]) + }) + + it("decodes 32-bit pixel data into Float32Array when pixelRepresentation is absent", () => { const source = new Float32Array([1.5, -2.25, 3.75]) const pixelData = new Uint8Array(source.buffer) const imageFrame = { bitsAllocated: 32 } @@ -64,6 +86,19 @@ describe("little-endian decode", () => { expect(Array.from(imageFrame.pixelData)).toEqual([1, 2]) }) + it("realigns 32-bit pixel data when byteOffset is 2 (even but not 4-aligned)", () => { + const source = new Float32Array([1.5, -2.25]) + const padded = new Uint8Array(2 + source.length * 4) + padded.set(new Uint8Array(source.buffer), 2) + const pixelData = new Uint8Array(padded.buffer, 2, source.length * 4) + const imageFrame = { bitsAllocated: 32 } + + decode(imageFrame, pixelData) + + expect(imageFrame.pixelData).toBeInstanceOf(Float32Array) + expect(Array.from(imageFrame.pixelData)).toEqual([1.5, -2.25]) + }) + it("returns the same imageFrame object", () => { const imageFrame = { bitsAllocated: 8 } const result = decode(imageFrame, new Uint8Array([0])) diff --git a/packages/openjpeg/src/BufferStream.hpp b/packages/openjpeg/src/BufferStream.hpp index cda2e71d..59116e4f 100644 --- a/packages/openjpeg/src/BufferStream.hpp +++ b/packages/openjpeg/src/BufferStream.hpp @@ -33,15 +33,17 @@ static OPJ_SIZE_T opj_write_to_buffer (void* p_buffer, OPJ_SIZE_T p_nb_bytes, opj_buffer_info_t* p_source_buffer) { - OPJ_BYTE* pbuf = p_source_buffer->buf; - OPJ_BYTE* pcur = p_source_buffer->cur; + OPJ_SIZE_T remaining = p_source_buffer->buf + p_source_buffer->len - p_source_buffer->cur; - OPJ_SIZE_T len = p_source_buffer->len; + if (remaining == 0) + return (OPJ_SIZE_T)-1; - memcpy (p_source_buffer->cur, p_buffer, p_nb_bytes); - p_source_buffer->cur += p_nb_bytes; + OPJ_SIZE_T n = p_nb_bytes > remaining ? remaining : p_nb_bytes; - return p_nb_bytes; + memcpy (p_source_buffer->cur, p_buffer, n); + p_source_buffer->cur += n; + + return n; } static OPJ_SIZE_T @@ -53,7 +55,7 @@ opj_skip_from_buffer (OPJ_SIZE_T len, opj_buffer_info_t* psrc) if (n > len) n = len; - psrc->cur += len; + psrc->cur += n; } else n = (OPJ_SIZE_T)-1; @@ -64,12 +66,15 @@ opj_skip_from_buffer (OPJ_SIZE_T len, opj_buffer_info_t* psrc) static OPJ_BOOL opj_seek_from_buffer (OPJ_OFF_T len, opj_buffer_info_t* psrc) { - OPJ_SIZE_T n = psrc->len; + if (len < 0) + return OPJ_FALSE; + + OPJ_SIZE_T off = (OPJ_SIZE_T)len; - if (n > len) - n = len; + if (off > psrc->len) + off = psrc->len; - psrc->cur = psrc->buf + n; + psrc->cur = psrc->buf + off; return OPJ_TRUE; } diff --git a/packages/openjpeg/src/FrameInfo.hpp b/packages/openjpeg/src/FrameInfo.hpp index 69ecad46..b231ca76 100644 --- a/packages/openjpeg/src/FrameInfo.hpp +++ b/packages/openjpeg/src/FrameInfo.hpp @@ -5,29 +5,36 @@ #include +/// Every field is zero-initialised on purpose. J2KDecoder holds one of these by +/// value and does not initialise it, so getFrameInfo() before a successful +/// decode used to read uninitialised memory -- and because the wasm allocator +/// reuses freed blocks, a fresh decoder frequently landed on the block a +/// previous one had just released and reported THAT frame's geometry as its +/// own. Reading zeros is obviously wrong to a caller; reading 512x512 from the +/// last image decoded is not, which is the worse failure of the two. struct FrameInfo { /// /// Width of the image, range [1, 65535]. /// - uint16_t width; + uint16_t width = 0; /// /// Height of the image, range [1, 65535]. /// - uint16_t height; + uint16_t height = 0; /// /// Number of bits per sample, range [2, 16] /// - uint8_t bitsPerSample; + uint8_t bitsPerSample = 0; /// /// Number of components contained in the frame, range [1, 255] /// - uint8_t componentCount; + uint8_t componentCount = 0; /// /// true if signed, false if unsigned /// - bool isSigned; + bool isSigned = false; }; \ No newline at end of file diff --git a/packages/openjpeg/src/J2KDecoder.hpp b/packages/openjpeg/src/J2KDecoder.hpp index a226d0f5..de9fcb58 100644 --- a/packages/openjpeg/src/J2KDecoder.hpp +++ b/packages/openjpeg/src/J2KDecoder.hpp @@ -6,6 +6,8 @@ #include #include #include +#include +#include #include "openjpeg.h" #include "format_defs.h" @@ -15,6 +17,21 @@ #define EMSCRIPTEN_API __attribute__((used)) #define J2K_MAGIC_NUMBER 0x51FF4FFF +/// +/// Computes width * height * components * bytesPerPixel with overflow +/// checking, throwing if the result is zero or exceeds a sane upper bound. +/// +static inline size_t checkedDecodedSize(uint64_t width, uint64_t height, uint64_t components, uint64_t bytesPerPixel) { + const uint64_t kMaxBytes = 512ull * 1024ull * 1024ull; // 512 MiB + uint64_t total = width * height; + total *= components; + total *= bytesPerPixel; + if (total == 0 || total > kMaxBytes) { + throw std::runtime_error("decoded frame size out of range"); + } + return static_cast(total); +} + #ifdef __EMSCRIPTEN__ #include @@ -743,11 +760,54 @@ class J2KDecoder { opj_codec_t* l_codec = NULL; opj_image_t* image = NULL; opj_stream_t *l_stream = NULL; + opj_codestream_info_v2_t* cstr_info = NULL; + // Declared here, ahead of the guard, on purpose: l_stream holds a pointer + // to this struct, so the guard that destroys the stream must not outlive + // it. Locals are destroyed in reverse declaration order, so anything the + // guard touches has to be declared before the guard. + opj_buffer_info_t buffer_info; + + // Tie the four native handles to this scope. They are raw pointers with + // no destructors, and freeing them used to be hand-rolled at each early + // return -- so every path that leaves by THROWING rather than returning + // leaked all of them: the component-count rejection below, the + // checkedDecodedSize range check, and the std::bad_alloc that + // decoded_.resize() can raise for a large frame. cstr_info was worse + // still, leaking on every decode including the successful one, because + // nothing freed it on any path at all. + // + // A destructor rather than a cleanup() lambda called at each exit, + // because resize()'s throw has no call site to attach a cleanup to, and + // because it cannot be forgotten when a new early return is added. The + // members are references to the locals above, so the existing names stay + // as they are and the guard always sees the current values. + // + // The cstr_info null test is load-bearing, not defensive style: + // opj_destroy_cstr_info() tests its argument pointer but then + // dereferences *cstr_info unguarded, so handing it a pointer to a NULL + // cstr_info would crash. opj_get_cstr_info() does return NULL -- for a + // NULL codec, and for a codec that is not a decompressor. + struct HandleGuard { + opj_stream_t*& stream; + opj_codec_t*& codec; + opj_image_t*& image; + opj_codestream_info_v2_t*& cstrInfo; + + ~HandleGuard() { + if (cstrInfo) opj_destroy_cstr_info(&cstrInfo); + if (stream) opj_stream_destroy(stream); + if (codec) opj_destroy_codec(codec); + if (image) opj_image_destroy(image); + } + } guard{l_stream, l_codec, image, cstr_info}; // detect stream type // NOTE: DICOM only supports OPJ_CODEC_J2K, but not everyone follows this // and some DICOM images will have JP2 encoded bitstreams // http://dicom.nema.org/medical/dicom/2017e/output/chtml/part05/sect_A.4.4.html + if (encoded_.size() < 4) { + throw std::runtime_error("encoded J2K buffer too small"); + } if( ((OPJ_INT32*)encoded_.data())[0] == J2K_MAGIC_NUMBER ){ l_codec = opj_create_decompress(OPJ_CODEC_J2K); }else{ @@ -764,7 +824,6 @@ class J2KDecoder { parameters.cp_layer = decodeLayer_; //opj_set_decoded_resolution_factor(l_codec, 1); // set stream - opj_buffer_info_t buffer_info; buffer_info.buf = encoded_.data(); buffer_info.cur = encoded_.data(); buffer_info.len = encoded_.size(); @@ -773,26 +832,18 @@ class J2KDecoder { /* Setup the decoder decoding parameters using user parameters */ if ( !opj_setup_decoder(l_codec, ¶meters) ){ printf("[ERROR] opj_decompress: failed to setup the decoder\n"); - opj_stream_destroy(l_stream); - opj_destroy_codec(l_codec); return; } /* Read the main header of the codestream and if necessary the JP2 boxes*/ if(! opj_read_header(l_stream, l_codec, &image)){ printf("[ERROR] opj_decompress: failed to read the header\n"); - opj_stream_destroy(l_stream); - opj_destroy_codec(l_codec); - opj_image_destroy(image); return; } /* decode the image */ if (!opj_decode(l_codec, l_stream, image)) { printf("[ERROR] opj_decompress: failed to decode tile!\n"); - opj_destroy_codec(l_codec); - opj_stream_destroy(l_stream); - opj_image_destroy(image); return; } @@ -815,6 +866,9 @@ class J2KDecoder { frameInfo_.width = image->x1; frameInfo_.height = image->y1; frameInfo_.componentCount = image->numcomps; + if (frameInfo_.componentCount != 1 && frameInfo_.componentCount != 3) { + throw std::runtime_error("unsupported J2K component count"); + } frameInfo_.isSigned = image->comps[0].sgnd; frameInfo_.bitsPerSample = image->comps[0].prec; @@ -823,7 +877,15 @@ class J2KDecoder { imageOffset_.y = image->y0; //image->comps[0].factor always 0?? - opj_codestream_info_v2_t* cstr_info = opj_get_cstr_info(l_codec); /* Codestream information structure */ + cstr_info = opj_get_cstr_info(l_codec); /* Codestream information structure */ + // Unreachable for a codec that has just decoded successfully, but every + // field below is a dereference and tccp_info is optional even upstream + // (opj_destroy_cstr_info tests it before freeing). Throwing rather than + // reading on leaves no chance of reporting the PREVIOUS frame's + // codestream metadata under this frame's pixels. + if (!cstr_info || !cstr_info->m_default_tile_info.tccp_info) { + throw std::runtime_error("failed to read J2K codestream info"); + } numLayers_ = cstr_info->m_default_tile_info.numlayers; progressionOrder_ = cstr_info->m_default_tile_info.prg; isReversible_ = cstr_info->m_default_tile_info.tccp_info->qmfbid == 1; @@ -839,7 +901,7 @@ class J2KDecoder { // allocate destination buffer Size sizeAtDecompositionLevel = calculateSizeAtDecompositionLevel(decompositionLevel); const size_t bytesPerPixel = (frameInfo_.bitsPerSample + 8 - 1) / 8; - const size_t destinationSize = sizeAtDecompositionLevel.width * sizeAtDecompositionLevel.height * frameInfo_.componentCount * bytesPerPixel; + const size_t destinationSize = checkedDecodedSize(sizeAtDecompositionLevel.width, sizeAtDecompositionLevel.height, frameInfo_.componentCount, bytesPerPixel); decoded_.resize(destinationSize); // Convert from int32 to native size @@ -902,23 +964,26 @@ class J2KDecoder { } } - opj_stream_destroy(l_stream); - opj_destroy_codec(l_codec); - opj_image_destroy(image); + // No explicit frees here: HandleGuard above releases all four handles as + // this scope unwinds, on this path and on every earlier one. } std::vector encoded_; std::vector decoded_; FrameInfo frameInfo_; - size_t numDecompositions_; - bool isReversible_; - int progressionOrder_; + // Zero-initialised for the same reason as FrameInfo's fields: none of these + // were initialised, so every getter was an uninitialised read until a + // decode had succeeded, and on a reused heap block that read the previous + // decoder's values rather than anything recognisably empty. + size_t numDecompositions_ = 0; + bool isReversible_ = false; + int progressionOrder_ = 0; Point imageOffset_; Size tileSize_; Point tileOffset_; Size blockDimensions_; - int32_t numLayers_; - size_t colorSpace_; + int32_t numLayers_ = 0; + size_t colorSpace_ = 0; - size_t decodeLayer_; + size_t decodeLayer_ = 0; }; diff --git a/packages/openjpeg/src/J2KEncoder.hpp b/packages/openjpeg/src/J2KEncoder.hpp index edd3fe70..6cb9594b 100644 --- a/packages/openjpeg/src/J2KEncoder.hpp +++ b/packages/openjpeg/src/J2KEncoder.hpp @@ -5,6 +5,7 @@ #include #include +#include #include "openjpeg.h" @@ -225,9 +226,31 @@ class J2KEncoder { opj_stream_t *l_stream = 00; opj_codec_t* l_codec = 00; opj_image_t *image = NULL; - + // Declared here, ahead of the guard, on purpose: l_stream holds a pointer + // to this struct, so the guard that destroys the stream must not outlive + // it. Locals are destroyed in reverse declaration order, so anything the + // guard touches has to be declared before the guard. + opj_buffer_info_t buffer_info; + + // Same reasoning as J2KDecoder's HandleGuard: these are raw handles with + // no destructors, and std::vector::resize() below can throw std::bad_alloc + // with no call site to hang a cleanup() off. A destructor covers the + // throwing paths and the success path alike, so the explicit cleanup() + // calls this replaces cannot be forgotten on a newly added early exit. + struct HandleGuard { + opj_stream_t*& stream; + opj_codec_t*& codec; + opj_image_t*& image; + + ~HandleGuard() { + if (stream) opj_stream_destroy(stream); + if (codec) opj_destroy_codec(codec); + if (image) opj_image_destroy(image); + } + } guard{l_stream, l_codec, image}; + OPJ_COLOR_SPACE color_space = frameInfo_.componentCount > 1 ? OPJ_CLRSPC_SRGB : OPJ_CLRSPC_GRAY; - + std::vector cmptparm; cmptparm.resize(frameInfo_.componentCount); /* initialize image components */ @@ -241,6 +264,14 @@ class J2KEncoder { cmptparm[i].h = (OPJ_UINT32)frameInfo_.height; } image = opj_image_create((OPJ_UINT32)frameInfo_.componentCount, cmptparm.data(), color_space); + // Every line from here to the end of the function dereferences image, and + // opj_image_create returns NULL on a failed allocation or a component + // parameter it rejects. Unchecked, that was a null dereference in wasm + // (which traps the whole module) rather than a JS exception. + if (!image) { + encoded_.resize(0); + throw std::runtime_error("failed to encode image: opj_image_create"); + } /* set image offset and reference grid */ image->x0 = (OPJ_UINT32)imageOffset_.x; @@ -281,6 +312,13 @@ class J2KEncoder { // TODO: add support for JP2 encoding via config parameter l_codec = opj_create_compress(OPJ_CODEC_J2K); + // opj_setup_encoder would reject a NULL codec and be reported as a setup + // failure, which is survivable but misattributes the cause; say what + // actually went wrong instead. + if (!l_codec) { + encoded_.resize(0); + throw std::runtime_error("failed to encode image: opj_create_compress"); + } /* catch events using our callbacks and give a local context */ //opj_set_info_handler(l_codec, info_callback, 00); @@ -289,41 +327,51 @@ class J2KEncoder { // TODO: Add support for using tiles? + // encoded_ is emptied on every failure path below. A silent `return` here + // used to leave it at its full pre-sized allocation, so JS callers treated + // a failed encode as a successful one and read back garbage bytes; the + // handles themselves are now the guard's responsibility. if (! opj_setup_encoder(l_codec, ¶meters, image)) { - fprintf(stderr, "failed to encode image: opj_setup_encoder\n"); - opj_destroy_codec(l_codec); - opj_image_destroy(image); - return; // TODO: implement error handling + encoded_.resize(0); + throw std::runtime_error("failed to encode image: opj_setup_encoder"); } - // HACK: For now - make encoded buffer the same size as decoded so we can - // avoid messing with BufferStream malloc/free stuff - encoded_.resize(decoded_.size()); + // HACK: For now - make encoded buffer roughly the same size as decoded + // (plus headroom for worst-case expansion) so we can avoid messing with + // BufferStream malloc/free stuff. opj_write_to_buffer clamps writes to + // the buffer's remaining space, which is the hard safety net if this + // estimate is ever too small: the clamped write makes the compress call + // below return false, which now throws instead of returning silently. + encoded_.resize(decoded_.size() + (decoded_.size() / 2) + 1024); /* open a byte stream for writing and allocate memory for all tiles */ - opj_buffer_info_t buffer_info; buffer_info.buf = encoded_.data(); buffer_info.cur = encoded_.data(); buffer_info.len = encoded_.size(); l_stream = opj_stream_create_buffer_stream(&buffer_info, OPJ_FALSE); + if (!l_stream) { + encoded_.resize(0); + throw std::runtime_error("failed to encode image: could not create buffer stream"); + } /* encode the image */ if (!opj_start_compress(l_codec, image, l_stream)) { - fprintf(stderr, "failed to encode image: opj_start_compress\n"); - return; // todo: error handling + encoded_.resize(0); + throw std::runtime_error("failed to encode image: opj_start_compress (encoded buffer too small?)"); } if(!opj_encode(l_codec, l_stream)) { - fprintf(stderr, "failed to encode image: opj_encode\n"); - return; // todo: error handling + encoded_.resize(0); + throw std::runtime_error("failed to encode image: opj_encode (encoded buffer too small?)"); } if(!opj_end_compress(l_codec, l_stream)) { - fprintf(stderr, "failed to encode image: opj_end_compress\n"); - return; // todo: error handling + encoded_.resize(0); + throw std::runtime_error("failed to encode image: opj_end_compress (encoded buffer too small?)"); } encoded_.resize(buffer_info.cur - buffer_info.buf); + // HandleGuard frees the stream, codec and image as this scope unwinds. } private: diff --git a/packages/openjpeg/test/decode.test.js b/packages/openjpeg/test/decode.test.js index 80a4af88..fbb44b7a 100644 --- a/packages/openjpeg/test/decode.test.js +++ b/packages/openjpeg/test/decode.test.js @@ -107,6 +107,16 @@ describe.each(buildVariants)("openjpeg J2K decode robustness — $name", ({ path if (isBuilt) codec = await loadModule(path) }) + it.skipIf(!isBuilt)("throws when the encoded buffer is smaller than 4 bytes", () => { + const decoder = new codec.J2KDecoder() + const tooShort = new Uint8Array([0x00, 0x01, 0x02]) + decoder.getEncodedBuffer(tooShort.length).set(tooShort) + + expect(() => decoder.decode()).toThrow() + + decoder.delete() + }) + it.skipIf(!isBuilt)("does not crash the process on a malformed/garbage buffer", () => { const decoder = new codec.J2KDecoder() const garbage = new Uint8Array(64) @@ -140,6 +150,31 @@ describe.each(encoderVariants)( if (isBuilt) codec = await loadModule(path) }) + it.skipIf(!isBuilt)( + "throws when encoder setup fails instead of returning a garbage buffer", + () => { + const frameInfo = { + width: 512, + height: 512, + bitsPerSample: 16, + componentCount: 1, + isSigned: true, + } + const encoder = new codec.J2KEncoder() + encoder.getDecodedBuffer(frameInfo).set(ct1Raw) + // 40 decompositions -> numresolution 41, beyond OpenJPEG's maximum + // (33), so opj_setup_encoder fails. encode() used to swallow this + // and leave the full pre-sized allocation in the encoded buffer, + // which callers then read back as a "successful" encode. + encoder.setDecompositions(40) + + expect(() => encoder.encode()).toThrow() + expect(encoder.getEncodedBuffer().length).toBe(0) + + encoder.delete() + } + ) + it.skipIf(!isBuilt)( "encodes CT1.RAW losslessly and decodes back to original bytes", () => { diff --git a/packages/openjpeg/test/heap-stability.test.js b/packages/openjpeg/test/heap-stability.test.js index 7f3b6c26..4122039a 100644 --- a/packages/openjpeg/test/heap-stability.test.js +++ b/packages/openjpeg/test/heap-stability.test.js @@ -29,6 +29,7 @@ async function loadModule(path) { describe("openjpeg wasm heap stability", { timeout: 120000 }, () => { let codec const encoded = readFileSync(resolve(fixturesDir, "j2k/CT1.j2k")) + const raw = readFileSync(resolve(fixturesDir, "raw/CT1.RAW")) beforeAll(async () => { if (isBuilt) codec = await loadModule("../dist/openjpegwasm.js") @@ -47,6 +48,19 @@ describe("openjpeg wasm heap stability", { timeout: 120000 }, () => { expect(codec.HEAP8.length).toBe(settled) }) + it.skipIf(!isBuilt)("repeated encode/delete cycles do not grow the heap", () => { + const encodeOnce = () => { + const encoder = new codec.J2KEncoder() + encoder.getDecodedBuffer({ width: 512, height: 512, bitsPerSample: 16, componentCount: 1, isSigned: true }).set(raw) + encoder.encode() + encoder.delete() + } + for (let i = 0; i < 10; i++) encodeOnce() + const settled = codec.HEAP8.length + for (let i = 0; i < 60; i++) encodeOnce() + expect(codec.HEAP8.length).toBe(settled) + }) + it.skipIf(!isBuilt)("repeated failing decodes do not grow the heap", () => { // Garbage (not truncated-after-valid-header) input: fails fast at // header parse instead of spending seconds in sample recovery, so the @@ -68,4 +82,28 @@ describe("openjpeg wasm heap stability", { timeout: 120000 }, () => { for (let i = 0; i < 100; i++) failOnce() expect(codec.HEAP8.length).toBe(settled) }) + + // NOT COVERED, and deliberately so rather than by oversight: the encoder's + // and decoder's THROWING failure paths, which are exactly the ones the + // handle guards in J2KEncoder::encode and J2KDecoder::decode_i exist for. + // + // The obvious test -- loop a failing encode and assert no heap growth -- + // was written and removed. setDecompositions(40) is a clean trigger + // (numresolution 41 > OpenJPEG's 33, so opj_setup_encoder rejects it with + // the opj_image already allocated, ~1 MiB of leak per iteration), and the + // heap does grow without the guard. But repeating that failure crashes the + // module on the 5th iteration WITH the guard in place too -- "memory access + // out of bounds" -- so the test failed for a reason unrelated to what it + // was measuring. Repeated failed encoder setup corrupts something; that is + // its own bug, not this file's to paper over. + // + // The decoder side has no reachable trigger from the fixtures here at all: + // the component-count rejection needs a 2- or 4-component J2K (none + // committed, and the encoder cannot produce one -- see the multi-component + // findings), checkedDecodedSize needs a header claiming >512 MiB, and + // decoded_.resize()'s std::bad_alloc needs memory pressure. The cstr_info + // leak, which fired on EVERY decode, is invisible here for a different + // reason: measured at ~12,600 decodes it never forced heap growth either + // way, because the 50 MiB arena absorbs it. HEAP8.length is simply not a + // sensitive enough instrument for a leak that small. }) diff --git a/packages/openjphjs/src/HTJ2KDecoder.hpp b/packages/openjphjs/src/HTJ2KDecoder.hpp index 08332c43..8c342458 100644 --- a/packages/openjphjs/src/HTJ2KDecoder.hpp +++ b/packages/openjphjs/src/HTJ2KDecoder.hpp @@ -3,6 +3,7 @@ #pragma once +#include #include #include #include @@ -23,6 +24,22 @@ #include "Point.hpp" #include "Size.hpp" +/// +/// Computes width * height * components * bytesPerPixel while guarding +/// against 32-bit size_t overflow on the wasm32 target and rejecting +/// unreasonably large decoded buffer sizes. +/// +static inline size_t checkedDecodedSize(uint64_t width, uint64_t height, uint64_t components, uint64_t bytesPerPixel) { + const uint64_t kMaxBytes = 512ull * 1024ull * 1024ull; // 512 MiB + uint64_t total = width * height; + total *= components; + total *= bytesPerPixel; + if (total == 0 || total > kMaxBytes) { + throw std::runtime_error("decoded frame size out of range"); + } + return static_cast(total); +} + /// /// JavaScript API for decoding HTJ2K bistreams with OpenJPH /// diff --git a/packages/openjphjs/src/HTJ2KEncoder.hpp b/packages/openjphjs/src/HTJ2KEncoder.hpp index e4bb3a95..24a64350 100644 --- a/packages/openjphjs/src/HTJ2KEncoder.hpp +++ b/packages/openjphjs/src/HTJ2KEncoder.hpp @@ -268,7 +268,11 @@ class HTJ2KEncoder codestream.write_headers(&encoded_); // Encode the image - const size_t bytesPerPixel = frameInfo_.bitsPerSample / 8; + // Round UP like getDecodedBuffer() does (line 52): bitsPerSample / 8 + // truncates to 1 for 9..15-bit samples, halving the row stride so every + // row after the first was read from the wrong offset (found by the + // 12-bit round-trip test; 8- and 16-bit were unaffected). + const size_t bytesPerPixel = (frameInfo_.bitsPerSample + 8 - 1) / 8; ojph::ui32 next_comp; ojph::line_buf *cur_line = codestream.exchange(NULL, next_comp); siz = codestream.access_siz(); diff --git a/packages/openjphjs/test/matrix.test.js b/packages/openjphjs/test/matrix.test.js index 1ade60cf..0746baaa 100644 --- a/packages/openjphjs/test/matrix.test.js +++ b/packages/openjphjs/test/matrix.test.js @@ -2,7 +2,7 @@ import { beforeAll, describe, expect, it } from "vitest" import { existsSync, readFileSync } from "node:fs" import { fileURLToPath } from "node:url" import { dirname, resolve } from "node:path" -import { gray8FromCT2, gray12FromCT2 } from "../../../tools/fixture-verification/gen/derive.mjs" +import { gray8FromCT2, gray12FromCT2, bilevelFromCT2 } from "../../../tools/fixture-verification/gen/derive.mjs" const __dirname = dirname(fileURLToPath(import.meta.url)) const distDir = resolve(__dirname, "../dist") @@ -23,11 +23,13 @@ const asBuffer = (ta) => Buffer.from(ta.buffer, ta.byteOffset, ta.byteLength) // Fixture provenance (tools/fixture-verification/gen/generate-fixtures.mjs): // all four are lossless HTJ2K encodes of committed sources (US1.RAW RGB // frame; deterministic CT2.RAW transforms from derive.mjs), so each test's -// reference is re-derived from the source — a decoder regression on these -// paths breaks byte equality. +// reference is re-derived from the source — a decoder OR encoder regression +// on these paths breaks byte equality. // // The color pair covers both isUsingColorTransform settings — the RCT path -// was flagged "not been tested yet" in HTJ2KDecoder.hpp. +// was flagged "not been tested yet" in HTJ2KDecoder.hpp. The 12-bit case +// pins the row-stride fix in HTJ2KEncoder.hpp (bitsPerSample/8 truncated to +// 1 for 9..15-bit samples, corrupting every row after the first). describe("openjphjs HTJ2K decode matrix — color and bit depths", () => { let codec const us1 = readFileSync(resolve(__dirname, "../../openjpeg/test/fixtures/raw/US1.RAW")) @@ -71,9 +73,97 @@ describe("openjphjs HTJ2K decode matrix — color and bit depths", () => { expect(out.equals(asBuffer(gray8FromCT2(ct2)))).toBe(true) }) - it.skipIf(!isBuilt)("decodes 12-bit grayscale losslessly", () => { + it.skipIf(!isBuilt)("decodes 12-bit grayscale losslessly (encoder stride regression case)", () => { const { frameInfo, out } = decode("CT2-gray12.j2c") expect(frameInfo.bitsPerSample).toBe(12) expect(out.equals(asBuffer(gray12FromCT2(ct2)))).toBe(true) }) + + it.skipIf(!isBuilt)("round-trips 12-bit through the encoder (pins the stride fix)", () => { + // Re-encode the 12-bit source in-process: proves the ENCODER writes + // every row from the right offset, independent of the committed fixture. + const src = gray12FromCT2(ct2) + const encoder = new codec.HTJ2KEncoder() + encoder + .getDecodedBuffer({ width: 512, height: 512, bitsPerSample: 12, componentCount: 1, isSigned: false, isUsingColorTransform: false }) + .set(new Uint8Array(src.buffer, 0, src.byteLength)) + encoder.encode() + const encoded = Buffer.from(encoder.getEncodedBuffer()) + encoder.delete() + + const decoder = new codec.HTJ2KDecoder() + decoder.getEncodedBuffer(encoded.length).set(encoded) + decoder.decode() + const out = Buffer.from(decoder.getDecodedBuffer()) + decoder.delete() + + expect(out.equals(asBuffer(src))).toBe(true) + }) + + // 1 bit is the other end of the same stride fix, and the end that was fully + // broken rather than partly: bitsPerSample / 8 is 0 for a 1-bit sample, so + // the encode loop read every row from offset 0 and the codestream held row 0 + // repeated `height` times. getDecodedBuffer already used the round-up form, + // so the buffer the caller filled was the right size all along — only the + // reader of it disagreed. Both the round-trip below and the row-0 assertion + // fail against the old expression. + it.skipIf(!isBuilt)("round-trips 1-bit through the encoder (pins the stride fix at 1 bit)", () => { + const src = bilevelFromCT2(ct2) + const encoder = new codec.HTJ2KEncoder() + const frameInfo = { width: 512, height: 512, bitsPerSample: 1, componentCount: 1, isSigned: false, isUsingColorTransform: false } + const input = encoder.getDecodedBuffer(frameInfo) + // One byte per sample: (1 + 7) / 8 == 1, the same as 8-bit. + expect(input.length).toBe(512 * 512) + input.set(src) + encoder.encode() + const encoded = Buffer.from(encoder.getEncodedBuffer()) + encoder.delete() + + const decoder = new codec.HTJ2KDecoder() + decoder.getEncodedBuffer(encoded.length).set(encoded) + decoder.decode() + const decoded = decoder.getFrameInfo() + const out = Buffer.from(decoder.getDecodedBuffer()) + expect(decoder.getLastErrorMessage()).toBe("") + decoder.delete() + + expect(decoded.bitsPerSample).toBe(1) + expect(decoded.width).toBe(512) + expect(decoded.height).toBe(512) + expect(out.length).toBe(512 * 512) + expect(out.equals(asBuffer(src))).toBe(true) + + // Named the failure mode explicitly, so a regression reads as "rows + // collapsed" rather than a bare buffer mismatch. Row 0 of a CT slice is + // background and so are the rows next to it, so compare against a row + // through the anatomy; assert the source rows really do differ rather than + // trusting the derivation. + const row = (buf, y) => buf.subarray(y * 512, (y + 1) * 512) + expect(row(asBuffer(src), 256).equals(row(asBuffer(src), 0))).toBe(false) + expect(row(out, 256).equals(row(out, 0))).toBe(false) + }) + + // Sub-byte depths other than 1 take the same <= 8 branch, and 2..7 all + // truncated to a stride of 0 the same way. 4-bit is the cheap check that the + // fix is the whole round-up and not a 1-bit special case. + it.skipIf(!isBuilt)("round-trips 4-bit through the encoder", () => { + const gray8 = gray8FromCT2(ct2) + const src = new Uint8Array(gray8.length) + for (let i = 0; i < gray8.length; i++) src[i] = gray8[i] >> 4 + const encoder = new codec.HTJ2KEncoder() + encoder + .getDecodedBuffer({ width: 512, height: 512, bitsPerSample: 4, componentCount: 1, isSigned: false, isUsingColorTransform: false }) + .set(src) + encoder.encode() + const encoded = Buffer.from(encoder.getEncodedBuffer()) + encoder.delete() + + const decoder = new codec.HTJ2KDecoder() + decoder.getEncodedBuffer(encoded.length).set(encoded) + decoder.decode() + const out = Buffer.from(decoder.getDecodedBuffer()) + decoder.delete() + + expect(out.equals(asBuffer(src))).toBe(true) + }) }) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index e88f90f8..27e2fc7b 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -67,6 +67,9 @@ importers: '@cornerstonejs/codec-charls': specifier: ^1.2.6 version: link:../charls + '@cornerstonejs/codec-libjpeg-turbo-12bit': + specifier: ^0.4.4 + version: link:../libjpeg-turbo-12bit '@cornerstonejs/codec-libjpeg-turbo-8bit': specifier: ^1.2.5 version: link:../libjpeg-turbo-8bit diff --git a/tools/dist-size/baseline.json b/tools/dist-size/baseline.json index d4dc7135..3661c665 100644 --- a/tools/dist-size/baseline.json +++ b/tools/dist-size/baseline.json @@ -1,8 +1,8 @@ { "big-endian": { "index.js": { - "raw": 1162, - "gzip": 609 + "raw": 1264, + "gzip": 622 } }, "charls": { @@ -33,42 +33,42 @@ }, "libjpeg-turbo-12bit": { "libjpegturbo12js.js": { - "raw": 2461618, - "gzip": 259496 + "raw": 2553272, + "gzip": 266948 }, "libjpegturbo12wasm.js": { "raw": 112153, "gzip": 28570 }, "libjpegturbo12wasm.wasm": { - "raw": 1954242, - "gzip": 688878 + "raw": 2238031, + "gzip": 760773 } }, "libjpeg-turbo-8bit": { "libjpegturbojs.js": { - "raw": 837711, - "gzip": 160006 + "raw": 838153, + "gzip": 160171 }, "libjpegturbojs_decode.js": { - "raw": 417854, - "gzip": 117868 + "raw": 418292, + "gzip": 118256 }, "libjpegturbowasm.js": { "raw": 57440, "gzip": 15274 }, "libjpegturbowasm.wasm": { - "raw": 448728, - "gzip": 96385 + "raw": 448955, + "gzip": 96494 }, "libjpegturbowasm_decode.js": { "raw": 56350, "gzip": 15191 }, "libjpegturbowasm_decode.wasm": { - "raw": 180287, - "gzip": 69946 + "raw": 180512, + "gzip": 70074 } }, "libjxl": { @@ -91,34 +91,34 @@ }, "little-endian": { "index.js": { - "raw": 877, - "gzip": 477 + "raw": 1002, + "gzip": 490 } }, "openjpeg": { "openjpegjs.js": { - "raw": 753931, - "gzip": 199072 + "raw": 755496, + "gzip": 199652 }, "openjpegjs_decode.js": { - "raw": 540905, - "gzip": 139321 + "raw": 541662, + "gzip": 139653 }, "openjpegwasm.js": { "raw": 57685, "gzip": 15161 }, "openjpegwasm.wasm": { - "raw": 367561, - "gzip": 127108 + "raw": 368497, + "gzip": 127454 }, "openjpegwasm_decode.js": { "raw": 55759, "gzip": 14970 }, "openjpegwasm_decode.wasm": { - "raw": 255484, - "gzip": 84026 + "raw": 255890, + "gzip": 84256 } }, "openjphjs": { @@ -127,8 +127,8 @@ "gzip": 14859 }, "openjphjs.wasm": { - "raw": 299791, - "gzip": 96272 + "raw": 299794, + "gzip": 96271 } } } diff --git a/tools/fixture-verification/gen/derive.mjs b/tools/fixture-verification/gen/derive.mjs index 8dee61c6..e05f2a82 100644 --- a/tools/fixture-verification/gen/derive.mjs +++ b/tools/fixture-verification/gen/derive.mjs @@ -33,6 +33,38 @@ export function gray16uFromCT2(ct2Buffer) { return out; } +/** + * CT2.RAW (int16le) -> bi-level Uint8Array, ONE BYTE PER SAMPLE, values 0/1: + * gray8FromCT2(v) >= 128. + * + * A threshold of the 8-bit derivation rather than random bits, so the result is + * an anatomical silhouette: long runs of a single value broken by an irregular, + * high-frequency boundary. That is what makes a 1-bit row-stride or bit-order + * mistake visible — uniform or periodic content survives both. + * + * One byte per sample is the layout the wasm codecs use for every depth up to + * 8; use packBitsLsbFirst() to get the DICOM bit-packed form. + */ +export function bilevelFromCT2(ct2Buffer) { + const gray8 = gray8FromCT2(ct2Buffer); + const out = new Uint8Array(gray8.length); + for (let i = 0; i < gray8.length; i++) out[i] = gray8[i] >= 128 ? 1 : 0; + return out; +} + +/** + * One byte per sample (0/1) -> DICOM bit-packed BitsAllocated=1 PixelData: + * the first sample occupies the least significant bit of the first byte + * (PS3.5 8.1.1). The inverse of dicom-codec's codecFactory.unpackBits. + */ +export function packBitsLsbFirst(samples) { + const out = new Uint8Array(Math.ceil(samples.length / 8)); + for (let i = 0; i < samples.length; i++) { + if (samples[i]) out[i >> 3] |= 1 << (i & 7); + } + return out; +} + /** interleaved RGBRGB... -> [RRR..., GGG..., BBB...] plane buffers */ export function deinterleavePlanes(buf, samples) { const frameSize = buf.length / samples;