Merge 6e3bb4b426e03d037e1bec3fd3fc126243731109 into 6fb4cd277ef7a804d4632297b2abe97bddda641d

This commit is contained in:
Bernardo Giordano 2026-02-06 20:55:23 -08:00 committed by GitHub
commit 76466e5ed9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 445 additions and 16 deletions

View File

@ -5,6 +5,7 @@ import (
"maps"
"sort"
"sync"
"sync/atomic"
"time"
"github.com/navidrome/navidrome/conf"
@ -38,10 +39,22 @@ type nowPlayingEntry struct {
position int
}
// playSession tracks an active play session for duration calculation.
// Keyed by userID:playerID to track what each player is currently playing.
type playSession struct {
TrackID string // The track being played
ScrobbleID string // The scrobble ID (set when Submit is called), empty if not yet scrobbled
Start time.Time // When playback started (wall clock time)
Position int // Position in seconds when playback started
UserID string // User ID for DB operations
}
type PlayTracker interface {
NowPlaying(ctx context.Context, playerId string, playerName string, trackId string, position int) error
GetNowPlaying(ctx context.Context) ([]NowPlayingInfo, error)
Submit(ctx context.Context, submissions []Submission) error
// StopPlayback finalizes the duration for an active session when playback stops.
StopPlayback(ctx context.Context, trackId string, positionInSeconds int)
}
// PluginLoader is a minimal interface for plugin manager usage in PlayTracker
@ -55,6 +68,9 @@ type playTracker struct {
ds model.DataStore
broker events.Broker
playMap cache.SimpleCache[string, NowPlayingInfo]
playSessionMap map[string]*playSession // key: userID:playerID
playSessionMu sync.Mutex
enableNowPlaying bool // Captured at creation time to avoid races with config changes
builtinScrobblers map[string]Scrobbler
pluginScrobblers map[string]Scrobbler
pluginLoader PluginLoader
@ -64,6 +80,7 @@ type playTracker struct {
npSignal chan struct{}
shutdown chan struct{}
workerDone chan struct{}
stopped atomic.Bool // Set to true when stopNowPlayingWorker is called
}
func GetPlayTracker(ds model.DataStore, broker events.Broker, pluginManager PluginLoader) PlayTracker {
@ -76,9 +93,13 @@ func GetPlayTracker(ds model.DataStore, broker events.Broker, pluginManager Plug
// the GetPlayTracker function above
func newPlayTracker(ds model.DataStore, broker events.Broker, pluginManager PluginLoader) *playTracker {
m := cache.NewSimpleCache[string, NowPlayingInfo]()
// Capture config value at creation time to avoid races with config changes in tests
enableNowPlaying := conf.Server.EnableNowPlaying
p := &playTracker{
ds: ds,
playMap: m,
playSessionMap: make(map[string]*playSession),
enableNowPlaying: enableNowPlaying,
broker: broker,
builtinScrobblers: make(map[string]Scrobbler),
pluginScrobblers: make(map[string]Scrobbler),
@ -88,11 +109,20 @@ func newPlayTracker(ds model.DataStore, broker events.Broker, pluginManager Plug
shutdown: make(chan struct{}),
workerDone: make(chan struct{}),
}
if conf.Server.EnableNowPlaying {
m.OnExpiration(func(_ string, _ NowPlayingInfo) {
// Set up expiration callback for NowPlaying entries
// When a NowPlaying entry expires (track finished), finalize the session duration
m.OnExpiration(func(playerId string, info NowPlayingInfo) {
// Skip if the tracker has been stopped (prevents races during test cleanup)
if p.stopped.Load() {
return
}
if p.enableNowPlaying {
broker.SendBroadcastMessage(context.Background(), &events.NowPlayingCount{Count: m.Len()})
})
}
}
// Finalize the session when NowPlaying expires (track finished naturally)
p.finalizeSessionOnExpiration(playerId, info)
})
var enabled []string
for name, constructor := range constructors {
@ -112,6 +142,7 @@ func newPlayTracker(ds model.DataStore, broker events.Broker, pluginManager Plug
// stopNowPlayingWorker stops the background worker. This is primarily for testing.
func (p *playTracker) stopNowPlayingWorker() {
p.stopped.Store(true) // Prevent expiration callbacks from running
close(p.shutdown)
<-p.workerDone // Wait for worker to finish
}
@ -193,6 +224,129 @@ func (p *playTracker) getActiveScrobblers() map[string]Scrobbler {
return combined
}
// sessionKey generates a unique key for tracking play sessions per user/player combination.
func sessionKey(userID, playerID string) string {
return userID + ":" + playerID
}
// finalizeSession calculates and updates the duration for a session's scrobble.
// Called when a track changes or when NowPlaying expires.
// NOTE: This should be called WITHOUT holding the playSessionMu lock since it makes DB calls.
func (p *playTracker) finalizeSession(session *playSession) {
if session == nil || session.ScrobbleID == "" {
// No scrobble recorded yet, nothing to update
return
}
duration := session.Position
// Update the scrobble in the database
ctx := context.Background()
err := p.ds.Scrobble(ctx).UpdateDuration(session.ScrobbleID, duration)
if err != nil {
log.Error(ctx, "Error updating scrobble duration", "scrobbleID", session.ScrobbleID, "duration", duration, err)
} else {
log.Debug(ctx, "Updated scrobble duration", "scrobbleID", session.ScrobbleID, "duration", duration, "trackID", session.TrackID)
}
}
// finalizeSessionOnExpiration is called when a NowPlaying entry expires.
// At TTL expiration, we can't know if the user kept listening or stopped earlier.
// We use the last known Position as the duration since it's the most reliable data.
func (p *playTracker) finalizeSessionOnExpiration(playerID string, info NowPlayingInfo) {
// Skip if the tracker has been stopped
if p.stopped.Load() {
return
}
p.playSessionMu.Lock()
var sessionToFinalize *playSession
// Find session by iterating (we need to match by playerID which is part of the key)
for key, session := range p.playSessionMap {
// Check if this session matches the expired NowPlaying entry
if session.TrackID == info.MediaFile.ID && key == sessionKey(session.UserID, playerID) {
sessionToFinalize = session
delete(p.playSessionMap, key)
break
}
}
p.playSessionMu.Unlock()
// Finalize outside the lock. This will calculate the duration up to the point of expiration.
if sessionToFinalize != nil {
p.finalizeSession(sessionToFinalize)
}
}
// getOrCreateSession gets the current session for a user/player, finalizing any previous one if track changed.
// Returns the session for the current track.
// NOTE: For the same track, this updates Start and Position on every call to improve duration accuracy.
func (p *playTracker) getOrCreateSession(userID, playerID, trackID string, start time.Time, position int) *playSession {
var sessionToFinalize *playSession
p.playSessionMu.Lock()
key := sessionKey(userID, playerID)
existing := p.playSessionMap[key]
// If there's an existing session for a different track, mark it for finalization
if existing != nil && existing.TrackID != trackID {
sessionToFinalize = existing
existing = nil
}
// If no session or session was for different track, create new one
if existing == nil {
existing = &playSession{
ScrobbleID: "", // Will be set when Submit is called
TrackID: trackID,
Start: start,
Position: position,
UserID: userID,
}
p.playSessionMap[key] = existing
} else {
existing.Start = start
existing.Position = position
}
result := existing
p.playSessionMu.Unlock()
// Finalize outside the lock to avoid holding it during DB operations
if sessionToFinalize != nil {
p.finalizeSession(sessionToFinalize)
}
return result
}
// setSessionScrobbleID sets the scrobble ID for an existing session.
// Called when Submit creates a scrobble record.
func (p *playTracker) setSessionScrobbleID(userID, playerID, trackID, scrobbleID string) {
p.playSessionMu.Lock()
defer p.playSessionMu.Unlock()
key := sessionKey(userID, playerID)
session := p.playSessionMap[key]
if session != nil && session.TrackID == trackID {
session.ScrobbleID = scrobbleID
}
}
// getSessionDuration returns the current duration (in seconds) for an active session.
// Returns 0 if no session exists or if trackID doesn't match.
func (p *playTracker) getSessionDuration(userID, playerID, trackID string) int {
p.playSessionMu.Lock()
defer p.playSessionMu.Unlock()
key := sessionKey(userID, playerID)
session := p.playSessionMap[key]
if session == nil || session.TrackID != trackID {
return 0
}
return session.Position
}
func (p *playTracker) NowPlaying(ctx context.Context, playerId string, playerName string, trackId string, position int) error {
mf, err := p.ds.MediaFile(ctx).GetWithParticipants(trackId)
if err != nil {
@ -201,9 +355,10 @@ func (p *playTracker) NowPlaying(ctx context.Context, playerId string, playerNam
}
user, _ := request.UserFrom(ctx)
now := time.Now()
info := NowPlayingInfo{
MediaFile: *mf,
Start: time.Now(),
Start: now,
Position: position,
Username: user.UserName,
PlayerId: playerId,
@ -222,6 +377,11 @@ func (p *playTracker) NowPlaying(ctx context.Context, playerId string, playerNam
if conf.Server.EnableNowPlaying {
p.broker.SendBroadcastMessage(ctx, &events.NowPlayingCount{Count: p.playMap.Len()})
}
// Get or create play session for duration tracking.
// This will finalize any previous session for a different track on this player.
_ = p.getOrCreateSession(user.ID, playerId, trackId, now, position)
player, _ := request.PlayerFrom(ctx)
if player.ScrobbleEnabled {
p.enqueueNowPlaying(ctx, playerId, user.ID, mf, position)
@ -307,7 +467,15 @@ func (p *playTracker) GetNowPlaying(_ context.Context) ([]NowPlayingInfo, error)
func (p *playTracker) Submit(ctx context.Context, submissions []Submission) error {
username, _ := request.UsernameFrom(ctx)
user, _ := request.UserFrom(ctx)
player, _ := request.PlayerFrom(ctx)
// Get player ID for session lookup
playerID, ok := request.ClientUniqueIdFrom(ctx)
if !ok {
playerID = player.ID
}
if !player.ScrobbleEnabled {
log.Debug(ctx, "External scrobbling disabled for this player", "player", player.Name, "ip", player.IP, "user", username)
}
@ -320,13 +488,29 @@ func (p *playTracker) Submit(ctx context.Context, submissions []Submission) erro
log.Error(ctx, "Cannot find track for scrobbling", "id", s.TrackID, "user", username, err)
continue
}
err = p.incPlay(ctx, mf, s.Timestamp)
// Get initial duration from the active session (if any).
// This ensures we have a duration value even if the server restarts before TTL expires.
initialDuration := p.getSessionDuration(user.ID, playerID, s.TrackID)
var durationPtr *int
if initialDuration > 0 {
durationPtr = &initialDuration
}
// Create scrobble with initial duration from session
scrobbleID, err := p.incPlay(ctx, mf, s.Timestamp, durationPtr)
if err != nil {
log.Error(ctx, "Error updating play counts", "id", mf.ID, "track", mf.Title, "user", username, err)
} else {
success++
event.With("song", mf.ID).With("album", mf.AlbumID).With("artist", mf.AlbumArtistID)
log.Info(ctx, "Scrobbled", "title", mf.Title, "artist", mf.Artist, "user", username, "timestamp", s.Timestamp)
log.Info(ctx, "Scrobbled", "title", mf.Title, "artist", mf.Artist, "user", username, "timestamp", s.Timestamp, "scrobbleID", scrobbleID, "initialDuration", initialDuration)
// Store the scrobble ID in the session so duration can be updated later when playback ends
if scrobbleID != "" {
p.setSessionScrobbleID(user.ID, playerID, s.TrackID, scrobbleID)
}
if player.ScrobbleEnabled {
p.dispatchScrobble(ctx, mf, s.Timestamp)
}
@ -339,8 +523,51 @@ func (p *playTracker) Submit(ctx context.Context, submissions []Submission) erro
return nil
}
func (p *playTracker) incPlay(ctx context.Context, track *model.MediaFile, timestamp time.Time) error {
return p.ds.WithTx(func(tx model.DataStore) error {
func (p *playTracker) StopPlayback(ctx context.Context, trackId string, position int) {
user, ok := request.UserFrom(ctx)
if !ok {
return
}
// Get player ID - same logic as Submit
player, _ := request.PlayerFrom(ctx)
playerID, ok := request.ClientUniqueIdFrom(ctx)
if !ok {
playerID = player.ID
}
var sessionToFinalize *playSession
var scrobbleID string
p.playSessionMu.Lock()
key := sessionKey(user.ID, playerID)
session := p.playSessionMap[key]
if session != nil && session.TrackID == trackId && session.ScrobbleID != "" {
scrobbleID = session.ScrobbleID
sessionToFinalize = session
delete(p.playSessionMap, key)
}
p.playSessionMu.Unlock()
// Update duration outside the lock
if sessionToFinalize != nil && scrobbleID != "" {
duration := position
if duration < 0 {
duration = 0
}
err := p.ds.Scrobble(ctx).UpdateDuration(scrobbleID, duration)
if err != nil {
log.Error(ctx, "Error updating scrobble duration on stop", "scrobbleID", scrobbleID, "duration", duration, err)
} else {
log.Debug(ctx, "Updated scrobble duration on stop", "scrobbleID", scrobbleID, "duration", duration, "trackID", trackId)
}
}
}
func (p *playTracker) incPlay(ctx context.Context, track *model.MediaFile, timestamp time.Time, duration *int) (string, error) {
var scrobbleID string
err := p.ds.WithTx(func(tx model.DataStore) error {
err := tx.MediaFile(ctx).IncPlayCount(track.ID, timestamp)
if err != nil {
return err
@ -356,10 +583,14 @@ func (p *playTracker) incPlay(ctx context.Context, track *model.MediaFile, times
}
}
if conf.Server.EnableScrobbleHistory {
return tx.Scrobble(ctx).RecordScrobble(track.ID, timestamp)
// Create scrobble with initial duration from session.
// Duration may be updated later when playback ends (track change or TTL expiration).
scrobbleID, err = tx.Scrobble(ctx).RecordScrobble(track.ID, timestamp, duration)
return err
}
return nil
})
return scrobbleID, err
}
func (p *playTracker) dispatchScrobble(ctx context.Context, t *model.MediaFile, playTime time.Time) {

View File

@ -0,0 +1,50 @@
-- +goose Up
-- +goose StatementBegin
-- Make id NOT NULL and primary key by recreating the table
CREATE TABLE scrobbles_new(
id VARCHAR(255) PRIMARY KEY NOT NULL,
media_file_id VARCHAR(255) NOT NULL
REFERENCES media_file(id)
ON DELETE CASCADE
ON UPDATE CASCADE,
user_id VARCHAR(255) NOT NULL
REFERENCES user(id)
ON DELETE CASCADE
ON UPDATE CASCADE,
submission_time INTEGER NOT NULL,
duration INTEGER
);
INSERT INTO scrobbles_new (id, media_file_id, user_id, submission_time, duration)
SELECT lower(hex(randomblob(16))), media_file_id, user_id, submission_time, NULL FROM scrobbles;
DROP TABLE scrobbles;
ALTER TABLE scrobbles_new RENAME TO scrobbles;
CREATE INDEX scrobbles_date ON scrobbles (submission_time);
CREATE INDEX scrobbles_user_track ON scrobbles (user_id, media_file_id);
-- +goose StatementEnd
-- +goose Down
-- +goose StatementBegin
CREATE TABLE scrobbles_old(
media_file_id VARCHAR(255) NOT NULL
REFERENCES media_file(id)
ON DELETE CASCADE
ON UPDATE CASCADE,
user_id VARCHAR(255) NOT NULL
REFERENCES user(id)
ON DELETE CASCADE
ON UPDATE CASCADE,
submission_time INTEGER NOT NULL
);
INSERT INTO scrobbles_old (media_file_id, user_id, submission_time)
SELECT media_file_id, user_id, submission_time FROM scrobbles;
DROP TABLE scrobbles;
ALTER TABLE scrobbles_old RENAME TO scrobbles;
CREATE INDEX scrobbles_date ON scrobbles (submission_time);
-- +goose StatementEnd

View File

@ -3,11 +3,16 @@ package model
import "time"
type Scrobble struct {
ID string
MediaFileID string
UserID string
SubmissionTime time.Time
Duration *int // Duration in seconds the user actually listened. Nil if unknown.
}
type ScrobbleRepository interface {
RecordScrobble(mediaFileID string, submissionTime time.Time) error
// RecordScrobble creates a new scrobble record and returns its ID
RecordScrobble(mediaFileID string, submissionTime time.Time, duration *int) (string, error)
// UpdateDuration updates the duration of an existing scrobble
UpdateDuration(id string, duration int) error
}

View File

@ -6,6 +6,7 @@ import (
. "github.com/Masterminds/squirrel"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/id"
"github.com/pocketbase/dbx"
)
@ -21,14 +22,26 @@ func NewScrobbleRepository(ctx context.Context, db dbx.Builder) model.ScrobbleRe
return r
}
func (r *scrobbleRepository) RecordScrobble(mediaFileID string, submissionTime time.Time) error {
func (r *scrobbleRepository) RecordScrobble(mediaFileID string, submissionTime time.Time, duration *int) (string, error) {
userID := loggedUser(r.ctx).ID
scrobbleID := id.NewRandom()
values := map[string]interface{}{
"id": scrobbleID,
"media_file_id": mediaFileID,
"user_id": userID,
"submission_time": submissionTime.Unix(),
"duration": duration,
}
insert := Insert(r.tableName).SetMap(values)
_, err := r.executeSQL(insert)
if err != nil {
return "", err
}
return scrobbleID, nil
}
func (r *scrobbleRepository) UpdateDuration(id string, duration int) error {
update := Update(r.tableName).Set("duration", duration).Where(Eq{"id": id})
_, err := r.executeSQL(update)
return err
}

View File

@ -41,7 +41,7 @@ var _ = Describe("ScrobbleRepository", func() {
})
Describe("RecordScrobble", func() {
It("records a scrobble event", func() {
It("records a scrobble event and returns an ID", func() {
submissionTime := time.Now().UTC()
// Insert User
@ -63,22 +63,114 @@ var _ = Describe("ScrobbleRepository", func() {
}).Execute()
Expect(err).ToNot(HaveOccurred())
err = repo.RecordScrobble(fileID, submissionTime)
scrobbleID, err := repo.RecordScrobble(fileID, submissionTime, nil)
Expect(err).ToNot(HaveOccurred())
Expect(scrobbleID).ToNot(BeEmpty())
// Verify insertion
var scrobble struct {
ID string `db:"id"`
MediaFileID string `db:"media_file_id"`
UserID string `db:"user_id"`
SubmissionTime int64 `db:"submission_time"`
Duration *int `db:"duration"`
}
err = rawRepo.db.Select("*").From("scrobbles").
Where(dbx.HashExp{"media_file_id": fileID, "user_id": userID}).
Where(dbx.HashExp{"id": scrobbleID}).
One(&scrobble)
Expect(err).ToNot(HaveOccurred())
Expect(scrobble.ID).To(Equal(scrobbleID))
Expect(scrobble.MediaFileID).To(Equal(fileID))
Expect(scrobble.UserID).To(Equal(userID))
Expect(scrobble.SubmissionTime).To(Equal(submissionTime.Unix()))
Expect(scrobble.Duration).To(BeNil())
})
It("records a scrobble event with initial duration", func() {
submissionTime := time.Now().UTC()
duration := 180
// Insert User
_, err := rawRepo.db.Insert("user", dbx.Params{
"id": userID,
"user_name": "user",
"password": "pw",
"created_at": time.Now(),
"updated_at": time.Now(),
}).Execute()
Expect(err).ToNot(HaveOccurred())
// Insert MediaFile
_, err = rawRepo.db.Insert("media_file", dbx.Params{
"id": fileID,
"path": "path",
"created_at": time.Now(),
"updated_at": time.Now(),
}).Execute()
Expect(err).ToNot(HaveOccurred())
scrobbleID, err := repo.RecordScrobble(fileID, submissionTime, &duration)
Expect(err).ToNot(HaveOccurred())
Expect(scrobbleID).ToNot(BeEmpty())
// Verify insertion
var scrobble struct {
ID string `db:"id"`
MediaFileID string `db:"media_file_id"`
UserID string `db:"user_id"`
SubmissionTime int64 `db:"submission_time"`
Duration *int `db:"duration"`
}
err = rawRepo.db.Select("*").From("scrobbles").
Where(dbx.HashExp{"id": scrobbleID}).
One(&scrobble)
Expect(err).ToNot(HaveOccurred())
Expect(scrobble.Duration).ToNot(BeNil())
Expect(*scrobble.Duration).To(Equal(180))
})
})
Describe("UpdateDuration", func() {
It("updates the duration of an existing scrobble", func() {
submissionTime := time.Now().UTC()
// Insert User
_, err := rawRepo.db.Insert("user", dbx.Params{
"id": userID,
"user_name": "user",
"password": "pw",
"created_at": time.Now(),
"updated_at": time.Now(),
}).Execute()
Expect(err).ToNot(HaveOccurred())
// Insert MediaFile
_, err = rawRepo.db.Insert("media_file", dbx.Params{
"id": fileID,
"path": "path",
"created_at": time.Now(),
"updated_at": time.Now(),
}).Execute()
Expect(err).ToNot(HaveOccurred())
// Create scrobble with nil duration
scrobbleID, err := repo.RecordScrobble(fileID, submissionTime, nil)
Expect(err).ToNot(HaveOccurred())
// Update duration
err = repo.UpdateDuration(scrobbleID, 240)
Expect(err).ToNot(HaveOccurred())
// Verify update
var scrobble struct {
Duration *int `db:"duration"`
}
err = rawRepo.db.Select("duration").From("scrobbles").
Where(dbx.HashExp{"id": scrobbleID}).
One(&scrobble)
Expect(err).ToNot(HaveOccurred())
Expect(scrobble.Duration).ToNot(BeNil())
Expect(*scrobble.Duration).To(Equal(240))
})
})
})

View File

@ -106,6 +106,13 @@ func (api *Router) SavePlayQueue(r *http.Request) (*responses.Subsonic, error) {
user, _ := request.UserFrom(r.Context())
client, _ := request.ClientFrom(r.Context())
// If there's a current track with a position, update the scrobble duration
// This handles the case where the user stops playback and the client saves the queue
if currentID != "" && position > 0 {
positionSecs := int(position / 1000)
api.scrobbler.StopPlayback(r.Context(), currentID, positionSecs)
}
items := slice.Map(ids, func(id string) model.MediaFile {
return model.MediaFile{ID: id}
})
@ -182,6 +189,14 @@ func (api *Router) SavePlayQueueByIndex(r *http.Request) (*responses.Subsonic, e
}
}
// If there's a current track with a position, update the scrobble duration
// This handles the case where the user stops playback and the client saves the queue
if len(ids) > 0 && currentIndex < len(ids) && position > 0 {
currentID := ids[currentIndex]
positionSecs := int(position / 1000)
api.scrobbler.StopPlayback(r.Context(), currentID, positionSecs)
}
items := slice.Map(ids, func(id string) model.MediaFile {
return model.MediaFile{ID: id}
})

View File

@ -127,6 +127,9 @@ func (f *fakePlayTracker) Submit(_ context.Context, submissions []scrobbler.Subm
return nil
}
func (f *fakePlayTracker) StopPlayback(_ context.Context, _ string, _ int) {
}
var _ scrobbler.PlayTracker = (*fakePlayTracker)(nil)
type fakeEventBroker struct {

View File

@ -2,23 +2,43 @@ package tests
import (
"context"
"sync"
"time"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/id"
"github.com/navidrome/navidrome/model/request"
)
type MockScrobbleRepo struct {
RecordedScrobbles []model.Scrobble
ctx context.Context
mu sync.Mutex
}
func (m *MockScrobbleRepo) RecordScrobble(fileID string, submissionTime time.Time) error {
func (m *MockScrobbleRepo) RecordScrobble(fileID string, submissionTime time.Time, duration *int) (string, error) {
m.mu.Lock()
defer m.mu.Unlock()
user, _ := request.UserFrom(m.ctx)
scrobbleID := id.NewRandom()
m.RecordedScrobbles = append(m.RecordedScrobbles, model.Scrobble{
ID: scrobbleID,
MediaFileID: fileID,
UserID: user.ID,
SubmissionTime: submissionTime,
Duration: duration,
})
return scrobbleID, nil
}
func (m *MockScrobbleRepo) UpdateDuration(scrobbleID string, duration int) error {
m.mu.Lock()
defer m.mu.Unlock()
for i := range m.RecordedScrobbles {
if m.RecordedScrobbles[i].ID == scrobbleID {
m.RecordedScrobbles[i].Duration = &duration
break
}
}
return nil
}