fix: encode Matrix auto-TTS voice as Ogg Opus - #54488
Conversation
Competing with #14900 for the same Matrix voice-bubble bug (#14841). This PR transcodes the generated MP3 to Ogg/Opus after generation (keeping the .mp3 path, which it notes is safer for Edge TTS) and additionally adds native Matrix MSC3245 voice metadata (ffprobe duration + MSC1767 waveform) for Element-compatible playback. #14900 takes the narrower one-line approach of extending the |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing the Matrix native-voice playback path; the current main implementation still requests an MP3 for Matrix in gateway/run.py:13153-13159 and marks it as MSC3245 voice in plugins/platforms/matrix/adapter.py:1964-1981.
Problems
- The new gateway conversion does not cover direct
text_to_speechdelivery.tools/tts_tool.py:2202still enables Opus only for Telegram, while gateway media delivery routes recognized Matrix audio tosend_voice(gateway/run.py:13293-13301). Matrix can therefore still receive a native voice event containing non-Opus audio outside_send_voice_reply. _matrix_voice_metadata_for_file()runsffprobeandffmpegsynchronously, but the PR invokes it from async_send_local_file. Its subprocess timeouts can block the Matrix adapter event loop during every voice upload. The gateway conversion already usesasyncio.to_thread; metadata extraction should do likewise.
Suggested changes
- Centralize Matrix voice conversion at the Matrix
send_voiceboundary, or update the tool-level Matrix Opus routing as well. - Offload metadata probing and add coverage for the non-blocking path and direct Matrix TTS routing.
Automated hermes-sweeper review.
| # Matrix and Telegram native voice renderers expect Ogg/Opus. | ||
| # Generate MP3 first for broad provider compatibility, then | ||
| # transcode below. Passing a .ogg path directly is unsafe for Edge | ||
| # TTS: it writes MP3 bytes to whatever path it is given. |
There was a problem hiding this comment.
This covers only _send_voice_reply. Direct text_to_speech media on Matrix still follows tools/tts_tool.py's Telegram-only want_opus predicate and is routed to MatrixAdapter.send_voice; please enforce the Matrix codec at the adapter boundary or extend the tool-level routing too.
| @@ -2165,9 +2262,19 @@ async def _send_local_file( | |||
| fname = file_name or p.name | |||
There was a problem hiding this comment.
_matrix_voice_metadata_for_file() may synchronously run ffprobe for 10s and ffmpeg for 15s. Calling it in this async method blocks the Matrix event loop; await it through asyncio.to_thread(...) instead.
|
Downstream confirmation + a concrete assist. We hit #14841 with a self-hosted OpenAI-compatible TTS (Moshi/Kyutai) and ran this PR's approach (Opus container + To help this land, I've opened #68063, which stacks on this branch unchanged (your commit stays yours) and resolves the two blockers from the automated review:
Tests: the affected suites pass (13 passed — your 8 plus new cases for Matrix tool routing, boundary transcode, and skip-when-already-Ogg); ruff clean. Whichever way the maintainers prefer — merge this PR then #68063 as a follow-up, or take #68063 wholesale (it contains this branch) — happy to rebase either way. |
Builds on NousResearch#54488 (kept as-is) and addresses its two review blockers: 1. Direct text_to_speech coverage — tools/tts_tool.py want_opus now includes Matrix, so model-invoked TTS on a Matrix session emits Ogg/Opus (native-Opus providers directly; MP3 providers via the existing tool-level _convert_to_opus step, which converts to a distinct path — no Edge in/out collision). As a safety net, MatrixAdapter.send_voice now enforces the MSC3245 codec at the adapter boundary: any non-Ogg audio reaching it is transcoded to Ogg/Opus (best-effort; original sent unchanged if ffmpeg is unavailable, preserving previous behaviour). 2. Non-blocking metadata — _matrix_voice_metadata_for_file() is now awaited via asyncio.to_thread from _send_local_file, so its ffprobe/ffmpeg subprocess timeouts can no longer stall the Matrix adapter event loop; the boundary transcode runs off-loop the same way. Tests: Matrix case added to tests/tools/test_tts_opus_routing.py (mirrors the Telegram case); adapter-boundary transcode + skip-when-Ogg cases added to tests/gateway/test_matrix_voice.py. Refs NousResearch#14841 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
One small observation while validating this downstream — not a blocker, just so it's a conscious trade-off on the record. The mp3-first + post-convert design (rightly recommended in review for Edge safety) has a minor side effect on the auto-reply path for native-Opus providers (openai / elevenlabs / mistral / gemini): previously a Telegram auto-reply requested In practice it's modest for 64k voice audio, and the Edge-safety rationale is sound, so this seems like a reasonable trade to land as-is. Flagging it mainly in case anyone later wonders why auto-reply audio differs slightly from direct |
|
The root cause here (gateway auto-TTS choosing MP3 vs Ogg/Opus via the cleared (Landed via #73508, merge |
Summary
Testing
PYTHONPATH="/tmp/hermes-agent-matrix-voice-pydeps${PYTHONPATH:+:$PYTHONPATH}" python -m pytest tests/gateway/test_voice_command.py::TestSendVoiceReply tests/gateway/test_matrix_voice.py::TestMatrixSendVoiceMSC3245 -q -o 'addopts='→8 passed in 1.34spython -m py_compile gateway/run.py plugins/platforms/matrix/adapter.py tests/gateway/test_voice_command.py tests/gateway/test_matrix_voice.py→ passeduv run --active --no-sync --with ruff==0.15.10 ruff check gateway/run.py plugins/platforms/matrix/adapter.py tests/gateway/test_voice_command.py tests/gateway/test_matrix_voice.py→All checks passed!git diff --check origin/main...HEAD -- gateway/run.py plugins/platforms/matrix/adapter.py tests/gateway/test_voice_command.py tests/gateway/test_matrix_voice.py→ passedNotes