feat(eot): allow configurable min_silence_duration for turn detectors (#7188) - #7252
feat(eot): allow configurable min_silence_duration for turn detectors (#7188)#7252rrfunde wants to merge 7 commits into
Conversation
…livekit#7188) Support exposing and configuring min_silence_duration on turn detectors and remove hardcoded +50ms buffer in AudioRecognition._check_vad_silence_requirement.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e18841348
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| detector_min_silence = getattr(detector, "min_silence_duration", None) | ||
| required = ( | ||
| detector_min_silence | ||
| if detector_min_silence is not None | ||
| else MIN_SILENCE_DURATION_MS / 1000 | ||
| ) |
There was a problem hiding this comment.
When min_silence_duration is configured below 0.2 seconds, this validation now accepts a matching low-latency VAD, but _on_vad_event still starts predictions only after the hard-coded MIN_SILENCE_DURATION_MS / 1000 threshold at line 1427. The VAD can therefore emit END_OF_SPEECH before any prediction is requested, causing _run_eou_detection to see no future and skip the turn detector entirely; values above 0.2 seconds conversely start inference before the detector's declared minimum. Use the detector/stream's configured threshold for the inference trigger as well.
Useful? React with 👍 / 👎.
…ce trigger in _on_vad_event
|
Addressed the review feedback:
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e7a79a6ec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| float(detector_min_silence) | ||
| if detector_min_silence is not None | ||
| else MIN_SILENCE_DURATION_MS / 1000 |
There was a problem hiding this comment.
When the VAD's min_silence_duration exactly equals the detector minimum, this validation now accepts the pairing, but the built-in and Silero VAD loops emit INFERENCE_DONE with the previous silence accumulator before incrementing it and possibly emitting END_OF_SPEECH (inference/vad.py:403-459 and plugins/silero/vad.py:495-552). For example, with 32 ms windows and both values set to 0.2 s, the last inference event reports 0.192 s, then the VAD increments to 0.224 s and ends speech, so predict() is never called and the turn commits without the detector. Fresh evidence after the prior trigger fix is this pre-increment event ordering; either require enough VAD headroom for another update or start a missing prediction at end-of-speech.
Useful? React with 👍 / 👎.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4eb19c366d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self._turn_detector_stream is not None | ||
| and self._turn_detector_prediction_fut is None | ||
| ): | ||
| self._turn_detector_prediction_fut = ( | ||
| self._turn_detector_stream.predict() |
There was a problem hiding this comment.
When endpointing commits before the VAD emits its delayed END_OF_SPEECH—for example, a 50 ms detector threshold with a 500 ms VAD threshold—the commit flushes the stream and clears _turn_detector_prediction_fut, so this block starts a second inference for the already-completed turn. With STT, _run_eou_detection then returns because the transcript was cleared, leaving an unnecessary cloud/local request pending until the next speech; the preceding vad_speech_started value already distinguishes this documented late-EOS path, so the replacement prediction should only be started for an active speech segment.
Useful? React with 👍 / 👎.
|
Fixed: Added fallback prediction trigger in |
|
Addressed review feedback: gated the fallback prediction trigger at |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 474666dd05
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ev.raw_accumulated_silence >= self._turn_detector_min_silence_duration | ||
| and self._speaking |
There was a problem hiding this comment.
When TurnDetector(min_silence_duration=0) (or a negative value) is used, every speech-bearing INFERENCE_DONE event satisfies this comparison because its accumulated silence is zero. The preceding block then cancels the current prediction and this block immediately starts another one on every VAD window, potentially issuing dozens of cloud inference requests per second while the user is speaking and preventing a stable prediction from completing. Validate that the new option is strictly positive, or require actual accumulated silence before calling predict().
Useful? React with 👍 / 👎.
… accumulated silence
|
Addressed review feedback:
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…ream threshold fallback
|
Addressed review feedback:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ebdf8d9b25
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| else: | ||
| required = MIN_SILENCE_DURATION_MS / 1000 |
There was a problem hiding this comment.
When a custom streaming detector exposes min_silence_duration only on its returned stream—a case _turn_detector_min_silence_duration explicitly supports—this fallback still validates against 0.2 seconds. _start() calls _update_vad() before adopting or creating the stream, so a VAD configured for 0.1 seconds is incorrectly rejected even if the stream requires only 0.08 seconds. Validation should use the candidate stream's threshold once available or occur after the stream is adopted.
Useful? React with 👍 / 👎.
| local_fallback: bool = True, | ||
| http_session: aiohttp.ClientSession | None = None, | ||
| conn_options: APIConnectOptions = DEFAULT_API_CONNECT_OPTIONS, | ||
| min_silence_duration: float = MIN_SILENCE_DURATION_MS / 1000, |
There was a problem hiding this comment.
When callers set this new behavior-affecting option, TurnDetector.describe_options() still reports only the model, provider, sample rate, fallback mode, and threshold overrides. Consequently, sessions that use different min_silence_duration values are indistinguishable in the uploaded session configuration, making latency or turn-detection experiments difficult to attribute and reproduce; include the resolved value in describe_options().
Useful? React with 👍 / 👎.
1 participant
Fixes #7188
Summary
Allows turn detectors to expose a configurable
min_silence_durationrather than enforcing a global 200ms minimum (+50ms buffer). This allows custom or local turn detection models to operate with their intended lower VAD silence windows without raising spurious validation errors.Changes
min_silence_durationparameter toTurnDetectorOptions,_BaseStreamingTurnDetector, andTurnDetector, defaulting toMIN_SILENCE_DURATION_MS / 1000(0.2s).min_silence_durationon_BaseStreamingTurnDetectorStream.AudioRecognition._check_vad_silence_requirement(), querydetector.min_silence_duration(falling back toMIN_SILENCE_DURATION_MS / 1000when unset) and remove the arbitrary+ 50msbuffer.tests/test_audio_recognition_turn_detection.pyverifying custom turn detector minimum silence validation.