Skip to content
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

Initial import of golang bindings #287

Merged
merged 6 commits into from Dec 20, 2022
Merged

Conversation

djthorpe
Copy link
Contributor

@djthorpe djthorpe commented Dec 18, 2022

I've added some bindings for golang here. They are pretty basic but I've tested on Darwin x64_86 and Debian arm64. There is one example (go-whisper) for the most basic case of using the bindings.

Linked to issue #269

@ggerganov
Copy link
Owner

This looks very well done - thank you for the contribution!

Regarding the need for the WHISPER_NO_AVX2=ON CMake option - maybe if you use make libwhisper.a instead of CMake it would be more portable. The Makefile has some logic to determine if AVX / AVX2 is available or not and add the necessary flags when possible.

I'll let this sit for a day before merging.
Also, hoping someone can help with adding a CI for this in the build.yml. Would make the bindings more resilient to future changes in the API. If not, we can merge and figure it out later.

@djthorpe
Copy link
Contributor Author

Thanks Georgi. On the AVX2 option, I'll take a look at using make and I can add the github actions for tests and building as well, but probably not going to get to them this week. I can send you some more pull requests for those changes later, and then begin to build out the full functionality and live streaming option in go examples. Thanks for your comments and continued efforts on this.

@jaybinks
Copy link
Contributor

@ggerganov any chance we can get this merged?
Its a bit hard for me to do any testing against this code before its merged, because of the way gomodules work with the module URL (which wont exist until you merge it)

@ggerganov ggerganov merged commit 231bebc into ggerganov:master Dec 20, 2022
@jaybinks
Copy link
Contributor

jaybinks commented Dec 20, 2022 via email

@djthorpe
Copy link
Contributor Author

Thanks. Yep, I'll fix that up and improve the documentation for it in the next few days:

  • Add a github action to make/test on PR merge
  • Use "make libwhisper.a" instead of cmake
  • Use LD_LIBRARY_PATH and INCLUDE_PATH to point to the compiled whisper library and include files

rock3125 pushed a commit to rock3125/whisper.cpp that referenced this pull request Feb 21, 2023
* Initial import of golang bindings

* Updated makefile rules

* Updated bindings

* Makefile update to add in more tests
anandijain pushed a commit to anandijain/whisper.cpp that referenced this pull request Apr 28, 2023
* Initial import of golang bindings

* Updated makefile rules

* Updated bindings

* Makefile update to add in more tests
jacobwu-b pushed a commit to jacobwu-b/Transcriptify-by-whisper.cpp that referenced this pull request Oct 24, 2023
* Initial import of golang bindings

* Updated makefile rules

* Updated bindings

* Makefile update to add in more tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants