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.

Sign up for GitHub

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

Jump to bottom

Fix OOM #30

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Fix OOM #30

wants to merge 7 commits into from

Conversation

WenqingZong
Copy link
Collaborator

@WenqingZong WenqingZong commented Oct 17, 2024 •
edited
Loading

Fix the OOM issue mentioned in #23

General Idea

Each time we find a SpeechEnd transition, remove the current active speech (and any silence before that) from internal buffer as they won't be used again for any future inferences.

Use a deleted_samples counter to make adjustment to any index calculations on internal buffer.

Also fixed a logic bug in main: When we find a SpeechEnd, we should reset self.speech_start_ms to None. This modification is missing in main.

Why the modified *.json files are correct

All modified part is current_speech_samples section. After we find a SpeechEnd but not yet find another SpeechStart, current speech samples should be 0 to reflect this fact.

@WenqingZong WenqingZong changed the title About to add delete part Fix OOM Oct 17, 2024
xd009642
xd009642 requested changes Oct 22, 2024
View reviewed changes
Copy link
Member

@xd009642 xd009642 left a comment

Choose a reason for hiding this comment

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

It all looks good so far, would be nice to open up some internal PRs using this branch just to test it more in context. A couple of small comments.

tests/snapshot.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blueridanus blueridanus Awaiting requested review from blueridanus

@xd009642 xd009642 Awaiting requested review from xd009642

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@WenqingZong @xd009642