Repository navigation
Add Go API for ten-vad - #2384
Conversation
|
Caution Review failedThe pull request is closed. """ WalkthroughThe changes update the voice activity detection (VAD) example to support both Silero and Ten-VAD models, switching audio input from portaudio to malgo with callback-based capture. The VAD configuration and model selection are now dynamic, and the code introduces helper functions and struct extensions for Ten-VAD support. The build script now ensures Ten-VAD model availability. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MainGo as main.go
participant Malgo as malgo
participant VAD as VoiceActivityDetector
User->>MainGo: Start program
MainGo->>Malgo: Initialize context & device
Malgo->>MainGo: Callback with audio data
MainGo->>MainGo: Convert & buffer audio samples
MainGo->>VAD: Feed window of samples
VAD-->>MainGo: Speech segment detected?
MainGo->>MainGo: Save speech as WAV if detected
User->>MainGo: Interrupt to exit
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR introduces support for the TenVad model in the Go bindings and updates the VAD example to download and use ten-vad.onnx.
- Added a new
TenVadModelConfigstruct and integrated it intoVadModelConfigand the C binding inNewVoiceActivityDetector - Enhanced the example shell script to download
ten-vad.onnxwhen missing - Updated the Go example (
main.go) to choose between Silero VAD and TenVad, switch tomalgofor audio I/O, and add helper utilities
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| scripts/go/sherpa_onnx.go | Added TenVadModelConfig and populated C struct fields for TenVad in constructor |
| go-api-examples/vad/run.sh | Added logic to download ten-vad.onnx |
| go-api-examples/vad/main.go | Implemented file‐based model selection, switched to malgo, and added helpers |
Comments suppressed due to low confidence (3)
go-api-examples/vad/main.go:47
- [nitpick] Local variable
window_sizeuses snake_case; consider renaming to camelCase (e.g.,windowSize) to follow Go naming conventions.
window_size := config.SileroVad.WindowSize
scripts/go/sherpa_onnx.go:1145
- [nitpick] The new
TenVadModelConfigAPI is introduced but lacks unit tests. Consider adding tests that instantiateTenVadModelConfigand callNewVoiceActivityDetectorto verify TenVad behavior.
type TenVadModelConfig struct {
go-api-examples/vad/main.go:146
- [nitpick] Consider adding a GoDoc comment for
samplesInt16ToFloatto explain its purpose and input/output conversion details.
func samplesInt16ToFloat(inSamples []byte) []float32 {
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
go-api-examples/vad/main.go (3)
47-50: Use camelCase for variable naming.Go convention prefers camelCase over snake_case for variable names.
- window_size := config.SileroVad.WindowSize + windowSize := config.SileroVad.WindowSize if config.TenVad.Model != "" { - window_size = config.TenVad.WindowSize + windowSize = config.TenVad.WindowSize }
84-87: Update variable reference to use camelCase.Following the previous suggestion to rename
window_sizetowindowSize.- for buffer.Size() >= window_size { + for buffer.Size() >= windowSize { head := buffer.Head() - s := buffer.Get(head, window_size) - buffer.Pop(window_size) + s := buffer.Get(head, windowSize) + buffer.Pop(windowSize)
159-166: Consider using a shared utility package to avoid code duplication.This
FileExistsfunction appears to be identical to the one inscripts/go/_internal/vad/main.go. Consider moving it to a shared utility package to follow DRY principles.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
go-api-examples/vad/main.go(2 hunks)go-api-examples/vad/run.sh(1 hunks)scripts/go/sherpa_onnx.go(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
go-api-examples/vad/main.go (2)
scripts/go/_internal/vad/main.go (1)
FileExists(159-166)scripts/go/sherpa_onnx.go (5)
NewVoiceActivityDetector(1221-1256)DeleteVoiceActivityDetector(1258-1261)NewCircularBuffer(1172-1176)DeleteCircularBuffer(1167-1170)GeneratedAudio(939-944)
🔇 Additional comments (4)
scripts/go/sherpa_onnx.go (2)
1145-1156: LGTM! Well-structured extension for Ten-VAD support.The new
TenVadModelConfigstruct and its integration intoVadModelConfigfollow the existing pattern established bySileroVadModelConfig, maintaining consistency in the API design.
1233-1240: Proper memory management for Ten-VAD configuration.The initialization correctly allocates and defers cleanup of C strings, following the same safe pattern used for Silero VAD configuration.
go-api-examples/vad/main.go (2)
21-40: Good implementation of dynamic VAD model selection.The code properly checks for the presence of model files and configures the appropriate VAD parameters. The window size differs between models (512 for Silero, 256 for Ten-VAD), which is correctly handled.
146-157: Correct implementation of int16 to float32 conversion.The function properly handles little-endian byte order and normalizes samples to the [-1, 1] range using the standard division by 32768.
Summary by CodeRabbit
New Features
Improvements