lyarenei / jellyfin-plugin-listenbrainz

ListenBrainz plugin for Jellyfin.
MIT License
78 stars 3 forks source link

Scrobble when receiving a request to the PlayedItems endpoint #25

Closed exitmouse closed 1 year ago

exitmouse commented 1 year ago

I use Jellyfin and jellyfin-plugin-listenbrainz, and I’d love if it would scrobble when I listen to a song in Symfonium. Currently it does scrobble if I listen on the web, and doesn’t if I listen in Symfonium (or Finamp, or Gelli, and it's inconsistent if I use Feishin). All of these players send PlaybackStopped events without PositionTicks set to the end of the track.

There are tracking issues for Finamp and Gelli https://github.com/jmshrv/finamp/issues/421 https://github.com/dkanada/gelli/issues/143 Feishin doesn't seem to have an issue for this but I am pretty sure it also doesn't always send the right PositionTicks. Of course this is also an issue in the jellyfin-plugin-listenbrainz repo: https://github.com/jesseward/jellyfin-plugin-lastfm/issues/41

In the case of Symfonium, I had a conversation with the author on the support forum: https://support.symfonium.app/t/playback-stopped-time-reporting-for-jellyfin/1777/1 Tolriq pointed out that Symfonium sends a zero PositionTicks value intentionally, and that the server should scrobble when a PlayedItem event is sent (as that's the only way for offline listens to be properly recorded).

It would be great if this plugin could do that, which would fix the issue for Symfonium, and give other players an easier path to be able to scrobble. Finamp has https://github.com/jmshrv/finamp/pull/200 with a workaround to always set PositionTicks to the end of the track.

The relevant endpoint in the jellyfin code is here: https://github.com/jellyfin/jellyfin/blob/1c72a8e0068fec8045884fa386a67c90a364ee0a/Jellyfin.Api/Controllers/PlaystateController.cs#L73 Although I don't understand Jellyfin plugin architecture enough to know if it's feasible to wrap that.

lyarenei commented 1 year ago

Hi, thanks for reporting. And yeah, this is a long-standing issue as you found out.

The relevant endpoint in the jellyfin code is here: ... Although I don't understand Jellyfin plugin architecture enough to know if it's feasible to wrap that.

Plugins cannot hook onto API endpoints directly, but they can react on various events emitted by the server. In this case, marking items as played emits a UserDataSaved event, so implementing this should be possible, assuming the event args contain all necessary info, which seems like it does.

Now, while this can be implemented, I have some mixed feelings about this - as it would take away any control of the plugin in regards to sending scrobbles. With this approach, it'd be up to the player to properly record if the played song was meaningfully played. Worst case, the player can just mark a song played, even just by playing for one second. Such scrobble would be invalid for ListenBrainz, while completely acceptable for Jellyfin, which has no rules.

On the other hand, one could argue that the plugin has no real control with current approrach anyway - as it still has to trust the client to send ReportPlaybackStopped with correct PositionTicks. So in the end, nothing would really change.

I think the best way to implement this would be as an optional alternative mode, with the current behavior being the default.

Let me know if you want to try it yourself, otherwise I'll implement this when I've got a bit more time.

lyarenei commented 1 year ago

Hi @exitmouse, if you are interested, you can test the new logic by grabbing the latest plugin build from jobs in #26. From what I was able to test with JF web client, it should work.

exitmouse commented 1 year ago

It works in Symfonium! And even Finamp, which I didn't realize already sent those requests! Thank you :)

lyarenei commented 1 year ago

Thanks for testing. I added some improvements and will do some more testing as well. I'll hopefully release this someday this week.

lyarenei commented 1 year ago

Released in plugin version 2.3.0.