Skip to content

Go binding: NewContext now returns a clean context #537

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

Merged
merged 1 commit into from
Mar 5, 2023

Conversation

polarmoon
Copy link
Contributor

It makes more sense to return a clean context upon NewContext call

@ggerganov
Copy link
Member

Setting the no_context to true will make the decoder ignore past transcription text.
I think for default settings, it's better to be false.

@polarmoon
Copy link
Contributor Author

polarmoon commented Feb 27, 2023

@ggerganov in what case the lib user wants to create a new Golang context but keep the past transcription text? Why not just use the existing Go context?

On the other hand, I believe it makes more sense that NewContext returns a truly new context, rather than a context remembers past transcription.

@ggerganov
Copy link
Member

ggerganov commented Feb 28, 2023

The meaning of this parameter is different:
The processing of the audio is done using a sliding window of chunk size of 30 seconds. For each new chunk we have the option to provide the past text as input for the decoder, or to ignore it.
When no_context == false it tells the whisper instance to provide it.

When you create a new context, it will always have empty past text, because no transcription has happened yet.

Edit: ignore this, it's wrong

@polarmoon
Copy link
Contributor Author

@ggerganov I made the change following our convo here. I've verified that w/o this change, Go context returned by NewContext may mess up the transcription (likely because it carries over previous context)
#312 (comment)

@ggerganov ggerganov merged commit 5e94129 into ggml-org:master Mar 5, 2023
@ggerganov
Copy link
Member

@polarmoon
I apologise - somehow I was confused about what no_context does.
You are correct - it is better to set it to true by default. I will also make it to be the default value in whisper.cpp as well.
Thanks 👍

anandijain pushed a commit to anandijain/whisper.cpp that referenced this pull request Apr 28, 2023
Co-authored-by: Ming <ming@localhost>
anandijain pushed a commit to anandijain/whisper.cpp that referenced this pull request Apr 28, 2023
jacobwu-b pushed a commit to jacobwu-b/Transcriptify-by-whisper.cpp that referenced this pull request Oct 24, 2023
Co-authored-by: Ming <ming@localhost>
jacobwu-b pushed a commit to jacobwu-b/Transcriptify-by-whisper.cpp that referenced this pull request Oct 24, 2023
jacobwu-b pushed a commit to jacobwu-b/Transcriptify-by-whisper.cpp that referenced this pull request Oct 24, 2023
Co-authored-by: Ming <ming@localhost>
jacobwu-b pushed a commit to jacobwu-b/Transcriptify-by-whisper.cpp that referenced this pull request Oct 24, 2023
landtanin pushed a commit to landtanin/whisper.cpp that referenced this pull request Dec 16, 2023
Co-authored-by: Ming <ming@localhost>
landtanin pushed a commit to landtanin/whisper.cpp that referenced this pull request Dec 16, 2023
iThalay pushed a commit to iThalay/whisper.cpp that referenced this pull request Sep 23, 2024
iThalay pushed a commit to iThalay/whisper.cpp that referenced this pull request Sep 23, 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