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

Offline recognizer fixes #80

Merged

Conversation

vlovich
Copy link
Contributor

@vlovich vlovich commented Jan 21, 2025

  • Make WhisperConfig / Moonshine config clonable; saves the owner having to do their own wrapping to make it clonable.
  • Change transcribe in WhisperRecognizer and MoonshineRecognizer take a &[f32] slice since ownership isn't needed. This is a backwards-incompatible change - not sure if a problem or not. Maybe the method could be made generic to somehow accept either Vec or &[f32] (AsRef?)?
  • Construct timestamps and samples for both whisper & moonshine.

One problem I've encountered is I'm not sure how to get the whisper recognizer to actually emit the timestamps to check if they're correct - is something missing in the config / stream setup? I've verified that the tokens do get generated and are converted into a Vec correctly.

@thewh1teagle
Copy link
Owner

One problem I've encountered is I'm not sure how to get the whisper recognizer to actually emit the timestamps to check if they're correct

I think that sherpa-onnx doesn't support getting timestamps

Looks good!

Taking an owned Vec is unnecessary since the ownership of the samples
array doesn't extend past the function.
@vlovich vlovich force-pushed the vitali/whisper-recognizer-fixes branch from dcfe136 to 3192f63 Compare January 21, 2025 18:12
@vlovich
Copy link
Contributor Author

vlovich commented Jan 21, 2025

Merge conflicts resolved. Please feel free to merge.

@thewh1teagle
Copy link
Owner

The tests failed

Return timestamps and tokens if the underlying FFI result contains it.
@vlovich vlovich force-pushed the vitali/whisper-recognizer-fixes branch from 3192f63 to 46e0a22 Compare January 22, 2025 04:04
@vlovich
Copy link
Contributor Author

vlovich commented Jan 22, 2025

Should be fixed now.

@thewh1teagle
Copy link
Owner

Thanks for contribution!

@thewh1teagle thewh1teagle merged commit 2643f13 into thewh1teagle:main Jan 22, 2025
3 checks passed
@vlovich vlovich deleted the vitali/whisper-recognizer-fixes branch January 22, 2025 20:02
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.

2 participants