node-jpeg-turbo
node-jpeg-turbo copied to clipboard
sorccu
cmake & latest node
This builds off the work of another fork (https://github.com/briskycat/node-tjpeg)
EDIT: This fork is published as @julusian/jpeg-turbo and will be updated until this PR is merged.
Changes:
- Update libjpeg-turbo to 2.0.2 (Thanks to briskycat)
- Replace node-gyp with cmake-js (libjpeg-turbo 2 needs cmake) (Thanks to briskycat)
- Add typescript definitions
- Update ci to use newer node versions
- Fix build issues with node 12
- Replace prebuilt-bindings with prebuild
- Fixed some segfaults
I have published a build of this as @julusian/jpeg-turbo v0.5.1, so feel free to use that package to do any testing. That is also populated with a set of prebuilds https://github.com/Julusian/node-jpeg-turbo/releases/tag/v0.5.1, so should be quick and easy to use.
CI is testing that this builds in each of node 8, 10 and 12 (except windows which is using 11 instead of 12). I have tested it myself on Windows 10 (node 10), macos (node 8) and ubuntu (node 8)
The fork tjpeg seemed unstable when tested. But might be that the api has changed?
I found a couple of segfaults in the node bindings, which could be the instability you found? In particular https://github.com/sorccu/node-jpeg-turbo/pull/13/commits/1640a1925cd3c8619a1b570387c7310f9bb053cb#diff-c3ffe5b9396eb0f8646374fdf35e0b3fL86 was consistently causing a crash if a preallocated buffer was supplied. And another crash if the source buffer was too short.
Could you do some testing with windows on node 9? If I don’t remeber totatally wrong, companion segfaulted when I tried to upgrade to tjpeg.
sure, I can do that. Ill write some unit tests too. It wont be hard to write some, there arent many api methods to do it for
@haakonnessjoen There are some unit tests in this now, and I have also done some manual tests for node 9 on windows, and it is working fine.
I intend to use this library myself with my StreamDeck XL, both through companion and my own stuff, so I will happily look into segfaults or bugs if any are found.
@sorccu https://github.com/sorccu/node-jpeg-turbo/issues/8#issuecomment-501674749 Let me know if you need anything from me to help you review this. The commits are pretty isolated to an area of changes per commit, so reviewing a commit at a time may make it easier.Or I could try and split it up into multiple isolated PRs? To verify usage is the same, perhaps it would help to run the tests I wrote against 0.4.2.
@Julusian OpenSTF maintainer here. Thanks for your work, sorry for no response for a long time and FYI this repository is not planned to be maintained as well as OpenSTF. However, we plan to replace node-jpeg-turbo with @julusian/jpeg-turbo in its successor - DeviceFarmer.
