-
-
Notifications
You must be signed in to change notification settings - Fork 110
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
Update whisper.cpp
to 8e46ba8
#85
Conversation
Looking at recent changes to whisper.cpp, it feels like this should be updated sooner rather than later. IMO the biggest "feature" is actually that it appears there were important fixes to the log_mel_spectrogram generation in whisper.cpp which improves WER for the smaller models significantly (ggerganov/whisper.cpp#1148). |
I found a related issue yesterday and appears if everything goes well upstream should have a new release by end of week. See ggerganov/whisper.cpp#1233 (comment) |
Figured I'd point out something possibly concerning for those who want to use this branch: ggerganov/whisper.cpp#1220 (comment)
It may be worth doing some local testing before upgrading whisper-rs, just in case this affects you. |
OpenVINO support is broken: see ggerganov/whisper.cpp#1289 |
I think I'm gonna pull out the OpenVINO support into a separate PR (given it's completely borked right now), merge everything else, and then push a prerelease to crates.io since upstream seems to be taking a bit to merge stuff. |
That would be fantastic :) |
|
Attempted to give 0.9.0-rc.1 a whirl but ran into a cmake error: |
@cenzovit Do a |
Hmm, tried to update today and running into a new cmake error:
|
Only macOS machine I have is a late 2012 MBP running 10.15, but should be recent enough to support whisper.cpp's Metal. Will give it a shot on there. |
Should be fixed in |
Got further this time, now running into a clang error on compile:
This one may be an issue in whisper.cpp though? |
That does seem like a whisper.cpp error since all we do is pass it off to upstream. Unless there's another missing file I don't think it's something we can fix. |
Maybe related: ggerganov/whisper.cpp#1344 |
I wonder if other language bindings are seeing similar issues with the latest whisper.cpp |
Further notes, if I link the missing frameworks myself (which I think are suppose to be getting linked by ldflags in the whisper.cpp makefile) via config.toml or my build.rs -- for example in build.rs:
I can get my app to build. However, at runtime I run into a different issue as whisper goes to initialize:
Which seems to be occurring because ggml can't find the path to metallib: https://github.com/ggerganov/ggml/blob/6549d12f2e3176050040a86334f17c001e170f13/src/ggml-metal.m#L200C8-L200C8 At this point I am trying to figure out how to just build whisper.cpp without metal via the WHISPER_NO_METAL env variable; however, I can't seem to figure out how to have it set properly at the time whisper.cpp is being compiled. |
It should just be as simple as setting it for the cargo build command I would think. |
That's what I would have thought, but when I do that it still tries to initialize metal during runtime and fails with the same issue above |
If you have a local copy you could probably just edit it out of the Makefile as a temporary workaround, but we'd need to find something else for real use. |
Oh I was able to hack macOS 14 onto this system so I can give it a shot again and see as well. Will do that now. |
Is there a chance that when whisper-rs-sys runs cmake on whisper.cpp that there is a metal file that needs to be copied to out as well (similar to coreml etc)? Separately, would it be possible to make metal configurable via a feature similar to how coreml is configured? |
With macOS 14 I can indeed reproduce this issue. However I get some extra missing lib errors that it appears you didn't get Full error
|
This is what I want to do in the end to align Metal with all other acceleration layers. |
I think I was able to hack together a Metal patch, will make a new branch and PR for it, you can try it out from there. |
When you add the libs and get past the undefined symbols errors, do you run into the same runtime issue where it can't find the path to the metal file? |
Another maybe related issue (even though it is in llama.cpp)? ggerganov/llama.cpp#1912 |
Interestingly in: ggerganov/whisper.cpp#1270 there is an incomplete todo for "Can't figure out how to make the CMake build look for ggml-metal.metal in the bin folder" |
whisper.cpp upstream error
However with
|
After thinking about it, chances are my error's related to trying to use Metal on a legacy Metal GPU (i5-3210M). Probably can't help further with this, as it appears to not just be an isolated case (the Photos app crashed a few days ago due to an internal shader compilation error as well). |
See full diff at ggerganov/whisper.cpp@a5defbc...ggerganov:whisper.cpp:8e46ba80d3c1dcf532a0029f9bcdf99ce9ce7d40
Closes #70, #78, #80
Big features:
Will not be merged until we get a stable release to fix to.