seek: move 15 seconds inside the playing track with Ctrl-b / Ctrl-f

The audio engine could already seek and nothing called it: no rpc, no
playback command, no binding. This wires it from every client.

The one real decision was where the arithmetic lives. A seek is relative
but the engine seeks to an absolute position, so either the client computes
a target from the last position update or it sends an offset and the engine
adds it to the live position. The offset wins on the ordinary case of
pressing the key twice: positions are broadcast on a 250 ms tick and then
cross the network, so three quick presses would all read the same stale
base and jump 15 s instead of 45. It also keeps the clamping policy in one
place instead of three clients, and matters more while paused, where no
position updates arrive at all.

So the wire carries sint32 delta_millis and the step is a client constant.

It also uncovered a live panic: seek_to did
`time.clamp(Duration::from_secs(1), duration)`, and `Ord::clamp` asserts
min <= max while `duration()` returns 0 for any source that reported no
length (HLS, some streams). That panicked the engine thread, killing audio.
Unreachable only because nothing called it; wiring seek made it reachable
from user input. It is now saturating arithmetic in a pure, exhaustively
tested function.

Boundaries: backwards saturates at 0 and never enters the previous track;
forwards stops 1 s short of the end so the track finishes through the
ordinary end-of-stream path (which advances the queue) instead of relying
on seek-to-exact-end, which decoders disagree about; an unknown duration
has no upper clamp. The engine emits Elapsed from the seek path itself,
because tick() skips a paused sink and a paused seek would otherwise show
the old position until playback resumed. An unseekable source (SoundCloud
HLS) warns server-side and changes nothing.

Ctrl-b/Ctrl-f join the existing control-chord family; plain f still toggles
the spectrum because lookup compares every modifier but SHIFT exactly. In
the browser Ctrl-f would open the find bar, but the keydown handler already
prevent_defaults any chord that resolves.

Seek is deliberately not tested through the playback loop: every test there
builds a real Player whose engine thread opens an audio device, so a test
that awaits a player reply passes or hangs depending on whether the machine
has working audio. The arithmetic is tested as a pure function, and the
rpc -> command mapping (the layer the paste bug lived in) in rpc.rs.

Verified: 20 audio-player tests (5 new: i64::MIN/MAX, zero duration,
sub-second tracks, composition, near-end saturation), 95 crabidy-server,
119 cbd-tui, 21 cbd-web, 58 server tests with --no-default-features,
workspace clippy clean under -D warnings, fmt clean, wasm bundle and book
build. Not exercised: an actual seek through an audio device.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Test User 2026-07-26 14:16:08 +02:00
parent 378066470e
commit d0b01c75d0
22 changed files with 924 additions and 16 deletions

224
architecture/seek.md Normal file
View File

@ -0,0 +1,224 @@
# 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.

View File

@ -109,6 +109,22 @@ impl Player {
rx.recv_async().await? rx.recv_async().await?
} }
/// Seeks `delta_millis` from the current position — negative seeks back —
/// and resolves to the position actually reached, clamped to the track.
///
/// Relative rather than absolute on purpose: the engine adds the offset to
/// the live position, so pressing a seek key several times in a row
/// composes instead of repeatedly targeting the same place
/// (architecture/seek.md D1). Fails when nothing is loaded, and when the
/// source cannot seek at all (HLS).
pub async fn seek_by(&self, delta_millis: i64) -> Result<Duration> {
let (tx, rx) = flume::bounded(1);
self.tx_engine
.send_async(PlayerEngineCommand::SeekBy(delta_millis, tx))
.await?;
rx.recv_async().await?
}
pub async fn volume(&self) -> Result<f32> { pub async fn volume(&self) -> Result<f32> {
let (tx, rx) = flume::bounded(1); let (tx, rx) = flume::bounded(1);
self.tx_engine self.tx_engine

View File

@ -38,6 +38,44 @@ fn display_source(source_str: &str) -> String {
/// Interval between elapsed-position updates while playing. /// Interval between elapsed-position updates while playing.
const TICK_INTERVAL: Duration = Duration::from_millis(250); const TICK_INTERVAL: Duration = Duration::from_millis(250);
/// How close to the end of a track a forward seek may land.
///
/// Seeking to the exact end is decoder-dependent — symphonia, the Opus
/// reader and a windowed http stream all disagree about whether that is an
/// error — so an overshooting forward seek stops here instead and lets the
/// track run out through the ordinary end-of-stream path, which advances the
/// queue (architecture/seek.md D4).
const SEEK_END_MARGIN: Duration = Duration::from_secs(1);
/// The absolute position a seek of `delta_millis` from `position` should land
/// on, given the track's `duration` if it is known.
///
/// Pure arithmetic, and deliberately free of assertions: it is driven
/// straight from user input, so every input — a delta larger than the track,
/// `i64::MIN`, a duration of zero, a track shorter than the step — has to
/// produce a position rather than a panic. (The previous implementation used
/// `clamp(Duration::from_secs(1), duration)`, which asserts `min <= max` and
/// therefore panicked on every duration-less stream.)
///
/// Saturates at 0 going back — a backward seek near the start means "go to
/// the beginning", not "go to the previous track". Going forward it stops
/// [`SEEK_END_MARGIN`] short of the end when the duration is known; a
/// length-less stream reports no duration, and then there is nothing to clamp
/// against, so the target passes through and the decoder decides whether it
/// can honour it.
fn seek_target(position: Duration, delta_millis: i64, duration: Option<Duration>) -> Duration {
let step = Duration::from_millis(delta_millis.unsigned_abs());
let target = if delta_millis < 0 {
position.saturating_sub(step)
} else {
position.saturating_add(step)
};
match duration {
Some(total) if !total.is_zero() => target.min(total.saturating_sub(SEEK_END_MARGIN)),
_ => target,
}
}
pub enum PlayerEngineCommand { pub enum PlayerEngineCommand {
Play(String, Sender<Result<MediaInfo>>), Play(String, Sender<Result<MediaInfo>>),
SetVolume(f32, Sender<f32>), SetVolume(f32, Sender<f32>),
@ -49,6 +87,10 @@ pub enum PlayerEngineCommand {
GetDuration(Sender<Result<Duration>>), GetDuration(Sender<Result<Duration>>),
GetElapsed(Sender<Result<Duration>>), GetElapsed(Sender<Result<Duration>>),
SeekTo(Duration, Sender<Result<Duration>>), SeekTo(Duration, Sender<Result<Duration>>),
/// Seek by a signed offset in milliseconds from the live position
/// (negative seeks back). Relative rather than absolute so repeated
/// presses compose (architecture/seek.md D1).
SeekBy(i64, Sender<Result<Duration>>),
GetVolume(Sender<f32>), GetVolume(Sender<f32>),
ToggleMute(Sender<bool>), ToggleMute(Sender<bool>),
GetPaused(Sender<Result<bool>>), GetPaused(Sender<Result<bool>>),
@ -243,6 +285,9 @@ impl PlayerEngine {
PlayerEngineCommand::GetDuration(tx) => send_reply(tx, self.duration()), PlayerEngineCommand::GetDuration(tx) => send_reply(tx, self.duration()),
PlayerEngineCommand::GetElapsed(tx) => send_reply(tx, self.elapsed()), PlayerEngineCommand::GetElapsed(tx) => send_reply(tx, self.elapsed()),
PlayerEngineCommand::SeekTo(time, tx) => send_reply(tx, self.seek_to(time)), PlayerEngineCommand::SeekTo(time, tx) => send_reply(tx, self.seek_to(time)),
PlayerEngineCommand::SeekBy(delta_millis, tx) => {
send_reply(tx, self.seek_by(delta_millis));
}
PlayerEngineCommand::SetVolume(volume, tx) => { PlayerEngineCommand::SetVolume(volume, tx) => {
send_reply(tx, self.set_volume(volume)); send_reply(tx, self.set_volume(volume));
} }
@ -541,13 +586,57 @@ impl PlayerEngine {
Ok(self.sink.get_pos()) Ok(self.sink.get_pos())
} }
/// Seeks to an absolute position, clamped by [`seek_target`]. Returns the
/// position actually reached.
pub fn seek_to(&self, time: Duration) -> Result<Duration> { pub fn seek_to(&self, time: Duration) -> Result<Duration> {
let duration = self.duration().unwrap_or_else(|_| self.sink.get_pos()); if self.is_stopped() {
let time = time.clamp(Duration::from_secs(1), duration); return Err(PlayerEngineError::NotPlaying.into());
}
self.seek_sink(seek_target(time, 0, self.known_duration()))
}
/// Seeks `delta_millis` from the **live** playback position (a negative
/// delta seeks back), clamped by [`seek_target`]. Returns the position
/// actually reached.
///
/// The offset is applied here rather than by the caller so that repeated
/// presses compose: each seek starts from where the previous one landed,
/// not from a position update that was broadcast up to a tick ago
/// (architecture/seek.md D1). Seeking with nothing loaded is an error, not
/// a way to start playback.
pub fn seek_by(&self, delta_millis: i64) -> Result<Duration> {
if self.is_stopped() {
return Err(PlayerEngineError::NotPlaying.into());
}
let target = seek_target(self.sink.get_pos(), delta_millis, self.known_duration());
self.seek_sink(target)
}
/// Seeks the sink to an already-clamped absolute position and reports the
/// position reached.
///
/// Reporting from here rather than leaving it to [`Self::tick`] is not
/// only about latency: the tick returns early for a paused sink, so
/// without this a seek while paused would leave every client showing the
/// pre-seek position until playback resumed (architecture/seek.md D6).
fn seek_sink(&self, target: Duration) -> Result<Duration> {
self.sink self.sink
.try_seek(time) .try_seek(target)
.map_err(|err| anyhow!("seek failed: {err}"))?; .map_err(|err| anyhow!("seek failed: {err}"))?;
Ok(self.sink.get_pos()) let elapsed = self.sink.get_pos();
debug!(?target, ?elapsed, "seeked");
self.notify(PlayerMessage::Elapsed {
duration: self.known_duration().unwrap_or_default(),
elapsed,
});
Ok(elapsed)
}
/// The current track's total duration when the source reported one.
/// `None` covers both "no track loaded" and "a length-less stream", which
/// clamping treats alike.
fn known_duration(&self) -> Option<Duration> {
self.media_info.as_ref().and_then(|info| info.duration)
} }
/// The user's intended volume — the level playback would resume at, /// The user's intended volume — the level playback would resume at,
@ -637,6 +726,107 @@ fn read_header<R: Read>(reader: &mut R, buf: &mut [u8]) -> Result<usize> {
mod tests { mod tests {
use super::*; use super::*;
/// A 10-minute track, the ordinary case.
fn ten_minutes() -> Option<Duration> {
Some(Duration::from_secs(600))
}
#[test]
fn seek_target_moves_by_the_delta() {
let at = Duration::from_secs(100);
assert_eq!(
seek_target(at, 15_000, ten_minutes()),
Duration::from_secs(115)
);
assert_eq!(
seek_target(at, -15_000, ten_minutes()),
Duration::from_secs(85)
);
}
/// Successive seeks compose because each one starts from the position the
/// previous one produced — the reason the delta is applied here and not by
/// the client (architecture/seek.md D1).
#[test]
fn successive_seeks_compose() {
let mut at = Duration::from_secs(100);
for _ in 0..3 {
at = seek_target(at, -15_000, ten_minutes());
}
assert_eq!(at, Duration::from_secs(55));
}
#[test]
fn seek_target_saturates_at_the_start() {
// Back past the beginning lands at 0 — not below, and not in the
// previous track.
assert_eq!(
seek_target(Duration::from_secs(3), -15_000, ten_minutes()),
Duration::ZERO
);
assert_eq!(
seek_target(Duration::ZERO, -15_000, ten_minutes()),
Duration::ZERO
);
}
/// Forward past the end stops a second short of it, so the track finishes
/// through the ordinary end-of-stream path and the queue advances.
#[test]
fn seek_target_saturates_near_the_end() {
assert_eq!(
seek_target(Duration::from_secs(595), 15_000, ten_minutes()),
Duration::from_secs(599)
);
}
/// A length-less stream reports no duration. The naive clamp turned that
/// into "seek to zero" (or panicked); the target has to pass through so
/// the decoder can decide.
#[test]
fn an_unknown_duration_does_not_clamp_forward() {
let at = Duration::from_secs(100);
assert_eq!(seek_target(at, 15_000, None), Duration::from_secs(115));
// `MediaInfo` carries `Some(0)` for some sources; same meaning.
assert_eq!(
seek_target(at, 15_000, Some(Duration::ZERO)),
Duration::from_secs(115)
);
}
/// Every extreme has to yield a position rather than a panic: the deltas
/// come from user input, and `duration` from whatever a feed claimed.
#[test]
fn seek_target_never_panics_on_extremes() {
let positions = [Duration::ZERO, Duration::from_secs(1), Duration::MAX];
let deltas = [i64::MIN, -1, 0, 1, i64::MAX];
let durations = [
None,
Some(Duration::ZERO),
// A track shorter than the end margin: the ceiling saturates to 0.
Some(Duration::from_millis(400)),
Some(Duration::from_secs(600)),
Some(Duration::MAX),
];
for position in positions {
for delta in deltas {
for duration in durations {
let _ = seek_target(position, delta, duration);
}
}
}
// The case that used to panic outright: unknown duration, and a
// sub-second track where the old floor exceeded the old ceiling.
assert_eq!(
seek_target(Duration::ZERO, 15_000, None),
Duration::from_secs(15)
);
assert_eq!(
seek_target(Duration::ZERO, 15_000, Some(Duration::from_millis(400))),
Duration::ZERO
);
}
#[test] #[test]
fn logged_sources_never_carry_url_tokens() { fn logged_sources_never_carry_url_tokens() {
// Stream URLs embed access tokens; only scheme and host may be // Stream URLs embed access tokens; only scheme and host may be

View File

@ -20,8 +20,8 @@ use crabidy_core::proto::crabidy::{
ChangeVolumeRequest, ClearQueueRequest, CreateLibraryNodeRequest, DeleteLibraryNodeRequest, ChangeVolumeRequest, ClearQueueRequest, CreateLibraryNodeRequest, DeleteLibraryNodeRequest,
GetLibraryNodeRequest, InitRequest, InsertRequest, LibraryNode, NextRequest, PrevRequest, GetLibraryNodeRequest, InitRequest, InsertRequest, LibraryNode, NextRequest, PrevRequest,
Queue, RemoveRequest, RenameLibraryNodeRequest, ReplaceRequest, RestartTrackRequest, Queue, RemoveRequest, RenameLibraryNodeRequest, ReplaceRequest, RestartTrackRequest,
SaveQueueRequest, SetCurrentRequest, StopRequest, ToggleMuteRequest, TogglePlayRequest, SaveQueueRequest, SeekRequest, SetCurrentRequest, StopRequest, ToggleMuteRequest,
ToggleRepeatRequest, ToggleShuffleRequest, Track, TogglePlayRequest, ToggleRepeatRequest, ToggleShuffleRequest, Track,
}; };
use crate::{Connection, GlobalCmd, LibraryCmd, QueueCmd, RemoteCmd}; use crate::{Connection, GlobalCmd, LibraryCmd, QueueCmd, RemoteCmd};
@ -281,6 +281,16 @@ async fn run_global(client: &mut Client, cmd: GlobalCmd) -> Result<(), Box<dyn s
.map_err(rpc_error)?; .map_err(rpc_error)?;
println!("restarted the current track"); println!("restarted the current track");
} }
GlobalCmd::Seek { seconds } => {
// Milliseconds on the wire; the server clamps to the track.
client
.seek(SeekRequest {
delta_millis: seconds.saturating_mul(1_000),
})
.await
.map_err(rpc_error)?;
println!("seeked {seconds:+} seconds");
}
GlobalCmd::Mute => { GlobalCmd::Mute => {
client client
.toggle_mute(ToggleMuteRequest {}) .toggle_mute(ToggleMuteRequest {})

View File

@ -113,6 +113,13 @@ pub enum GlobalCmd {
Prev, Prev,
/// Restart the current track. /// Restart the current track.
Restart, Restart,
/// Seek inside the current track by a number of seconds (negative seeks
/// back, e.g. `-15`). Stays within the track: it saturates at the start,
/// and overshooting the end lets the track finish and the queue advance.
Seek {
#[arg(allow_negative_numbers = true)]
seconds: i32,
},
/// Toggle mute. /// Toggle mute.
Mute, Mute,
/// Change the volume by a delta in [-1.0, 1.0] (e.g. `0.05`, `-0.1`). /// Change the volume by a delta in [-1.0, 1.0] (e.g. `0.05`, `-0.1`).

View File

@ -42,6 +42,14 @@ pub enum Action {
ToggleRepeat, ToggleRepeat,
NextTrack, NextTrack,
PrevTrack, PrevTrack,
/// Move the playing position back inside the current track by
/// [`super::SEEK_STEP_MILLIS`]. Never leaves the track: near the start it
/// simply lands at 0 (architecture/seek.md A1).
SeekBackward,
/// Move the playing position forward inside the current track by
/// [`super::SEEK_STEP_MILLIS`]. Overshooting the end lets the track
/// finish, which advances the queue.
SeekForward,
/// Show or hide the frequency-spectrum visualizer in the now-playing /// Show or hide the frequency-spectrum visualizer in the now-playing
/// pane (client-side only; the server keeps streaming the bars). /// pane (client-side only; the server keeps streaming the bars).
ToggleSpectrum, ToggleSpectrum,
@ -229,6 +237,20 @@ pub const BINDINGS: &[Binding] = &[
action: Action::PrevTrack, action: Action::PrevTrack,
description: "Previous track", description: "Previous track",
}, },
Binding {
scope: Scope::Global,
mods: KeyModifiers::CONTROL,
code: KeyCode::Char('b'),
action: Action::SeekBackward,
description: "Seek back 15 seconds",
},
Binding {
scope: Scope::Global,
mods: KeyModifiers::CONTROL,
code: KeyCode::Char('f'),
action: Action::SeekForward,
description: "Seek forward 15 seconds",
},
Binding { Binding {
scope: Scope::Global, scope: Scope::Global,
mods: KeyModifiers::NONE, mods: KeyModifiers::NONE,
@ -858,6 +880,41 @@ mod tests {
); );
} }
/// Seek rides the control chords in every focus, and taking `Ctrl-f` must
/// not disturb plain `f` — the modifier comparison is exact for anything
/// but `SHIFT`.
#[test]
fn seek_is_on_the_control_chords_in_any_focus() {
for focus in [UiFocus::Library, UiFocus::Queue] {
assert_eq!(
lookup(focus, false, key(KeyCode::Char('f'), KeyModifiers::CONTROL)),
Some(Action::SeekForward)
);
assert_eq!(
lookup(focus, false, key(KeyCode::Char('b'), KeyModifiers::CONTROL)),
Some(Action::SeekBackward)
);
// Unmodified, these keep their old meanings (or none at all).
assert_eq!(
lookup(focus, false, key(KeyCode::Char('f'), KeyModifiers::NONE)),
Some(Action::ToggleSpectrum)
);
assert_eq!(
lookup(focus, false, key(KeyCode::Char('b'), KeyModifiers::NONE)),
None
);
}
// And the help modal still swallows them.
assert_eq!(
lookup(
UiFocus::Library,
true,
key(KeyCode::Char('f'), KeyModifiers::CONTROL)
),
None
);
}
#[test] #[test]
fn spectrum_moved_off_v_to_f() { fn spectrum_moved_off_v_to_f() {
// The frequency-spectrum toggle now lives on Global `f`, in any focus. // The frequency-spectrum toggle now lives on Global `f`, in any focus.

View File

@ -79,6 +79,15 @@ pub const COLOR_GREEN: Color = Color::Rgb(163, 190, 140);
/// provider). /// provider).
const CURRENT_QUEUE_PATH: &str = "/crabidy/current"; const CURRENT_QUEUE_PATH: &str = "/crabidy/current";
/// How far one press of a seek key moves the playing position, in
/// milliseconds — the unit the wire speaks, so any step is expressible
/// (architecture/seek.md D3).
///
/// The step lives here, in the client: the server is handed an offset and adds
/// it to the live position, which is what makes repeated presses compose. The
/// help text and the docs spell the number out, so change those too.
pub(crate) const SEEK_STEP_MILLIS: i64 = 15_000;
// FIXME: Rename this // FIXME: Rename this
pub enum MessageToUi { pub enum MessageToUi {
Init(InitialData), Init(InitialData),
@ -127,6 +136,11 @@ pub enum MessageFromUi {
NextTrack, NextTrack,
PrevTrack, PrevTrack,
RestartTrack, RestartTrack,
/// Move the position inside the playing track by a signed millisecond
/// offset. Relative: the server adds it to the live position, so holding
/// the key composes instead of aiming at the same spot every time
/// (architecture/seek.md D1).
Seek(i64),
SetCurrentTrack(usize), SetCurrentTrack(usize),
TogglePlay, TogglePlay,
ChangeVolume(f32), ChangeVolume(f32),
@ -525,6 +539,12 @@ impl App {
} }
Action::NextTrack => self.queue.play_next(), Action::NextTrack => self.queue.play_next(),
Action::PrevTrack => self.queue.play_prev(), Action::PrevTrack => self.queue.play_prev(),
Action::SeekBackward => {
let _ = self.tx.send(MessageFromUi::Seek(-SEEK_STEP_MILLIS));
}
Action::SeekForward => {
let _ = self.tx.send(MessageFromUi::Seek(SEEK_STEP_MILLIS));
}
Action::ToggleSpectrum => self.now_playing.toggle_spectrum(), Action::ToggleSpectrum => self.now_playing.toggle_spectrum(),
Action::LibraryFirst => self.library_move(|l| l.first()), Action::LibraryFirst => self.library_move(|l| l.first()),
Action::LibraryLast => self.library_move(|l| l.last()), Action::LibraryLast => self.library_move(|l| l.last()),
@ -834,6 +854,19 @@ mod tests {
let _ = app.dispatch(Action::RestartTrack); let _ = app.dispatch(Action::RestartTrack);
assert!(matches!(rx.try_recv(), Ok(MessageFromUi::RestartTrack))); assert!(matches!(rx.try_recv(), Ok(MessageFromUi::RestartTrack)));
// Seek sends the *step*, signed — never a target position: the server
// adds it to the live position (architecture/seek.md D1).
let _ = app.dispatch(Action::SeekForward);
assert!(
matches!(rx.try_recv(), Ok(MessageFromUi::Seek(d)) if d == SEEK_STEP_MILLIS),
"forward seek must send +one step"
);
let _ = app.dispatch(Action::SeekBackward);
assert!(
matches!(rx.try_recv(), Ok(MessageFromUi::Seek(d)) if d == -SEEK_STEP_MILLIS),
"backward seek must send -one step"
);
let _ = app.dispatch(Action::ToggleMute); let _ = app.dispatch(Action::ToggleMute);
assert!(matches!(rx.try_recv(), Ok(MessageFromUi::ToggleMute))); assert!(matches!(rx.try_recv(), Ok(MessageFromUi::ToggleMute)));

View File

@ -146,6 +146,9 @@ async fn poll(
MessageFromUi::RestartTrack => { MessageFromUi::RestartTrack => {
rpc_client.restart_track().await? rpc_client.restart_track().await?
} }
MessageFromUi::Seek(delta_millis) => {
rpc_client.seek(delta_millis).await?
}
MessageFromUi::SetCurrentTrack(pos) => { MessageFromUi::SetCurrentTrack(pos) => {
rpc_client.set_current_track(pos).await? rpc_client.set_current_track(pos).await?
} }

View File

@ -4,7 +4,7 @@ use crabidy_core::proto::crabidy::{
GetLibraryNodeRequest, GetUpdateStreamRequest, GetUpdateStreamResponse, InitRequest, GetLibraryNodeRequest, GetUpdateStreamRequest, GetUpdateStreamResponse, InitRequest,
InitResponse, InsertRequest, LibraryNode, NextRequest, PrevRequest, QueueRequest, InitResponse, InsertRequest, LibraryNode, NextRequest, PrevRequest, QueueRequest,
RemoveRequest, RenameLibraryNodeRequest, ReplaceRequest, RestartTrackRequest, SaveQueueRequest, RemoveRequest, RenameLibraryNodeRequest, ReplaceRequest, RestartTrackRequest, SaveQueueRequest,
SetCurrentRequest, ToggleMuteRequest, TogglePlayRequest, ToggleRepeatRequest, SeekRequest, SetCurrentRequest, ToggleMuteRequest, TogglePlayRequest, ToggleRepeatRequest,
ToggleShuffleRequest, ToggleShuffleRequest,
}; };
@ -325,6 +325,17 @@ impl RpcClient {
Ok(()) Ok(())
} }
/// Moves the playing position by a signed millisecond offset. The server
/// applies it to the live position and clamps it to the track; a source
/// that cannot seek leaves the position untouched.
pub async fn seek(&mut self, delta_millis: i64) -> Result<(), Box<dyn Error>> {
let seek_request = Request::new(SeekRequest {
delta_millis: delta_millis as i32,
});
self.client.seek(seek_request).await?;
Ok(())
}
pub async fn set_current_track(&mut self, pos: usize) -> Result<(), Box<dyn Error>> { pub async fn set_current_track(&mut self, pos: usize) -> Result<(), Box<dyn Error>> {
let set_current_request = Request::new(SetCurrentRequest { let set_current_request = Request::new(SetCurrentRequest {
position: pos as u32, position: pos as u32,

View File

@ -20,6 +20,12 @@ use crate::state::{
}; };
const VOLUME_STEP: f32 = 0.1; const VOLUME_STEP: f32 = 0.1;
/// How far one seek press moves the playing position, in milliseconds — the
/// unit the wire speaks (architecture/seek.md D3). The step lives in the
/// client; the server only ever receives an offset and applies it to the live
/// position, which is what makes repeated presses compose.
pub(crate) const SEEK_STEP_MILLIS: i32 = 15_000;
const JUMP: isize = 15; const JUMP: isize = 15;
/// Stream reconnect backoff bounds, milliseconds. /// Stream reconnect backoff bounds, milliseconds.
const BACKOFF_MIN_MS: u32 = 1_000; const BACKOFF_MIN_MS: u32 = 1_000;
@ -397,6 +403,10 @@ impl Store {
Action::RestartTrack => self.call(async |mut rpc: Rpc| rpc.restart_track().await), Action::RestartTrack => self.call(async |mut rpc: Rpc| rpc.restart_track().await),
Action::NextTrack => self.call(async |mut rpc: Rpc| rpc.next().await), Action::NextTrack => self.call(async |mut rpc: Rpc| rpc.next().await),
Action::PrevTrack => self.call(async |mut rpc: Rpc| rpc.prev().await), Action::PrevTrack => self.call(async |mut rpc: Rpc| rpc.prev().await),
Action::SeekBackward => {
self.call(async |mut rpc: Rpc| rpc.seek(-SEEK_STEP_MILLIS).await)
}
Action::SeekForward => self.call(async |mut rpc: Rpc| rpc.seek(SEEK_STEP_MILLIS).await),
Action::VolumeUp => { Action::VolumeUp => {
self.call(async |mut rpc: Rpc| rpc.change_volume(VOLUME_STEP).await) self.call(async |mut rpc: Rpc| rpc.change_volume(VOLUME_STEP).await)
} }
@ -1152,8 +1162,12 @@ fn Transport(store: Store) -> impl IntoView {
<div class="controls"> <div class="controls">
<button class="ghost" title="previous (Ctrl-p)" <button class="ghost" title="previous (Ctrl-p)"
on:click=move |_| store.dispatch(Action::PrevTrack)>""</button> on:click=move |_| store.dispatch(Action::PrevTrack)>""</button>
<button class="ghost" title="back 15 seconds (Ctrl-b)"
on:click=move |_| store.dispatch(Action::SeekBackward)>""</button>
<button class="ghost big" title="play/pause (Space)" <button class="ghost big" title="play/pause (Space)"
on:click=move |_| store.dispatch(Action::TogglePlay)>{state_symbol}</button> on:click=move |_| store.dispatch(Action::TogglePlay)>{state_symbol}</button>
<button class="ghost" title="forward 15 seconds (Ctrl-f)"
on:click=move |_| store.dispatch(Action::SeekForward)>""</button>
<button class="ghost" title="next (Ctrl-n)" <button class="ghost" title="next (Ctrl-n)"
on:click=move |_| store.dispatch(Action::NextTrack)>""</button> on:click=move |_| store.dispatch(Action::NextTrack)>""</button>
<button class="ghost" title="restart track (r)" <button class="ghost" title="restart track (r)"

View File

@ -21,6 +21,13 @@ pub enum Action {
ToggleRepeat, ToggleRepeat,
NextTrack, NextTrack,
PrevTrack, PrevTrack,
/// Move the playing position back inside the current track by
/// [`crate::app::SEEK_STEP_MILLIS`]. Near the start it lands at 0 rather
/// than stepping into the previous track (architecture/seek.md A1).
SeekBackward,
/// Move the playing position forward by the same step; overshooting the
/// end lets the track finish, which advances the queue.
SeekForward,
LibraryFirst, LibraryFirst,
LibraryLast, LibraryLast,
LibraryNext, LibraryNext,
@ -89,6 +96,11 @@ pub const HELP: &[HelpEntry] = &[
key: "r", key: "r",
description: "Restart current track", description: "Restart current track",
}, },
HelpEntry {
scope: "Global",
key: "Ctrl-b / Ctrl-f",
description: "Seek back / forward 15 seconds",
},
HelpEntry { HelpEntry {
scope: "Global", scope: "Global",
key: "K", key: "K",
@ -297,6 +309,10 @@ pub fn lookup(focus: Focus, help_open: bool, key: &str, ctrl: bool) -> Option<Ac
Focus::Library => Action::LibraryJumpUp, Focus::Library => Action::LibraryJumpUp,
Focus::Queue => Action::QueueJumpUp, Focus::Queue => Action::QueueJumpUp,
}), }),
// The caller `prevent_default`s a resolved chord, so these take
// precedence over the browser's own Ctrl-f / Ctrl-b.
"f" => Some(Action::SeekForward),
"b" => Some(Action::SeekBackward),
_ => None, _ => None,
}; };
} }
@ -305,6 +321,8 @@ pub fn lookup(focus: Focus, help_open: bool, key: &str, ctrl: bool) -> Option<Ac
"Tab" => Some(Action::CycleFocus), "Tab" => Some(Action::CycleFocus),
" " => Some(Action::TogglePlay), " " => Some(Action::TogglePlay),
"r" => Some(Action::RestartTrack), "r" => Some(Action::RestartTrack),
"," => Some(Action::SeekBackward),
"." => Some(Action::SeekForward),
"K" => Some(Action::VolumeUp), "K" => Some(Action::VolumeUp),
"J" => Some(Action::VolumeDown), "J" => Some(Action::VolumeDown),
"m" => Some(Action::ToggleMute), "m" => Some(Action::ToggleMute),
@ -377,6 +395,24 @@ mod tests {
); );
} }
/// Seek is a control chord in either pane, and claiming `Ctrl-f` leaves
/// plain `f` alone (the ctrl branch is checked first and separately).
#[test]
fn seek_is_on_the_control_chords_in_both_panes() {
for focus in [Focus::Library, Focus::Queue] {
assert_eq!(lookup(focus, false, "f", true), Some(Action::SeekForward));
assert_eq!(lookup(focus, false, "b", true), Some(Action::SeekBackward));
}
// Unmodified `b` is unbound; unmodified `f` is not seek.
assert_eq!(lookup(Focus::Library, false, "b", false), None);
assert_ne!(
lookup(Focus::Library, false, "f", false),
Some(Action::SeekForward)
);
// Help is modal for control chords too.
assert_eq!(lookup(Focus::Library, true, "f", true), None);
}
#[test] #[test]
fn globals_win_in_both_panes() { fn globals_win_in_both_panes() {
for focus in [Focus::Library, Focus::Queue] { for focus in [Focus::Library, Focus::Queue] {

View File

@ -9,7 +9,7 @@ use crabidy_core::proto::crabidy::{
ChangeVolumeRequest, ClearQueueRequest, CreateLibraryNodeRequest, DeleteLibraryNodeRequest, ChangeVolumeRequest, ClearQueueRequest, CreateLibraryNodeRequest, DeleteLibraryNodeRequest,
GetLibraryNodeRequest, GetUpdateStreamRequest, GetUpdateStreamResponse, InitRequest, GetLibraryNodeRequest, GetUpdateStreamRequest, GetUpdateStreamResponse, InitRequest,
InsertRequest, LibraryNode, NextRequest, PrevRequest, QueueRequest, RemoveRequest, InsertRequest, LibraryNode, NextRequest, PrevRequest, QueueRequest, RemoveRequest,
RenameLibraryNodeRequest, ReplaceRequest, RestartTrackRequest, SaveQueueRequest, RenameLibraryNodeRequest, ReplaceRequest, RestartTrackRequest, SaveQueueRequest, SeekRequest,
SetCurrentRequest, ToggleMuteRequest, TogglePlayRequest, ToggleRepeatRequest, SetCurrentRequest, ToggleMuteRequest, TogglePlayRequest, ToggleRepeatRequest,
ToggleShuffleRequest, ToggleShuffleRequest,
}; };
@ -260,6 +260,15 @@ impl Rpc {
Ok(()) Ok(())
} }
/// Moves the playing position by a signed millisecond offset. The server
/// applies it to the live position and clamps it to the track, so this
/// carries the *step*, never a target (architecture/seek.md D1).
pub async fn seek(&mut self, delta_millis: i32) -> Result<(), Status> {
let request = Request::new(SeekRequest { delta_millis });
let _ = self.client.seek(request).await?;
Ok(())
}
pub async fn next(&mut self) -> Result<(), Status> { pub async fn next(&mut self) -> Result<(), Status> {
let _ = self.client.next(Request::new(NextRequest {})).await?; let _ = self.client.next(Request::new(NextRequest {})).await?;
Ok(()) Ok(())

View File

@ -55,6 +55,11 @@ service CrabidyService {
rpc Next(NextRequest) returns (NextResponse); rpc Next(NextRequest) returns (NextResponse);
rpc Prev(PrevRequest) returns (PrevResponse); rpc Prev(PrevRequest) returns (PrevResponse);
rpc RestartTrack(RestartTrackRequest) returns (RestartTrackResponse); rpc RestartTrack(RestartTrackRequest) returns (RestartTrackResponse);
// Moves the playing position inside the current track. Relative, never
// absolute: the server adds the offset to the live position, so repeated
// presses of a seek key compose instead of all computing from the same
// (already stale) position update.
rpc Seek(SeekRequest) returns (SeekResponse);
} }
// System // System
@ -249,6 +254,24 @@ message PrevResponse {}
message RestartTrackRequest {} message RestartTrackRequest {}
message RestartTrackResponse {} message RestartTrackResponse {}
message SeekRequest {
// Signed offset from the current position; negative seeks backwards.
// Milliseconds, like TrackPosition, so a client is free to pick any step.
//
// Clamped by the server: backwards saturates at the start of the track (it
// never steps into the previous one), and forwards stops just short of the
// end, so an overshooting seek lets the track finish and the queue advance.
// A track whose duration is unknown (a length-less stream) has no upper
// clamp and the decoder may refuse the seek.
//
// Fire-and-forget, like the other playback rpcs: the response says the
// command was accepted, not that the seek happened. A source that cannot
// seek at all (SoundCloud's HLS streams) leaves the position unchanged; the
// position updates on the stream remain the truth.
sint32 delta_millis = 1;
}
message SeekResponse {}
// Data types // Data types
message LibraryNodeChild { message LibraryNodeChild {
string path = 1; string path = 1;

View File

@ -1075,6 +1075,13 @@ pub enum PlaybackCommand {
Next, Next,
Prev, Prev,
RestartTrack, RestartTrack,
/// Moves the position inside the playing track by a signed offset in
/// milliseconds. The offset is passed straight to the player, which adds
/// it to the live position and clamps it — the playback loop keeps no
/// position of its own to go stale (architecture/seek.md D1).
Seek {
delta_millis: i64,
},
StateChanged { StateChanged {
state: PlayState, state: PlayState,
}, },
@ -1114,6 +1121,7 @@ impl PlaybackCommand {
Self::Next => "next", Self::Next => "next",
Self::Prev => "prev", Self::Prev => "prev",
Self::RestartTrack => "restart_track", Self::RestartTrack => "restart_track",
Self::Seek { .. } => "seek",
Self::StateChanged { .. } => "state_changed", Self::StateChanged { .. } => "state_changed",
Self::VolumeChanged { .. } => "volume_changed", Self::VolumeChanged { .. } => "volume_changed",
Self::MuteChanged { .. } => "mute_changed", Self::MuteChanged { .. } => "mute_changed",

View File

@ -463,6 +463,20 @@ impl Playback {
} }
} }
PlaybackCommand::Seek { delta_millis } => {
debug!(delta_millis, "seeking");
// The player owns both the live position and the clamping, so
// this arm only forwards. A source that cannot seek (HLS) or a
// player with nothing loaded is a warning: clients render the
// position from the update stream and never moved it
// themselves, so there is nothing to correct
// (architecture/seek.md D7).
match self.player.seek_by(delta_millis).await {
Ok(position) => trace!(?position, "seeked"),
Err(err) => warn!("seek failed: {err:?}"),
}
}
PlaybackCommand::VolumeChanged { volume } => { PlaybackCommand::VolumeChanged { volume } => {
trace!(volume, "volume changed"); trace!(volume, "volume changed");
self.broadcast(StreamUpdate::Volume(volume)); self.broadcast(StreamUpdate::Volume(volume));
@ -1125,6 +1139,15 @@ mod tests {
); );
} }
// Seek is not covered here on purpose. The handler's only job is to call
// `player.seek_by`, and every test in this module builds a real `Player`
// whose engine thread opens an audio device — so a test that *awaits* a
// player reply passes or hangs depending on whether the machine running it
// has working audio output, which is not a property of this code. The
// clamping arithmetic is tested as a pure function in `audio-player`, and
// the mapping from the RPC to the command (the layer the paste bug lived
// in) is tested in `rpc.rs`.
/// And position 0 reaches the very front — what `P` on the first row needs. /// And position 0 reaches the very front — what `P` on the first row needs.
#[tokio::test] #[tokio::test]
async fn insert_command_at_zero_reaches_the_front() { async fn insert_command_at_zero_reaches_the_front() {

View File

@ -11,10 +11,10 @@ use crabidy_core::proto::crabidy::{
InsertResponse, NextRequest, NextResponse, PrevRequest, PrevResponse, QueueRequest, InsertResponse, NextRequest, NextResponse, PrevRequest, PrevResponse, QueueRequest,
QueueResponse, RemoveRequest, RemoveResponse, RenameLibraryNodeRequest, QueueResponse, RemoveRequest, RemoveResponse, RenameLibraryNodeRequest,
RenameLibraryNodeResponse, ReplaceRequest, ReplaceResponse, RestartTrackRequest, RenameLibraryNodeResponse, ReplaceRequest, ReplaceResponse, RestartTrackRequest,
RestartTrackResponse, SaveQueueRequest, SaveQueueResponse, SetCurrentRequest, RestartTrackResponse, SaveQueueRequest, SaveQueueResponse, SeekRequest, SeekResponse,
SetCurrentResponse, StopRequest, StopResponse, ToggleMuteRequest, ToggleMuteResponse, SetCurrentRequest, SetCurrentResponse, StopRequest, StopResponse, ToggleMuteRequest,
TogglePlayRequest, TogglePlayResponse, ToggleRepeatRequest, ToggleRepeatResponse, ToggleMuteResponse, TogglePlayRequest, TogglePlayResponse, ToggleRepeatRequest,
ToggleShuffleRequest, ToggleShuffleResponse, ToggleRepeatResponse, ToggleShuffleRequest, ToggleShuffleResponse,
}; };
use crabidy_core::ProviderError; use crabidy_core::ProviderError;
use std::pin::Pin; use std::pin::Pin;
@ -589,4 +589,66 @@ impl CrabidyService for RpcService {
self.send_playback(PlaybackCommand::RestartTrack).await?; self.send_playback(PlaybackCommand::RestartTrack).await?;
Ok(Response::new(RestartTrackResponse {})) Ok(Response::new(RestartTrackResponse {}))
} }
/// Moves the position inside the playing track by a signed millisecond
/// offset. The offset is forwarded verbatim: the player adds it to the
/// live position and clamps it to the track (architecture/seek.md D1).
///
/// Fire-and-forget like the other playback rpcs — an unseekable source or
/// an idle player is logged, not reported, and the position updates on the
/// stream stay the single truth.
#[instrument(skip(self, request), fields(delta_millis))]
async fn seek(&self, request: Request<SeekRequest>) -> Result<Response<SeekResponse>, Status> {
let delta_millis = request.into_inner().delta_millis;
tracing::Span::current().record("delta_millis", delta_millis);
debug!("received seek request");
self.send_playback(PlaybackCommand::Seek {
delta_millis: delta_millis as i64,
})
.await?;
Ok(Response::new(SeekResponse {}))
}
}
#[cfg(test)]
mod tests {
use super::*;
fn service() -> (RpcService, flume::Receiver<PlaybackMessage>) {
let (update_tx, _) = tokio::sync::broadcast::channel(16);
let (playback_tx, playback_rx) = flume::bounded(16);
let (provider_tx, _provider_rx) = flume::bounded(16);
(
RpcService::new(update_tx, playback_tx, provider_tx, false),
playback_rx,
)
}
/// The seek offset must reach the playback loop **unchanged**, sign
/// included: it is a signed `sint32` on the wire widened to `i64`, and a
/// wrong widening (via `u32`, say) would turn every backward seek into a
/// forward jump of ~50 days.
///
/// This layer is worth pinning because it is exactly where the queue-paste
/// bug lived: the primitive and the ops were tested, the command mapping
/// was not. Nothing here touches the player, so no audio device is needed.
#[tokio::test]
async fn seek_forwards_the_signed_delta_unchanged() {
let (service, playback_rx) = service();
for delta in [-15_000, 15_000, i32::MIN, i32::MAX, 0] {
service
.seek(Request::new(SeekRequest {
delta_millis: delta,
}))
.await
.expect("seek is accepted");
let message = playback_rx.recv_async().await.expect("command sent");
match message.command {
PlaybackCommand::Seek { delta_millis } => {
assert_eq!(delta_millis, delta as i64, "delta must survive the hop");
}
other => panic!("expected a seek command, got {}", other.name()),
}
}
}
} }

View File

@ -47,8 +47,9 @@ cbd global volume -- -0.1 # lower the volume
`replace <PATH>…`, `queue remove <POS>…`, `queue clear `replace <PATH>…`, `queue remove <POS>…`, `queue clear
[--keep-current]`, `queue set-current <POS>`, `queue save`/`capture [--keep-current]`, `queue set-current <POS>`, `queue save`/`capture
<NAME>`, `queue shuffle`, and `queue repeat` change it. <NAME>`, `queue shuffle`, and `queue repeat` change it.
- `global play`/`stop`/`next`/`prev`/`restart`/`mute` and `global - `global play`/`stop`/`next`/`prev`/`restart`/`mute`, `global
volume <DELTA>` control playback. volume <DELTA>`, and `global seek <SECONDS>` (negative seeks back, e.g.
`global seek -15`) control playback.
## Server commands ## Server commands

View File

@ -156,6 +156,7 @@ pane).
| Global | `Tab` | Switch between library and queue | | Global | `Tab` | Switch between library and queue |
| Global | `Space` | Play/pause | | Global | `Space` | Play/pause |
| Global | `r` | Restart current track | | Global | `r` | Restart current track |
| Global | `Ctrl-b`/`Ctrl-f` | Seek back / forward 15 seconds |
| Global | `K` | Volume up | | Global | `K` | Volume up |
| Global | `J` | Volume down | | Global | `J` | Volume down |
| Global | `m` | Toggle mute | | Global | `m` | Toggle mute |

View File

@ -14,8 +14,10 @@ marks (`s`), visual mode (`v`/`V`) in both panes, and the register — `y`
yanks, `d`/`c`/`C` fill it as they remove, and `p`/`P` paste it after or yanks, `d`/`c`/`C` fill it as they remove, and `p`/`P` paste it after or
before the cursor (see [the TUI's register before the cursor (see [the TUI's register
section](./tui.md#the-register-y-d-and-pp), which behaves identically here; section](./tui.md#the-register-y-d-and-pp), which behaves identically here;
the queue toolbar shows how many entries are waiting). The `/` live filter the queue toolbar shows how many entries are waiting). Seek is there too —
is TUI-only for now. `Ctrl-b` and `Ctrl-f` move 15 seconds back and forward, as do the `⏪`/`⏩`
buttons in
the transport bar. The `/` live filter is TUI-only for now.
## How it is served ## How it is served

View File

@ -158,6 +158,29 @@ Two consequences worth knowing:
node expands it to tracks at paste time, and a path that no longer node expands it to tracks at paste time, and a path that no longer
resolves simply does not come back. resolves simply does not come back.
## Seeking inside a track
`Ctrl-b` and `Ctrl-f` in either client (and `cbd global seek <SECONDS>`) move the
playing position by 15 seconds. The wire carries the **offset**, not a
target position: the server adds it to the live position, so pressing the
key three times moves 45 seconds rather than three times to the same spot —
which is what would happen if a client computed the target from a position
update that is already up to a tick old.
The step stays inside the current track:
- Backwards it saturates at the start. It never steps into the previous
track; that is `Ctrl-p`.
- Forwards it stops a second short of the end, so an overshooting seek lets
the track run out through the normal end-of-track path — which advances
the queue, obeying shuffle and repeat like any other track change.
- A track whose duration the source never reported has no upper bound to
clamp to, so the decoder decides whether it can honour the seek.
- **SoundCloud tracks cannot seek at all.** They arrive as HLS playlists,
which are decoded as a forward-only stream on purpose; a seek there is
logged by the server and leaves the position where it was. The position
you see is always the truth — clients never move it themselves.
## Persistence ## Persistence
The live queue survives a server restart. It is mirrored to the reserved The live queue survives a server restart. It is mirrored to the reserved

68
plan/seek.md Normal file
View File

@ -0,0 +1,68 @@
# Plan — seek
Executes `architecture/seek.md` against `quality/seek.md`. One commit:
the change is a single thin path from key to sink, and half-wiring it
would leave a dead RPC.
## A — The engine (where the only real logic is)
- [x] **A1 — `PlayerEngine::seek_target`** — a pure function of
(current position, delta millis, known duration) returning the absolute
target: saturating on both ends, 0 as the floor, `duration - 1 s` as
the ceiling when the duration is known, no ceiling when it is 0.
Extracted as a free function so the arithmetic is testable without an
audio device. *Verifies:* G1, G5, G6, G7.
- [x] **A2 — Rewrite `PlayerEngine::seek_to`** to route through the same
helper, dropping the `clamp(1s, duration)` that panics whenever the
duration is unknown or under a second. *Verifies:* G1.
- [x] **A3 — `PlayerEngine::seek_by`** — reads `sink.get_pos()`, applies
A1, calls `try_seek`, and on success notifies
`PlayerMessage::Elapsed` with the new position so a paused seek is
visible. Refuses when stopped. *Verifies:* G4, G8, G9.
- [x] **A4 — `PlayerEngineCommand::SeekBy` + `Player::seek_by`** on the
existing bounded reply pattern. *Verifies:* G3.
- [x] **A5 — Engine tests**: extremes (`i64::MIN`/`MAX`), zero duration,
short track, composition, near-end saturation. *Verifies:* G1, G4G7.
## B — Wire and server
- [x] **B1 — Proto**: `rpc Seek(SeekRequest) returns (SeekResponse)` with
`sint32 delta_millis`, documented with the clamping contract.
*Verifies:* G10.
- [x] **B2 — `PlaybackCommand::Seek { delta_millis }`** + its `name()`
arm, and the playback handler that forwards to `player.seek_by` and
warns on failure. No state. *Verifies:* G2, G12.
- [x] **B3 — `RpcService::seek`** following the `change_volume` shape
(record the field, send, return empty). *Verifies:* G10.
- [x] **B4 — Server test** that the command reaches the player with the
delta unchanged. *Verifies:* G11.
## C — Clients
- [x] **C1 — TUI**: `Action::SeekForward`/`SeekBackward`, `Ctrl-f`/`Ctrl-b`
bindings in `Scope::Global` with help text, `MessageFromUi::Seek(i64)`,
the `lib.rs` arm, `rpc.rs` wrapper, `SEEK_STEP` constant, dispatch
tests. *Verifies:* G13, G14, G15.
- [x] **C2 — Web**: the same two actions in `keymap.rs` (global, with
`HELP` rows), `Rpc::seek`, the `app.rs` dispatch arms, and `⏪`/`⏩`
buttons in the transport bar. *Verifies:* G13, G14, G15.
- [x] **C3 — CLI**: `GlobalCmd::Seek { seconds: i32 }` with
`allow_negative_numbers`, and its executor arm. *Verifies:* G14.
## D — Docs and gates
- [x] **D1 — Docs**: key tables (`docs/src/clients/tui.md`,
`docs/src/clients/web.md`), the CLI page's `global` list, and the
within-track/end-of-track/HLS sentences in `docs/src/queue.md`.
*Verifies:* G16.
- [x] **D2 — Gates**: workspace tests, `check-features`, `cargo fmt`, a
wasm build of `cbd-web`, and the book build. *Verifies:* G17.
- [x] **D3 — `plan/summary.md`** entry, then commit.
## Deferred (recorded, not dropped)
- Absolute seek + click-to-seek on the web progress gauge (the gauge is
already rendered; needs one proto field and a click handler).
- Configurable step size, and a larger step on `<`/`>`.
- Chapter-aware seek for podcasts/audiobooks — no provider exposes
chapter marks through the library model.

87
quality/seek.md Normal file
View File

@ -0,0 +1,87 @@
# Quality gates — seek
Criteria an implementation of `architecture/seek.md` must satisfy.
Automated coverage lives in `audio-player/src/player_engine.rs` (the
clamping arithmetic, which is where the interesting behaviour is),
`crabidy-server/src/playback.rs`, and the two clients' binding tests.
## Hard rules (highest priority)
- [ ] **G1 — No panic on any delta, from any state.** The clamp is
saturating arithmetic with no `assert`/`clamp(min, max)` where
`min > max` is reachable. Specifically: an unknown duration (reported
as 0), a track shorter than the step, a delta larger than the track,
`i64::MIN`/`i64::MAX` as the delta, and a stopped or empty sink must
all be handled without panicking. This closes the existing bug in
`seek_to` (D5). *(tests: `seek_target_never_panics_on_extremes`,
`seek_target_saturates_at_the_start`,
`an_unknown_duration_does_not_clamp_forward`.)*
- [ ] **G2 — A failed seek stays server-side.** `Seek` never returns a
`color-eyre`/`anyhow` report to a client; an unseekable source produces
a `warn!` and an unchanged position, not an RPC error (D7).
- [ ] **G3 — No new unbounded channel and no new blocking call on an
async task.** The seek reply rides the existing bounded reply pattern;
`try_seek` is called only from the engine thread.
## Behaviour
- [ ] **G4 — Deltas compose.** Two successive backward seeks from the
same starting position land at `start - 2 × step`, not
`start - step` — the engine reads the live position each time (D1).
*(test: `successive_seeks_compose`.)*
- [ ] **G5 — Backward saturates at zero**, never below, and never into
the previous track (A1).
- [ ] **G6 — Forward past the end lands one second before the end**
when the duration is known, so the track finishes through the normal
end-of-stream path and the queue advances — the engine never seeks to
the exact end (D4). *(test: `seek_target_saturates_near_the_end`.)*
- [ ] **G7 — An unknown duration does not clamp forward.** A
length-less stream reports duration 0; that must not turn every
forward seek into "seek to 0" (the naive clamp) — it must pass the
target through and let the decoder refuse it.
- [ ] **G8 — The new position is broadcast immediately**, including
while **paused**`tick()` returns early for a paused sink, so the
engine emits `Elapsed` from the seek path itself (D6).
- [ ] **G9 — Seeking while stopped does nothing** and does not start
playback (A4).
## Wire and plumbing
- [ ] **G10 — One RPC, relative only**: `Seek(SeekRequest{sint32
delta_millis}) -> SeekResponse`, fire-and-forget like the other
playback RPCs (D2, A3). No absolute field, no oneof — the deferred
extension stays compatible.
- [ ] **G11 — The delta is never re-derived client-side.** No client
computes a target position from a broadcast `TrackPosition`; grep for
arithmetic on `position` in the clients and find none (D1, A2).
*(test: `seek_command_forwards_the_delta_unchanged`.)*
- [ ] **G12 — The playback loop holds no seek state.** `PlaybackCommand::Seek`
forwards to the player and does nothing else — no position cache to go
stale.
## Clients
- [ ] **G13 — `Ctrl-b` and `Ctrl-f` are global in both the TUI and the
web client** and collide with nothing in any scope — in particular plain
`f` still toggles the spectrum, and the TUI's binding-table uniqueness
invariant still holds. In the browser the chord must reach the client
rather than the find bar. *(tests: the existing `bindings`/`keymap`
tables and their uniqueness tests, extended.)*
- [ ] **G14 — Every client reaches it**: TUI binding → `MessageFromUi`
→ RPC; web binding + `⏪`/`⏩` buttons → RPC; `cbd global seek
<SECONDS>` accepting a negative value (D8).
- [ ] **G15 — Both help surfaces list it** — the TUI help modal (from
`BINDINGS`) and the web help overlay (`HELP`) — so the two tables stay
in lockstep with dispatch, which their tests enforce.
## Documentation
- [ ] **G16 — Documented where a user looks**: the key tables in
`docs/src/clients/tui.md` and the web page, the CLI page's `global`
list, and a sentence in `docs/src/queue.md` saying seek is
within-track, that a forward seek past the end advances, and that
SoundCloud's HLS streams cannot seek.
- [ ] **G17 — Clippy clean under `-D warnings`** across the feature
matrix (`check-features`), fmt clean, and the wasm target builds — the
web client's `mod app` is `#[cfg(target_arch = "wasm32")]` and native
clippy does not see it.