Skip to content

fix: un-freeze the seek bar after a stream restart or state round-trip - #61

Merged
chrisuthe merged 2 commits into
masterfrom
fix/seek-bar-freeze
Jul 20, 2026
Merged

fix: un-freeze the seek bar after a stream restart or state round-trip#61
chrisuthe merged 2 commits into
masterfrom
fix/seek-bar-freeze

Conversation

@chrisuthe

Copy link
Copy Markdown
Owner

Summary

The progress bar could stop advancing permanently — audio kept playing while the bar sat still for the rest of the track. Found by comparing our implementation against the protocol spec and the reference implementations, not from logs.

Root cause: the anchor lifecycle was driven by PlaybackState, but only fresh progress could ever rebuild an anchor. Two real paths reached the dead state:

  1. Stream restart. The SDK synthesizes state transitions the server never sent — it forces Playing on stream/start and Idle on stream/end. A format renegotiation (or a server implementing seek as end+start) emits Idle then Playing, both carrying the same carried-forward PlaybackProgress instance. Freeze() ran on Idle; nothing was fresh, so the Playing event could not re-anchor.
  2. Resume from a server that signals via group/update only, which carries just playback_state/group_id/group_name — no progress to re-anchor from.

Spec basis

Validated against Sendspin/spec @ 3632c68:

  • playback_state is only 'playing' | 'stopped' (README:681) — there is no paused state, so playback state cannot be the pause signal.
  • playback_speed: 0 means paused (README:1448) — this is the protocol's only pause signal.
  • Inside progress, track_progress/track_duration/playback_speed are all required (README:1446-1448), so a conformant server always sends the speed when it sends progress.
  • Progress is mandated only on play/pause/resume/seek/speed-change (README:1445), with no periodic guarantee — so a frozen bar may never self-heal.

Changes

  • Speed factor derives from playback_speed alone; the group playback state no longer overrides it.
  • New TrackProgressTracker.Resume(now) re-anchors at the current displayed position with now as the anchor time and the last known non-zero speed. Anchoring at now is what prevents a jump forward by the paused duration. No-op when already advancing, when no position is known yet, or after a pending track-change reset.
  • MainViewModel.OnPlaybackStateChanged: Playing → Resume(), otherwise Freeze().
  • Removed the PlaybackState != Playing early return in the position timer. It was wrong in both directions: on a conformant pause the state stays playing so it did nothing, and during a synthesized Idle it discarded ticks the tracker had already judged correct. The tracker is now authoritative; genuine stops are still covered by Freeze() nulling the anchor.
  • The retained playbackState parameter now emits a Debug log when speed and state disagree — useful signal for diagnosing non-conformant servers.
  • Design note: docs/superpowers/specs/2026-07-20-seek-bar-freeze-design.md.

Tests

8 new cases covering both trigger paths, resume-in-place (no pause-duration jump), speed-0 anchors, and the no-op guards. Tracker suite 43/43. Full suite 140/142 — the 2 failures are the known pre-existing resampler ones, unchanged. Release build clean at the 553-warning baseline.

Four existing tests were rewritten because they encoded the bug: two asserted that a non-playing state overrides an explicit playback_speed, and depended on a PlaybackState.Paused the protocol never emits plus a playback_speed the spec makes required. They passed only because the test supplied both sides of a boundary that never behaves that way on the wire.

Manual verification

  1. Pause ~20s in, wait ~30s, resume → bar holds while paused, resumes from ~20s (not ~50s, not frozen).
  2. Track change from MA and from the client's own transport → resets to 0 and advances.
  3. Stream restart (seek in MA, or change output device in Settings) → bar resumes from the correct position. This is the case that killed the bar for the rest of the track before this change.

The progress bar could stop advancing permanently. Four pieces combined so
that nothing could rebuild the extrapolation anchor:

- ApplyMetadata let the group playback_state override the progress object's
  playback_speed, anchoring at effective speed 0 for any non-Playing state.
- Freeze() nulls the anchor, and only fresh progress (a new PlaybackProgress
  instance, detected by ReferenceEquals) ever rebuilt it.
- OnPlaybackStateChanged called Freeze() on every transition away from Playing
  with no counterpart on the way back.
- OnPositionTimerTick returned early unless PlaybackState was Playing.

The SDK synthesizes state transitions the server never sent, forcing Playing on
stream/start and Idle on stream/end. A stream restart (format renegotiation, or
a server implementing seek as end + start) therefore produces Idle then Playing,
both carrying the same carried-forward progress instance. Nothing was fresh,
Freeze() had run on Idle, and the Playing event could not re-anchor: the bar was
dead for the rest of the track. A server signalling resume via a group/update
carrying only playback_state hit the same dead end.

Per spec (Sendspin/spec @ 3632c68) there is no 'paused' playback_state
(README:681); playback_speed = 0 is the protocol's only pause signal
(README:1448), and it is a required field whenever progress is sent
(README:1446-1448). The state override was modelling a state the protocol does
not have, using a signal that is not the pause signal.

Derive advance/freeze from playback_speed alone, and add Resume(now) as the
counterpart to Freeze(): with no anchor or a speed-0 anchor it re-anchors at the
currently displayed position, using now as the anchor instant so the paused wall
time is never added to the position, and the last known non-zero speed factor
(1.0 until the server reports otherwise). It is a no-op while the position is
already advancing and while no server-confirmed position exists, including after
ResetForPendingTrackChange(). This matches the SendspinDroid reference. Freeze()
itself is unchanged - capturing the last position on a genuine stop was always
correct.

Also remove the PlaybackState gate in OnPositionTimerTick. The tracker is now
authoritative: it returns null with no anchor and holds the position still on a
speed-0 anchor, so the gate could only suppress updates the tracker had already
decided were correct. For a conformant pause the state stays 'playing', so it
did nothing useful there, while for the SDK-synthesized Idle it silently
discarded ticks - re-introducing exactly the coupling this fix removes.

Two existing tests encoded the old semantics and were rewritten.
PauseWithFreshProgress_OmittingSpeed_StaysFrozen becomes
ConformantPause_SpeedZeroWhilePlaying_StaysFrozen: a spec-conformant pause
carries speed 0, so it no longer needs the state to freeze.
PauseWithFreshProgress_ExplicitNormalSpeed_StaysFrozen asserted that a
not-playing state wins over an explicit speed of 1000 - the bug itself - and is
replaced by FreshProgressWithSpeed_AdvancesRegardlessOfGroupState, which asserts
what the spec requires. Two more now express a pause as speed 0 rather than a
PlaybackState.Paused the protocol never emits.
Records the root cause, the spec lines that settle it (Sendspin/spec @ 3632c68 README:681, 1445-1448), the fix, and a behavior matrix covering conformant pause/resume, stream restart, non-conformant resume, and track change.
@chrisuthe
chrisuthe merged commit 5cd88a7 into master Jul 20, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant