-
Notifications
You must be signed in to change notification settings - Fork 6
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
Implement Zstandard streaming decompression #47
Conversation
Is this ready for review? |
@@ -49,7 +49,7 @@ cd ../../../ | |||
${OPTIMIZE} \ | |||
-I "$CODEC_DIR/lib" \ | |||
--closure 1 \ | |||
-fwasm-exceptions | |||
-fwasm-exceptions \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see you did use wasm exceptions!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this is listed as "legacy" now, but the alternative proposal isn't supported in any runtimme at all so I'm good with using for now: https://webassembly.org/features/
Almost. I got a few notes from Janelia Scientific Computing Code Review this morning, so I have a few changes to make this evening. I also think a little more work is needed to catch the exceptions properly. I'm slightly unclear which exception handling feature I'm using on that chart. |
According to Mozilla, all major browsers have supported this since 2021 or 2022. https://developer.mozilla.org/en-US/docs/WebAssembly/JavaScript_interface/Exception#see_also |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, just some minor comments! Thanks for looking in to the exceptions and the nice tests!
* Initial attempt at implementing Zstandard streaming decompression * Drop Node 16 due to exeptions support, bump setup-node to v4 * Add streaming decompression tests for streaming compression * Apply prettier * Add EXPORT_EXCEPTION_HANDLING_HELPERS * Code clean up * Fix typo in build.sh * Free decompression stream when throwing or exiting * Throw error if there is additional input data after decompressing frame * Run prettier on test/zstd.test.js * Update with recommendations from Janelia SciCompSoft code review * Update codecs/zstd/zstd_codec.cpp * Add changeset
Thanks! Released in v0.3.2 https://github.com/manzt/numcodecs.js/releases/tag/v0.3.2 |
The output of ZSTD_getFrameContentSize is now checked.
If ZSTD_CONTENTSIZE_UNKNOWN, then we try to stream decompress into a 1 MiB output buffer
that grows in
1 MiB128 KiB incrementsWe use
WebAssembly.Exception
to report errors.Fix #46