image-js / iobuffer

Read and write binary data in ArrayBuffers
http://image-js.github.io/iobuffer/
MIT License
43 stars 6 forks source link

fix(readArray): account for endianness #62

Closed ghost closed 1 year ago

ghost commented 1 year ago

When the underlying AB bytes are transformed into a TypedArray it is done using the host Endianness and not the user defined endianness.

This would work well for LE stored data and LE user system, but not when they differ.

This PR tries to fix that case, and accounts for uint8 and int8 arrays being endianness-independent according to my understanding.

The IIFE function to determine endianness was taken from this SO post.

I added several tests that hopefully also spot any error in this logic.

Could be useful to plug this code into some parsers and check that nothing breaks. (I have done it in netcdfjs and wdf-parser, both use readArray with different endianness.)

Close #61

codecov[bot] commented 1 year ago

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage :thumbsup:

Coverage data is based on head (75be52a) compared to base (20ae4b6). Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #62 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 2 2 Lines 230 244 +14 Branches 22 26 +4 ========================================= + Hits 230 244 +14 ``` | [Impacted Files](https://codecov.io/gh/image-js/iobuffer/pull/62?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=image-js) | Coverage Δ | | |---|---|---| | [src/IOBuffer.ts](https://codecov.io/gh/image-js/iobuffer/pull/62/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=image-js#diff-c3JjL0lPQnVmZmVyLnRz) | `100.00% <100.00%> (ø)` | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=image-js). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=image-js)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

targos commented 1 year ago

I'd also change the title to "account for endianness". It's good to support big-endian hosts (rare), but IIUC this also fixes the code for big-endian data formats (more likely to happen)

targos commented 1 year ago

BTW I have access to an IBM AIX machine where I ran this to confirm the fix:

const isBE = (() => {
  const array = new Uint8Array(4);
  const view = new Uint32Array(array.buffer);
  return !((view[0] = 1) & array[0]);
})();

console.log(isBE);

const dataFromBE = new Uint8Array([2, 1, 3, 1]);

const uint16 = new Uint16Array(dataFromBE.buffer);

console.log(uint16[0], uint16[1]);

const slice = new Uint8Array(uint16.buffer.slice(0, 4));
slice.reverse();
const returnArray = new Uint16Array(slice.buffer);
returnArray.reverse();

console.log(returnArray);

Result:

true
513 769
Uint16Array(2) [ 258, 259 ]