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

hotfix[whsiperASR]: Remove word_timestamp from getTranscriptionWhisperASR #54

Closed
wants to merge 1 commit into from

Conversation

dahifi
Copy link
Contributor

@dahifi dahifi commented Apr 10, 2024

Not supported by WhisperASR. Fixes #52

I'm not actually sure whether the newer ASR versions support this, we can always add back with something that parses the toggle setting properly.

@dahifi
Copy link
Contributor Author

dahifi commented Apr 10, 2024

Needs more work, mainly a git blame between the last known good version 3.1.6: e019f0e

Copy link
Contributor

@bscholer bscholer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just removing this will break some other functionality introduced in #50, although this may be appropriate for a hotfix. I'll take a closer look this evening if I have time.

@bscholer
Copy link
Contributor

bscholer commented Apr 10, 2024

@dahifi did you test this? did it fix the issue?

After looking into it further, I don't think this change will fix the issue, and I believe it would only break other functionality. Even without word_timestamps, ASR_ENGINE=openai_whisper still responds in the way identified here (with field names in segments).

I recommend closing this PR.

@djmango
Copy link
Owner

djmango commented Apr 11, 2024

Closing in favor of #55

@djmango djmango closed this Apr 11, 2024
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.

Whisper ASR is broken
3 participants