Skip to content

fix(audio): resample MLX outputs to the declared sample rate - #2106

Merged
debpalash merged 4 commits into
debpalash:mainfrom
kapelame:fix/voicestudio-mlx-sample-rate
Sep 17, 2026
Merged

debpalash merged 4 commits into
debpalash:mainfrom
kapelame:fix/voicestudio-mlx-sample-rate

Conversation

@kapelame

@kapelame kapelame commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Summary

MLX-Audio results from a non-24 kHz model are currently played at the wrong speed and pitch. The adapter always advertises 24,000 Hz but discards each GenerationResult.sample_rate; the locked mlx-audio 0.5.0 Dia implementation emits 44,100 Hz. A one-second Dia waveform is consequently treated as 1.8375 seconds of audio. This also affects duration calculations, inline pauses, crossfades and export.

_run_backend_inference reads the rate before calling generate, so changing the adapter's rate only after generation would leave that first render inconsistent. The GPT-SoVITS adapter already resamples foreign-rate outputs to its declared rate.

Changes

  • Resample each MLX result to the adapter's existing 24 kHz contract before concatenation, including the reduced-kwargs fallback path.
  • Keep already-24 kHz audio and legacy bare-array outputs unchanged.
  • Document normalized output and cover duration/pitch for 24, 44.1 and 48 kHz, fallback generation, and mixed-rate pieces.

Type

  • 🐛 Bug fix

Testing

On this standalone MLX branch, the regression and neighboring VoiceDesign tests were rerun with the existing isolated Python environment: python -m pytest tests/test_mlx_audio_sample_rate.py tests/test_mlx_audio_instruct_1405.py -q --tb=short13 passed. The test harness explicitly asserts before and after pytest that services.tts_backend.__file__ resolves inside this worktree. git apply --check and git diff --cached --check passed.

Current-main integration check: this patch was also applied alone to dad59c1318f18492b93ec81e55f49bb2bee458e5 in a separate worktree. The two targeted files reproduced 5 failures / 8 passes before the production fix; the seven MLX/engine/chunk/dub test files listed below passed 88 tests afterward. The imported adapter path was asserted before and after pytest, and its MLX class matches this PR. The environment used temporary application data, HF_HUB_OFFLINE=1 and an empty HF cache. The newer main branch's full backend/frontend/native matrix was not rerun.

Validation scope: the broader checks listed below (87 related Python tests, 25 policy checks, frontend, Rust, and full-suite attempts) were run earlier on the reviewed SHA with both the MLX sample-rate patch and the independent ffprobe sibling-path patch applied. They were not rerun wholesale on this standalone branch. The ffprobe patch is absent from this PR.

  • New regression file before the fix: 5 failed, 3 passed. After the fix: 8 passed. Synthetic one-second 440 Hz tones retain their duration and pitch through the real NumPy/Torch/torchaudio conversion path; model generation is stubbed.
  • HF_HUB_OFFLINE=1 HF_HUB_CACHE=<empty-cache> .venv/bin/python -m pytest tests/test_mlx_audio_sample_rate.py tests/test_mlx_audio_instruct_1405.py tests/test_mlx_gate_390.py tests/test_mlx_supported.py tests/test_engine_routing.py tests/test_chunked_tts.py tests/test_dub_batch_engine_selection.py -q --tb=short87 passed on macOS arm64, Python 3.11, Torch/torchaudio 2.8.0.
  • Changelog, CJK allowlist, structure, app-version and engine-docs checks: 25 passed.
  • A wider registry run reached 91 passed, 1 failed: the existing isolation-mode test expects in-process OmniVoice while this Mac uses subprocess isolation. The identical test also fails on untouched eaf8bb953855cab3b687d547b3835f86fa38b308.
  • The full Python suite was attempted with this minimal environment: pytest tests/ -q --tb=short --maxfail=5 stopped at 510 passed, 5 failed, 10 skipped, 1 xfailed; failures include missing Transformers/ASR packages and existing Mac isolation expectations. pytest backend/tests/ -q --tb=short --maxfail=3 finished 353 passed, 2 failed, 2 skipped, with the same native-preload assumption on this Mac. This is not a full CI-green claim.
  • No model weights were downloaded and no MLX synthesis performance/voice quality was measured. Windows/Linux native smoke remains for the upstream platform matrix.
  • git diff --check passes.

Checklist

  • I've tested this locally
  • I've updated relevant documentation
  • No local machine paths, logs, or personal env details in this PR
  • Version files are in sync (no version bump)
  • The regression fixture still loads green on the smoke-matrix CI job (macOS + Windows + Linux)

