Skip to content

fix(voice): capture at the input device's native sample rate - #61463

Closed
bricelb wants to merge 2 commits into
NousResearch:mainfrom
bricelb:fix/voice-input-samplerate
Closed

fix(voice): capture at the input device's native sample rate#61463
bricelb wants to merge 2 commits into
NousResearch:mainfrom
bricelb:fix/voice-input-samplerate

Conversation

@bricelb

@bricelb bricelb commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes broken voice recording on capture devices that reject 16 kHz input.

AudioRecorder hard-codes SAMPLE_RATE (16 kHz) when opening the input stream, but some devices (e.g. USB microphones exposed directly through ALSA hw) do not support 16 kHz capture — sd.InputStream fails with PaErrorCode -9997 (Invalid sample rate) and voice mode is unusable. Reproduced with a common "USB PnP Sound Device" microphone whose only supported rates are 44.1/48 kHz.

The fix queries the default input device for its native default_samplerate at recording start, opens the stream and writes the WAV at that rate, and falls back to the Whisper-friendly 16 kHz constant when the backend does not expose a usable rate. STT providers accept standard WAV sample rates, so downstream transcription is unaffected.

Related Issue

No existing issue found for this; happy to open one if preferred.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • tools/voice_mode.py: add _default_input_samplerate(); AudioRecorder tracks the capture rate per recording (self._sample_rate) and uses it for the input stream, the min-duration check, and the WAV header (_write_wav becomes an instance method).
  • tests/tools/test_voice_mode.py: new TestDefaultInputSamplerate class (5 tests) covering device-rate selection, fallback on query failure / non-numeric rate, stream opening at the device rate, and WAV written at the capture rate.

How to Test

  1. On a machine whose default input device does not support 16 kHz (check with python -c "import sounddevice as sd; sd.check_input_settings(samplerate=16000, channels=1, dtype='int16')"), enable voice mode and start a recording.
  2. Before this fix: recording fails with PortAudio error -9997.
  3. With this fix: recording works at the device's native rate and transcription proceeds normally; devices that do support 16 kHz keep the previous behavior via the fallback.
  4. pytest tests/tools/test_voice_mode.py -q — 78 passed.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature
  • I've run the targeted tests and they pass (tests/tools/test_voice_mode.py, 78 passed)
  • I've added tests for my changes
  • I've tested on my platform: Raspberry Pi OS (Debian 12, arm64), Python 3.11, USB PnP microphone
  • Cross-platform impact considered: uses the sounddevice API only, no platform-specific paths

Documentation & Housekeeping

  • Documentation: N/A (no user-facing behavior change beyond the fix)
  • cli-config.yaml.example: N/A (no config keys changed)
  • CONTRIBUTING.md / AGENTS.md: N/A

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have tool/tts Text-to-speech and transcription duplicate This issue or pull request already exists labels Jul 9, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #20788 (OPEN, earliest) -- both add _default_input_samplerate(sd) querying the input device's default_samplerate (falling back to the 16 kHz SAMPLE_RATE constant), track self._sample_rate per recording on AudioRecorder, convert _write_wav from a @staticmethod to an instance method, and use the device rate for the stream, the min-duration check, and the WAV header. Byte-identical mechanism and helper name; #20788 is the canonical open PR. Maintainer to pick one.

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: LGTM

What the PR Does

Capture audio at the input device native sample rate instead of forcing 16 kHz, fixing compatibility with mics that reject 16 kHz capture.

Assessment

  • Correctness: Device native rate is used via query_devices; falls back gracefully to SAMPLE_RATE (16 kHz) on any error.
  • Testing: Comprehensive test coverage: device default rate, query failure fallback, non-numeric fallback, stream opened at device rate, WAV written at capture rate.

Note: PR was previously unreviewed.


Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for adding coverage for devices that reject 16 kHz. The reported hard-coded capture rate is still present on current main at tools/voice_mode.py:637, so the core direction is valid.

Problems

  • AudioRecorder deliberately retains its stream across recordings (tools/voice_mode.py:525-534; introduced by eb79dda04 to avoid a CoreAudio reopen hang). This PR assigns a newly queried _sample_rate on every start() but leaves that persistent stream open. If the default input device/rate changes, subsequent WAV headers and minimum-duration checks use the new value although the audio was captured by the original stream. The CLI reuses this recorder across recordings (cli.py:11169-11213).

Suggested changes

  • Bind the selected rate to stream creation, or retain the existing stream's rate when it is reused.
  • Add a two-start regression test that changes the queried rate and verifies the persistent stream and WAV header stay consistent.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 11, 2026
@bricelb
bricelb force-pushed the fix/voice-input-samplerate branch from c043134 to 7ce9e6e Compare July 24, 2026 04:06
Daemon and others added 2 commits July 24, 2026 06:10
The TUI launcher installs dependencies with npm install --workspace
ui-tui, which intentionally leaves unrelated workspaces (apps/desktop,
web) out of npm's hidden lockfile. _tui_need_npm_install() then compares
the entire root package-lock.json against that hidden lockfile, so every
subsequent launch sees the missing unrelated entries as staleness and
re-runs npm install — a false-positive reinstall on every TUI start from
a source checkout.

Walk the lockfile dependency graph from the ui-tui workspace entry
(following workspace links and npm's nested node_modules resolution
candidates) and restrict the comparison to that closure, so only
packages the scoped install is actually responsible for can mark the
install stale.
AudioRecorder hard-codes SAMPLE_RATE (16 kHz) when opening the input
stream, but some capture devices (e.g. USB microphones exposed through
ALSA hw) reject 16 kHz outright — sd.InputStream fails with
PaErrorCode -9997 (Invalid sample rate) and voice recording is broken.

Query the default input device for its native default_samplerate at
recording start and open the stream / write the WAV at that rate,
falling back to the Whisper-friendly 16 kHz constant when the backend
does not expose a usable rate. STT providers accept standard WAV rates,
so downstream transcription is unaffected.
@teknium1

Copy link
Copy Markdown
Contributor

Merged into main via consolidated salvage PR #73520 (merge e04c2a9ebd). Your native-sample-rate capture fix (devices without 16 kHz support) was cherry-picked with your authorship.

Your contribution is credited to you in git history. Thank you! Closing this PR as merged-via-salvage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades tool/tts Text-to-speech and transcription type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants