From 13a64bd63ab6b8e86a96349ab770e75892901308 Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Fri, 29 May 2026 14:40:45 +0000 Subject: [PATCH] refactor(now playing): Improve now playing to generically handle clients that have real-time updates * Break out player status checks and thresholds into separate functions * Consolidate all should update functionality into shouldUpdatePlayingNow for easier testing * Introduce realtime flag for teal/discord and check if clearing is possible/desired * Remove equivalent functionality in individual clients * (tealfm): Correctly set max threshold using track duration * (tealfm): Skip clearing update if close to/after expiration time * (tealfm): Increase min threshold to avoid spamming pds updates for pauses/skip track --- .../bluesky/AbstractBlueSkyApiClient.ts | 34 ++- .../scrobblers/AbstractScrobbleClient.ts | 195 ++++++++++++++---- src/backend/scrobblers/DiscordScrobbler.ts | 41 ++-- src/backend/scrobblers/LastfmScrobbler.ts | 2 +- .../scrobblers/ListenbrainzScrobbler.ts | 2 +- src/backend/scrobblers/TealfmScrobbler.ts | 57 ++--- .../PlayerState/AbstractPlayerState.ts | 5 +- .../tests/scrobbler/scrobblers.test.ts | 10 +- 8 files changed, 244 insertions(+), 102 deletions(-) diff --git a/src/backend/common/vendor/bluesky/AbstractBlueSkyApiClient.ts b/src/backend/common/vendor/bluesky/AbstractBlueSkyApiClient.ts index 771cbd99..32a6c532 100644 --- a/src/backend/common/vendor/bluesky/AbstractBlueSkyApiClient.ts +++ b/src/backend/common/vendor/bluesky/AbstractBlueSkyApiClient.ts @@ -4,7 +4,7 @@ import { ListRecord, ScrobbleRecord, StatusRecord, TealClientData } from "../../ import AbstractApiClient from "../AbstractApiClient.js"; import { Agent, ComAtprotoRepoCreateRecord, ComAtprotoRepoListRecords, ComAtprotoRepoPutRecord } from "@atproto/api"; import { MSCache } from "../../Cache.js"; -import { BrainzMeta, PlayObject, PlayObjectLifecycleless, ScrobbleActionResult, UnixTimestamp, MBID } from "../../../../core/Atomic.js"; +import { BrainzMeta, PlayObject, PlayObjectLifecycleless, ScrobbleActionResult, UnixTimestamp, MBID, NowPlayingUpdateThreshold, SourcePlayerObj, Second } from "../../../../core/Atomic.js"; import { musicServiceToCononical } from '../listenbrainz/lzUtils.js'; import { parseRegexSingle } from "@foxxmd/regex-buddy-core"; import { RecordOptions } from "../../infrastructure/config/client/tealfm.js"; @@ -15,6 +15,7 @@ import { baseFormatPlayObj } from "../../../utils/PlayTransformUtils.js"; import { ScrobbleSubmitError } from "../../errors/MSErrors.js"; import { UpstreamError } from "../../errors/UpstreamError.js"; import { decodeTid, generateTID } from '@ewanc26/tid'; +import { Duration } from "dayjs/plugin/duration.js"; export abstract class AbstractBlueSkyApiClient extends AbstractApiClient implements PagelessTimeRangeListens { @@ -141,15 +142,12 @@ export const playToStatusRecord = (play: PlayObject, notPlaying: boolean, positi ? { trackName: "", artists: [] } : playToRecord(play); - // default "fallback" value - let expiry: Dayjs = dayjs().add(10, 'minute'); - // 1min ago if paused -> try now + (duration - position) -> try now + duration -> fallback + let expiry: Dayjs; if(notPlaying) { + // if clearing status we set expiration as one minute in the past expiry = dayjs().subtract(1, 'minute'); - } else if(position !== undefined && play.data.duration !== undefined) { - expiry = dayjs().add(play.data.duration - position, 'second'); - } else if(play.data.duration !== undefined) { - expiry = dayjs().add(play.data.duration, 'second'); + } else { + expiry = dayjs().add(nowPlayingExpirationDuration({play, position})); } return { @@ -160,6 +158,26 @@ export const playToStatusRecord = (play: PlayObject, notPlaying: boolean, positi }; } +export const nowPlayingExpirationDuration = (data: Pick): Duration => { + let expiry: Dayjs = dayjs().add(10, 'minute'); + + const { + position, + play + } = data; + + // if we have position and duration then expiration is set as calculated end of listening session + if(position !== undefined && play?.data.duration !== undefined) { + expiry = dayjs().add(play.data.duration - position, 'second'); + } else if(play?.data.duration !== undefined) { + // else if we have duration but not position then use track duration + expiry = dayjs().add(play.data.duration, 'second'); + } + + // otherwise use 10 minutes + return dayjs.duration(expiry.diff(dayjs(), 'ms')); +} + export const listRecordToPlay = (listRecord: ListRecord): PlayObject => { const opts: RecordOptions = {}; const uriRes = parseRegexSingle(ATPROTO_URI_REGEX, listRecord.uri); diff --git a/src/backend/scrobblers/AbstractScrobbleClient.ts b/src/backend/scrobblers/AbstractScrobbleClient.ts index 71ddea89..3fbfc2b4 100644 --- a/src/backend/scrobblers/AbstractScrobbleClient.ts +++ b/src/backend/scrobblers/AbstractScrobbleClient.ts @@ -30,6 +30,7 @@ import { FormatPlayObjectOptions, PaginatedTimeRangeOptions, REFRESH_STALE_DEFAULT, + ReportedPlayerStatus, ScrobbledPlayObject, SourceIdentifier, TIME_WEIGHT, @@ -79,6 +80,7 @@ import { ComponentMigrationNew, PlaySelect, PlaySelectWithQueueStates, QueueStat import { asPlay } from "../../core/PlayMarshalUtils.js"; import { DrizzleQueueRepository } from "../common/database/drizzle/repositories/QueueRepository.js"; import { GenericRepository } from "../common/database/drizzle/repositories/BaseRepository.js"; +import assert from "node:assert"; type PlatformMappedPlays = Map; type NowPlayingQueue = Map; @@ -120,6 +122,7 @@ export default abstract class AbstractScrobbleClient extends AbstractComponent i deadLetterQueued: number = 0; supportsNowPlaying: boolean = false; + nowPlayingIsRealtime: boolean = false; nowPlayingInit: boolean = false; nowPlayingEnabled: boolean; nowPlayingFilter: (queue: NowPlayingQueue) => SourcePlayerObj | undefined; @@ -1529,13 +1532,13 @@ export default abstract class AbstractScrobbleClient extends AbstractComponent i if(sourcePlayerData === undefined) { return; } - let shouldUpdate: boolean, - clientReason: string | undefined; - const [npUpdateTop, npUpdateTopReason] = this.shouldUpdatePlayingNowResult(sourcePlayerData); - shouldUpdate = npUpdateTop; - if(!npUpdateTop) { - this.npLogger.trace(`Not updating because ${npUpdateTopReason}`); - } else { + let [shouldUpdate, npUpdateTopReason] = this.shouldUpdatePlayingNow(sourcePlayerData); + let clientReason: string | undefined; + if(!shouldUpdate) { + this.npLogger.trace(`Not updating, ${npUpdateTopReason}`); + } + + if(shouldUpdate) { const [clientUpdate, clientUpdateReason, level] = await this.shouldUpdatePlayingNowPlatformSpecific(sourcePlayerData); clientReason = clientUpdateReason; shouldUpdate = clientUpdate; @@ -1543,6 +1546,8 @@ export default abstract class AbstractScrobbleClient extends AbstractComponent i this.npLogger[level ?? 'trace'](`Not updating, ${npUpdateTopReason} --BUT-- ${clientUpdateReason}`); } } + + // finally, do the update if(shouldUpdate) { this.npLogger.verbose(`Updating because ${npUpdateTopReason}${clientReason !== undefined ? ` --AND-- ${clientReason}` : ''}`); try { @@ -1559,46 +1564,129 @@ export default abstract class AbstractScrobbleClient extends AbstractComponent i } } - shouldUpdatePlayingNowResult = (data: SourcePlayerObj): [boolean, string?] => { + nowPlayingHasDiscrepancy = (data: SourcePlayerObj): [boolean, string?] => { if(this.nowPlayingLastPlay === undefined || this.nowPlayingLastUpdated === undefined) { return [true, 'Now Playing has not yet been set']; } - if(data.play?.data?.track === undefined) { - return [false, 'play is missing track information']; + const playExistingDiscrepancy = (this.nowPlayingLastPlay.play !== undefined && data.play === undefined) || (this.nowPlayingLastPlay === undefined && data.play !== undefined); + if(playExistingDiscrepancy) { + return [true, `previous update ${this.nowPlayingLastPlay.play !== undefined ? 'exists' : 'does not exist'} and current update ${data.play !== undefined ? 'exists' : 'does not exist'}`]; } - if((data.play?.data?.artists ?? []).length === 0) { - return [false, 'play is missing artist information']; + + if(this.nowPlayingLastPlay.play === undefined && data.play === undefined) { + return [false, 'both previous and current update do not exist, nothing to update']; } - const lastUpdateDiff = Math.abs(dayjs().diff(this.nowPlayingLastUpdated, 's')); + if(this.nowPlayingLastPlay.status.calculated !== data.status.calculated) { + return [true, 'player state has changed']; + } + + if(!playObjDataMatch(data.play, this.nowPlayingLastPlay.play)) { + return [true, 'previous update play data does not match current']; + } - const playExistingDiscrepancy = (this.nowPlayingLastPlay.play !== undefined && data.play === undefined) || (this.nowPlayingLastPlay === undefined && data.play !== undefined); - const bothPlaysExist = this.nowPlayingLastPlay.play !== undefined && data.play !== undefined; + return [false, 'previous update data matches current']; + } - const playerStatusChanged = this.nowPlayingLastPlay.status.calculated !== data.status.calculated; + protected nowPlayingThresholdsMet = (data: SourcePlayerObj) => { + const lastUpdateDiff = Math.abs(dayjs().diff(this.nowPlayingLastUpdated, 's')); + const minMet = this.nowPlayingMinThreshold(data.play) < lastUpdateDiff; + const minReason = `time since last update (${lastUpdateDiff}s) is ${minMet ? 'greater' : 'less'} than min threshold ${this.nowPlayingMinThreshold(data.play)}s`; - // update if play *has* changed and time since last update is greater than min interval - // this prevents spamming scrobbler API with updates if user is skipping tracks and source updates frequently - if(this.nowPlayingMinThreshold(data.play) < lastUpdateDiff && (playExistingDiscrepancy || playerStatusChanged || (bothPlaysExist && !playObjDataMatch(data.play, this.nowPlayingLastPlay.play)))) { - return [true, `New Play differs from previous Now Playing and time since update ${lastUpdateDiff}s, greater than threshold ${this.nowPlayingMinThreshold(data.play)}`]; - } - // update if play *has not* changed but last update is greater than max interval - // this keeps scrobbler Now Playing fresh ("active" indicator) in the event play is long - if(this.nowPlayingMaxThreshold(data.play) < lastUpdateDiff && (bothPlaysExist && playObjDataMatch(data.play, this.nowPlayingLastPlay.play))) { - return [true, `Now Playing last updated ${lastUpdateDiff}s ago, greater than threshold ${this.nowPlayingMaxThreshold(data.play)}s`]; - } + const maxMet = this.nowPlayingMaxThreshold(data.play) < lastUpdateDiff; + const maxReason = `time since last update (${lastUpdateDiff}s) is ${maxMet ? 'greater' : 'less'} than max threshold ${this.nowPlayingMaxThreshold(data.play)}s`; - return [false, `Now Playing ${bothPlaysExist && playObjDataMatch(data.play, this.nowPlayingLastPlay.play) ? 'matches' : 'does not match'} and was last updated ${lastUpdateDiff}s ago (threshold ${this.nowPlayingMaxThreshold(data.play)}s)`]; + return { + minMet, + minReason, + maxMet, + maxReason + } } - shouldUpdatePlayingNow = (data: SourcePlayerObj): boolean => { - return this.shouldUpdatePlayingNowResult(data)[0]; + shouldUpdatePlayingNow = (sourcePlayerData: SourcePlayerObj): [boolean, string] => { + let shouldUpdate: boolean; + const thresholds = this.nowPlayingThresholdsMet(sourcePlayerData); + // first we check if there is an obvious discrepancy between last updated and current update data + // such as one missing, status change, no stored previous, etc... + const [npUpdateTop, npUpdateTopReason] = this.nowPlayingHasDiscrepancy(sourcePlayerData); + shouldUpdate = npUpdateTop; + if(!npUpdateTop) { + + if(npUpdateTopReason === 'previous update data matches current') { + if(thresholds.maxMet) { + return [true, `previous matches current update --AND-- ${thresholds.maxReason}`]; + } else { + return [false, `previous matches current update --BUT-- ${thresholds.maxReason}`]; + } + } + + return [false, npUpdateTopReason]; + } + + let validStatusReason: string; + if(shouldUpdate) { + // next we check if new player state is even valid to use for an update + const [statusValid, reason] = this.nowPlayingIsRealtime ? playerInValidNPUpdateState(sourcePlayerData) : playerInNPPlayingOnlyState(sourcePlayerData); + validStatusReason = reason; + shouldUpdate = statusValid; + if(!statusValid) { + return [false, `${npUpdateTopReason} --BUT-- ${validStatusReason}`]; + } + } + + if(shouldUpdate && this.nowPlayingLastPlay !== undefined) { + // at this point its possible we could update but we should respect minimum update intervals + // and triggering this early means less, deeper checks + const thresholds = this.nowPlayingThresholdsMet(sourcePlayerData); + if (!thresholds.minMet) { + shouldUpdate = false; + return [false, `${npUpdateTopReason} and ${validStatusReason} --BUT-- ${thresholds.minReason}`]; + } + else if ( + // status hasn't changed + this.nowPlayingLastPlay.status?.calculated === sourcePlayerData.status?.calculated + // and both plays are defined and have not changed + && (this.nowPlayingLastPlay.play !== undefined && sourcePlayerData.play !== undefined) + && playObjDataMatch(sourcePlayerData.play, this.nowPlayingLastPlay.play)) { + + // only update if we are passed max threshold + shouldUpdate = thresholds.maxMet; + if(!thresholds.maxMet) { + return [false, `${npUpdateTopReason} and ${validStatusReason} --BUT-- ${thresholds.maxReason}`]; + } + } + } + + if(shouldUpdate) { + // check for valid play data if the update should be for a playing track + if(playerInNPPlayingOnlyState(sourcePlayerData)) { + if(sourcePlayerData.play?.data?.track === undefined) { + shouldUpdate = false; + return [false, `${npUpdateTopReason} and ${validStatusReason} --BUT-- play is missing track information`]; + } + if((sourcePlayerData.play?.data?.artists ?? []).length === 0) { + shouldUpdate = false; + return [false, `${npUpdateTopReason} and ${validStatusReason} --BUT-- play is missing artist information`]; + } + } + } + + if(shouldUpdate && this.nowPlayingIsRealtime) { + // prevent multiple clearing updates + if(this.nowPlayingLastPlay !== undefined && shouldClearNPStatus(sourcePlayerData) && shouldClearNPStatus(this.nowPlayingLastPlay)) { + shouldUpdate = false; + return [false, `${npUpdateTopReason} and ${validStatusReason} --BUT-- last update already cleared now playing`]; + } + } + + return [true, `${npUpdateTopReason} and ${validStatusReason}`]; } /** Implement this for specific requirements for updating playing now based on the scrobbler platform */ - protected shouldUpdatePlayingNowPlatformSpecific(data: SourcePlayerObj): Promise<[boolean, string?, LogLevel?]> { - return shouldUpdatePlayingNowPlatformWhenPlayingOnly(data); + protected async shouldUpdatePlayingNowPlatformSpecific(data: SourcePlayerObj): Promise<[boolean, string?, LogLevel?]> { + return [true]; } protected doPlayingNow = (data: SourcePlayerObj): Promise => Promise.resolve(undefined) @@ -1636,9 +1724,46 @@ export const nowPlayingUpdateByPlayDuration: NowPlayingUpdateThreshold = (play?: return (play?.data?.duration ?? 30) + 1; } -export const shouldUpdatePlayingNowPlatformWhenPlayingOnly = async (data: SourcePlayerObj): Promise<[boolean, string]> => { - if(data.status.calculated === CALCULATED_PLAYER_STATUSES.playing || (data.nowPlayingMode && !CALCULATED_PLAYER_STATUSES.stopped)) { - return [true, `calculated player status is ${data.status.calculated}`]; +export const shouldClearNPStatus = (data: SourcePlayerObj) => { + return [CALCULATED_PLAYER_STATUSES.stopped, CALCULATED_PLAYER_STATUSES.paused].includes(data.status.calculated as ReportedPlayerStatus); +} + +export const playerInNPPlayingOnlyState = (data: SourcePlayerObj): [boolean, string] => { + // for lower-interval update clients (like listenbrainz, lastfm) IE not real-time + // we don't want to create updates for paused/stopped because the NP data for these services + // is only supposed to be updated intermittently + // + // so only allow an update if the player is actually playing + if(!data.nowPlayingMode) { + if(data.status.calculated === CALCULATED_PLAYER_STATUSES.playing) { + return [true, `calculated player status is ${data.status.calculated}`]; + } + return [false, `calculated player status is ${data.status.calculated} but must be playing`]; + } + return npPlayerInValidNPUpdateState(data); +} + +export const playerInValidNPUpdateState = (data: SourcePlayerObj): [boolean, string] => { + // if the source player is not a "Now Playing" type (lz, endpoint Source, etc...) + // then we only want to allow an update if the player state is a known "good" type IE don't allow on unknown + if(!data.nowPlayingMode) { + if([CALCULATED_PLAYER_STATUSES.stopped, CALCULATED_PLAYER_STATUSES.paused, CALCULATED_PLAYER_STATUSES.playing].includes(data.status.calculated as ReportedPlayerStatus)) { + return [true, `player in valid update state: '${data.status.calculated }'`]; + } + return [false,`player is not in state: stopped | paused | playing => Found '${data.status.calculated }'`]; + } + + return npPlayerInValidNPUpdateState(data); +} + +export const npPlayerInValidNPUpdateState = (data: SourcePlayerObj): [boolean, string] => { + assert(data.nowPlayingMode === true, 'data is not in nowPlayingMode'); + + // if the source player *is* a "Now Playing" type + // then we allow update on anything that isn't explicitly stopped + // since these sources have limited reporting capability for calculating a valid state + if(CALCULATED_PLAYER_STATUSES.stopped !== data.status.calculated as ReportedPlayerStatus) { + return [true, `NP player in valid update state: '${data.status.calculated }'`]; } - return [false, `calculated player status is ${data.status.calculated} but must be played/stopped`]; + return [false, `NP player is is invalid update state: stopped`]; } \ No newline at end of file diff --git a/src/backend/scrobblers/DiscordScrobbler.ts b/src/backend/scrobblers/DiscordScrobbler.ts index 1fc68439..b2b287ca 100644 --- a/src/backend/scrobblers/DiscordScrobbler.ts +++ b/src/backend/scrobblers/DiscordScrobbler.ts @@ -4,7 +4,7 @@ import { PlayMatchResult, PlayObject, SourcePlayerObj } from "../../core/Atomic. import { CALCULATED_PLAYER_STATUSES, FormatPlayObjectOptions, REPORTED_PLAYER_STATUSES, ReportedPlayerStatus, SINGLE_USER_PLATFORM_ID_STR, TimeRangeListensFetcher } from "../common/infrastructure/Atomic.js"; import { Notifiers } from "../notifier/Notifiers.js"; -import AbstractScrobbleClient, { nowPlayingUpdateByPlayDuration } from "./AbstractScrobbleClient.js"; +import AbstractScrobbleClient, { nowPlayingUpdateByPlayDuration, shouldClearNPStatus } from "./AbstractScrobbleClient.js"; import { DiscordClientConfig, DiscordStrongData } from "../common/infrastructure/config/client/discord.js"; import { DiscordWSClient } from "../common/vendor/discord/DiscordWSClient.js"; import { configToStrong } from "../common/vendor/discord/DiscordUtils.js"; @@ -18,6 +18,7 @@ export default class DiscordScrobbler extends AbstractScrobbleClient { api: DiscordWSClient | DiscordIPCClient; requiresAuth = true; requiresAuthInteraction = false; + override nowPlayingIsRealtime: boolean = true; apiMode!: 'ws' | 'ipc'; declare config: DiscordClientConfig & {data: DiscordStrongData }; @@ -146,7 +147,7 @@ export default class DiscordScrobbler extends AbstractScrobbleClient { doPlayingNow = async (data: SourcePlayerObj) => { try { - if([CALCULATED_PLAYER_STATUSES.stopped, CALCULATED_PLAYER_STATUSES.paused].includes(data.status.calculated as ReportedPlayerStatus)) { + if(shouldClearNPStatus(data)) { await this.api.sendActivity(undefined); } else { await this.api.sendActivity(data); @@ -157,33 +158,19 @@ export default class DiscordScrobbler extends AbstractScrobbleClient { } shouldUpdatePlayingNowPlatformSpecific = async (data: SourcePlayerObj): Promise<[boolean, string?, LogLevel?]> => { - if ([CALCULATED_PLAYER_STATUSES.stopped, CALCULATED_PLAYER_STATUSES.paused, CALCULATED_PLAYER_STATUSES.playing].includes(data.status.calculated as ReportedPlayerStatus) - || (data.nowPlayingMode && !CALCULATED_PLAYER_STATUSES.stopped) - || data.status.stale) { - - const [sendOk, reasons, level = 'warn'] = await this.api.checkOkToSend(); - if (!sendOk) { - return [false, `Cannot update playing now because api client is ${reasons}`, level as LogLevel]; - } - - if(this.api instanceof DiscordWSClient) { - const [allowed, reason] = this.api.presenceIsAllowed(); - if(!allowed) { - this.npLogger.debug(reason); - return [false, reason]; - } - } + const [sendOk, reasons, level = 'warn'] = await this.api.checkOkToSend(); + if (!sendOk) { + return [false, `Cannot update playing now because api client is ${reasons}`, level as LogLevel]; + } - return [true]; - } else { - if(!data.nowPlayingMode && ![CALCULATED_PLAYER_STATUSES.stopped, CALCULATED_PLAYER_STATUSES.paused, CALCULATED_PLAYER_STATUSES.playing].includes(data.status.calculated as ReportedPlayerStatus)) { - return [false,`player is not in state: stopped | paused | playing => Found '${data.status.calculated }'`]; - } else if(data.nowPlayingMode && CALCULATED_PLAYER_STATUSES.stopped) { - this.npLogger.trace(`Will not update because now playing player is stopped => Found ${data.status.calculated}`); - return [false,`playing player is stopped => Found ${data.status.calculated}` ] - } else { - return [false, 'player is in an unexpected state for discord usage'] + if(this.api instanceof DiscordWSClient) { + const [allowed, reason] = this.api.presenceIsAllowed(); + if(!allowed) { + this.npLogger.debug(reason); + return [false, reason]; } } + + return [true]; } } diff --git a/src/backend/scrobblers/LastfmScrobbler.ts b/src/backend/scrobblers/LastfmScrobbler.ts index e4e1bdf5..39d2c37b 100644 --- a/src/backend/scrobblers/LastfmScrobbler.ts +++ b/src/backend/scrobblers/LastfmScrobbler.ts @@ -8,7 +8,7 @@ import { FormatPlayObjectOptions, InternalConfigOptional, TimeRangeListensFetche import { LastfmClientConfig } from "../common/infrastructure/config/client/lastfm.js"; import LastfmApiClient, { LastFMIgnoredScrobble, playToClientPayload, formatPlayObj } from "../common/vendor/LastfmApiClient.js"; import { Notifiers } from "../notifier/Notifiers.js"; -import AbstractScrobbleClient, { nowPlayingUpdateByPlayDuration, shouldUpdatePlayingNowPlatformWhenPlayingOnly } from "./AbstractScrobbleClient.js"; +import AbstractScrobbleClient, { nowPlayingUpdateByPlayDuration, playerInNPPlayingOnlyState } from "./AbstractScrobbleClient.js"; import { findCauseByReference } from "../utils/ErrorUtils.js"; import { createGetScrobblesForTimeRangeFunc } from "../utils/ListenFetchUtils.js"; diff --git a/src/backend/scrobblers/ListenbrainzScrobbler.ts b/src/backend/scrobblers/ListenbrainzScrobbler.ts index fdcfa303..775da1fd 100644 --- a/src/backend/scrobblers/ListenbrainzScrobbler.ts +++ b/src/backend/scrobblers/ListenbrainzScrobbler.ts @@ -12,7 +12,7 @@ import { playToListenPayload } from '../common/vendor/listenbrainz/lzUtils.js'; import { ListenPayload } from '../common/vendor/listenbrainz/interfaces.js'; import { Notifiers } from "../notifier/Notifiers.js"; -import AbstractScrobbleClient, { nowPlayingUpdateByPlayDuration, shouldUpdatePlayingNowPlatformWhenPlayingOnly } from "./AbstractScrobbleClient.js"; +import AbstractScrobbleClient, { nowPlayingUpdateByPlayDuration, playerInNPPlayingOnlyState } from "./AbstractScrobbleClient.js"; import { isDebugMode } from "../utils.js"; import { createGetScrobblesForTimeRangeFunc } from "../utils/ListenFetchUtils.js"; diff --git a/src/backend/scrobblers/TealfmScrobbler.ts b/src/backend/scrobblers/TealfmScrobbler.ts index 94b2c2e9..ee9989a6 100644 --- a/src/backend/scrobblers/TealfmScrobbler.ts +++ b/src/backend/scrobblers/TealfmScrobbler.ts @@ -7,16 +7,20 @@ import { FormatPlayObjectOptions, CALCULATED_PLAYER_STATUSES, ReportedPlayerStat import { playToListenPayload } from '../common/vendor/listenbrainz/lzUtils.js'; import { Notifiers } from "../notifier/Notifiers.js"; -import AbstractScrobbleClient from "./AbstractScrobbleClient.js"; +import AbstractScrobbleClient, { nowPlayingUpdateByPlayDuration, shouldClearNPStatus } from "./AbstractScrobbleClient.js"; import { TealClientConfig } from "../common/infrastructure/config/client/tealfm.js"; import { BlueSkyAppApiClient } from "../common/vendor/bluesky/BlueSkyAppApiClient.js"; import { BlueSkyOauthApiClient } from "../common/vendor/bluesky/BlueSkyOauthApiClient.js"; -import { AbstractBlueSkyApiClient, listRecordToPlay, playToRecord, playToStatusRecord, recordToPlay } from "../common/vendor/bluesky/AbstractBlueSkyApiClient.js"; +import { AbstractBlueSkyApiClient, listRecordToPlay, nowPlayingExpirationDuration, playToRecord, playToStatusRecord, recordToPlay } from "../common/vendor/bluesky/AbstractBlueSkyApiClient.js"; +import dayjs, { Dayjs } from "dayjs"; +import { durationToHuman } from "../utils.js"; export default class TealScrobbler extends AbstractScrobbleClient { requiresAuth = true; requiresAuthInteraction = false; + override nowPlayingIsRealtime: boolean = true; + protected lastExpirationDate: Dayjs; declare config: TealClientConfig; @@ -35,6 +39,8 @@ export default class TealScrobbler extends AbstractScrobbleClient { } else { throw new Error(`Must define either 'baseUri' or 'appPassword' in configuration!`); } + this.nowPlayingMaxThreshold = nowPlayingUpdateByPlayDuration; + this.nowPlayingMinThreshold = (_) => 20; } formatPlayObj = (obj: any, options: FormatPlayObjectOptions = {}) => recordToPlay(obj); @@ -123,36 +129,39 @@ export default class TealScrobbler extends AbstractScrobbleClient { } doPlayingNow = async (data: SourcePlayerObj) => { - const notPlaying = [CALCULATED_PLAYER_STATUSES.stopped, CALCULATED_PLAYER_STATUSES.paused].includes(data.status.calculated as ReportedPlayerStatus); + + const isClearing = shouldClearNPStatus(data); + + // we can avoid additional calls to PDS for clearing a status if the status is about to expire, or is already expired. + // this will usually happen if a player stops playing the last track in a queue + // -- worth doing since PDS calls have a daily rate limit + if(isClearing && (this.statusAlreadyExpired() || this.statusAlreadyExpired())) { + this.npLogger.debug(`Not calling status record update because status is about to expire (or has already), expiring ${durationToHuman(dayjs.duration(dayjs().diff(this.lastExpirationDate)))}`); + return; + } + try { - await this.client.updateStatusRecord(playToStatusRecord(data.play, notPlaying, data.position)); + await this.client.updateStatusRecord(playToStatusRecord(data.play, isClearing, data.position)); + if(!isClearing) { + this.lastExpirationDate = dayjs().add(nowPlayingExpirationDuration(data)); + } } catch (e) { throw e; } } - wasLastStatusCleared = () => { - return this.nowPlayingLastPlay !== undefined - && [CALCULATED_PLAYER_STATUSES.stopped, CALCULATED_PLAYER_STATUSES.paused].includes(this.nowPlayingLastPlay.status.calculated as ReportedPlayerStatus) + protected statusExpiresSoon = () => { + if(this.lastExpirationDate === undefined) { + return false; + } + // may want to make this configurable in the future? + return Math.abs(dayjs().diff(this.lastExpirationDate, 's')) < 15; } - - shouldUpdatePlayingNowPlatformSpecific = async (data: SourcePlayerObj): Promise<[boolean, string?, LogLevel?]> => { - if ([CALCULATED_PLAYER_STATUSES.stopped, CALCULATED_PLAYER_STATUSES.paused].includes(data.status.calculated as ReportedPlayerStatus) && !this.wasLastStatusCleared() - || [CALCULATED_PLAYER_STATUSES.playing].includes(data.status.calculated as ReportedPlayerStatus) - || (data.nowPlayingMode && !CALCULATED_PLAYER_STATUSES.stopped)) { - return [true]; - } else { - if(!data.nowPlayingMode && ![CALCULATED_PLAYER_STATUSES.stopped, CALCULATED_PLAYER_STATUSES.paused, CALCULATED_PLAYER_STATUSES.playing].includes(data.status.calculated as ReportedPlayerStatus)) { - return [false,`player is not in state: stopped | paused | playing => Found '${data.status.calculated }'`]; - } else if (this.wasLastStatusCleared()) { - return [false, 'teal.fm status has already been set to expired']; - } else if (data.nowPlayingMode && CALCULATED_PLAYER_STATUSES.stopped) { - this.npLogger.trace(`Will not update because now playing player is stopped => Found ${data.status.calculated}`); - return [false,`playing player is stopped => Found ${data.status.calculated}` ] - } else { - return [false, 'player is in an unexpected state for teal.fm usage'] - } + protected statusAlreadyExpired = () => { + if(this.lastExpirationDate === undefined) { + return false; } + return dayjs().isAfter(this.lastExpirationDate); } } diff --git a/src/backend/sources/PlayerState/AbstractPlayerState.ts b/src/backend/sources/PlayerState/AbstractPlayerState.ts index cddd6c55..7b1c1e3e 100644 --- a/src/backend/sources/PlayerState/AbstractPlayerState.ts +++ b/src/backend/sources/PlayerState/AbstractPlayerState.ts @@ -302,7 +302,10 @@ export abstract class AbstractPlayerState { playDateCompleted: completed ? dayjs() : undefined, repeat: this.isRepeatPlay }, - meta: this.currentPlay.meta + meta: { + ...this.currentPlay.meta, + trackProgressPosition: this.getPosition() ?? this.currentPlay.meta.trackProgressPosition + } } } return undefined; diff --git a/src/backend/tests/scrobbler/scrobblers.test.ts b/src/backend/tests/scrobbler/scrobblers.test.ts index 8bcd6b3b..8c52beb0 100644 --- a/src/backend/tests/scrobbler/scrobblers.test.ts +++ b/src/backend/tests/scrobbler/scrobblers.test.ts @@ -970,7 +970,7 @@ describe('Now Playing', function() { await using npScrobbler = new NowPlayingScrobbler(); await npScrobbler.initialize(); - const res = npScrobbler.shouldUpdatePlayingNow(generateSourcePlayerObj({play:generatePlay({}, {deviceId: genGroupIdStr(generatePlayPlatformId())})})); + const res = npScrobbler.shouldUpdatePlayingNow(generateSourcePlayerObj({play:generatePlay({}, {deviceId: genGroupIdStr(generatePlayPlatformId())})}))[0]; expect(res).to.be.true; }); @@ -983,7 +983,7 @@ describe('Now Playing', function() { npScrobbler.nowPlayingLastUpdated = dayjs().subtract(npScrobbler.nowPlayingMaxThreshold(lastUpdate.play) + 1, 's'); npScrobbler.nowPlayingLastPlay = lastUpdate; - const res = npScrobbler.shouldUpdatePlayingNow(lastUpdate); + const res = npScrobbler.shouldUpdatePlayingNow(lastUpdate)[0]; expect(res).to.be.true; }); @@ -996,7 +996,7 @@ describe('Now Playing', function() { npScrobbler.nowPlayingLastUpdated = dayjs().subtract(npScrobbler.nowPlayingMaxThreshold(lastUpdate.play) - 1, 's'); npScrobbler.nowPlayingLastPlay = lastUpdate; - const res = npScrobbler.shouldUpdatePlayingNow(lastUpdate); + const res = npScrobbler.shouldUpdatePlayingNow(lastUpdate)[0]; expect(res).to.be.false; }); @@ -1009,7 +1009,7 @@ describe('Now Playing', function() { npScrobbler.nowPlayingLastUpdated = dayjs().subtract(npScrobbler.nowPlayingMinThreshold(lastUpdate.play) + 1, 's'); npScrobbler.nowPlayingLastPlay = lastUpdate; - const res = npScrobbler.shouldUpdatePlayingNow(generateSourcePlayerObj({play:generatePlay({}, {deviceId: genGroupIdStr(generatePlayPlatformId())})})); + const res = npScrobbler.shouldUpdatePlayingNow(generateSourcePlayerObj({play:generatePlay({}, {deviceId: genGroupIdStr(generatePlayPlatformId())})}))[0]; expect(res).to.be.true; }); @@ -1022,7 +1022,7 @@ describe('Now Playing', function() { npScrobbler.nowPlayingLastUpdated = dayjs().subtract(npScrobbler.nowPlayingMinThreshold(lastUpdate.play) - 1, 's'); npScrobbler.nowPlayingLastPlay = lastUpdate; - const res = npScrobbler.shouldUpdatePlayingNow(generateSourcePlayerObj({play:generatePlay({}, {deviceId: genGroupIdStr(generatePlayPlatformId())})})); + const res = npScrobbler.shouldUpdatePlayingNow(generateSourcePlayerObj({play:generatePlay({}, {deviceId: genGroupIdStr(generatePlayPlatformId())})}))[0]; expect(res).to.be.false; }); -- 2.51.2