fix: consolidated codec correctness fixes (supersedes #71)

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;

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]))

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",

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;