mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-28 22:27:10 +01:00
fix(jellyfin): only count a play past the scrobble threshold
reportPlaybackStopped set IgnoreScrobble and force-submitted a play on every Stopped report, so a briefly-played track (e.g. an immediate skip) was marked played. Finamp sends Stopped on every track switch, so the threshold must be applied server-side. Let ReportPlayback's StateStopped logic decide (play + scrobble only past 50% of the track / 4-minute cap) instead. Subsonic differs because there the client gates submission.
This commit is contained in:
parent
c3f1e29e71
commit
cd18a6fa77
3 changed files with 41 additions and 32 deletions
|
|
@ -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)))
|
||||
})
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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())
|
||||
})
|
||||
})
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue