crabidy/architecture/seek.md

225 lines
11 KiB
Markdown

# Seek within the playing track
## Context
Playback exposes track-level controls only: `TogglePlay`, `Next`, `Prev`,
`RestartTrack`. There is no way to move inside a track, which hurts most
where tracks are longest — the podcast providers (`/rss`, `/fyyd`) and
audiobooks (`/abs`), where "I missed that sentence" and "skip the ad" are
the two most common wishes.
The audio engine can already do it: `PlayerEngine::seek_to` and
`PlayerEngineCommand::SeekTo` exist and are wired through
`Player::seek_to`. **Nothing calls them.** There is no RPC, no playback
command, no binding — and the one implementation that exists carries a
panic (D5). So this feature is almost entirely *wiring*, plus a decision
about where the arithmetic lives.
Goal: a step of about 15 seconds forward and backward, from every client.
## Assumptions
Stated explicitly and settled by reading the code rather than by asking:
- **A1 — Within the current track only.** A backward seek at 3 s lands at
0, it does not step into the previous track; `Ctrl-p` is the
track-level control and stays that way. Crossing tracks would need the
queue lock and the previous track's duration, for a gesture nobody
expects to do that.
- **A2 — The server owns the position.** Clients render `TrackPosition`
from the update stream and never predict it, exactly as they do for
volume and play state. A seek therefore needs no optimistic UI and no
rollback when it is refused.
- **A3 — Fire-and-forget.** Like every other playback RPC, `Seek` returns
as soon as the command is queued. A refused seek is a server-side
warning, not a client-visible error (D7).
- **A4 — No autoplay.** Seeking with nothing loaded does nothing; it does
not start the queue. `TogglePlay` and `RestartTrack` are the controls
that resume an idle player.
## Options
The only real question is **where the arithmetic lives**: a seek is
relative ("15 seconds back"), but the engine seeks to an absolute
position.
### Option A — the client computes the target
`Seek(position_millis)`. Each client takes the last `TrackPosition` it
received, adds ±15 s, clamps against the duration it was told, and sends
an absolute target.
- The wire is a plain absolute seek, which a click-on-the-progress-bar
gesture also wants.
- But the base is **stale**: positions are broadcast on a 250 ms tick and
then cross the network. The step is 15 s, so 250 ms of drift is not the
problem — **repetition** is. Press `.` three times quickly and all
three presses read the *same* broadcast position, compute the *same*
target, and the track advances 15 s instead of 45. That is the normal
way people use a seek key.
- Every client re-implements clamping, so the end-of-track and
unknown-duration edges have to be right in three places (TUI, web,
CLI) instead of one.
- While **paused** no position updates arrive at all (the tick loop skips
a paused sink), so a client's base position goes stale the moment you
pause, and a paused seek would be computed from wherever playback
stopped rather than from where the last seek left it.
### Option B — the client sends a delta *(chosen)*
`Seek(delta_millis)`, signed. The engine adds it to the live sink
position, clamps, and seeks.
- Repeated presses compose exactly: each one reads the position the
previous one produced, because the engine is a single-threaded command
loop and `try_seek` updates the position before returning.
- Clamping policy lives in one place, next to the duration the engine
already tracks.
- Clients get simpler, not more complex: a constant and one RPC call.
- Cost: absolute seek is not on the wire. It is a compatible proto3
addition when a click-to-seek gauge wants it (deferred, below).
**Decision: Option B.** The composition argument decides it — Option A is
wrong in the ordinary case of pressing the key twice.
## Decisions
- **D1 — The delta crosses the wire; the engine accumulates.** Clients
send a signed offset, never a target.
- **D2 — One new RPC, relative only.** `Seek(SeekRequest) -> SeekResponse`
with `sint32 delta_millis = 1`. Milliseconds because `TrackPosition`
already speaks milliseconds, so a future sub-second step needs no wire
change; `sint32` because zigzag encoding keeps negatives at one byte.
Absolute seek stays deferred — adding a field later is compatible.
- **D3 — The step size is a client constant.** `SEEK_STEP` = 15 s, one
named constant per client. The wire carries milliseconds, so making the
step configurable later is a client-only change.
- **D4 — Clamping.** Backward past the start lands at 0. Forward past the
end clamps to `duration - 1 s` when the duration is known, so the track
runs out through the ordinary end-of-stream path and advances to the
next one; the engine never has to seek *to* the exact end, whose
behaviour differs per decoder. With an **unknown** duration (a
length-less stream reports 0) there is no upper clamp — the decoder
decides, and a refusal is just a warning.
- **D5 — Fix the panic in `seek_to`.** It currently does
`time.clamp(Duration::from_secs(1), duration)`. `Ord::clamp` asserts
`min <= max`, and `duration()` yields **0** whenever the duration is
unknown (HLS, some network streams) — so that call panics the engine
thread, taking audio down with it, on any duration-less source. It is
unreachable today only because nothing calls `seek_to`; wiring seek
makes it reachable from **user input**, which the hard rules forbid.
Replaced by saturating arithmetic with no assertions. The 1-second
floor also went: landing at 0 is exactly what a backward seek near the
start means.
- **D6 — The engine reports the new position immediately.** After a
successful seek it emits `PlayerMessage::Elapsed` rather than waiting
for the next 250 ms tick. This is not just about latency: `tick()`
returns early for a paused or empty sink, so without this a seek while
paused would leave every client showing the old position until playback
resumed.
- **D7 — Unseekable sources warn and change nothing.** HLS (SoundCloud)
is decoded with `seekable = false` precisely so symphonia never seeks
it, and `try_seek` reports `NotSupported`. The server logs a warning
and broadcasts nothing; clients keep showing the true position because
they never moved it (A2). Consistent with how `pause` and
`set_volume` failures are handled — no new status codes, no new
client-side error path.
- **D8 — Bindings: `Ctrl-b` back, `Ctrl-f` forward**, in the **global**
scope of both the TUI and the web client (the user's choice; an earlier
round used `,`/`.`). They join the existing control-chord family for
playback and movement — `Ctrl-n`/`Ctrl-p` for tracks, `Ctrl-d`/`Ctrl-u`
for paging — and read as vim's back/forward-a-screen pair. Neither chord
was bound in any scope of either client, and plain `f` (spectrum) is
untouched because the TUI's `lookup` compares every modifier except
`SHIFT` exactly. In the browser, `Ctrl-f` would otherwise open the find
bar; the web keydown handler already calls `prevent_default` on any
chord that resolves to an action, so binding it is enough to claim it.
The web transport bar also gets `⏪`/`⏩` buttons, and the CLI gets
`cbd global seek <SECONDS>`, which accepts negatives.
- **D9 — No feature flag.** Seek is a few lines in the engine and one RPC
arm; it carries no dependency of its own, so by the rule in
`architecture/build-features.md` ("a feature must pay for itself in
dependencies") it does not earn one.
## Structure
```d2
direction: right
clients: Clients {
tui: cbd-tui\n`,` / `.`
web: cbd-web\n`,` / `.` / ⏪⏩
cli: cbd-cli\nglobal seek N
}
server: crabidy-server {
rpc: RpcService::seek
loop: playback loop\nPlaybackCommand::Seek
fwd: poll_play_bus
}
engine: audio-player {
player: Player::seek_by
eng: PlayerEngine\nseek_by -> seek_clamped
sink: rodio Player\ntry_seek
}
clients.tui -> server.rpc: Seek{delta_millis}
clients.web -> server.rpc: Seek{delta_millis}
clients.cli -> server.rpc: Seek{delta_millis}
server.rpc -> server.loop: bounded channel
server.loop -> engine.player: seek_by(delta)
engine.player -> engine.eng: SeekBy(delta, reply)
engine.eng -> engine.sink: try_seek(clamped target)
engine.eng -> server.fwd: PlayerMessage::Elapsed
server.fwd -> server.loop: PositionChanged
server.loop -> clients: TrackPosition broadcast
```
The delta stays a delta all the way down to the engine; the only place an
absolute position is computed is `PlayerEngine::seek_by`, which is also
the only place that knows the live sink position and the track duration.
## Boundaries
- **`audio-player`** gains `Player::seek_by(delta_millis: i64)` and
`PlayerEngineCommand::SeekBy`. The clamping helper is private; both
`seek_to` (absolute, kept for the deferred extension) and `seek_by` go
through it, so there is one clamping policy, not two.
- **`crabidy-core`** gains the `Seek` RPC and its two messages.
- **`crabidy-server`** gains `PlaybackCommand::Seek { delta_millis }` and
the RPC arm. The playback loop only forwards; it holds no seek state.
- **Clients** gain an action, a binding, an RPC wrapper, and a constant
each. No client-side position arithmetic anywhere (A2, D1).
## Risks
- **`try_seek` blocks the engine thread** for up to ~5 ms (it waits for
the audio callback to pick the order up). That thread already blocks
for up to 30 s opening a network stream, so this is not a new class of
stall — but it does mean seek is serialized behind an in-flight track
open, which is correct anyway.
- **Seeking while paused** relies on rodio's `periodic_access` sitting
*outside* `pausable` in the chain: a paused sink keeps being polled for
silence, so the seek order is still picked up. Verified in rodio 0.22.2
(`src/player.rs`); if a future rodio inverts that order, a paused seek
would block until unpause. Worth re-checking on a rodio bump.
- **Network-backed sources** seek inside `stream_download`'s temp
storage. A seek outside the downloaded window triggers a fresh range
request, so a long forward seek can stall audio briefly. Bounded by the
existing HTTP timeouts; no new failure mode.
- **A 15-second step on a very short track** always lands in the clamp,
which is why the clamp has to be arithmetic that cannot assert (D5).
## Deferred
Recorded, not dropped:
- **Absolute seek** (`position_millis`) and a click-to-seek progress
gauge in the web client. The gauge is already rendered; only the wire
field and a click handler are missing. A compatible proto3 addition.
- **Configurable step size** in the client configs, and a larger step on
`<`/`>` (same physical keys, shifted). Client-only once wanted.
- **Chapter-aware seek** for podcasts and audiobooks. No provider exposes
chapter marks through the library model today.