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

Add NFO metadata source #2305

Merged
merged 8 commits into from
Nov 26, 2023
Merged

Add NFO metadata source #2305

merged 8 commits into from
Nov 26, 2023

Conversation

mikiher
Copy link
Contributor

@mikiher mikiher commented Nov 12, 2023

In many cases, audiobooks come with .nfo files. While there's no standard to this text format, most audiobook .nfo files follow some de-facto standards specifying metadata (e.g. "Title", "Author", "Read by"). In some cases the .nfo files have metadata attributes that are missing/wrong in the folder structure or the audio meta tags.

This adds NFO as an additional metadata source, parsing the .nfo files according to the de-facto standards.

I put the NFO default priority just above audio meta tags, which I find reasonable, but this could be of course changed if needed.

I also have unit test for the parser. If you approve PR #2300, I'll commit that as well.

@mikiher mikiher marked this pull request as ready for review November 12, 2023 13:47
@mikiher
Copy link
Contributor Author

mikiher commented Nov 21, 2023

Unit test now committed.

@advplyr
Copy link
Owner

advplyr commented Nov 25, 2023

Can you share one or more sample NFO files so I can test and that we can add to the guide? I don't remember seeing an NFO with an audiobook before

@mikiher
Copy link
Contributor Author

mikiher commented Nov 25, 2023

Here are a few examples: nfo examples.zip
Let's just make sure we sanitize them before adding to the guide.

@advplyr
Copy link
Owner

advplyr commented Nov 26, 2023

Since the description at the top of the scanner settings was incorrect now that there are 6 metadata sources I refactored that settings tab.

Previously it was not intuitive because 1 was lowest priority. For the UI only I updated it so that 1 is the highest priority.

image

@advplyr
Copy link
Owner

advplyr commented Nov 26, 2023

I thought it made more sense to trim the metadata description after it gets parsed.
For example, one of the sample NFO files had 2 empty lines in the start and end of the description. This didn't seem necessary to include but let me know if I'm missing some edge case there.

Book Description
================


A Swedish crime writer as thrilling as Mankell, a detective as compelling 
as Wallander....

Håkan Nesser's astonishingly successful Van Veeteren series continues 
with the eighth book, The Weeping Girl.

Another thing to note with the description is that it has to be placed last or the rest of the NFO file will be included in the description. This looked intentional on your part and it works for me but I just wanted to highlight that. I don't see a better way of handling it.

@advplyr
Copy link
Owner

advplyr commented Nov 26, 2023

Thanks!

@advplyr advplyr merged commit 2e5822b into advplyr:master Nov 26, 2023
1 check passed
@mikiher
Copy link
Contributor Author

mikiher commented Nov 26, 2023

I thought it made more sense to trim the metadata description after it gets parsed. For example, one of the sample NFO files had 2 empty lines in the start and end of the description. This didn't seem necessary to include but let me know if I'm missing some edge case there.

Sounds good.

Another thing to note with the description is that it has to be placed last or the rest of the NFO file will be included in the description. This looked intentional on your part and it works for me but I just wanted to highlight that. I don't see a better way of handling it.

Indeed, it was intentional.

Thanks for the thorough review!

advplyr added a commit that referenced this pull request Dec 8, 2023
Follow up for sso-redirecturi and #2305 and #2333
@mikiher mikiher deleted the nfo-metadata branch December 10, 2023 22:00
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.

2 participants