Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problematic API issue with artists / albums #6048

Closed
Tolriq opened this issue May 15, 2021 · 21 comments
Closed

Problematic API issue with artists / albums #6048

Tolriq opened this issue May 15, 2021 · 21 comments
Labels
bug Something isn't working

Comments

@Tolriq
Copy link

Tolriq commented May 15, 2021

Describe the bug

API returns non matching data for artists causing problems for API consumers.

System (please complete the following information):
Any OS, reported mostly with 10.7.X but maybe present before.

To Reproduce

Query the artists :

http://xxxxx:8096/Artists?Recursive=true&ParentId=xxxx&Fields=Genres,SortName,ProviderIds,DateCreated&startIndex=0&limit=1000&UserId=xxxxx

You will get for example

{"Name":"Adele","ServerId":"xxxxxxx","Id":"935931fba2250076087b1dcfe0d9d39c","DateCreated":"2020-06-21T18:39:28.2497865Z","SortName":"adele","ChannelId":null,"Genres":["Pop","Pop Soul","Folk"],"RunTimeTicks":105382797312,"ProviderIds":{},"Type":"MusicArtist","GenreItems":[{"Name":"Pop","Id":"f1f202f389018ad2c0766af4b0fcb155"},{"Name":"Pop Soul","Id":"2ca505ceb89ddb4b3def5271883f7cf9"},{"Name":"Folk","Id":"9a542eba8ecf66ca66dc99ecf3452477"}],"UserData":{"PlaybackPositionTicks":0,"PlayCount":0,"IsFavorite":false,"Played":false,"Key":"Artist-Adele"},"ImageTags":{},"BackdropImageTags":[],"ImageBlurHashes":{},"LocationType":"FileSystem"}

Notice the ID: 935931fba2250076087b1dcfe0d9d39c

Then request the albums:

http://xxxx:8096/Users/xxx/Items?Recursive=true&IncludeItemTypes=MusicAlbum&ParentId=7e64e319657a9516ec78490da03edccb&ExcludeLocationTypes=Virtual&Fields=Genres,SortName,Path,DateCreated,ProductionYear,CommunityRating&startIndex=0&limit=1000&UserId=xxxx

And the albums will have:

"ArtistItems":[{"Name":"Adele","Id":"fe0ce443bd0a8c005e05e62c4b47b45b"}],"AlbumArtist":"Adele","AlbumArtists":[{"Name":"Adele","Id":"fe0ce443bd0a8c005e05e62c4b47b45b"}]

Another ID for the artist making it impossible to match both from the API.

Expected behavior
The same ID or both IDS should be returned by both end points when Jellyfin have multiple IDs internally.

@Tolriq Tolriq added the bug Something isn't working label May 15, 2021
@Tolriq
Copy link
Author

Tolriq commented May 26, 2021

Small bump, this is quite problematic as breaks Music handling in those cases.

@dkanada
Copy link
Member

dkanada commented May 26, 2021

This might have to wait for larger changes in the database. Artists are internally handled as two separate entities depending on what you're requesting and no one has decided to tackle the huge rebase it would require yet.

@Tolriq
Copy link
Author

Tolriq commented May 26, 2021

How is it handled internally ? No way to expose the 2 Ids at least at one of the end points ?

@dkanada
Copy link
Member

dkanada commented May 26, 2021

That might be possible :) I'm just explaining why it works the way it does haha. Usually everyone on the client teams say something will take ages to implement and then someone on the server team chooses that moment to generate a pull request though, so who can really say how involved it would be.

@cvium
Copy link
Member

cvium commented May 26, 2021

It would require an API change to return both (or more) IDs, so that's mostly out of the question.

@Tolriq
Copy link
Author

Tolriq commented May 26, 2021

Non breaking API changes are normal and wanted :)

For the record there were a few breaking changes recently, like no more accepting strings for Boolean or empty string as default Int for the transcoding API ;) Way more impactful.

@cvium
Copy link
Member

cvium commented May 26, 2021

For the record there were a few breaking changes recently, like no more accepting strings for Boolean or empty string as default Int for the transcoding API ;) Way more impactful.

