Fix replace logic

[Shadowghost]

We never passed the flag for metadata replacement to the remote provider logic, hence the remote result would never replace what's already there.
Additionally, the logic did not repect if an item was locked or not

Changes

• Pass replace flag to remote providers
• Fix merge logic to only override empty fields, except when merging with the actual item
  • this will gradually build up metadata beginning with local providers and going through remote providers later on

Issues
Fixes #8532 (potentially)
Fixes #11294 (the part where it is repopulated)
Fixes #11656
Fixes #11717
Fixes #11773 (potentially)
Fixes #11831

Testing

• Add series, manually remove some metadata and replace all metadata -> should refill
• Add series, manually remove some metadata and fetch missing metadata -> should refill
• Add series, manually identify, remove some metadata and replace metadata -> should keep identified series
• Any other combination of the above and with replace images

[Shadowghost]

Not sure if this is correct.

[oddstr13]

I haven't studied the code, but I have the following questions;
How does this interact with multiple enabled metadata providers and the ordering of those?
What about fields that contain multiple values, such as Genres, People, Tags and such?

[sjorge]

What about fields that contain multiple values, such as Genres, People, Tags and such?

First provider wins I think? (Otherwise you get duplicates if there are small variations in spelling, which I've seen happen with actor names)

Would be nice if you could disable those in the provider configuration, I think for some of those you already can. I know you can set genre limits for anidb and anilist, so you could just tell it not to add any I think. Although I am not sure the next one in line will get a chance to add those if the first one has that disabled. (Maybe warrants a feature request?)

[Shadowghost]

How does this interact with multiple enabled metadata providers and the ordering of those?

NFO/Local provider metadata is always preferred, after that providers only add metadata not already there (in the order you defined)
Maybe a note of caution here: This is how it was supposed to work already but the current logic replaced all data with the next provider's data if you were running a replaceAll refresh

What about fields that contain multiple values, such as Genres, People, Tags and such?

The new logic merges them from the different providers while removing duplicates

Would be nice if you could disable those in the provider configuration

Purely provider dependant

[TimGels]

Test Results:

I pulled your branch and started testing. I executed 4 different test cases. The first three were based on your description tests. Number four was made up by me. 1 - 3 seem to have passed. the 4th one I had a surprising finding, but that finding could be unrelated to your changes.

To start off, the following screenshot displays the information of the show right after the library scan. This will be the starting point for the test cases 1 and 2.

Executing test 1 (passed):

I started by manually emptying the fields: "Original title", "Community rating" and "Overview". This is shown in the following screenshot:

I saved the changes and proceeded by executing a "Refresh Metadata"-> "Replace all metadata" on this show.

I refreshed the page and the manually deleted information is now present again:

Executing test 2 (passed):

I manually emptied the fields: "Original title", "Community rating" and "Overview".

I then proceeded by executing a "Refresh Metadata"-> "Search for missing metadata" on this show.

I refreshed the page and the manually deleted information is now present again:

Executing test 3 (passed):

I manually identified the series as: "A Certain Magical Index". The metadata fields are updated:

I started by manually emptying the fields: "Original title", "Community rating" and "Overview".

I then proceeded by executing a "Refresh Metadata"-> "Replace all metadata" on the library level.

I refreshed the page and the manually deleted information is now present again:

Executing test 4 (passes):

I manually emptied the fields: "Original title", "Community rating" and "Overview".

You can see the fields getting emptied also show nothing on the shows page anymore:

I proceed by enabling "Lock this item to prevent future changes".

I initiate a "Replace all metadata" on this show level.

I refreshed the page and the manually deleted information is still empty (as it should due to it being locked):

Actors are also still showing:

[TimGels]

I also tested the moving part mentioned in issue #11294. The user did not provide a clear reproduction so I tested the moving of a show between 2 libraries. This resulted in metadata being pulled freshly again, even when originally in the original library it being set to locked. In my opinion this is working as intended.

I also tested moving a folder from the root in the library, to it's own folder. This also resulted in the movie metadata being pulled again. I can see why someone would think that this is not working as intended as the movie stayed within the same library, just with a different file path (see bottom of comment, this is working as intended).

To clarify what I did with the movie:

Firstly I added a movie (violet evergarden) to the root directory of what the movie library uses:

I then did a scan and edited the metadata of the movie. I then removed the ratings and the overview description and the original title. After that i locked the metadata fields/ item and saved it.

The next step I did was creating a directory for the movie and moved the movie inside it. See Violet Evergarden The Movie (2020) [tmdbid-533514]:

After that I performed a replace all refresh on the movies library.

The result is that the movie has all it's metadata again:

What is our view on this behavior?
Edit: Discussed with @cvium and we do not support retention of metadata or it's settings when moving files. Jellyfin only has a understanding of Deletion and Additions of files, not moving. Both my movie and show example are working as intended.

[sjorge]

IIRC jellyfin has all the metadata tied to a path, so moving the media == loosing the lock would be expected.

I'm pretty sure this is also why the same path in two libraries share metadata.

[TimGels]

I can confirm that #11773 is solved with this PR. I identified a movie as another movie. After that I performed a replace all on the library and on the movie itself. It is still showing as the movie I manually identified it with.

[Shadowghost]

@TimGels Test case 4 should now work too.

[TimGels]

@Shadowghost
Test case 4 indeed passes now. I locked it, re-scanned everything, refreshed the page and it still showed people. Basically I re executed step 4. Another thing we discussed testing was playing around with removing some actors and combining that with locking and not locking in different test scenario's. I also tested that and there were no duplicated and the actors worked as expected.

[xd003]

Can confirm that everything is working as mentioned in the PR

[cvium]

If it's equal to 0, it doesn't change anything?

[Shadowghost]

Since we're oly in this check if we either not replace or the target already has studios this is intended. If souce has studios, we append, if it does not we ignore.

[cvium]

Unless target.Studios contains duplicates, then target.Studios.Concat(source.Studios).Distinct().ToArray() is the same as target.Studios when length == 0.

[Shadowghost]

Well, I can just remove the check, that should fix the issue

[cvium]

I was just wondering why the check was >= 0 and not > 0

[Shadowghost]

I think removing the checks to delete any duplicates is sensible regardless

[nodje]

Was this merged into 10.9.4?

I still have the issue

[DrMcCoy]

For me, replacing works now with 10.9.4...mostly. I does grab description and ratings again now. But it doesn't get season names, despite that tick being on in the settings. So that's still a bug.

However, it also now doesn't break season names anymore when they've been manually changed in the nfo file. Which is good, because I kinda only want fix season names in my library and keep the other info as-is. I wrote myself a small Python script that goes over my library, gets the season names from TMDB and patches the nfo files.

In case that's useful for anyone else, it's here: https://github.com/DrMcCoy/fix-season-names . No warranties, use at your own peril, etc., though, of course.

[nodje]

OK, to be very specific, here's the test I perform:

• go to a movie, Edit Metadata
• In Metadata Settings->Preferred download language change language to a specific one to override the Library's default
• then Refresh Metadata->mode: Replace All Metadata

and it doesn't change the metadata like it used to in 10.8.x

Logs from the operation

[16:01:44] [INF] [31] MediaBrowser.MediaEncoding.Encoder.MediaEncoder: Starting /usr/lib/jellyfin-ffmpeg/ffprobe with args -analyzeduration 200M -probesize 1G -i file:"/media/Directors/France/Dupieux, Quentin - 1974-/2022 - Dupieux, Quentin - Fumer fait tousser [imdbid-tt15471560]/2022 - Dupieux, Quentin - Fumer fait tousser - BD1080pHEVC.mp4" -threads 0 -v warning -print_format json -show_streams -show_chapters -show_format
[mov,mp4,m4a,3gp,3g2,mj2 @ 0x55abeff24ec0] stream 0, timescale not set
[16:01:45] [WRN] [31] MediaBrowser.XbmcMetadata.Providers.MovieNfoProvider: Trailer URL uses a deprecated format : plugin://plugin.video.youtube/?action=play_video&videoid=u80Bh3sMrzU. Using plugin://plugin.video.youtube/play/?video_id=u80Bh3sMrzU instead is advised.
[16:01:45] [WRN] [31] MediaBrowser.XbmcMetadata.Providers.MovieNfoProvider: Trailer URL uses a deprecated format : plugin://plugin.video.youtube/?action=play_video&videoid=Q0X8LXlE9pA. Using plugin://plugin.video.youtube/play/?video_id=Q0X8LXlE9pA instead is advised.
[16:01:45] [WRN] [31] MediaBrowser.XbmcMetadata.Providers.MovieNfoProvider: Trailer URL uses a deprecated format : plugin://plugin.video.youtube/?action=play_video&videoid=fxc6TyTdW50. Using plugin://plugin.video.youtube/play/?video_id=fxc6TyTdW50 instead is advised.

Incidently, I'm not sure why the msg " Trailer URL uses a deprecated format" shows up as I never enabled/used Trailers.

[Shadowghost]

@nodje if you remove the NFO, does it work then? I think the issue is that NFOs take precedence over anything else and since they are language-independent, they'll fill up the metadata before the remote providers do.
Another option would be to identify the item with the correct language.

[NiklasBr]

But should the NFO really take precedence when Replace All Metadata is checked? Previous behaviour was that the NFO content was replaced in that case.

[Shadowghost]

I think this kind of is an apple and egg problem since technically NFOs are metadata sources to query for metadata and nothing more. They are the ultimate source of truth in my opinion. I agree and know (since I changed it) that before remote provider results overrode the NFOs if you replaced all metadata, but I'm unsure what to do about it.
Maybe we need a switch for ignoring local metadata all together - but that would be a 10.10 thing. You should be able to fix it by removing the NFO and force refreshing afterwards.

Regardless, I think this conversation should be moved to an issue.

[DrMcCoy]

You're right, that doesn't work for me either.

I add a new TV show, it defaults to English (because most of what I have is English, so that's okay). But this show is a German one and I want German metadata. So I change it to German and Germany, then replace all metadata. That used to work, but now it doesn't.

I can delete all the nfos now, but that's a bit cumbersome? Also, isn't the language selection saved to the tvshow.nfo in the first place?

[DrMcCoy]

Maybe we need a switch for ignoring local metadata all together - but that would be a 10.10 thing. You should be able to fix it by removing the NFO and force refreshing afterwards

I disagree that this is a 10.10 thing (I mean, that's going to take, what, another 2 years?) or that deleting the NFO is a "good" solution, even in the meantime.

You basically broke adding items to mixed-language metadata libraries. Since there is no way to signal "I want this new stuff to be in $LANGUAGE instead of the default", "replace all metadata" basically the only way to do this. You shouldn't need fuzz around with the generated file in the filesystem afterwards, that's just...bad.

[michaelkrieger]

There 100% needs to be a “fetch metadata from remote sources” vs “refresh data from local NFO”. A checkbox will do. One looks past the NFO. One looks only to the NFO. I do modify NFO files on occasion and want it read. That said, most of the time I want to refresh a show that got uploaded before the metadata was available and once the NFO is done, there’s no way to achieve it without manually deleting files

[NiklasBr]

They are the ultimate source of truth in my opinion

This was not the case before 10.9, and before 10.9 it was intuitive and worked. This new behaviour should have been a separate feature.

And on top of that, they are never the ultimate source of truth in my opinion since the the remote source is likely to have better, more up-to-date data in > 99% of the cases because at most one person contributes to the NFO file, while potentially thousands contribute to the remote source.

[Shadowghost]

#11921 should take care of it - skipping local NFOs when replacing and saving is enabled. Please test if you have the time.

[nodje]

It does work to remove the NFO.
Identifying the movie with the Preferred download language set doesn't seem to work unfortunately. It would have been a good work around.

Unrelated to the case but I'd appreciate a pointer to an explanation: what puzzles me with NFO, is that I get two of them for existing movies after the 10.9 upgrade. The new movie.nfo one an the old one that bears the name of the movie file.
E.G.

• movie.nfo
• 2011 - Winding Refn, Nicholas - Drive - 2160pHEVC.HDR10.nfo

Shall I delete all NFO with the old format?

[michaelkrieger]

#11921 should take care of it - skipping local NFOs when replacing and saving is enabled. Please test if you have the time.

Agreed. And with that logic, the way to replace files with data from NFO is on initial import- so remove and re-add if it's already there. You'd likely rarely want to import from NFO unless it's the initial addition. Agree with that logic.