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

Fix using movie.nfo first when <filename>.nfo also exists #10339

Merged
merged 1 commit into from
Oct 10, 2023
Merged

Fix using movie.nfo first when <filename>.nfo also exists #10339

merged 1 commit into from
Oct 10, 2023

Conversation

leovan
Copy link
Contributor

@leovan leovan commented Oct 5, 2023

<filename>.nfo is used for store custom information of movie uploader on some private tracker bittorrent website, which is not a valid movie metadata nfo file. Overwrite <filename>.nfo will lead errors with file sharing. Therefor using movie.nfo first may be better when <filename>.nfo also exists.

Changes
Use movie.nfo first when <filename>.nfo also exists.

Issues
Fixes #1558

Copy link
Contributor

@Shadowghost Shadowghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see an issue with this.

@barronpm
Copy link
Member

barronpm commented Oct 5, 2023

If there are multiple movies in a folder, would this force the same nfo onto all of them even if they have their own?

@Shadowghost
Copy link
Contributor

That is correct and a problem. Likely the reason why this check was placed after the direct match.

Not sure how we could remedy this though. I'd rather drop this PR if we don't find a way to handle this.

@leovan
Copy link
Contributor Author

leovan commented Oct 5, 2023

If there are multiple movies in a folder, would this force the same nfo onto all of them even if they have their own?

In this code, I see only when item.IsInMixedFolder is false, movie.nfo will be the candidate metadata file. And in MovieResolver.cs when resolverResult.Count > 1, which I think it means there are multiple video files in parent folder, then isInMixedFolder will be true, then movie.nfo won't be the candidate metadata file.

var isInMixedFolder = resolverResult.Count > 1 || parent?.IsTopParent == true;

Sorry I didn't fully read the codes. So please check whether the condition !item.IsInMixedFolder && item.ItemType == typeof(Movie) can avoid the problem you mentioned or not.

@leovan
Copy link
Contributor Author

leovan commented Oct 5, 2023

That is correct and a problem. Likely the reason why this check was placed after the direct match.

Not sure how we could remedy this though. I'd rather drop this PR if we don't find a way to handle this.

If the condition !item.IsInMixedFolder && item.ItemType == typeof(Movie) cannot avoid the problem. A better solution is to check whether <filename>.nfo is valid metadata file. If not then use movie.nfo. Is there a helper function to check whether a nfo file is a valid metadata file (at least is a valid xml file)? In the situation I mentioned before, the <filename.nfo> created by movie uploader is just a plain text file not a valid xml file.

@leovan
Copy link
Contributor Author

leovan commented Oct 6, 2023

I made a minimal test on my machine and it seems working fine. I think item.IsInMixedFolder can tell whether a movie fold has multiple video files or not.

test-01

test-02

It uses their own nfo file to show the title.

@Shadowghost
Copy link
Contributor

Shadowghost commented Oct 6, 2023

What you tested is only different movies in a single folder if I understand the pictures correctly?
What we're concerned about is multiple versions in the same folder:

Matrix (1999)
| - Matrix (1999) - 1080p.mkv
| - Matrix (1999) - 1080p.nfo
| - Matrix (1999) - 2160p.mkv
| - Matrix (1999) - 2160p.nfo
| - movie.nfo

@leovan
Copy link
Contributor Author

leovan commented Oct 6, 2023

I made the test of multiple versions in the same folder, it works too. Besides, 1080p.mkv and 2160p.mkv are the same sample videos, I just copy and change the filename.

test-03

test-04

@Shadowghost
Copy link
Contributor

Your structure is not correct though, you'd need to test it with a folder structure like this:

<library root>
| - Matrix (1999)
     | - Matrix (1999) - 1080p.mkv
     | - Matrix (1999) - 1080p.nfo
     | - Matrix (1999) - 2160p.mkv
     | - Matrix (1999) - 2160p.nfo
     | - movie.nfo

This is because the multi-version matching depends on the folder name (see our naming guidelines)

@leovan
Copy link
Contributor Author

leovan commented Oct 6, 2023

Now I can see the problem after changing the parent folder name to Matrix (1999). It use movie.nfo for the collection title which I think is reasonable.

屏幕截图 2023-10-06 163902

Problem is that it also uses movie.nfo for details of different version. Same title for 1080p and 2160p, Matrix movie.nfo.

屏幕截图 2023-10-06 164026
屏幕截图 2023-10-06 164034

After removing movie.nfo, it use Matrix (1999) - 1080p.nfo for the collection title which I think it choose the first nfo file ordered by filename.

屏幕截图 2023-10-06 164351

But it still has the problem, it also has the same title for 1080p and 2160p, Matrix 1080p.

屏幕截图 2023-10-06 164431
屏幕截图 2023-10-06 164441

This problem may be beyond this PR to fix.

So a better way to fix the issue is to check whether <filename>.nfo is a valid xml file before add it to the candidate nfo file list. I will try to fix this issue in this way later.

@Shadowghost
Copy link
Contributor

This problem is know, so it's working as expected even if we have multiple versions. I guess we're fine now. Thanks for testing!

@leovan
Copy link
Contributor Author

leovan commented Oct 6, 2023

This problem is know, so it's working as expected even if we have multiple versions. I guess we're fine now. Thanks for testing!

You mean current simple solution of this PR for #1558 is OK?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prefer movie.nfo when other nfo files may exist in the same directory within a Movie folder
4 participants