lyarenei / jellyfin-plugin-listenbrainz

ListenBrainz plugin for Jellyfin.
MIT License
65 stars 2 forks source link

Concept: Provide listen import endpoint #51

Open Maxr1998 opened 8 months ago

Maxr1998 commented 8 months ago

A big problem of the current Jellyfin API is that currently only “live” listens can be submitted when using the PlaybackStoppped event. An alternative exists, but this has its own share of disadvantages. Instead, I suggest creating an import endpoint in the plugin, where offline listens can be submitted by (third-party) music players. The API could be designed as follows: The import endpoint simply receives a list of StoredListens as JSON (which are already used for the submission cache), and either submits them immediately, or adds them to the cache to be submitted by the resubmission task later. To ensure that stale/removed IDs don't cause listens to be dropped, the endpoint could optionally receive additional metadata like title, artist, album, and a MusicBrainz track ID, if available. Thus, if the Jellyfin item ID is missing from the database, the items can be rediscovered though the alternative metadata. What are your thoughts on this feature suggestion?

lyarenei commented 7 months ago

This is something I have been thinking about even before I got a suggestion for the alternative mode. Ultimately, I decided against it at that time. In short, while I'd like to have the whole situation with this resolved, I am not really a fan of a custom endpoint.

The biggest issue I see is with adoption. Technically, it would be a special endpoint of some "random" plugin, which is not even an official one. So, in my opinion, it is not much likely it will be adopted by clients. Also, if the client would adopt such endpoint, I think at that point it would be just better to integrate with ListenBrainz directly instead. Not only the API token would be stored more securely, it would be also better over all as the client has the ultimate control over what's being played. But, I am aware this server-side plugin approach is at least more convenient over all, be it for users - as they have the configuration at one place and client developers as well, as they only have to implement Jellyfin APIs and that's it. The only issue with that approach is the security of the whole thing, but that's specific to the plugin limitations (obfuscated tokens in configs, admin has access to the tokens of all users, ...).

With that being said, I know, I kind of undermined my point about the whole direct client integration with the plugin alternative mode. But, at least, it still uses Jellyfin API endpoints, nothing custom. It'd be much nicer to have some sort of support on the Jellyfin server itself. Or at least have some formal specification of the endpoint somewhere, so that it could be adopted by plugins for other services, not just by this one (I guess you can argue that the readme of this repo would fit nicely).

But maybe I am overlooking something?

Maxr1998 commented 7 months ago

The biggest issue I see is with adoption. Technically, it would be a special endpoint of some "random" plugin, which is not even an official one. So, in my opinion, it is not much likely it will be adopted by clients.

Yeah, I definitely agree. I was planning to add support to at least Finamp (see also jmshrv/finamp#521), but that's just a single client. For the official apps, support for an API of custom plugins is very unlikely.

Also, if the client would adopt such endpoint, I think at that point it would be just better to integrate with ListenBrainz directly instead. Not only the API token would be stored more securely, it would be also better over all as the client has the ultimate control over what's being played. But, I am aware this server-side plugin approach is at least more convenient over all, be it for users - as they have the configuration at one place and client developers as well, as they only have to implement Jellyfin APIs and that's it. The only issue with that approach is the security of the whole thing, but that's specific to the plugin limitations (obfuscated tokens in configs, admin has access to the tokens of all users, ...).

Whether it's stored more securely or not depends on whether you trust the server or the client more. Since I'm running my Jellyfin server only for myself, I prefer not having API tokens for LB on every client - there's only a single type of authentication necessary, that is the Jellyfin login. Having config files with API tokens on the server isn't a huge issue for me - if any attacker gets access to my server, that's already the worst possible situation. But the security aspect isn't really a concern for me anyway, the convenience factor is by far the most important one for me. I also prefer having a central instance (the plugin) submit my listens, as it's always consistent with its metadata (regarding the played recordings but also the "player" information).

With that being said, I know, I kind of undermined my point about the whole direct client integration with the plugin alternative mode. But, at least, it still uses Jellyfin API endpoints, nothing custom. It'd be much nicer to have some sort of support on the Jellyfin server itself. Or at least have some formal specification of the endpoint somewhere, so that it could be adopted by plugins for other services, not just by this one (I guess you can argue that the readme of this repo would fit nicely).

I'd love if there was a formal specification or API to hook into, if only an official stub that would then be left to plugins to implement. The issue is just that server development is slow (out of necessity, of course), and new APIs aren't added just like that. Maybe starting with a third-party API, iterating on it if necessary (which is much easier when you can test it in the real world), and slowly turning it into an official API would be the best approach?

lyarenei commented 7 months ago

Whether it's stored more securely or not depends on whether you trust the server or the client more. Since I'm running my Jellyfin server only for myself, I prefer not having API tokens for LB on every client - there's only a single type of authentication necessary, that is the Jellyfin login. Having config files with API tokens on the server isn't a huge issue for me - if any attacker gets access to my server, that's already the worst possible situation. But the security aspect isn't really a concern for me anyway, the convenience factor is by far the most important one for me.

Well, to be fair, I'd say majority of all Jellyfin instances would be like that. While security is not much of a concern for local deployments (which is reasonable), I think there is still large group of users that have Jellyfin instances somewhere hosted (myself included in the past). Yes, it's their own responsibility to properly secure the server and yes, if someone has an access to your server, you have much bigger problem, but it's also a good idea to minimize the damage in such case (attacker might get the data, but they are useless if they are secured in some way). But let's not get hung up on this too much.

I'd love if there was a formal specification or API to hook into, if only an official stub that would then be left to plugins to implement. The issue is just that server development is slow (out of necessity, of course), and new APIs aren't added just like that. Maybe starting with a third-party API, iterating on it if necessary (which is much easier when you can test it in the real world), and slowly turning it into an official API would be the best approach?

I see you have started a discussion in the meta repo, so for now I think it'd be best to wait and see if this goes somewhere. But if not, we can think about the API spec and implementation, sure. 🙂

Maxr1998 commented 7 months ago

I see you have started a discussion in the meta repo, so for now I think it'd be best to wait and see if this goes somewhere. But if not, we can think about the API spec and implementation, sure.

Right, let's do that :+1: