image-js / iobuffer

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

Idea: read multiple bytes of a kind and return typed array #58

Closed ghost closed 1 year ago

ghost commented 1 year ago

What is solves

A user could type buffer.arrayOf("int16", 8) and get a typed array of int16s, moving also the pointer forward.

This will simplify some lines like new TypedArray(size) and looping over it explicitly in the code.


It may not be useful enough. If it is I will improve it.

targos commented 1 year ago

Can you show an example code that can be simplified with this API (before/after)?

ghost commented 1 year ago

Can you show an example code that can be simplified with this API (before/after)?

Yes ( I will have to update the PR anyways as there are some old commits, appended)

For example netcdfjs code ``` /** * Auxiliary function to read numeric data * @param size - Size of the element to read * @param bufferReader - Function to read next value * @return */ function readNumber( size: number, bufferReader: () => number, ): number | number[] { if (size !== 1) { let numbers = new Array(size); for (let i = 0; i < size; i++) { numbers[i] = bufferReader(); } return numbers; } else { return bufferReader(); } } /** * Given a type and a size reads the next element * @param buffer - Buffer for the file data * @param type - Type of the data to read * @param size - Size of the element to read * @return */ export function readType( buffer: IOBuffer, type: number, size: number, ): string | number | number[] { switch (type) { case types.BYTE: return Array.from(buffer.readBytes(size)); case types.CHAR: return trimNull(buffer.readChars(size)); case types.SHORT: return readNumber(size, buffer.readInt16.bind(buffer)); case types.INT: return readNumber(size, buffer.readInt32.bind(buffer)); case types.FLOAT: return readNumber(size, buffer.readFloat32.bind(buffer)); case types.DOUBLE: return readNumber(size, buffer.readFloat64.bind(buffer)); default: throw new Error(`non valid type ${type}`); } } ```

In this case there are strings as well, but it would be simplified to

buffer.arrayOf(size, type)
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 (768646a) compared to base (b5216a9). Patch coverage: 100.00% of modified lines in pull request are covered.

:exclamation: Current head 768646a differs from pull request most recent head a4a6cc9. Consider uploading reports for the commit a4a6cc9 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #58 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 2 2 Lines 225 230 +5 Branches 22 22 ========================================= + Hits 225 230 +5 ``` | [Impacted Files](https://codecov.io/gh/image-js/iobuffer/pull/58?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/58/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.

ghost commented 1 year ago

It slices copying the underlying AB now. Maybe I am just confused but for Uint8 seems faster than this.readBytes at least in simple tests locally.

targos commented 1 year ago

Maybe I am just confused but for Uint8 seems faster than this.readBytes at least in simple tests locally.

That makes sense to me, as readBytes copies the bytes one by one. Feel free to update its implementation so it calls this new method.

ghost commented 1 year ago

Thanks. Sorry for the first snippet. I think I was impressed by using bind for the first time :-(

Should we add Big Int arrays to the list as well @targos ? I have read them using loops and `readBigUint64 previously in some parsers.

targos commented 1 year ago

Sure, I don't see why we shouldn't.

ghost commented 1 year ago

Sure, I don't see why we shouldn't.

I added the items from the prev discussion. Uses InstanceType to give the user information about the returned TypedArray type.

May need another look to make sure everything is fine.

targos commented 1 year ago

The "lint" action is failing.

ghost commented 1 year ago

The "lint" action is failing.

Yes, this seems because the node types need to be updated. I was just unsure whether I should do it, we can update several dependencies.

targos commented 1 year ago

Ok, please try to update the dependencies. We can't merge this with red github actions.