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

Use silero v3.1 #142

Merged
merged 6 commits into from
Nov 30, 2023
Merged

Use silero v3.1 #142

merged 6 commits into from
Nov 30, 2023

Conversation

Jeronymous
Copy link
Member

This seems to improve VAD.
See problems spotted in #74

In the figure below:

  • left: VAD with silero-vad v4.0 (latest)
  • right: VAD with silero-vad v3.1
    image

Notes:

Comment on lines 1803 to 1807
if os.path.exists(repo_or_dir_master):
tmp_folder = repo_or_dir_master + ".tmp"
shutil.move(repo_or_dir_master, tmp_folder)
# Make a symlink to the v3.1 model, otherwise it fails
os.symlink(repo_or_dir_v31, repo_or_dir_master)
Copy link
Member Author

Choose a reason for hiding this comment

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

If I don't make a symbolic link snakers4_silero-vad_master -> snakers4_silero-vad_v3.1 in ~/.cache/torch/hub/, it does not work.
It fails with

  File "/home/jlouradour/.cache/torch/hub/snakers4_silero-vad_v3.1/utils_vad.py", line 16, in __init__
    self.session = onnxruntime.InferenceSession(path)
  File "/home/jlouradour/.local/lib/python3.10/site-packages/onnxruntime/capi/onnxruntime_inference_collection.py", line 360, in __init__
    self._create_inference_session(providers, provider_options, disabled_optimizers)
  File "/home/jlouradour/.local/lib/python3.10/site-packages/onnxruntime/capi/onnxruntime_inference_collection.py", line 397, in _create_inference_session
    sess = C.InferenceSession(session_options, self._model_path, True, self._read_config_from_model)
onnxruntime.capi.onnxruntime_pybind11_state.NoSuchFile: [ONNXRuntimeError] : 3 : NO_SUCHFILE : Load model from /home/jlouradour/.cache/torch/hub/snakers4_silero-vad_master/files/silero_vad.onnx failed:Load model /home/jlouradour/.cache/torch/hub/snakers4_silero-vad_master/files/silero_vad.onnx failed. File doesn't exist

I also don't want to rename "v3.1" into "master" so that both versions can co-exist.
So I temporarily move master to make the symbolic link so that it works.
But I have to it at every call of the VAD. This is awkward and... imagine user does not have write permission on the folder...

Choose a reason for hiding this comment

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

@Jeronymous
I found a commit that addresses this issue in silero repository. But judging from commit dates, it seems to have been merged after default switched to v4.0? I don't know what is the best option here. I personally don't use the silero repo anymore. Because I wanted a near-instant inference start on demand (to skip non-speech in my local mpv player from any playback position) I switched to a self-contained minimal C program that calls to onnxruntime's C api in a dll. I just pipe the audio from ffmpeg and it immediately returns the timestamps. Switching silero versions for me was just a matter of renaming the model file and adjusting the onnxruntime api (the V4 IIRC changed output tensor dimension).

snakers4/silero-vad@df1d520

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks a lot for sharing this.

Given this, my solution in python makes sense. Just I don't like the fact of meshing like this with folders when processing an audio.
Maybe the right solution is just to fork silero from v3.1 and fix that particular issue inside...

Concerning the quality of silero VAD, apparently, there is no free lunch : it's not obvious that solving this case would not degrade in other circumstances (speech with very noisy background). See comments here: snakers4/silero-vad#396 (comment)

Choose a reason for hiding this comment

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

@Jeronymous In my case, the hypothetical issues of detecting speech with very noisy background accurately are so rare that they are indeed hypothetical for me. I can't speak for others using the whisper-timestamped, and I don't know your intended main usage for the project, so I will offer a suggestion strictly from my own perspective and usage patterns. For my use cases I would settle on the silero-vad version that suits best for my needs and set that as a reasonable default, and implement it first and foremost. Support the other version later if there is enough demand. And since silero-vad models are so small, and the inference code is so simple and short, I would just bundle the models (if license permits) and inference code with the whisper-timestamped project, and by removing a dependency we get rid of a potential failure point, downloading the entire repository for like 50 lines of python code, and dealing with all the symlinking and paths nonsense.
Unfortunately it wouldn't address my main issue with the project, (unacceptably high VRAM usage, making it impossible to run whisper-v3-large), but that's better addressed in topical issues like #110

@Jeronymous
Copy link
Member Author

In the end I implemented the choice of the VAD method.
Default remains the same (silero latest / 4.0), but former versions of silero can be specified (e.g. "silero:3.1").
And also "auditok" can be used. See this PR that I'm gonna close: #78

@Jeronymous Jeronymous merged commit fadbfb7 into master Nov 30, 2023
1 check passed
@Jeronymous Jeronymous mentioned this pull request Nov 30, 2023
@Jeronymous Jeronymous deleted the bugfix/use_silero_v3.1 branch January 15, 2024 19:03
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