And very likely unintended as the aim was for 1:1 compatibility (barring a few security and framework related issues). But we only know of the issues we know of ;)

@Tolriq
Copy link
Author

Tolriq commented May 26, 2021

You know this one :p Unlike most other things, this one have 0 workaround :(

@cvium
Copy link
Member

cvium commented May 26, 2021

Well, is this an issue in 10.6? If it's not, this is really low priority due to the nature of the issue.

@Tolriq
Copy link
Author

Tolriq commented May 26, 2021

This was reported a few times but I have no idea what and how to trigger this never happened on my library. For 98% of the users it works there's just 1 ID.

I don't know how to have them to fix on Jellyfin side, I know the issue was present in 10.6 and before and some users reported than starting fresh did fix this. But since I don't know the Jellyfin reason to get into that state, it's hard to tell more.

@Tolriq
Copy link
Author

Tolriq commented Jul 15, 2021

Getting recurrent issues about this and nothing I can do about.

Is there really no way to have a new field that expose all the IDs on one of the end points until the proper fix is done?

@Tolriq
Copy link
Author

Tolriq commented Jul 21, 2021

@cvium So it seems this issue is spreading a lot as I'm getting multiple reports per week now.

Again adding a new field while an API change is not a breaking change and can occurs and does occurs very often like when adding the image hashes and a few other cases.
Maybe the issue can be solved by just adding some sorts on the returned IDs internally so always the same is returned (no idea how the data is handled).

I'd really like an official statement about usage of Jellyfin from advanced clients that use the API cached and not online, as it might be easier for me to drop support for Jellyfin than dealing with the generated support.

@mcarlton00
Copy link
Member

Assuming you're still looking at this, you should be using the values pulled from the /Artists endpoint as ArtistIds= or AlbumArtistIds= (Not ParentId=) in your /Items call. It's annoying inherited behavior that clients supporting music just have to work around for now. This is how I'm doing it in mopidy:

https://github.com/jellyfin/mopidy-jellyfin/blob/760901a386ecaa500fa90374773cd1d617630d2e/mopidy_jellyfin/remote.py#L488-L500

@Tolriq
Copy link
Author

Tolriq commented Dec 30, 2021

The problem is that my app is fully offline and rebuild the DB locally if the Ids does not match I can't match them :) Currently I'm forced to do another query on the unknown ID to get some artist details then find out by fuzzy matching + item counts what is the proper IDs, this is error prone and slow.

Doing 6000 queries to ask for each artists albums instead of getting all albums at once is also incredibly slow and inefficient.

But even the Web interface of Jellyfin is bugged in those cases, the artist is different depending on how you browse and have the metadata or not and the properly affected media or not.

In all cases, adding a new field that would return all the IDs would not be a breaking change and would easily fix the issue, there was already millions of additions to the API. But it seems external clients support is not that important seeing that no answer was an answer.

@Tolriq
Copy link
Author

Tolriq commented Dec 30, 2021

Thanks @nielsvanvelzen for the very contributive thumb down :) Maybe you have a proper solution or something so say to solve this issue?

Explain how an application is supposed to be able to get all artists and all albums and match them for offline use in an efficient way?

Get all artists ArtistX have an ID a, get all albums album say artistX have ID b. Query artist a get some info not containing artist b Id, query artist b get some info not containing artist a id.
Only solutions:

  1. match artist a and b by name and checking the childitemcounts are matching too (hoping there's no duplicate name with same values) (That's what I do as an ugly workaround)
  2. Query all artist then query all albums for each artists 1 by 1, takes eternity to complete (more or less 15 more times for a sync that is completely unacceptable).

What is your proposal for advanced applications? (Highly rated, millions downloads things like that ...)

@nielsvanvelzen
Copy link
Member

The thumbs down is for this part of your comment:

But it seems external clients support is not that important seeing that no answer was an answer.

Third party clients are extremely important for the Jellyfin ecosystem.

So back to this issue:

We (the Jellyfin contributors) are 100% aware of the limitations of the current API and multiple people are working hard to fix it (rewriting the database, moving the API over to ASP.NET, planning Jellyfin 11, etc.). This takes time though, as all of us are unpaid developers doing this in their free time. If you hit a limitation of the current API and believe it can be easily fixed you're free to open a pull request.

Even the official clients run into issues like this. As an example: I'm working on our Android TV app and can't implement the quick connect functionality until 10.8, because it required changes to get it to work properly.

It's not like we don't want to fix this and all the other issues. We just don't have the capacity to do it.

@Tolriq
Copy link
Author

Tolriq commented Dec 30, 2021

This might have to wait for larger changes in the database.

It would require an API change to return both (or more) IDs, so that's mostly out of the question.

Made it clear that a PR was unwanted as API change, then vanish without any further answer, when asking about support for external clients again no answer.

I have made many PR to many things, but never without approval first, since as you said, time is precious.

About the issue itself, Jellyfin have the data as it can match the albums internally, it's just a matter of exposing it or ensuring a sort so that the IDs matches. Without discussions and vision of where Jellyfin is headed, this can't be just PRed. Adding the field if the issue is planned to be removed at scrapping level makes no sense, adding sort can be a breaking change too as a different ID may be returned for people who previously cached them. Those decisions are not possible to be taken randomly by external contributors.

BTW about:

Third party clients are extremely important for the Jellyfin ecosystem.

I did ask

I'd really like an official statement about usage of Jellyfin from advanced clients that use the API cached and not online, as it might be easier for me to drop support for Jellyfin than dealing with the generated support.

Without answer since July, so yes the lack of answer was an answer by itself, leading to my comment, there's nothing to thumbdown in facts.

@ferferga
Copy link
Member

ferferga commented Dec 30, 2021

@Tolriq I think this has been clear since almost the beginning of this issue:

It would require an API change to return both (or more) IDs, so that's mostly out of the question.

But for some reason you've been repeating yourself over and over around the same idea

As for your second point: I don't actively work in server but just a 1 minute search led me to the following results:
#1431
#1431