Earlier local gates on the combined two-patch tree

  • Frontend (Bun 1.3.14, Node 25.9): bun install --frozen-lockfile, bun run lint, bun run typecheck:ci, bun run format:check, and bun run build passed. bun run test --maxWorkers=4 passed 346 files / 2,872 tests; node --experimental-strip-types --no-warnings --test ../tests/frontend/*.test.mjs passed 100 tests. Manifests and lockfiles are unchanged.
  • CI=1 E2E_PROD_PORT=44173 PLAYWRIGHT_CHROMIUM=<system-Chrome> bun run test:prod-bundle passed 4 tests against a freshly built production bundle. Offline Python pytest tests/smoke/ -q --tb=short passed 4 tests. Install-document validation passed; AppRun's model-free shell harness passed 26 cases.
  • Native Mac: cargo check --locked --target aarch64-apple-darwin --message-format=short -j 4 passed. cargo test --locked --lib --target aarch64-apple-darwin --message-format=short -j 4 -- --test-threads=1 passed 271 tests; the corresponding --test backend_lifecycle invocation passed 25 tests. The first parallel lib-test run had 270 passed / 1 failed, because an existing test temporarily sets HOME while another asserts the log directory; serial execution resolves that race without a code change.
  • Optional non-CI scripts remain red on both this tree and untouched reviewed SHA: bun run typecheck reports 4,028 existing checkJs errors (the actual CI typecheck passes), bun run lint:hooks reports 113 errors / 23 warnings, and bun run knip reports existing unused/duplicate exports. Development bun run e2e --workers=2 was attempted without a full seeded backend: 12 passed / 6 failed waiting for gallery/support elements; this does not establish full development E2E success.
  • bash scripts/inject-apprun.test.sh exits 2 on macOS because the injector intentionally does nothing outside Linux and its fixture is absent. bash scripts/verify-apprun-bundle.test.sh exits 1 at GNU-style sed -i under BSD sed; the first two checks pass. Linux CI owns these native packaging checks.
  • Docker builds could not run: docker info --format '{{.ServerVersion}}' exits 1, with no daemon socket. Windows/Linux native packaging requires those hosts and their platform SDKs. No release bundle or signing/publishing action was performed. The current thin installer bundles source and uv/ffmpeg/ffprobe, not model weights; Cargo's zero-byte sidecar placeholders cannot validate release binaries. The full desktop smoke script was not run because it resets application data and stops/launches local app processes; it needs a dedicated installation test host.

MLXAudioBackend.generate now groups consecutive chunks by sample rate and resamples each group to the declared 24 kHz rate before concatenation, including the fallback path. This fixes duration and pitch errors for 44.1 kHz and 48 kHz models while preserving 24 kHz and bare-array behavior. No additional merge-blocking risk is identified in the supplied findings.

@greptile-apps

greptile-apps Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Retrigger

The PR appears safe to merge with no concrete correctness, security, or repository-rule violations identified.

Summary

Normalizes MLX-Audio output to the backend’s declared 24 kHz contract.

  • Groups contiguous chunks with the same native rate before resampling.
  • Covers native-rate, mixed-rate, fallback-generation, duration, and pitch behavior.
  • Updates engine documentation and the changelog.

Reviews (3) · Last reviewed commit: "fix(mlx): retain resampling context betw..."

@coderabbitai

coderabbitai Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 9fa264ce-8e2d-4372-8124-436553de244d

📥 Commits

Reviewing files that changed from the base of the PR and between 7d77f15 and 06c675f.

📒 Files selected for processing (3)
  • backend/services/tts_backend.py
  • docs/engines/mlx-audio.md
  • tests/test_mlx_audio_sample_rate.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/engines/mlx-audio.md

Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

MLX audio output now groups consecutive chunks by reported sample rate and resamples each group to the backend rate before concatenation. Tests cover primary and fallback generation, mixed rates, grouped resampling, and raw audio without rate metadata. Documentation and the changelog describe the behavior.

Changes

MLX audio sample-rate normalization

Layer / File(s) Summary
Group and normalize generated audio pieces
backend/services/tts_backend.py
collect groups consecutive chunks with the same sample_rate, resamples each mismatched group once, and supports the primary and TypeError fallback generation paths.
Validate and document declared-rate output
tests/test_mlx_audio_sample_rate.py, tests/test_mlx_audio_instruct_1405.py, docs/engines/mlx-audio.md, CHANGELOG.md
Tests cover differing rates, grouped resampling, fallback generation, mixed-rate concatenation, and raw audio without metadata. The fixture sets the declared rate. Documentation and the changelog describe resampling before stitching and export.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: debpalash

Merge Risk: ⚪ Minimal · up to 06c67

Audio from supported MLX generation paths is normalized to the declared output rate while preserving the expected output shape. No actionable merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 7 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title follows Conventional Commit style and accurately describes the MLX sample-rate fix, but it does not include the required issue reference. The provided description also does not contain an is… Add the related issue reference to the title or pull request body, such as fix(audio): resample MLX outputs to the declared sample rate (#2106) or an equivalent tracker reference.
Docstring Coverage ⚠️ Warning Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 3 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (7 passed)
Check name Status Explanation
Description check ✅ Passed The description includes the required Summary, Changes, Type, Testing, and Checklist sections. It provides detailed validation results and clearly records known environment-dependent failures; the inf…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cross-Platform Default Parity ✅ Passed PASS — The PR changes only MLXAudioBackend.generate; it does not change the application default backend, which remains omnivoice in active_backend_id(). MLX-Audio is an explicit engine/model sel…
I18n Completeness (21 Locales) ✅ Passed The reviewed range changes five non-frontend files and changes no files under frontend/. It introduces no frontend t('...') keys and no changed frontend user-facing strings that bypass i18n. Locale co…
Local-First Guarantee ✅ Passed The PR changes only local audio buffering and resampling, documentation, changelog, and tests. The production diff adds NumPy concatenation and torchaudio.functional.resample; it adds no cloud call,…
Backward Compatibility ✅ Passed No backward-compatibility failure is introduced. The authoritative PR diff changes only MLX audio processing, documentation, changelog, and tests; it contains no database schema, Alembic migration, vo…
Full details: Title check

Explanation

The title follows Conventional Commit style and accurately describes the MLX sample-rate fix, but it does not include the required issue reference. The provided description also does not contain an issue reference.

Full details: Docstring Coverage

Explanation

Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 3 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@debpalash debpalash added the ready-for-agent Fully specified, ready for an AFK agent label Sep 17, 2026
@debpalash
debpalash merged commit 39cdd4b into debpalash:main Sep 17, 2026
10 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-agent Fully specified, ready for an AFK agent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants