fix(discord): render real voice-message waveform + accurate duration_secs - #11359
fix(discord): render real voice-message waveform + accurate duration_secs#11359malaiwah wants to merge 1 commit into
Conversation
9ab4701 to
f8bbe77
Compare
Reviewer findings addressedNew — operator opt-out. Full Discord test suite. |
f8bbe77 to
9034786
Compare
Added — fix the "1:04 → 0:14" voice-bubble duration flickerIn-field observation: voice-message bubbles first rendered with a wildly wrong duration (e.g. "1:04" on a 14 s clip) and then silently corrected themselves a second later to the real length. That's Discord's UI showing the Root cause: Fix bundled into this PR (same attachment payload, same ffmpeg toolchain):
PR title updated to reflect the bundled scope. Full Discord suite: 237 passed, 1 skipped, 0 failed across 5 consecutive runs. |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing both the native waveform and duration fallback; the current Discord adapter still has the flat waveform and byte-rate duration fallback at plugins/platforms/discord/adapter.py:2650-2658.
Problems
- The branch targets the pre-migration adapter and config bridge. Commit
cc8e5ec2amovedgateway/platforms/discord.pytoplugins/platforms/discord/adapter.py; current YAML handling belongs in_apply_yaml_configatplugins/platforms/discord/adapter.py:8229. Consequently, the addedgateway/config.py:623bridge will not run on current main. - The new test imports
gateway.platforms.discordattests/gateway/test_discord_voice_waveform.py:57, a module absent from current main after that migration.
Suggested changes
- Salvage the helper and
send_voicechanges intoplugins/platforms/discord/adapter.py, add the YAML translation to that plugin's_apply_yaml_config, and retarget the test imports/mocks to the plugin path.
Automated hermes-sweeper review.
| # voice_message_waveform: compute a real loudness waveform for | ||
| # voice messages (default true). Set false to skip ffmpeg + | ||
| # numpy work and always ship a flat 128-byte waveform. | ||
| if "voice_message_waveform" in discord_cfg and not os.getenv("DISCORD_VOICE_MESSAGE_WAVEFORM"): |
There was a problem hiding this comment.
Current main no longer owns Discord YAML translation here: cc8e5ec2a moved it to plugins/platforms/discord/adapter.py:_apply_yaml_config (line 8229). Port this setting to that hook or the config value will not be applied.
|
|
||
| _ensure_discord_mock() | ||
|
|
||
| from gateway.platforms.discord import ( # noqa: E402 |
There was a problem hiding this comment.
Current main removed this import path when cc8e5ec2a migrated the adapter to plugins.platforms.discord.adapter. Update this import and the test's patch targets to the plugin module so collection succeeds.
9034786 to
b7575db
Compare
Retargeted to current main HEADThe original branch targeted the pre-migration Code changes (plugins/platforms/discord/adapter.py):
Tests (tests/gateway/test_discord_voice_waveform.py):
Docs:
Ready for re-review. |
…secs Discord's voice-message bubble renders a ``waveform`` field as a loudness bar graph next to the play button. The old code shipped a flat ``bytes([128] * 256)`` — a featureless line that's the tell-tale sign of a bot voice message. Replace the flat waveform with real RMS/dBFS-based computation: - Decode the audio to mono 48 kHz s16le PCM via ffmpeg (already a Hermes voice dependency) - Window into target_samples buckets (one per 100ms, capped at 256) - Compute per-bucket RMS, map to dBFS, then to uint8 on a [-60, 0] scale - Any failure (missing ffmpeg, decode error, empty audio) falls back to the flat 128-waveform — Discord accepts it and the voice message still sends Also fix the duration_secs probe: ffprobe → mutagen → byte-rate fallback chain. The old mutagen-only path misreported non-OGG inputs (Edge TTS / OpenAI TTS can ship MP3), causing the voice bubble to show "1:04" on a 14-second clip then silently correct itself a second later. Operators can opt out via ``DISCORD_VOICE_MESSAGE_WAVEFORM=false`` to skip the ffmpeg decode and always ship the flat fallback. The waveform computation uses pure Python (struct + math) — no numpy dependency. This keeps the feature available in all environments without adding an optional dependency. Retargeted from gateway/platforms/discord.py to plugins/platforms/discord/adapter.py after the Discord platform migration.
b7575db to
b60b889
Compare
Rebased + CI failures fixed — ready for re-reviewRebased onto current main HEAD ( CI failures — root cause and fixAll 9 failures in Fix: Replaced the numpy implementation with pure Python ( teknium1's inline comments — both addressed
Test resultsAll 44 tests pass locally (was 35 passed / 9 failed before the fix): Current state
Ready for re-review. |
|
Discord Feature Parity & Alignment Campaign interlock: tracked by EPIC #79564. Original contributor lane; preservation and integration routing remain explicit in the campaign ledger. |
Fixes #11358.
Summary
Discord voice messages currently ship with a flat waveform (
bytes([128] * 256)). Discord's voice-bubble UI renders thewaveformfield as a loudness bar graph next to the play button, so a flat array looks like a featureless straight line.This PR computes a real waveform from the audio file:
s16lePCM viaffmpeg(already a Hermes voice dep).min(256, max(1, round(duration_secs * 10)))buckets per the Discord spec (max 256 samples, at most one per 100 ms).Fallback paths all return a flat waveform (Discord still accepts it) so voice-message sending is never blocked by waveform trouble: numpy missing, ffmpeg missing / timeout (30 s) / decode error / permission / OOM / other exceptions, empty output, short PCM, very long clips (>10 min — don't buffer hundreds of MB of PCM), NaN / ±inf / negative duration.
Defense in depth on the subprocess argv:
-nostdinso ffmpeg never hangs on piped input.file:<path>so a leading-dash filename can't be interpreted as an ffmpeg flag (argv isolation already prevents shell injection; this closes the ffmpeg-flag-injection angle).Test plan
pytest tests/gateway/test_discord_voice_waveform.py -v— 21/21 passpytest tests/gateway/ -k discord -p asyncio— 214 passed, 1 skipped, 0 failed (full Discord suite)file:prefix,-nostdin)Peer review
Peer-reviewed by a subagent against the Discord spec before shipping. Feedback addressed:
file:prefix +-nostdinexceptlist → broadexcept Exceptionin both subprocess and post-decode numeric paths, all withlogger.warningpcm.size < target_samplesguard + regression testmath.isfiniteguard + parametrized testnp.nan_to_numon the dBFS→uint8 mapping as belt-and-bracesRisk
Zero behavior change when anything goes wrong (same flat-128 output as before). Only visible difference is: for a healthy ffmpeg + numpy setup, voice-message bubbles now render an actual loudness graph. No new dependency — numpy is already under
hermes-agent[voice]/[vad], and the helper falls back gracefully if absent.🤖 Generated with Claude Code