From ffe59b2c78673153a62d2bcf8966e3caa38f52a8 Mon Sep 17 00:00:00 2001 From: jeffvli Date: Tue, 12 May 2026 22:04:46 -0700 Subject: [PATCH] refactor scrobbling to use duration instead of progress (#2010) - add scrobble status debug and indicator - add force / reset scrobble --- src/i18n/locales/en.json | 3 +- .../components/playerbar-slider.module.css | 17 +- .../player/components/playerbar-slider.tsx | 14 +- .../player/components/scrobble-status.tsx | 124 ++++++++ .../features/player/hooks/use-scrobble.ts | 299 +++++++++++++++--- src/renderer/store/index.ts | 1 + src/renderer/store/scrobble-debug.store.ts | 45 +++ 7 files changed, 438 insertions(+), 65 deletions(-) create mode 100644 src/renderer/features/player/components/scrobble-status.tsx create mode 100644 src/renderer/store/scrobble-debug.store.ts diff --git a/src/i18n/locales/en.json b/src/i18n/locales/en.json index 8e0f675cf..58c179f37 100644 --- a/src/i18n/locales/en.json +++ b/src/i18n/locales/en.json @@ -696,7 +696,8 @@ "sleepTimer_off": "Off", "sleepTimer_timeRemaining": "{{time}} remaining", "sleepTimer_setCustom": "Set timer", - "sleepTimer_cancel": "Cancel timer" + "sleepTimer_cancel": "Cancel timer", + "scrobbleForceSubmit": "Force scrobble" }, "queryBuilder": { "standardTags": "Standard tags", diff --git a/src/renderer/features/player/components/playerbar-slider.module.css b/src/renderer/features/player/components/playerbar-slider.module.css index 2d7f8d20e..b03f4caf4 100644 --- a/src/renderer/features/player/components/playerbar-slider.module.css +++ b/src/renderer/features/player/components/playerbar-slider.module.css @@ -59,9 +59,24 @@ .slider-value-wrapper { display: flex; flex: 1; - align-self: center; + align-items: center; justify-content: center; max-width: 50px; + min-height: 0; + + @media (width < 768px) { + display: none; + } +} + +.slider-value-wrapper-elapsed { + display: flex; + flex: 1; + align-items: center; + justify-content: center; + min-width: 0; + max-width: 4.75rem; + min-height: 0; @media (width < 768px) { display: none; diff --git a/src/renderer/features/player/components/playerbar-slider.tsx b/src/renderer/features/player/components/playerbar-slider.tsx index 021977dee..c1cc18ece 100644 --- a/src/renderer/features/player/components/playerbar-slider.tsx +++ b/src/renderer/features/player/components/playerbar-slider.tsx @@ -4,6 +4,7 @@ import { lazy, Suspense } from 'react'; import { PlayerbarSeekSlider } from './playerbar-seek-slider'; import styles from './playerbar-slider.module.css'; +import { ScrobbleStatus } from '/@/renderer/features/player/components/scrobble-status'; import { useAppStore, useAppStoreActions, @@ -41,17 +42,8 @@ export const PlayerbarSlider = () => { return ( <>
-
- - {formattedTime} - +
+
{isWaveform ? ( diff --git a/src/renderer/features/player/components/scrobble-status.tsx b/src/renderer/features/player/components/scrobble-status.tsx new file mode 100644 index 000000000..8faa24a41 --- /dev/null +++ b/src/renderer/features/player/components/scrobble-status.tsx @@ -0,0 +1,124 @@ +import { useTranslation } from 'react-i18next'; + +import { + invokeScrobbleForceSubmit, + invokeScrobbleResetListenedState, +} from '/@/renderer/features/player/hooks/use-scrobble'; +import { useAppStore, useScrobbleDebugStore, useSettingsStore } from '/@/renderer/store'; +import { Button } from '/@/shared/components/button/button'; +import { Group } from '/@/shared/components/group/group'; +import { HoverCard } from '/@/shared/components/hover-card/hover-card'; +import { Icon } from '/@/shared/components/icon/icon'; +import { Progress } from '/@/shared/components/progress/progress'; +import { Stack } from '/@/shared/components/stack/stack'; +import { Text } from '/@/shared/components/text/text'; +import { PlaybackSelectors } from '/@/shared/constants/playback-selectors'; + +const scrobbleProgressProps = { + 'aria-hidden': true, + color: 'var(--theme-colors-primary)', + size: 'xs' as const, +}; + +const clampPct = (n: number) => Math.min(100, Math.max(0, n)); + +const ScrobbleConditionProgress = ({ value }: { value: number }) => ( + +); + +export const ScrobbleStatus = ({ formattedTime }: { formattedTime: string }) => { + const { t } = useTranslation(); + const scrobbleEnabled = useSettingsStore((state) => state.playback.scrobble.enabled); + const privateMode = useAppStore((state) => state.privateMode); + const snapshot = useScrobbleDebugStore((state) => state.snapshot); + + const hookInactive = !scrobbleEnabled || privateMode; + + const listenedSec = (snapshot.listenedMs / 1000).toFixed(1); + const listenPercentOfTrack = + snapshot.trackDurationMs > 0 ? (snapshot.listenedMs / snapshot.trackDurationMs) * 100 : 0; + + const durationConditionPct = + snapshot.targetDurationSec > 0 + ? clampPct((snapshot.listenedMs / 1000 / snapshot.targetDurationSec) * 100) + : 0; + const percentConditionPct = + snapshot.targetPercentage > 0 && snapshot.trackDurationMs > 0 + ? clampPct((listenPercentOfTrack / snapshot.targetPercentage) * 100) + : 0; + + return ( + + + e.stopPropagation()} + style={{ userSelect: 'none' }} + wrap="nowrap" + > + + + {formattedTime} + + + + e.stopPropagation()}> + + {hookInactive ? ( + {t('form.privateMode.enabled')} + ) : ( + <> + + + {`${listenedSec}s / ${snapshot.targetDurationSec}s`} + + + + + + {`${listenPercentOfTrack.toFixed(1)}% / ${snapshot.targetPercentage}%`} + + + + + + + + + )} + + + + ); +}; diff --git a/src/renderer/features/player/hooks/use-scrobble.ts b/src/renderer/features/player/hooks/use-scrobble.ts index 2e16f67e7..787612248 100644 --- a/src/renderer/features/player/hooks/use-scrobble.ts +++ b/src/renderer/features/player/hooks/use-scrobble.ts @@ -1,9 +1,10 @@ -import React, { useCallback, useEffect, useRef, useState } from 'react'; +import React, { useCallback, useEffect, useRef } from 'react'; import { useItemImageUrl } from '/@/renderer/components/item-image/item-image'; import { usePlayerEvents } from '/@/renderer/features/player/audio-player/hooks/use-player-events'; import { useSendScrobble } from '/@/renderer/features/player/mutations/scrobble-mutation'; import { + publishScrobbleDebug, useAppStore, usePlaybackSettings, usePlayerSong, @@ -16,34 +17,64 @@ import { logMsg } from '/@/renderer/utils/logger-message'; import { LibraryItem, QueueSong, ServerType } from '/@/shared/types/domain-types'; import { PlayerStatus } from '/@/shared/types/types'; +type ScrobbleManualHandlers = { + forceSubmitScrobble: () => void; + resetListenedState: () => void; +}; + +let scrobbleManualHandlers: null | ScrobbleManualHandlers = null; + +export const registerScrobbleManualHandlers = (next: null | ScrobbleManualHandlers) => { + scrobbleManualHandlers = next; +}; + +export const invokeScrobbleForceSubmit = () => { + scrobbleManualHandlers?.forceSubmitScrobble(); +}; + +export const invokeScrobbleResetListenedState = () => { + scrobbleManualHandlers?.resetListenedState(); +}; + /* - Scrobble Conditions (match any): - - If the song has been played for the required percentage - - If the song has been played for the required duration + Submission (Last.fm / etc.) eligibility uses accumulated listen time: + - If listened time meets the required percentage of track duration + - If listened time meets the required duration (seconds) -Scrobble Events: - - On song timestamp update: - - If the song has been played for the required percentage - - If the song has been played for the required duration +Listen time advances only while PLAYING, from consecutive timestamp deltas. +Seeks and other timeline jumps re-baseline the next sample without counting +the jump as listen time; accumulated listen time is kept across seeks. - - When the song changes (or is completed): - - Current song: Sends the 'playing' scrobble event - - Resets the 'isCurrentSongScrobbled' state to false +Listen time and submission state reset when the playhead returns to the start +of the track (position before SCROBBLE_TRACK_BEGIN_SEC), e.g. seek-to-start or +restart-from-near-zero. Song change and repeat still reset for a new play-through. - - When the song is restarted: - - Sends the 'submission' scrobble event if conditions are met AND the 'isCurrentSongScrobbled' state is false - - Resets the 'isCurrentSongScrobbled' state to false +Jellyfin progress APIs still use playback position (ticks), not listen time: + - Periodic timeupdate while playing + - timeupdate on seek + - pause / unpause - - When the song is seeked: - - Sends the 'timeupdate' scrobble event (Jellyfin only) +Other events: + - When the song changes: sends 'start' when the new track is playing; + clears submission flag and listen accumulator for the new track. + - When the song is restarted (near 0 after 10s+): clears submission flag + and listen accumulator. -Progress Events: - - When the song is playing (Jellyfin only): - - Sends the 'progress' scrobble event on an interval - + - When the song is seeked: Jellyfin sends timeupdate (throttled). Seeking from + at/after the intro into the start of the track clears listen accumulator and + submission flag; other seeks keep accumulated listen time. */ +// Positions before this time (seconds) count as the start of the track for listen/scrobble resets. +const SCROBBLE_TRACK_BEGIN_SEC = 5; + +// Min previous position (seconds) to treat a jump to the start as a full restart. +const SCROBBLE_RESTART_PREVIOUS_MIN_SEC = 10; + +// Max seconds between timestamp samples to count as continuous play (above poll interval, below a teleport). +const MAX_LISTEN_DELTA_SEC = 5; + const checkScrobbleConditions = (args: { scrobbleAtDurationMs: number; scrobbleAtPercentage: number; @@ -56,10 +87,10 @@ const checkScrobbleConditions = (args: { ? (songCompletedDurationMs / songDurationMs) * 100 : 0; - const shouldScrobbleBasedOnPercetange = percentageOfSongCompleted >= scrobbleAtPercentage; + const shouldScrobbleBasedOnPercentage = percentageOfSongCompleted >= scrobbleAtPercentage; const shouldScrobbleBasedOnDuration = songCompletedDurationMs >= scrobbleAtDurationMs; - return shouldScrobbleBasedOnPercetange || shouldScrobbleBasedOnDuration; + return shouldScrobbleBasedOnPercentage || shouldScrobbleBasedOnDuration; }; export const useScrobble = () => { @@ -77,7 +108,12 @@ export const useScrobble = () => { }); const imageUrlRef = useRef(imageUrl); - const [isCurrentSongScrobbled, setIsCurrentSongScrobbled] = useState(false); + const isCurrentSongScrobbledRef = useRef(false); + const listenedMsRef = useRef(0); + const lastListenSampleTimeRef = useRef(null); + const scrobbleAtDurationMsRef = useRef(0); + const scrobbleAtPercentageRef = useRef(75); + const previousSongRef = useRef(undefined); const previousTimestampRef = useRef(0); const lastProgressEventRef = useRef(0); @@ -89,30 +125,101 @@ export const useScrobble = () => { imageUrlRef.current = imageUrl; }, [imageUrl]); + useEffect(() => { + scrobbleAtDurationMsRef.current = (scrobbleSettings?.scrobbleAtDuration ?? 0) * 1000; + scrobbleAtPercentageRef.current = scrobbleSettings?.scrobbleAtPercentage ?? 75; + }, [scrobbleSettings?.scrobbleAtDuration, scrobbleSettings?.scrobbleAtPercentage]); + + const flushScrobbleDebug = useCallback(() => { + const song = usePlayerStore.getState().getCurrentSong(); + const status = usePlayerStore.getState().player.status; + const positionSec = useTimestampStoreBase.getState().timestamp; + const trackDurationMs = song?.duration ?? 0; + + const eligibilityMet = Boolean( + song?.id && + checkScrobbleConditions({ + scrobbleAtDurationMs: scrobbleAtDurationMsRef.current, + scrobbleAtPercentage: scrobbleAtPercentageRef.current, + songCompletedDurationMs: listenedMsRef.current, + songDurationMs: trackDurationMs, + }), + ); + + publishScrobbleDebug({ + eligibilityMet, + lastListenSampleTimeSec: lastListenSampleTimeRef.current, + listenedMs: listenedMsRef.current, + playerStatus: status, + positionSec, + songId: song?.id, + songName: song?.name, + submitted: isCurrentSongScrobbledRef.current, + targetDurationSec: scrobbleAtDurationMsRef.current / 1000, + targetPercentage: scrobbleAtPercentageRef.current, + trackDurationMs, + }); + }, []); + const handleScrobbleFromProgress = useCallback( (properties: { timestamp: number }, prev: { timestamp: number }) => { if (!isScrobbleEnabled || isPrivateModeEnabled) return; const currentSong = usePlayerStore.getState().getCurrentSong(); const currentStatus = usePlayerStore.getState().player.status; - - if (!currentSong?.id || currentStatus !== PlayerStatus.PLAYING) return; - const currentTime = properties.timestamp; const previousTime = prev.timestamp; - // Detect song restart: when timestamp resets to near 0 and was playing for at least 10 seconds + if (!currentSong?.id) { + return; + } + + if (currentStatus !== PlayerStatus.PLAYING) { + lastListenSampleTimeRef.current = currentTime; + return; + } + + // Detect song restart: when timestamp resets to near 0 and was playing past the intro if ( currentTime < previousTime && - currentTime < 5 && // Reset to near 0 - previousTime >= 10 // Was playing for at least 10 seconds + currentTime < SCROBBLE_TRACK_BEGIN_SEC && + previousTime >= SCROBBLE_RESTART_PREVIOUS_MIN_SEC ) { - setIsCurrentSongScrobbled(false); + isCurrentSongScrobbledRef.current = false; lastProgressEventRef.current = 0; previousTimestampRef.current = 0; + listenedMsRef.current = 0; + lastListenSampleTimeRef.current = null; return; } + const lastSample = lastListenSampleTimeRef.current; + if (lastSample === null) { + const prevSec = prev.timestamp; + if (currentTime > prevSec && currentTime - prevSec <= MAX_LISTEN_DELTA_SEC) { + listenedMsRef.current += (currentTime - prevSec) * 1000; + } + lastListenSampleTimeRef.current = currentTime; + } else { + const deltaSec = currentTime - lastSample; + const jumpedBackToTrackStart = + currentTime < lastSample && + currentTime < SCROBBLE_TRACK_BEGIN_SEC && + lastSample >= SCROBBLE_TRACK_BEGIN_SEC; + + if (jumpedBackToTrackStart) { + listenedMsRef.current = 0; + isCurrentSongScrobbledRef.current = false; + lastProgressEventRef.current = 0; + lastListenSampleTimeRef.current = currentTime; + } else if (currentTime < lastSample || deltaSec > MAX_LISTEN_DELTA_SEC) { + lastListenSampleTimeRef.current = currentTime; + } else if (deltaSec > 0) { + listenedMsRef.current += deltaSec * 1000; + lastListenSampleTimeRef.current = currentTime; + } + } + // Send Jellyfin progress events every 10 seconds if (currentSong._serverType === ServerType.JELLYFIN) { const timeSinceLastProgress = currentTime - lastProgressEventRef.current; @@ -144,12 +251,12 @@ export const useScrobble = () => { } } - // Check if we should submit scrobble based on conditions - if (!isCurrentSongScrobbled) { + // Check if we should submit scrobble based on listened time + if (!isCurrentSongScrobbledRef.current) { const shouldSubmitScrobble = checkScrobbleConditions({ - scrobbleAtDurationMs: (scrobbleSettings?.scrobbleAtDuration ?? 0) * 1000, - scrobbleAtPercentage: scrobbleSettings?.scrobbleAtPercentage, - songCompletedDurationMs: currentTime * 1000, + scrobbleAtDurationMs: scrobbleAtDurationMsRef.current, + scrobbleAtPercentage: scrobbleAtPercentageRef.current, + songCompletedDurationMs: listenedMsRef.current, songDurationMs: currentSong.duration, }); @@ -177,25 +284,18 @@ export const useScrobble = () => { category: LogCategory.SCROBBLE, meta: { id: currentSong.id, - reason: 'from song progress', + reason: 'from listened time', }, }); }, }, ); - setIsCurrentSongScrobbled(true); + isCurrentSongScrobbledRef.current = true; } } }, - [ - isScrobbleEnabled, - isPrivateModeEnabled, - scrobbleSettings?.scrobbleAtDuration, - scrobbleSettings?.scrobbleAtPercentage, - isCurrentSongScrobbled, - sendScrobble, - ], + [isScrobbleEnabled, isPrivateModeEnabled, sendScrobble], ); const handleScrobbleFromSongChange = useCallback( @@ -240,11 +340,16 @@ export const useScrobble = () => { if (!isScrobbleEnabled || isPrivateModeEnabled) { previousSongRef.current = currentSong; previousTimestampRef.current = 0; + listenedMsRef.current = 0; + lastListenSampleTimeRef.current = null; + flushScrobbleDebug(); return; } - setIsCurrentSongScrobbled(false); + isCurrentSongScrobbledRef.current = false; lastProgressEventRef.current = 0; + listenedMsRef.current = 0; + lastListenSampleTimeRef.current = null; // Use a timeout to prevent spamming the server when switching songs quickly clearTimeout(songChangeTimeoutRef.current); @@ -280,8 +385,15 @@ export const useScrobble = () => { previousSongRef.current = currentSong; previousTimestampRef.current = 0; + flushScrobbleDebug(); }, - [scrobbleSettings?.notify, isScrobbleEnabled, isPrivateModeEnabled, sendScrobble], + [ + flushScrobbleDebug, + scrobbleSettings?.notify, + isScrobbleEnabled, + isPrivateModeEnabled, + sendScrobble, + ], ); const handleScrobbleFromSeek = useCallback( @@ -297,8 +409,21 @@ export const useScrobble = () => { return; } + const sampleBeforeSeek = lastListenSampleTimeRef.current; + lastListenSampleTimeRef.current = properties.timestamp; + + if ( + properties.timestamp < SCROBBLE_TRACK_BEGIN_SEC && + (sampleBeforeSeek === null || sampleBeforeSeek >= SCROBBLE_TRACK_BEGIN_SEC) + ) { + listenedMsRef.current = 0; + isCurrentSongScrobbledRef.current = false; + lastProgressEventRef.current = 0; + } + // Position scrobbles are only relevant for Jellyfin if (currentSong._serverType !== ServerType.JELLYFIN) { + flushScrobbleDebug(); return; } @@ -307,6 +432,7 @@ export const useScrobble = () => { // Only allow seek scrobble once per second if (timeSinceLastSeek < 1000) { + flushScrobbleDebug(); return; } @@ -337,8 +463,9 @@ export const useScrobble = () => { }, }, ); + flushScrobbleDebug(); }, - [isScrobbleEnabled, isPrivateModeEnabled, sendScrobble], + [flushScrobbleDebug, isScrobbleEnabled, isPrivateModeEnabled, sendScrobble], ); const handleScrobbleFromStatus = useCallback( @@ -412,8 +539,10 @@ export const useScrobble = () => { }, ); } + + flushScrobbleDebug(); }, - [isScrobbleEnabled, isPrivateModeEnabled, sendScrobble], + [flushScrobbleDebug, isScrobbleEnabled, isPrivateModeEnabled, sendScrobble], ); const handleScrobbleFromRepeat = useCallback(() => { @@ -428,9 +557,11 @@ export const useScrobble = () => { return; } - setIsCurrentSongScrobbled(false); + isCurrentSongScrobbledRef.current = false; lastProgressEventRef.current = 0; previousTimestampRef.current = 0; + listenedMsRef.current = 0; + lastListenSampleTimeRef.current = null; sendScrobble.mutate( { @@ -455,17 +586,81 @@ export const useScrobble = () => { }, }, ); - }, [isScrobbleEnabled, isPrivateModeEnabled, sendScrobble]); + flushScrobbleDebug(); + }, [flushScrobbleDebug, isScrobbleEnabled, isPrivateModeEnabled, sendScrobble]); // Update previous timestamp on progress for use in status change handler const handleProgressUpdate = useCallback( (properties: { timestamp: number }, prev: { timestamp: number }) => { previousTimestampRef.current = properties.timestamp; handleScrobbleFromProgress(properties, prev); + flushScrobbleDebug(); }, - [handleScrobbleFromProgress], + [flushScrobbleDebug, handleScrobbleFromProgress], ); + useEffect(() => { + registerScrobbleManualHandlers({ + forceSubmitScrobble: () => { + if (!isScrobbleEnabled || isPrivateModeEnabled) { + return; + } + + const song = usePlayerStore.getState().getCurrentSong(); + if (!song?.id) { + return; + } + + const position = + song._serverType === ServerType.JELLYFIN ? song.duration * 1e7 : undefined; + + sendScrobble.mutate( + { + apiClientProps: { serverId: song._serverId || '' }, + query: { + albumId: song.albumId, + id: song.id, + position, + submission: true, + }, + }, + { + onSuccess: () => { + logFn.debug(logMsg[LogCategory.SCROBBLE].scrobbledSubmission, { + category: LogCategory.SCROBBLE, + meta: { + id: song.id, + reason: 'forced from UI', + }, + }); + }, + }, + ); + + isCurrentSongScrobbledRef.current = true; + flushScrobbleDebug(); + }, + resetListenedState: () => { + if (!isScrobbleEnabled || isPrivateModeEnabled) { + return; + } + + const song = usePlayerStore.getState().getCurrentSong(); + if (!song?.id) { + return; + } + + listenedMsRef.current = 0; + isCurrentSongScrobbledRef.current = false; + lastProgressEventRef.current = 0; + lastListenSampleTimeRef.current = useTimestampStoreBase.getState().timestamp; + flushScrobbleDebug(); + }, + }); + + return () => registerScrobbleManualHandlers(null); + }, [flushScrobbleDebug, isPrivateModeEnabled, isScrobbleEnabled, sendScrobble]); + usePlayerEvents( { onCurrentSongChange: handleScrobbleFromSongChange, diff --git a/src/renderer/store/index.ts b/src/renderer/store/index.ts index 18fa2f4d3..bd43ffdd9 100644 --- a/src/renderer/store/index.ts +++ b/src/renderer/store/index.ts @@ -2,6 +2,7 @@ export * from './app.store'; export * from './auth.store'; export * from './full-screen-player.store'; export * from './player.store'; +export * from './scrobble-debug.store'; export * from './scroll.store'; export * from './settings.store'; export * from './timestamp.store'; diff --git a/src/renderer/store/scrobble-debug.store.ts b/src/renderer/store/scrobble-debug.store.ts new file mode 100644 index 000000000..e06e743c2 --- /dev/null +++ b/src/renderer/store/scrobble-debug.store.ts @@ -0,0 +1,45 @@ +import { createWithEqualityFn } from 'zustand/traditional'; + +import { PlayerStatus } from '/@/shared/types/types'; + +export type ScrobbleDebugSnapshot = { + eligibilityMet: boolean; + lastListenSampleTimeSec: null | number; + listenedMs: number; + playerStatus: PlayerStatus; + positionSec: number; + songId?: string; + songName?: string; + submitted: boolean; + targetDurationSec: number; + targetPercentage: number; + trackDurationMs: number; +}; + +const initialSnapshot: ScrobbleDebugSnapshot = { + eligibilityMet: false, + lastListenSampleTimeSec: null, + listenedMs: 0, + playerStatus: PlayerStatus.PAUSED, + positionSec: 0, + songId: undefined, + songName: undefined, + submitted: false, + targetDurationSec: 240, + targetPercentage: 75, + trackDurationMs: 0, +}; + +type ScrobbleDebugStore = { + snapshot: ScrobbleDebugSnapshot; +}; + +export const useScrobbleDebugStore = createWithEqualityFn()(() => ({ + snapshot: initialSnapshot, +})); + +export const publishScrobbleDebug = (partial: Partial) => { + useScrobbleDebugStore.setState((state) => ({ + snapshot: { ...state.snapshot, ...partial }, + })); +};