diff --git a/server/jellyfin/e2e/sessions_test.go b/server/jellyfin/e2e/sessions_test.go index 87fcdd1a6..2057b43c6 100644 --- a/server/jellyfin/e2e/sessions_test.go +++ b/server/jellyfin/e2e/sessions_test.go @@ -2,6 +2,7 @@ package e2e import ( "net/http" + "strconv" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -10,26 +11,42 @@ import ( var _ = Describe("Sessions", func() { BeforeEach(func() { setupTestDB() }) - reportBody := func(itemID string) string { - return `{"ItemId":"` + enc(itemID) + `","PositionTicks":1200000000}` + ticks := func(ms int64) int64 { return ms * 10_000 } + reportBody := func(itemID string, positionTicks int64) string { + return `{"ItemId":"` + enc(itemID) + `","PositionTicks":` + strconv.FormatInt(positionTicks, 10) + `}` } Describe("playback reporting", func() { It("accepts a playback start report", func() { - Expect(post("/Sessions/Playing", reportBody(songID("Come Together"))).Code).To(Equal(http.StatusNoContent)) + Expect(post("/Sessions/Playing", reportBody(songID("Come Together"), 0)).Code).To(Equal(http.StatusNoContent)) }) It("accepts a playback progress report", func() { - Expect(post("/Sessions/Playing/Progress", reportBody(songID("Come Together"))).Code).To(Equal(http.StatusNoContent)) + Expect(post("/Sessions/Playing/Progress", reportBody(songID("Come Together"), ticks(5000))).Code).To(Equal(http.StatusNoContent)) }) - It("accepts a playback stopped report and records the play", func() { + It("counts a play stopped past the threshold", func() { id := songID("So What") - Expect(post("/Sessions/Playing/Stopped", reportBody(id)).Code).To(Equal(http.StatusNoContent)) + mf, err := ds.MediaFile(ctx).Get(id) + Expect(err).ToNot(HaveOccurred()) + // Report a stop at the end of the track — comfortably past 50% / the 4-minute cap. + Expect(post("/Sessions/Playing/Stopped", reportBody(id, ticks(int64(mf.Duration*1000)))).Code).To(Equal(http.StatusNoContent)) + + mf, err = ds.MediaFile(ctx).Get(id) + Expect(err).ToNot(HaveOccurred()) + Expect(mf.PlayCount).To(BeNumerically(">=", 1)) + }) + + It("does not count a brief play stopped before the threshold", func() { + // Regression: Finamp sends a Stopped report on every track switch, so an immediate skip + // (1 second in) must not mark the track played. Seeded tracks are >= 120s, so the 50% + // threshold is always well above 1s. + id := songID("Help!") + Expect(post("/Sessions/Playing/Stopped", reportBody(id, ticks(1000))).Code).To(Equal(http.StatusNoContent)) mf, err := ds.MediaFile(ctx).Get(id) Expect(err).ToNot(HaveOccurred()) - Expect(mf.PlayCount).To(BeNumerically(">=", 1)) + Expect(mf.PlayCount).To(Equal(int64(0))) }) }) diff --git a/server/jellyfin/sessions.go b/server/jellyfin/sessions.go index 0275468c2..4ed45222a 100644 --- a/server/jellyfin/sessions.go +++ b/server/jellyfin/sessions.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "net/http" - "time" "github.com/navidrome/navidrome/core/scrobbler" "github.com/navidrome/navidrome/log" @@ -90,30 +89,28 @@ func (api *Router) reportPlaybackProgress(w http.ResponseWriter, r *http.Request // reportPlaybackStopped handles POST /Sessions/Playing/Stopped, sent once when // playback of an item ends. +// +// Unlike Subsonic — where the client decides a play "counts" and only then calls +// scrobble?submission=true — Jellyfin clients (Finamp) send a Stopped report on *every* stop, +// including an immediate track switch. So the play threshold has to be applied server-side: we let +// ReportPlayback's own StateStopped logic decide (play counts + external scrobble only when the +// reported position passes 50% of the track, capped at 4 minutes). Force-submitting here instead +// would mark even a one-second skip as played. func (api *Router) reportPlaybackStopped(w http.ResponseWriter, r *http.Request) { ctx := r.Context() body := decodeReport(r) clientId, clientName := clientIdentity(ctx) - // IgnoreScrobble: Submit below is the sole source of the play-count increment - // and external scrobble for this stop event. Without it, ReportPlayback's own - // threshold-based scrobble logic would double-count the same play. err := api.scrobbler.ReportPlayback(ctx, scrobbler.ReportPlaybackParams{ - MediaId: body.ItemId, - PositionMs: body.PositionTicks / 10_000, - State: scrobbler.StateStopped, - IgnoreScrobble: true, - ClientId: clientId, - ClientName: clientName, + MediaId: body.ItemId, + PositionMs: body.PositionTicks / 10_000, + State: scrobbler.StateStopped, + ClientId: clientId, + ClientName: clientName, }) if err != nil { log.Warn(ctx, "Jellyfin API: report playback stopped failed", "id", body.ItemId, err) } - - err = api.scrobbler.Submit(ctx, []scrobbler.Submission{{TrackID: body.ItemId, Timestamp: time.Now()}}) - if err != nil { - log.Warn(ctx, "Jellyfin API: scrobble failed", "id", body.ItemId, err) - } w.WriteHeader(http.StatusNoContent) } diff --git a/server/jellyfin/sessions_test.go b/server/jellyfin/sessions_test.go index b21b59750..a08443e62 100644 --- a/server/jellyfin/sessions_test.go +++ b/server/jellyfin/sessions_test.go @@ -6,7 +6,6 @@ import ( "net/http" "net/http/httptest" "strings" - "time" "github.com/navidrome/navidrome/core" "github.com/navidrome/navidrome/core/scrobbler" @@ -117,8 +116,7 @@ var _ = Describe("Sessions", func() { }) Describe("reportPlaybackStopped", func() { - It("reports the stopped state and submits a scrobble", func() { - before := time.Now().Add(-time.Second) + It("reports the stopped state and lets the scrobbler apply its play threshold", func() { w := httptest.NewRecorder() r := authed(httptest.NewRequest("POST", "/Sessions/Playing/Stopped", strings.NewReader(`{"ItemId":"s1","PositionTicks":600000000}`))) @@ -130,13 +128,10 @@ var _ = Describe("Sessions", func() { Expect(pt.reported[0].MediaId).To(Equal("s1")) Expect(pt.reported[0].State).To(Equal(scrobbler.StateStopped)) Expect(pt.reported[0].PositionMs).To(Equal(int64(60000))) - // IgnoreScrobble must be set: Submit below is the sole play-count/scrobble trigger, - // otherwise ReportPlayback's own threshold logic would double-count the play. - Expect(pt.reported[0].IgnoreScrobble).To(BeTrue()) - - Expect(pt.submitted).To(HaveLen(1)) - Expect(pt.submitted[0].TrackID).To(Equal("s1")) - Expect(pt.submitted[0].Timestamp).To(BeTemporally(">", before)) + // IgnoreScrobble stays false so ReportPlayback's own StateStopped threshold decides + // whether the play counts; we no longer force a Submit that would bypass it. + Expect(pt.reported[0].IgnoreScrobble).To(BeFalse()) + Expect(pt.submitted).To(BeEmpty()) }) })