Add 1x, 1.5x, and 2x playback speed for voice messages - #41
Conversation
A chip beside the waveform cycles 1x, 1.5x, and 2x, like the phone, keeping the speaker's pitch: playing samples at a higher rate sharpens the voice, so speeds above 1x queue a time-compressed copy of the clip (waveform-similarity overlap-add in timestretch.rs) built once in the background, and the sink always plays at 1x. The choice is kept in settings and applies to later messages, and the position stays put when the speed changes mid-clip.
There was a problem hiding this comment.
🟡 Changes recommended
Three moderate playback issues and one layout nit remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds persistent 1x, 1.5x, and 2x pitch-preserving playback speeds for voice and audio messages.
Changes:
- Adds background WSOLA time compression and playback integration.
- Adds a speed chip and persisted preference.
- Updates tests and user documentation.
File summaries
| File | Summary |
|---|---|
src/ui/conversation.rs |
Adds the speed chip. Nit: undownloaded audio reserves chip space even when the chip is hidden. |
src/timestretch.rs |
Adds WSOLA processing and unit tests. |
src/settings.rs |
Adds persisted playback-speed settings. |
src/model.rs |
Adds the speed-cycle action. |
src/lib.rs |
Registers the timestretch module. |
src/audio.rs |
Integrates speed processing. Moderate: workers need cancellation or generation checks. Moderate: retain completed buffers keyed by speed. Moderate: preserve the currently queued speed while a new buffer builds. |
src/app.rs |
Applies and persists playback speed. |
README.md |
Documents playback-speed controls. |
docs/_guide/using-zapfast.md |
Updates voice-message guidance. |
Review details
Suppressed comments (1)
src/ui/conversation.rs:3291
- When the attachment has not been downloaded, the chip is not rendered below because
media.path.is_some()is false, but this width still reserves its 38px plus gap. Every undownloaded audio row consequently leaves an empty chip-sized hole and a needlessly narrow waveform; reserve that space only when the chip will be shown.
let wave_width = width - button - 10.0 - chip - 10.0;
- Files reviewed: 9/9 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Choosing a speed whose compression is still building keeps playing the buffer already queued instead of dropping to 1x, and completed compressions stay kept per speed, so cycling back to one reuses it. Replacing or abandoning a compression job sets a flag the WSOLA loop checks every frame, so stale jobs stop instead of running in parallel. While the clip is still downloading, the waveform takes back the space reserved for the speed chip.
|
Also addressed the layout nit on |
There was a problem hiding this comment.
🟡 Changes recommended
Four moderate findings remain involving stale work, memory growth, UI-thread copying, and long-clip CPU cost.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
src/timestretch.rs:124
- This correlation loop evaluates 113 candidate shifts and 256 decimated samples for every synthesis frame. At the existing 15-minute recording limit that is roughly 1.2 to 1.6 billion inner iterations for one speed, and cycling to the other factor repeats the scan, so selecting a speed can keep a CPU core busy for a long time while playback is already running. Restrict the comparison to the overlap region or use an incremental search so long clips do not incur this cost.
src/audio.rs:77
- The raw clip remains in
Loaded.sampleswhilestretchesretains both compressed copies. At 48 kHz f32, a 15-minute clip is about 173 MB raw, and the 1.5x and 2x copies add roughly 115 MB and 86 MB, before the WSOLA scratch buffers. Cycling speeds can therefore push a maximum-length recording toward hundreds of megabytes and transiently much higher. Add a memory-bounded cache or eviction policy before retaining both transforms indefinitely.
/// Time-compressed copies of the loaded clip, one per speed already
/// built, dropped when the clip changes.
stretches: Vec<(f32, Arc<Vec<f32>>)>,
src/audio.rs:377
- This new completion path runs on the UI thread and
apply_speed()immediately callsrestart, whosebuffer[offset..].to_vec()copies the entire remaining compressed clip before it can be queued. For a long clip that can copy tens or hundreds of megabytes during a frame, causing a visible freeze despite moving WSOLA off-thread. Prepare the owned queue buffer off-thread or use a streaming/shared source so the swap does not perform a full synchronous copy.
self.apply_speed();
- Files reviewed: 9/9 changed files
- Comments generated: 1
- Review effort level: Lite
1x plays the clip as recorded, so a 1.5x or 2x job still building is no longer needed; dropping it sets its cancel flag and the worker stops instead of finishing the scan and caching an unused buffer.
There was a problem hiding this comment.
🔵 Needs a closer look
Two confirmed edge-case bugs need fixing (unvalidated non-finite speeds and potentially negative waveform width leading to broken layout/math).
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
src/audio.rs:148
Player::set_speedaccepts anyf32and stores it directly. Non-finite values (NaN/Inf) or very large/small speeds can break comparisons inensure_stretch(NaN never equals itself, so jobs can be spawned repeatedly) and can panic inDuration::mul_f32whenloaded.factorbecomes invalid. Clamp and reject non-finite speeds at the API boundary.
src/ui/conversation.rs:3294wave_widthcan become negative when the chat window is narrow (or when padding + button + chip exceedwidth). Passing a negative width intoallocate_exact_sizecan break layout and can lead to divide-by-zero/NaN in the seek fraction calculation.
- Files reviewed: 9/9 changed files
- Comments generated: 0 new
- Review effort level: Lite
set_speed clamps speeds to 1x through 2x and plays non-finite ones at 1x, so a hand-edited setting cannot spawn endless compressions or panic in the position maths. The waveform keeps a positive width in a very narrow window, so the layout and the seek fraction stay defined.
|
Also addressed the two suppressed notes from the latest Copilot review in 85e2dc4: |
|
Goat Feature! Audios de 8min da minha tia precisam disso. |
There was a problem hiding this comment.
🟡 Changes recommended
A very narrow window can make the waveform width less than 10 px, which can trigger a runtime panic in the seek knob clamp during playback.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 1
- Review effort level: Lite
# Conflicts: # src/ui/conversation.rs
There was a problem hiding this comment.
🟡 Changes recommended
timestretch::speed_up_unless currently mishandles non-finite factors (returning an empty buffer), and the speed chip can disagree with the effective playback rate while a new compression is still building.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 2
- Review effort level: Lite
The chip shows the chosen speed at once, so a click is never mistaken for a miss, but its label is faded while the clip keeps playing at the earlier speed as the compression builds. speed_up_unless now returns the samples unchanged for a non-finite factor instead of an empty buffer.
|
Nice feat! |
|
Tested this on Windows 11 (x86_64 MSVC, rustc 1.98.0) against a live linked account, and it works: the chip sits on both incoming and outgoing bubbles, cycles 1x, 1.5x and 2x, keeps the speaker's pitch, remembers the choice for later messages, playback continues while a compression builds, and seeking keeps the speed. Checks on this machine:
One layout nit I hit: the pill is not vertically centered with the play button, it sits about 5.5 px lower on both incoming and outgoing bubbles. The row is declared at the button's height, but the waveform and its duration line stretch it to about 47 px, and since the pill is drawn last it gets cross-aligned to the stretched rect. I fixed it in a one commit PR against your branch, so you can take it with a click if you agree with the approach: gustavobragac#1 The fix centers the pill on the centre line the play button uses and moves the click target with it, nothing else changes. |
There was a problem hiding this comment.
🟢 Approval recommended
The implementation matches the stated behavior (including settings persistence and pitch preservation), follows the UI action boundary, and includes focused unit tests for both timestretching and player speed behavior.
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 0 new
- Review effort level: Lite
The phone client plays voice notes at 1x, 1.5x, or 2x without changing the speaker's pitch; ZapFast had no speed control at all. This adds the same control.
A small chip beside the waveform shows the current speed and cycles 1x, 1.5x, and 2x on click. The choice is remembered in settings and applies to every later voice or audio message. It works while a clip is playing, paused, or idle, and seeking keeps the selected speed.
How the pitch stays put: playing samples at a higher rate sharpens the voice, so the sink always plays at 1x. Speeds above 1x queue a time-compressed copy of the clip instead, built with waveform-similarity overlap-add (WSOLA) in the new
src/timestretch.rs, on a background thread like decoding. Finished copies are kept per speed for the loaded clip, so cycling back to a speed reuses it. At most one build runs: replacing it, stopping the clip, or returning to 1x sets a cancel flag that the WSOLA loop checks on every frame. While a compression builds, playback continues at the speed already queued and swaps in place, at the same position, once it is ready; meanwhile the chip already shows the chosen speed, with its label faded until that speed takes effect. The waveform and the time counter keep running on the clip's own timeline, so a 0:42 note at 2x still shows 0:42 with the position advancing at twice the clock. The chip rests on the hover surface step because incoming bubbles share the resting surface colour.Testing:
timestretchunit tests: a 440 Hz tone stays at about 440 Hz after 1.5x and 2x, lengths land exactly on the rounded target, silence stays silent, loudness survives, short clips fall back to dropping samples, and unit speed is the identity.timestretchalso returns unusable factors (NaN, infinity, 1 or below) unchanged and gives up once cancelled.audiotests: the cycle order and the labels, speeds clamped to 1x through 2x, a building speed keeping the queued buffer, both built speeds staying available, 1x cancelling an outstanding build, and the chip's preparing state; an ignored on-hardware test (audio::tests::doubles_the_position_rate_on_this_machine) checks that the position outruns the clock at 2x.voicescenario lays out with the chip inevery_surface_lays_out, and a--demo-shotrender shows the chip on both incoming and outgoing bubbles.Docs: the README feature bullet and the Voice messages section of the user guide mention the control and the unchanged pitch.
Checks run on Linux: fmt, clippy (default and all features), tests (default and all features), and cargo doc, all clean. Other platforms compile through the same code paths with no new platform-specific code.