Allow loading model from buffer in nodejs

coqui-ai / STT

🐸STT - The deep learning toolkit for Speech-to-Text. Training and deploying STT models has never been so easy.

https://coqui.ai

Mozilla Public License 2.0

2.27k stars 275 forks source link

Allow loading model from buffer in nodejs #2269

Closed dsouza95 closed 2 years ago

dsouza95 commented 2 years ago

This PR allows loading models from buffer for the nodejs bindings. Some important remarks:

I am not very familiar with SWIG, so it would be nice to have someone that is reviewing these changes.
I tried to keep aModelData as a string to avoid the multiple allowed types, but converting the string back to an UInt8Array would provide different results. Could we keep the UInt8Array as an allowed input type?

CLAassistant commented 2 years ago

All committers have signed the CLA.

reuben commented 2 years ago

Hm, I guess the CI is ignoring definitions.mk and just using the freshly built SWIG binaries :/

dsouza95 commented 2 years ago

With the swig version issue out of the way, I will mark this PR as ready for review :)

dsouza95 commented 2 years ago

Oops, I thought the pre-commit was running on my PC, but apparently not. Should be ok now :)

reuben commented 2 years ago

Looks like the Windows NodeJS package build is broken: https://github.com/coqui-ai/STT/runs/7580319577?check_suite_focus=true#step:12:2597

dsouza95 commented 2 years ago

Looks like the Windows NodeJS package build is broken: https://github.com/coqui-ai/STT/runs/7580319577?check_suite_focus=true#step:12:2597

It seems the linux one failed as well. I was able to reproduce this behaviour on my linux computer when using node v12. I believe older versions of node probably rely on the now deprecated GetContents API which was replaced by the GetBackingStore API causing this issue. Should be a simple case of testing if the newer API exists, I will update the PR shortly.

dsouza95 commented 2 years ago

I have added the check and it should compile with the appropriate API now (tested on node v12, v14, and v16).

dsouza95 commented 2 years ago

@reuben the Win build fail seems to be related to this: https://github.com/electron/electron/issues/29893 There seems to be a problem with the GetBackingStore API on Windows after all, but from the thread it seems to be possible to use node buffers instead, so I will try that. The macOS build fail seems to be related to void_t, which I believe is not available for c++11 on macOS, so I will try to replace that. I will make those changes and validate them on my linux, but I don't have a Win or macOS system available right now. Could we run only those steps on the CI when the changes are done?

dsouza95 commented 2 years ago

@reuben as it turns out, the existing buffer to short* typemap could be leveraged, which largely reduces the changes necessary. By using the existing typemap and the Buffer API which is more cross platform friendly, it should compile :)

reuben commented 2 years ago

Thanks for the PR!