-
-
Notifications
You must be signed in to change notification settings - Fork 290
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
feat: Added music support through Lidarr #1238
base: develop
Are you sure you want to change the base?
Conversation
This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged. |
@0-Pierre do not merge develop into this PR. Rebase on develop instead. Could you please rebase and remove all theses merge commits? |
Apologies, I'm not very familiar with the GitHub processes and features. I'll do my best, but the first commit was automatically created during a rebase when I used the "Rebase current branch..." option. |
A preview for this feature is available as (In about 15-20 mins) |
Hi! I briefly tested the PR. Great work!
Thank you again for the work! |
@AlessandroTischer Hey! Thanks for your notes, here's some points : On my end, I’m using it in production with all my users, and so far, I haven’t encountered any major bugs or errors causing server crashes. I’ll continue monitoring the logs. 👀 🔥 |
Hey, @0-Pierre just popping by to say awesome work! It's fun to finally be able to play around with music requests, even though it's just a preview. Sharing some additional feedback as a Plex/Jellyseerr user:
Unmonitoring either the artist/album and making the request again results with the same error. The only work around that I've found so far is to remove the artist altogether from Lidarr and request it again. That way, it get added to Lidarr the way you described above; however, the following is observed:
Additionally, the artist gets added to Lidarr without a metadata profile; unsure if that is expected behavior. For context, I am running everything on Windows 11 and have tried using both the official Lidarr service as well as with Once again, thanks for putting in the work and time to make this! |
@gssariev Thank you for your tests! I actually haven't tested it in a Plex setup situation yet. I'll review all the issues you reported and will get back with a commit to fix them. 😉 |
… and applied prettier to migration file
@0-Pierre great work so far, I was hoping one day we would get this added. Some items I am seeing so far; Running in docker with emby "play on" not showing for available media player |
This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should only have the en.json
using the pnpm i18n:extract
for the i18n locales. The rest of the translation strings should be updated through weblate.
Could you remove those translation files and rebase?
Sure, i'm fixing all the requested issues and I'll take care of that 👍 |
@gssariev Hello! I'll be pushing a new commit that resolves all the Plex-related issues. However, I'm having trouble reproducing your other issues, as everything seems to be working perfectly on my end — requests are sent and downloaded properly. I even set up a Plex test server, and everything is running smoothly with the same Docker version you're using. I've also implemented proper mapping for the Plex music library since Plex doesn't handle it like other libraries. They rely solely on release IDs, so we’re now mapping those to release-group IDs to ensure the database gets the correct entries. Gotta love Jellyfin for handling things more consistently in this area, lol. Let me know if you encounter any additional issues! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just started a review, not done yet, here is what i noticed for now.
I stopped it because i'm pretty sure you messed up a lot of things during merge.
For instance: you removed a bunch of updates related to OverrideRules in MediaRequest.request() and changed the behavior to the old one.
I don't want to be rude, but please be careful when you rebase on the develop branch. This is already huge and hard to review, so please help us by making it easier.
Can you please fix these rebase/merge issues? Thanks for your work!
@@ -50,51 +51,57 @@ export const defaultSliders: Partial<DiscoverSlider>[] = [ | |||
order: 3, | |||
}, | |||
{ | |||
type: DiscoverSliderType.POPULAR_MOVIES, | |||
type: DiscoverSliderType.POPULAR_ALBUMS, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious why would you put music above all movies/series? IMO it makes more sense to add it at the end of the list
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding an album slider to the default sliders will not make albums appear in everyone's instances.
You should probably add a migration that adds an album slider to the list of the existing sliders, so people with an already configured instance can see albums.
LYRICS = 4, | ||
OTHER = 5, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing the value of OTHER
from 4 to 5 will break existing rows that have an OTHER
value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think your migrations are not properly generated. I see missing things (an example: the change of tmdbId
from non-nullable to nullable).
And the PostgreSQL migrations are missing too.
See how to generate migrations in our contribution guide: https://github.com/Fallenbagel/jellyseerr/blob/develop/CONTRIBUTING.md#migrations
if (!Array.isArray(tmdbIds)) { | ||
finalIds = [tmdbIds]; | ||
} else { | ||
finalIds = tmdbIds; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happened to this part? If the ids
argument is not a list but a primitive?
const whereClause = | ||
typeof id === 'string' | ||
? { mbId: id, mediaType } | ||
: { tmdbId: id, mediaType }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it better to assign the where clause by checking the mediaType
instead of the type of the arg?
if ( | ||
requestBody.mediaType === MediaType.MOVIE && | ||
requestBody.mediaType === MediaType.MUSIC && | ||
existing[0].status !== MediaRequestStatus.DECLINED | ||
) { | ||
logger.warn('Duplicate request for media blocked', { | ||
tmdbId: tmdbMedia.id, | ||
mbId: requestBody.mediaId, | ||
mediaType: requestBody.mediaType, | ||
is4k: requestBody.is4k, | ||
label: 'Media Request', | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happened to the check for MediaType.MOVIE
?
@@ -208,131 +257,116 @@ export class MediaRequest { | |||
} | |||
} | |||
|
|||
// Apply overrides if the user is not an admin or has the "advanced request" permission |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this deleted?
const overrideRules = await overrideRuleRepository.find({ | ||
where: | ||
requestBody.mediaType === MediaType.MOVIE | ||
? { radarrServiceId: defaultRadarrId } | ||
: { sonarrServiceId: defaultSonarrId }, | ||
}); | ||
|
||
const appliedOverrideRules = overrideRules.filter((rule) => { | ||
const hasAnimeKeyword = | ||
'results' in tmdbMedia.keywords && | ||
tmdbMedia.keywords.results.some( | ||
(keyword: TmdbKeyword) => keyword.id === ANIME_KEYWORD_ID | ||
); | ||
|
||
// Skip override rules if the media is an anime TV show as anime TV | ||
// is handled by default and override rules do not explicitly include | ||
// the anime keyword | ||
if ( | ||
requestBody.mediaType === MediaType.TV && | ||
hasAnimeKeyword && | ||
(!rule.keywords || | ||
!rule.keywords.split(',').map(Number).includes(ANIME_KEYWORD_ID)) | ||
) { | ||
return false; | ||
} | ||
if (requestBody.mediaType !== MediaType.MUSIC) { | ||
const defaultRadarrId = requestBody.is4k | ||
? settings.radarr.findIndex((r) => r.is4k && r.isDefault) | ||
: settings.radarr.findIndex((r) => !r.is4k && r.isDefault); | ||
const defaultSonarrId = requestBody.is4k | ||
? settings.sonarr.findIndex((s) => s.is4k && s.isDefault) | ||
: settings.sonarr.findIndex((s) => !s.is4k && s.isDefault); | ||
|
||
const overrideRuleRepository = getRepository(OverrideRule); | ||
const overrideRules = await overrideRuleRepository.find({ | ||
where: | ||
requestBody.mediaType === MediaType.MOVIE | ||
? { radarrServiceId: defaultRadarrId } | ||
: { sonarrServiceId: defaultSonarrId }, | ||
}); | ||
|
||
if ( | ||
rule.users && | ||
!rule.users | ||
.split(',') | ||
.some((userId) => Number(userId) === requestUser.id) | ||
) { | ||
return false; | ||
} | ||
if ( | ||
rule.genre && | ||
!rule.genre | ||
.split(',') | ||
.some((genreId) => | ||
tmdbMedia.genres | ||
.map((genre) => genre.id) | ||
.includes(Number(genreId)) | ||
) | ||
) { | ||
return false; | ||
} | ||
if ( | ||
rule.language && | ||
!rule.language | ||
.split('|') | ||
.some((languageId) => languageId === tmdbMedia.original_language) | ||
) { | ||
return false; | ||
} | ||
if ( | ||
rule.keywords && | ||
!rule.keywords.split(',').some((keywordId) => { | ||
let keywordList: TmdbKeyword[] = []; | ||
|
||
if ('keywords' in tmdbMedia.keywords) { | ||
keywordList = tmdbMedia.keywords.keywords; | ||
} else if ('results' in tmdbMedia.keywords) { | ||
keywordList = tmdbMedia.keywords.results; | ||
const appliedOverrideRules = overrideRules.filter((rule) => { | ||
if (isTmdbMedia(tmdbMedia)) { | ||
if ( | ||
rule.language && | ||
!rule.language | ||
.split('|') | ||
.some( | ||
(languageId) => languageId === tmdbMedia.original_language | ||
) | ||
) { | ||
return false; | ||
} | ||
|
||
return keywordList | ||
.map((keyword: TmdbKeyword) => keyword.id) | ||
.includes(Number(keywordId)); | ||
}) | ||
) { | ||
return false; | ||
} | ||
return true; | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same, why is all this deleted?
…operly display artist name in the slide-over menu
…ensured media are properly removed from Lidarr.
@0-Pierre any update with the fix of the merge issues? If you're unsure on how to proceed, it's ok we can help and do it ourselves, but please let us know |
I'd be glad if you could take care of this, as you're much more comfortable with the rebase process than I am. 🤝 Should I stop commiting for the moment ? |
Yes please. I'll try to do it by 1/2 days. |
Hi All, one of the Servarr devs here (we maintain Lidarr and it's backend). The Lidarr API exists solely for Lidarr's use and is aimed at it. We run a MusicBrainz mirror and ultimately do not test any changes with any products that are not Lidarr nor do we expect to support alternative products. As much as I appreciate what you are trying to do, there has been no conversation with ourselves surroudning the user of our API and our general expectation would be that Jellyseerr would run a similar service themselves to support this instead of using our API as then changes to it can be tested/managed for compatibility with Jellyseerr. |
Hi, sorry for the use of your API. We (maintainers) didn't catch that from this PR as we have not yet reviewed this and it wasn't our intention to use something we shouldn't have (we thank you for the confirmation). However, would you be willing to discuss with us about the use of this API? If not, that's fine, thanks for your awesome work on Servarr and Lidarr 🙂 |
Our options are quite limited. The MusicBrainz API is too slow and has a low rate limit, which prevents it from delivering a smooth experience like TMDB. A possible solution would be to set up a Jellyseerr mirror API, similar to Lidarr's approach. This mirror API could prefetch data, populate the database with CCA images, and retrieve artist images from Fanart.tv using a global API key and maybe the missing one with LastFM with a global key for the API. This approach could offer additional benefits, such as more detailed data, preloaded charts, and an overall enhanced experience. |
It's always possible to self-host a music brainz instance, they provide a full setup guide and everything. Issue is that it requires comparably potent hardware. |
Yep, I started to setup a mirror on a VPS I don't really use, but the issue is the hardware requirements that are quite high, especially if we want something fast enough for thousands of users. |
Using MusicBrainz would work but take more time to gather all the data, which I think is an alright compromise, isn't it? Even it it takes a couple of days to scan a complete library, it shouldn't be a big deal. |
I think there are multiple aproaches to attack this problem (some can be combined)
Other ideas? |
Why not use arl deezer? and other alternative sources for information. The MB database does not have the same content as streaming services. And their moderation in 7 days is depressing. |
I can easily adjust the API to use MusicBrainz instead, but this would require all images to be fetched from CCA for covers. Since CCA servers are quite slow, these images will need to be cached. Additionally, we would need to use a global API key from Fanart (similar to the TMDB key) to retrieve artist covers and images. I will create a new branch lidarr-mb to make tests. |
I've already considered this, but the main issue is that Lidarr relies on unique mbIds, while Deezer does not, which would inevitably cause mismatches over time. To ensure accuracy in media requests without manually mapping each item to the correct ID and to avoid running into rate limits we must adhere to the source app's identification system. The only viable solution would be a version of Lidarr built entirely around using Deezer as its indexer. |
hm, maybe a unified metadata service with a standard api set that we can work into all the arrs and jellyfin and jellyseerr, that we can self-host, with a pull-through mode, would be an interesting side-project in case I ever get time. |
After discussions with @fallenbagel and the Lidarr dev, we're going to host our own MusicBrainz mirror. Since the costs of hosting the mirror are going to be high, we're going to launch a fundraising initiative to finance these servers. We'll be discussing with Fallen how we're going to proceed. @0-Pierre, could you please list all the MusicBrainz endpoints used in this PR (as well as the parameters used)? So that I can think about how to make the api for the MusicBrainz mirror (i'm not sure we will use the api in Perl they provide) |
@gauthier-th I've started reworking the API in a new branch called lidarr-mb, based on the latest version of the current branch. So far, I've achieved response times comparable to Lidarr's mirror by applying some optimizations. Would it make sense to give me a few days to fully align the lidarr branch with lidarr-mb? After that, We could run some preview tests to evaluate its reliability for end use. |
Sure. Let us know how it goes. |
Great news! I'm pretty sure we will not need a mirror server, the main issue with using the MusicBrainz API was: Occasional 503 Errors: During searches or API calls, 503 errors would sometimes occur (server busy not a rate limit issue). I've added an immediate, invisible retry mechanism that resolves this problem effectively. Slow Loading Times Due to CAA Image Fetching: Unlike the Lidarr API, which always has properly mapped URLs for album covers in its database, MusicBrainz doesnt provide it. The solution I implemented is simple and works seamlessly. After analyzing how mirrors like ListenBrainz operate, I discovered they store pre-mapped CAA URLs for each album ID. To address this: I will now review the process of fetching artist images from fanart.tv or other sources. If it introduces significant delays to requests, I will work on implementing a similar caching system to optimize performance. This approach ensures instant loading as the smallest image versions are cached and ready for display. Actually, 500 prefetched album covers and their mappings take up less than 10 MB in the cache folder. Using the MB API resolves the mismatches present in the Lidarr API, such as incorrect artist-to-album associations. Additionally, it provides more album images and significantly improves the accuracy of the results, so I will be able to add country flags on album details, tags etc.... |
You are an absolute god |
any chance to get a config option to point to our own selfhosted mb instance? for inconsistent internet situations and / or data capped (think rural 4g) I have a relative that likes to queue up their requests on their server and just lets it do its thing behind the scenes but hates how slow things load sometimes |
No worries, it's actually very fast, maybe even faster than using the Lidarr API. It will be pushed soon. |
Just wanted to chime in and say great work to @0-Pierre and all involved. Looking forward to this |
Out of curiosity, how is the caching system designed to handle large data sets efficiently? |
@formeo14 We're not replicating the entire MB database in the cache only storing the image paths and TMDb IDs associated with an Artist MBID. On each call, we check the cache to see if any previously missing data is now available. For instance, if a person in TMDb has been created or a new image has been added to CAA, it will be added to the cache for the corresponding artist. |
Description
Jellyseerr fully support music trough Lidarr 🚀 🎵
Metadata
We’re using the Lidarr API (api.lidarr.audio) to improve speed, as MusicBrainz (MB) often suffers from high latency and server overload issues.
Trade-off: The results are slightly less precise compared to the MB Search API.
Overview
The overview section pulls data from Wikipedia, ensuring multilingual support for more flexible and accurate descriptions.
Image Handling
Images are initially loaded from the provided links and cached locally.
The Lidarr API offers additional artist images that aren’t available in the Cover Art Archive (CAA).
If an image is missing, the system automatically falls back to CAA.
Caching
By default, images from CAA and Fanart are cached locally, as these sources tend to load slowly.
Cached images are stored in the /cache folder and can be cleared via the settings page using a dedicated job.
Other images are only cached if specific options are enabled.
Similar Artists / Trending
We use the ListenBrainz API to provide recommendations for similar artists and trending music. Missing images are completed through the Lidarr API, with a fallback to CAA.
Artist Groups
The /group endpoint is dedicated to artist groups (bands). For individual artists (persons), we merge data with the /person endpoint from TMDb to avoid duplicate entries.
Informations Points
There are cases where images aren’t available on either the CAA or the Lidarr API. In such instances, we need to upload them to the MusicBrainz database to ensure they are displayed. This aligns with the core purpose of MusicBrainz, a community-driven platform for sharing metadata.
For this reason, I’ve removed Deezer as a fallback. Encouraging community contributions is more in line with MusicBrainz’s philosophy. Additionally, using Deezer’s fallback wasn’t always reliable, as the matching wasn’t based on precise IDs, though it worked correctly about 95% of the time.
Screenshot (if UI-related)
To-Dos
pnpm build
pnpm i18n:extract
Issues Fixed or Closed