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

When DTW timestamps are enabled, defer new_segment_callback until after DTW compute step #2515

Merged

Conversation

jettoblack
Copy link
Contributor

I'm using the whisper.net wrapper for .NET which uses the new_segment_callback to get all results from whisper.cpp. I'm extending it to support enabling DTW timestamps which I find to be useful. However, new_segment_callback gets called on segments before DTW timestamps are computed, so I can't get the DTW timestamps this way.

This commit changes it so that when dtw_token_timestamps is true, new_segment_callback are deferred until after DTW timestamps are computed. This change has no effect except when dtw_token_timestamps is true AND new_segment_callback is not null.

Copy link
Owner

@ggerganov ggerganov left a comment

Choose a reason for hiding this comment

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

I wonder if it makes sense to move the new_segment_callback at the end of the decode loop, regardless if DTW is on or off. What do you think - do you see any problems with it?

@jettoblack
Copy link
Contributor Author

I wonder if it makes sense to move the new_segment_callback at the end of the decode loop, regardless if DTW is on or off. What do you think - do you see any problems with it?

I considered that but I wanted to limit the scope of the change just in case. It might increase latency when doing streaming transcription with a small window? I'm not sure if anyone would notice..

@ggerganov ggerganov merged commit 1626b73 into ggerganov:master Oct 29, 2024
45 checks passed
@ggerganov
Copy link
Owner

It might increase latency when doing streaming transcription with a small window?

Hm, yes. I guess it is fine as proposed. Thanks 👍

lyapple2008 pushed a commit to lyapple2008/whisper.cpp.mars that referenced this pull request Nov 2, 2024
adutilleul pushed a commit to adutilleul/whisper.cpp that referenced this pull request Nov 19, 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.

2 participants