fix(audio): re-trigger playhead when the same tile is pressed twice#150
Conversation
Pressing the same sound tile while it was still playing restarted the audio but froze the smooth playhead at 0. The engine's handle_play replaces the active voice and emits PlaybackFinished for the *displaced* voice carrying the SAME sound_id as the re-triggered one; the app could not distinguish that stale event from a genuine end, so it ran clear_playback_state() and nulled the freshly-created playhead. The trailing PlaybackStarted restored only the highlight, never the playhead. Tag each play with a monotonic generation token threaded through AudioCommand::Play and AudioEvent::PlaybackFinished. The app clears playback state only when a Finished's generation matches the current play generation, so the displaced voice's stale Finished (older generation) is ignored and the re-triggered playhead survives. Genuine end-of-playback carries the current generation and still clears as before; the #111 sound_id guard is retained. Not a favorites/filtering bug: every tile dispatches the identical PlaySound(id) (Favorites is only a view filter), so it reproduces on any same-tile re-press. Closes #149 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The generation token relies on the invariant that every Play(generation)
yields exactly one matching-generation PlaybackFinished(generation). The two
early-return error paths in handle_play (VirtualSinkNotRegistered,
SinkStreamCreation) violated it: a rejected Play emitted only Error and no
terminal event.
Before the generation token, a rejected re-press self-corrected because the
displaced voice's Finished matched on sound_id alone; with the token it
carries the older generation and is ignored, so the app's optimistically-set
playing/playhead state for the new generation would stick forever (and the
vsync Frame subscription would spin indefinitely).
Emit PlaybackFinished { sound_id, generation } for the rejected play before
each error return so the app clears that generation's state. The app side
(clearing on a current-generation Finished) is already covered by
re_pressing_same_sound_keeps_playhead_alive and
playback_finished_clears_playhead_and_display_progress; the engine error
paths require a live PipeWire stream-creation failure and remain
integration-only, like the surrounding Error emissions.
Found by an adversarial review pass. Refs #149
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 35 minutes and 23 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
1 participant
Why
Re-pressing the same sound tile while it is still playing restarted the audio
but the smooth waveform playhead froze at 0 instead of sweeping again. First
noticed on the Favorites tab, but it is not a favorites/filtering bug — every
tile in every tab dispatches the identical
Message::PlaySound(id)(Favorites isonly a view filter), so it reproduces on any same-tile re-press.
Root cause: the engine's
handle_playreplaces the active voice and emitsPlaybackFinishedfor the displaced voice carrying the samesound_idasthe re-triggered one. The app's drain handler could not distinguish that stale
event from a genuine end (
self.playing == Some(id)), so it ranclear_playback_state()and nulled the freshly-createdplayhead. The trailingPlaybackStartedrestored only the highlight — never the playhead.What
Tag each play with a monotonic
generationtoken threaded throughAudioCommand::Play→ActivePlayback→AudioEvent::PlaybackFinished. The appclears playback state only when a
Finishedmatches both the currentsound_id(the #111 guard) andgeneration == self.play_generation. A staledisplaced
Finishedcarries an older generation and is ignored, so there-triggered playhead survives; a genuine end carries the current generation and
clears as before.
Establishes the invariant every
Play(gen)yields exactly one matching-generationFinished(gen)— including the twohandle_playerror returns, which previouslyemitted only
Error. (An adversarial review pass caught that gap: without it arejected re-press would leave the playhead stuck and the vsync
Framesubscription spinning.)
Engine play params bundled into a
PlayRequeststruct to stay under theargument-count lint.
Testing
cargo test(477 unit), incl. newre_pressing_same_sound_keeps_playhead_alive(watched RED → GREEN)cargo test --features pipewire-test(22)cargo clippy --all-targets -- -D warningsclean (default and--features pipewire-test)cargo fmt --all --checkcleanre-review confirmed sound
→ playhead sweeps from 0 again (engine error-path emit is integration-only)
🤖 Co-authored by Claude Opus 4.8 (1M context). Closes #149.