-
Notifications
You must be signed in to change notification settings - Fork 23
test: verify exact pixel output for every codec + fail CI on silently-skipped suites #72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 30 commits
3cfbdd9
fff5057
1b72d13
c293e39
eab8b32
0f609bd
ace5cf9
ffd155e
6943d76
3179998
c9dc902
6b04753
110fa1f
3280dcc
30d160a
80fbe1d
d93b80f
c20d46f
d03c65c
35e8074
1b00da1
c267e8c
67a8e5b
2e7b8d7
e8d29cb
8794920
d9ba7b9
6769ccc
9b968ea
6c46189
538e335
ff46061
472089f
a53a394
a8fbc9d
aaa5154
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,18 +2,28 @@ | |
| 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` | ||
| * | ||
| * otherwise it will be an `Int16Array`. 32-bit data is byte-swapped | ||
| * into a `Float32Array`, 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) { | ||
|
|
@@ -38,8 +48,25 @@ 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) { | ||
|
sedghi marked this conversation as resolved.
Outdated
|
||
| imageFrame.pixelData = pixelData; | ||
| } else if (imageFrame.bitsAllocated === 32) { | ||
|
sedghi marked this conversation as resolved.
Outdated
|
||
| let arrayBuffer = pixelData.buffer; | ||
|
|
||
| let offset = pixelData.byteOffset; | ||
| const length = pixelData.length; | ||
| // Float32Array views must be 4-byte aligned; shift unaligned data | ||
| if (offset % 4) { | ||
| arrayBuffer = arrayBuffer.slice(offset); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is probably an exception, not a just a shift since it isn't clear where the actual data is any longer. |
||
| offset = 0; | ||
| } | ||
|
|
||
| const swapView = new Uint32Array(arrayBuffer, offset, length / 4); | ||
| for (let i = 0; i < swapView.length; i++) { | ||
| swapView[i] = swap32(swapView[i]); | ||
| } | ||
|
|
||
| imageFrame.pixelData = new Float32Array(arrayBuffer, offset, length / 4); | ||
| } | ||
|
|
||
| return imageFrame; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,6 +43,43 @@ 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 pixel data into Float32Array", () => { | ||
| const source = new Float32Array([1.5, -2.25, 3.75]) | ||
| // 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.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", () => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is really problematic - it should throw since it is completely unclear what this means. |
||
| 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])) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.