As you can see both PRs are referenced by newer PRs that improve upon the original one (like #6049 from me).
If you broaden your search, you'll see that we even have a repository for RFCs and roadmaps: jellyfin-meta

If you take a look there, you'll see there's only an OpenAPI schema that makes reference to an "hypotetical" version 11. Why nothing else? Basically because there are no roadmaps yet. Why? Because it makes no sense to acomplish such major task without understanding the codebase properly and when we're still facing many inherited bugs.

I'll give you props to the lack of documentation, because it's true that we mentioned multiple times the point of 11.0 of being the release with the new API, sane DB design (and, overall, the one where we're happy to improve upon and we don't need to be stuck with all the Emby shenanigans), but all of that has been done in the public Matrix's rooms and Reddit. We don't have a clear vision for what we want 11.0 to be, so what exactly would go to an RFC or roadmap file regarding this is probably not very useful still.

RFCs implies that anybody can participate in the design process. How so, if this is so concerning to you, you (and nobody really) did a proper RFC process? Why we need to be the first ones? For further context, the main contributors here in server are waiting for us, main client devs, to start proposing designs for the new API. Clients are the one that are going to consume the API, so it makes sense that they are the ones that puppet the API design process.

Why we, client devs, haven't done it? Well, for the same reasons as the server guys face: we're still fighting to get a good codebase we're happy to work with and stable clients. As you said in your original message:

time is precious

And that's exactly what we lack the most here, time, as we don't earn any money from this and some of us need to work or study along the way.

You seem to be a Yatse developer, a client that has premium features, so you're benefitting and earning money from using our API in some way or another, right? If Jellyfin support is so important to you and your customers, instead of going here setting things in fire, I suggest you can start by creating the:

discussions and vision of where Jellyfin is headed

You're consumer of our API, so you're also the community (but you don't have a single PR in server, despite your claims). We don't have to do everything. We might have the final say of what gets approved or what not, but at the end of the day every proposal is useful. No standard has been made without a mix of different visions.

If you're earning money with Yatse thanks to our API support and it makes sense to help us, you're more than welcome. If not, we honestly don't care if you don't want to support our API, but please don't make us lose our precious time in this kind of pointless discussions that leads to nowhere. As the GPL license states:

EXCEPT WHEN OTHERWISE STATED IN WRITING THE COPYRIGHT HOLDERS AND/OR OTHER PARTIES
PROVIDE THE PROGRAM "AS IS"

If you don't like how Jellyfin AS IS, no problem for us, but we don't have to support your inquiries, less so if made in this kind of impositive manner.

@Tolriq
Copy link
Author

Tolriq commented Dec 30, 2021

It would require an API change to return both (or more) IDs, so that's mostly out of the question.

But for some reason you've been repeating yourself over and over around the same idea

I just tried to get an answer about why, API change at each releases, I do understand breaking change, but refusing to update API to fix things without breaking happens every day, no API change ever means stop of updating the product ....

You seem to be a Yatse developer, a client that has premium features, so you're benefitting and earning money from using our API in some way or another, right?

I was asked by users and some Jellyfin members to support Jellyfin early, making millions of people aware of the fork in the early days and encouraging them to use it, I'm still labelled Kodi remote and do not gain much here but brought a lot of publicity to you.

You're consumer of our API, so you're also the community (but you don't have a single PR in server, despite your claims).
I never claimed anything about PR for Jellyfin, I said I made many PR to many projects but always after discussions, here there was no discussion so no PR. Do not change the meaning of what I write.

If you don't like how Jellyfin AS IS, no problem for us, but we don't have to support your inquiries, less so if made in this kind of impositive manner.

You mean report a problem, get an answer that makes no sense, ask for details being ignored? Where did I impose anything? You seems a little angry but should take time to properly read what I wrote. I asked for details, get none, stopped asking, today someone post a new message, this does not solve the issue, I add again the details, end of story.

but please don't make us lose our precious time in this kind of pointless discussions that leads to nowhere

You mean discussing about a major issue and trying to find how to fix it? Where does it leads to nowhere? If we can't report issues and propose solutions why having issues opened?

About API changes and future, it's nowhere advertised that external contributor should ask things, until now it was said that API was going to be stable until a rework, and joshua boniface did warn me a couple of time when there was breaking change.
I from the start asked about API #96 and future and everything there were a few discussion on other places too.

So please take time to calm down and see the global situation before assaulting :)

@ferferga
Copy link
Member

ferferga commented Dec 30, 2021

I just tried to get an answer about why, API change at each releases, I do understand breaking change, but refusing to update API to fix things without breaking happens every day, no API change ever means stop of updating the product....

@Tolriq Again, I think the response from us have been crystal clear since the beginning and you just don't want to accept it:

dkanada pointed you out that Artists and AlbumArtists are different entities in the database

cvium confirmed you that merging artists and album artists means changes to the original Emby's API spec

cvium asked if this was happening in 10.6 and you confirmed it, verifying the fact that this is a "feature" of the API and not a bug (although implicitly everybody here agreed with you that this makes no sense).

mcarlton provided a workaround so you can circumvent this poor design choice, confirming you that even us need to circumvent our API flaws and it's not a matter that we're forgetting or don't want to fix, it's just clear that we can't. If we could fix them, we would as it's even affecting ourselves.

API has been historically changed multiple times to include new endpoints or for security reasons, but we've been striving for keeping all the original things untouched (besides a few stuff that changed because the ASP.NET migration, which were fixed afterwards. Examples here and here

I was asked by users and some Jellyfin members to support Jellyfin early, making millions of people aware of the fork in the early days and encouraging them to use it, I'm still labelled Kodi remote and do not gain much here but brought a lot of publicity to you.

Again, we don't care about publicity. We don't need to make money, surpass other solutions or anything like that. We just want to build the best possible solution in a voluntary-driven way. Publicity are necessary for businesses, for us it's just a welcomed thing. If we cared about it, we would've spend money on it

I never claimed anything about PR for Jellyfin, I said I made many PR to many projects but always after discussions, here there was no discussion so no PR. Do not change the meaning of what I write.

You're right about the PRs here, but my point of your ability to start these discussions on your own still stands.

You mean report a problem, get an answer that makes no sense, ask for details being ignored?

Explained above, doesn't make sense for you but it's the API we inherited and, unfortunately, we need to maintain it until we come up with a new one in 11.0 that phases out this one.

Where did I impose anything?

Implicitly insisting us in "fixing" this when you've been told many times we can't break the API counts as imposition, in my opinion.

You mean discussing about a major issue and trying to find how to fix it? Where does it leads to nowhere? If we can't report issues and propose solutions why having issues opened?

Again, there's no issue or bug, this is the API design we inherited. Leads to nowhere because everything has already been said from us.

About API changes and future, it's nowhere advertised that external contributor should ask things, until now it was said that API was going to be stable until a rework, and joshua boniface did warn me a couple of time when there was breaking change.
I from the start asked about API #96 and future and everything there were a few discussion on other places too.

Take a look at our first paragraph in our Contributing code guidelines:

Even if you can't contribute code, you can still help Jellyfin! The two main things you can help with are testing and creating issues. Contributing to code, documentation, translations, and other non-code components are all outlined in the sections below.

We also have Matrix rooms for this exact purpose. The whole point of open source is open contribution between everybody, so I don't know what exactly made you think otherwise.

So please take time to calm down and see the global situation before assaulting :)

Sorry if you felt assaulted, that wasn't my intention. My goal was to hopefully point you properly to all the facts that you brought up (incorrectly) to this issue, such as your misconception of considering that we vanished without any further answer (when our reply was clear, as I said above), that we don't want a PR for this (dkanada pointed you that no one tackled the DB rewrite library stuff requires yet, why not you? Is this not enough indication?) and the one where you think external users are not welcome to contribute here.

(For the record, all of us were "external users" before getting into the team, we got here contributing stuff we wanted/needed for ourselves. This is why sometimes we call JF a doo-cracy)

Before finishing, I would like to comment in this statement:

What is your proposal for advanced applications? (Highly rated, millions downloads things like that ...)

Our applications are also used by millions and @nielsvanvelzen is one of the leaders of the Android app, which is also highly rated. Play Store is our only real source for ratings, for the rest we rely in Docker pulls (we don't collect telemetry), which currently are sitting over 100M downloads (and not everybody uses Docker, so this number might be well off). He told you that we know the workarounds are bad, but there's nothing we can do to make this more efficient rather than using workarounds until we got a sane design - yet you still kept asking for something efficient.

I don't know what you want to imply with you application being "advanced" and "with millions of downloads", as if that makes our work less worthy. Our proposals are to use the workarounds we already gave you.

In Vue (the one I work the most) we also had to use workarounds for music.


Please feel free to open a new one with your proposal for tackling the database rewrite (the schema is already merged, but you can also contribute changes if you feel something's off) and the API changes required to tackle this issue, so we can continue and get the original problem solved.

I'm going to close this issue now as you already got the answer for your original question and, going forward, this issue can only be tackled with the aforementioned design proposals (which are a unrelated to the topic of this particular issue).

Thank you very much for pointing out this design issues so somebody coming up later are already aware of this distinction the DB makes between artists.

@Tolriq
Copy link
Author

Tolriq commented Dec 30, 2021

  1. Don't see "Implicitly insisting us" anywhere, I'm not native English speaker, what is to be understand is what is written.
  2. About publicity we are talking about 3 years ago when the fork happened, without users, no contributors either as you point out too. Just explaining why I added support of Jellyfin and that's no I'm not a vilain money maker taking advantages of open source as you implied.
  3. By advanced and highly rated (4.7/5), mean by the features the app propose that none of other Jellyfin client propose, high rating implies that all the features works. Fully offline with smart filters and smart downloads, require a complete copy of the data on the client side, no workaround was proposed to achieve that, even my workaround is not 100% guaranteed. So nothing wrong about your clients or any other clients, just that the needs are not the same at all between an online client that do calls for each subscreen and clients that works 100% offline with complete sync.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants