From 13df5683488920244010b48fbff83da9d5400e2e Mon Sep 17 00:00:00 2001 From: chris Date: Tue, 6 Oct 2026 12:48:29 -0400 Subject: [PATCH] Keep progress bars aligned The server now sends each fresh position on every poll that has one, and sends positions advanced to send time; clients count only the time since they received it. Bars realign every poll and no longer drift by however far the viewer's clock is off. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01MzFywujuH9TiuXbr9vjoX8 --- apps/api/src/services/nowPlaying.ts | 34 +++++++++------- apps/api/test/now-playing.test.ts | 7 +++- apps/api/test/progress.test.ts | 47 +++++++++++++++------- apps/mobile/src/components/ProgressBar.tsx | 22 +++++----- apps/web/src/components/ProgressBar.tsx | 19 +++++---- packages/shared/src/progress.test.ts | 17 +++----- packages/shared/src/progress.ts | 14 +++---- 7 files changed, 93 insertions(+), 67 deletions(-) diff --git a/apps/api/src/services/nowPlaying.ts b/apps/api/src/services/nowPlaying.ts index 0d983e0..88d5e80 100644 --- a/apps/api/src/services/nowPlaying.ts +++ b/apps/api/src/services/nowPlaying.ts @@ -27,20 +27,32 @@ export async function getNowPlaying(userIds: string[]): Promise .innerJoin(schema.users, eq(schema.users.id, schema.nowPlaying.userId)) .where(inArray(schema.nowPlaying.userId, userIds)); const tracks = await getTrackDTOs(rows.map((r) => r.np.trackId)); + const now = Date.now(); return rows - .map(({ np, user }) => ({ + .map(({ np, user }) => ({ np, user, track: tracks.get(np.trackId)! })) + .map(({ np, user, track }) => ({ user: toUserSummary(user), - track: tracks.get(np.trackId)!, - isPlaying: np.isPlaying && !(PUSH_SOURCES.has(np.source) && Date.now() - np.updatedAt.getTime() > DEVICE_STALE_MS), + track, + isPlaying: np.isPlaying && !(PUSH_SOURCES.has(np.source) && now - np.updatedAt.getTime() > DEVICE_STALE_MS), source: np.source, sourceApp: np.sourceApp, sourceUrl: np.sourceUrl, - progressMs: np.progressMs, + progressMs: positionNow(np, track.durationMs, now), updatedAt: np.updatedAt.toISOString(), })) .sort((a, b) => Number(b.isPlaying) - Number(a.isPlaying) || b.updatedAt.localeCompare(a.updatedAt)); } +/** + * The position as of right now, so clients only count time since they received + * it (their clock can be off from ours by any amount). + */ +function positionNow(np: { progressMs: number | null; isPlaying: boolean; updatedAt: Date }, durationMs: number | null, now: number) { + if (np.progressMs == null || !np.isPlaying) return np.progressMs; + const position = np.progressMs + Math.max(0, now - np.updatedAt.getTime()); + return durationMs ? Math.min(position, durationMs) : position; +} + /** Plays kept per user. Profiles show the latest few; the rest leaves room for a future history page. */ export const LISTEN_HISTORY_LIMIT = 50; /** Coming back to the same track after this long counts as a new play. */ @@ -94,8 +106,6 @@ export async function publishNowPlaying(userId: string) { if (dto) await redis.publish(nowPlayingChannel(userId), JSON.stringify(dto)); } -const SEEK_THRESHOLD_MS = 5_000; - /** Apply one provider's poll result to the user's now-playing state. Returns whether they're playing. */ export async function applyPollResult( userId: string, @@ -121,13 +131,9 @@ export async function applyPollResult( const track = await upsertTrack(result.track); const changed = !current || current.trackId !== track.id || !current.isPlaying; - // Same song, but not where it should be by now: a seek (or a restart). Tell - // watchers, or their progress bars would drift. - const jumped = - !changed && - current.progressMs != null && - result.progressMs != null && - Math.abs(result.progressMs - (current.progressMs + (now.getTime() - current.updatedAt.getTime()))) > SEEK_THRESHOLD_MS; + // A fresh position from the source: send it so viewers' progress bars realign + // (seeks, buffering, pauses we missed between polls). + const realign = !changed && result.progressMs != null; const newPlay = !current || current.trackId !== track.id || (!current.isPlaying && now.getTime() - current.updatedAt.getTime() > REPLAY_GAP_MS); const values = { @@ -146,7 +152,7 @@ export async function applyPollResult( .onConflictDoUpdate({ target: schema.nowPlaying.userId, set: values }); if (newPlay) await recordListen(userId, track.id, source, now); - if (jumped) await publishNowPlaying(userId); + if (realign) await publishNowPlaying(userId); if (changed) { await publishNowPlaying(userId); const [t] = await db diff --git a/apps/api/test/now-playing.test.ts b/apps/api/test/now-playing.test.ts index 2f58836..da2df58 100644 --- a/apps/api/test/now-playing.test.ts +++ b/apps/api/test/now-playing.test.ts @@ -23,14 +23,17 @@ async function published(userId: string, fn: () => Promise): Promise<{ const track = (title: string, extra = {}) => ({ track: { title, artist: 'Daft Punk', ...extra }, isPlaying: true, progressMs: 1000 }); describe('applyPollResult', () => { - it('publishes on a new track and queues link resolution, but not on repeat polls', async () => { + it('publishes on a new track and queues link resolution, but not on repeat polls without a position', async () => { const { user } = await createUser(app(), 'bob'); const first = await published(user.id, () => applyPollResult(user.id, 'spotify', track('One More Time'))); expect(first).toHaveLength(1); expect(first[0]).toMatchObject({ track: { title: 'One More Time' }, isPlaying: true }); expect(await resolveQueue.getJobCounts('waiting')).toMatchObject({ waiting: 1 }); - const repeat = await published(user.id, () => applyPollResult(user.id, 'spotify', track('One More Time'))); + // (polls that carry a position do republish, to realign progress bars: see progress.test.ts) + const repeat = await published(user.id, () => + applyPollResult(user.id, 'spotify', { ...track('One More Time'), progressMs: null }), + ); expect(repeat).toHaveLength(0); }); diff --git a/apps/api/test/progress.test.ts b/apps/api/test/progress.test.ts index 02d3d35..064bd00 100644 --- a/apps/api/test/progress.test.ts +++ b/apps/api/test/progress.test.ts @@ -3,7 +3,7 @@ import { afterEach, describe, expect, it } from 'vitest'; import { db, schema } from '../src/db'; import { createRedis, nowPlayingChannel } from '../src/lib/redis'; import { reportedPosition } from '../src/routes/devices'; -import { applyPollResult } from '../src/services/nowPlaying'; +import { applyPollResult, getNowPlaying } from '../src/services/nowPlaying'; import { nextPollDelay } from '../src/services/scheduler'; import { createUser, useTestApp } from './helpers'; @@ -54,27 +54,44 @@ describe('phone-reported positions', () => { }); }); -describe('seeking', () => { - it('tells watchers when the position jumps, but not as it ticks along', async () => { +describe('realigning progress bars', () => { + it('sends the fresh position on every poll that has one, advanced to send time', async () => { const { user } = await createUser(app(), 'alice'); await applyPollResult(user.id, 'plex', playing(0)); - // Pretend the last poll was 10 seconds ago. - await db - .update(schema.nowPlaying) - .set({ updatedAt: new Date(Date.now() - 10_000) }) - .where(eq(schema.nowPlaying.userId, user.id)); - const messages: string[] = []; await sub.subscribe(nowPlayingChannel(user.id)); sub.on('message', (_channel, msg) => messages.push(msg)); - await applyPollResult(user.id, 'plex', playing(10_500)); // right where it should be - await new Promise((r) => setTimeout(r, 150)); - expect(messages).toHaveLength(0); - + await applyPollResult(user.id, 'plex', playing(10_500)); + await db + .update(schema.nowPlaying) + .set({ updatedAt: new Date(Date.now() - 3_000) }) // as if that poll was 3s ago + .where(eq(schema.nowPlaying.userId, user.id)); await applyPollResult(user.id, 'plex', playing(120_000)); // skipped ahead await new Promise((r) => setTimeout(r, 150)); - expect(messages).toHaveLength(1); - expect(JSON.parse(messages[0]!)).toMatchObject({ progressMs: 120_000 }); + const sent = messages.map((m) => JSON.parse(m).progressMs as number); + expect(sent).toHaveLength(2); + expect(sent[0]).toBeGreaterThanOrEqual(10_500); + expect(sent[0]).toBeLessThan(11_500); + expect(sent[1]).toBeGreaterThanOrEqual(120_000); + expect(sent[1]).toBeLessThan(121_000); + + // Fetched later, the position has moved on by the time since the poll. + const [np] = await getNowPlaying([user.id]); + await db.update(schema.nowPlaying).set({ updatedAt: new Date(Date.now() - 4_000) }).where(eq(schema.nowPlaying.userId, user.id)); + const [later] = await getNowPlaying([user.id]); + expect(np!.progressMs).toBeGreaterThanOrEqual(120_000); + expect(later!.progressMs).toBeGreaterThanOrEqual(124_000); + }); + + it("doesn't send anything extra for sources without a position", async () => { + const { user } = await createUser(app(), 'alice'); + await applyPollResult(user.id, 'lastfm', playing(null)); + const messages: string[] = []; + await sub.subscribe(nowPlayingChannel(user.id)); + sub.on('message', (_channel, msg) => messages.push(msg)); + await applyPollResult(user.id, 'lastfm', playing(null)); + await new Promise((r) => setTimeout(r, 150)); + expect(messages).toHaveLength(0); }); }); diff --git a/apps/mobile/src/components/ProgressBar.tsx b/apps/mobile/src/components/ProgressBar.tsx index acdf454..4c29ca7 100644 --- a/apps/mobile/src/components/ProgressBar.tsx +++ b/apps/mobile/src/components/ProgressBar.tsx @@ -7,26 +7,28 @@ import { colors } from '@/lib/theme'; /** Where they are in the song. Renders nothing when the source doesn't report a position. */ export function ProgressBar({ np }: { np: NowPlayingDTO }) { - const [now, setNow] = useState(() => Date.now()); - const pos = playbackPosition(np, now); - const visible = np.isPlaying && pos !== null; - const fraction = pos ? pos.positionMs / pos.durationMs : 0; + if (!np.isPlaying || playbackPosition(np, 0) === null) return null; + // Each fresh position from the server restarts the clock, realigning the bar. + return ; +} +function Bar({ np }: { np: NowPlayingDTO }) { + const [receivedAt] = useState(() => Date.now()); + const [now, setNow] = useState(receivedAt); useEffect(() => { - if (!visible) return; const id = setInterval(() => setNow(Date.now()), 1000); return () => clearInterval(id); - }, [visible]); + }, []); + const pos = playbackPosition(np, now - receivedAt); + const fraction = pos ? pos.positionMs / pos.durationMs : 0; const width = useSharedValue(fraction); useEffect(() => { - // Glide between ticks; jump straight there after a seek or a new song. - const jump = Math.abs(fraction - width.value) > 0.05; - width.set(jump ? fraction : withTiming(fraction, { duration: 1000, easing: Easing.linear })); + width.set(withTiming(fraction, { duration: 1000, easing: Easing.linear })); // glide between ticks }, [fraction, width]); const fill = useAnimatedStyle(() => ({ width: `${width.value * 100}%` })); - if (!visible || !pos) return null; + if (!pos) return null; return ( diff --git a/apps/web/src/components/ProgressBar.tsx b/apps/web/src/components/ProgressBar.tsx index 6850c5a..c90cd05 100644 --- a/apps/web/src/components/ProgressBar.tsx +++ b/apps/web/src/components/ProgressBar.tsx @@ -5,22 +5,25 @@ import { formatDuration, playbackPosition, type NowPlayingDTO } from '@musicshar /** Where they are in the song. Renders nothing when the source doesn't report a position. */ export function ProgressBar({ np }: { np: NowPlayingDTO }) { - const [now, setNow] = useState(() => Date.now()); - const pos = playbackPosition(np, now); - const ticking = np.isPlaying && pos !== null; + if (!np.isPlaying || playbackPosition(np, 0) === null) return null; + // Each fresh position from the server restarts the clock, realigning the bar. + return ; +} +function Bar({ np }: { np: NowPlayingDTO }) { + const [receivedAt] = useState(() => Date.now()); + const [now, setNow] = useState(receivedAt); useEffect(() => { - if (!ticking) return; const id = setInterval(() => setNow(Date.now()), 1000); return () => clearInterval(id); - }, [ticking]); + }, []); - if (!pos || !np.isPlaying) return null; - const pct = (pos.positionMs / pos.durationMs) * 100; + const pos = playbackPosition(np, now - receivedAt); + if (!pos) return null; return (
-
+
{formatDuration(pos.positionMs)} diff --git a/packages/shared/src/progress.test.ts b/packages/shared/src/progress.test.ts index e2389cd..fe0f7fd 100644 --- a/packages/shared/src/progress.test.ts +++ b/packages/shared/src/progress.test.ts @@ -1,30 +1,25 @@ import { describe, expect, it } from 'vitest'; import { formatDuration, playbackPosition } from './progress'; -const at = Date.parse('2026-10-06T12:00:00Z'); const np = (over: Partial<{ progressMs: number | null; isPlaying: boolean; durationMs: number | null }> = {}) => ({ progressMs: 'progressMs' in over ? over.progressMs! : 60_000, isPlaying: over.isPlaying ?? true, - updatedAt: new Date(at).toISOString(), track: { durationMs: 'durationMs' in over ? over.durationMs! : 200_000 }, }); describe('playbackPosition', () => { - it('advances while playing', () => { - expect(playbackPosition(np(), at + 15_000)).toEqual({ positionMs: 75_000, durationMs: 200_000 }); + it('advances by the time since it was received while playing', () => { + expect(playbackPosition(np(), 15_000)).toEqual({ positionMs: 75_000, durationMs: 200_000 }); }); it('holds still while paused', () => { - expect(playbackPosition(np({ isPlaying: false }), at + 15_000)?.positionMs).toBe(60_000); + expect(playbackPosition(np({ isPlaying: false }), 15_000)?.positionMs).toBe(60_000); }); it('stops at the end of the song', () => { - expect(playbackPosition(np(), at + 10 * 60_000)?.positionMs).toBe(200_000); + expect(playbackPosition(np(), 10 * 60_000)?.positionMs).toBe(200_000); }); it("is null when the source doesn't report a position or there's no length", () => { - expect(playbackPosition(np({ progressMs: null }))).toBeNull(); - expect(playbackPosition(np({ durationMs: null }))).toBeNull(); - }); - it("doesn't go backwards if the viewer's clock is behind", () => { - expect(playbackPosition(np(), at - 30_000)?.positionMs).toBe(60_000); + expect(playbackPosition(np({ progressMs: null }), 0)).toBeNull(); + expect(playbackPosition(np({ durationMs: null }), 0)).toBeNull(); }); }); diff --git a/packages/shared/src/progress.ts b/packages/shared/src/progress.ts index f81ecb1..b86fd8d 100644 --- a/packages/shared/src/progress.ts +++ b/packages/shared/src/progress.ts @@ -6,18 +6,18 @@ export interface PlaybackPosition { } /** - * Where in the song someone is right now, or null when the source doesn't say - * (Last.fm, ListenBrainz, Apple Music). While playing, the last reported - * position is advanced by the time since it was reported, so a progress bar can - * tick smoothly between updates. + * Where in the song someone is, or null when the source doesn't say (Last.fm, + * ListenBrainz, Apple Music). The server sends the position as of when it sent + * it; pass the time since this client received it, so the bar ticks smoothly and + * the viewer's clock being off doesn't matter. */ export function playbackPosition( - np: Pick & { track: Pick }, - now = Date.now(), + np: Pick & { track: Pick }, + msSinceReceived: number, ): PlaybackPosition | null { const durationMs = np.track.durationMs; if (np.progressMs == null || !durationMs) return null; - const elapsed = np.isPlaying ? Math.max(0, now - Date.parse(np.updatedAt)) : 0; + const elapsed = np.isPlaying ? Math.max(0, msSinceReceived) : 0; return { positionMs: Math.min(Math.max(np.progressMs + elapsed, 0), durationMs), durationMs }; }