Radarr / Radarr

Movie organizer/manager for usenet and torrent users.
https://radarr.video
GNU General Public License v3.0
10.25k stars 993 forks source link

Original Language not properly parsed for a movie - [TMDb Country Codes for Languages; not Language codes] #5970

Closed clinta closed 6 months ago

clinta commented 3 years ago

Describe the bug

To Reproduce

  1. Add Drunken Master (1978)
  2. Query the database for the original language
    sqlite3 config/radarr.db 'select Title, TMDBID, OriginalLanguage from Movies where Title like "Drunken Master";'
    Drunken Master|11230|1

Expected behavior OriginalLanguage should be 10 matching the value of Cantonese in https://www.themoviedb.org/movie/11230-jui-kuen

Platform Information (please complete the following information):

Trace Logs radarr.trace.txt

AB#635

bakerboy448 commented 3 years ago

I'm going to guess the issue is Cantonese != Chinese https://github.com/Radarr/Radarr/blob/60ac9d3ad75902384ba3cc1721d064216c281867/src/NzbDrone.Core/Languages/Language.cs#L83

clinta commented 3 years ago

Is this logic not intended to be used for parsing OriginalLanguge?

https://github.com/Radarr/Radarr/blob/6702c7d21b9eb8118e54be2ed4b2a97ab34c47fd/src/NzbDrone.Core/Parser/LanguageParser.cs#L65-L68

bakerboy448 commented 3 years ago

you did more digging than I :) yeah something is probably weird then; reproduced/confirmed on 3.1.0.4648

clinta commented 3 years ago

And maybe a separate bug is required for this, but I'd expect if the original language is not known or matched, it should be set to -1 for Any, or maybe a new meta-value. Not default to English.

I want to use the original language value for a script to strip out unnecessary audio streams. But I don't want to accidentally strip out an original language stream because it defaulted to English when the language was unknown.

clinta commented 3 years ago

Further digging, original language is set by looking up the iso code here

https://github.com/Radarr/Radarr/blob/74c89e1d4412f2dde8fed83fb5aa04cb1fcfef2c/src/NzbDrone.Core/MetadataSource/SkyHook/SkyHookProxy.cs#L180

And this function does not contain any of the necessary logic to map Cantonese to Chinese the way LanguageParser does.

https://github.com/Radarr/Radarr/blob/74c89e1d4412f2dde8fed83fb5aa04cb1fcfef2c/src/NzbDrone.Core/Parser/IsoLanguages.cs#L42-L80

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. Please verify that this is still an issue with the latest version of Radarr and report back. Otherwise this issue will be closed.

bakerboy448 commented 3 years ago

Just adding some additional context/example for what was seen in discord today.

"OriginalLanguage": "cn",

So original is CN and it seems that is NOT mapped to CHINESE https://radarrapi.servarr.com/v1/movie/9470 thus falls back to english movie.OriginalLanguage = IsoLanguages.Find(resource.OriginalLanguage.ToLower())?.Language ?? Language.English; is the TMDB lookup and map (ref https://github.com/Radarr/Radarr/blob/627ab64fd023269c8bedece61e529329600a3419/src/NzbDrone.Core/MetadataSource/SkyHook/SkyHookProxy.cs)

and where it maps. https://github.com/Radarr/Radarr/blob/627ab64fd023269c8bedece61e529329600a3419/src/NzbDrone.Core/Parser/IsoLanguages.cs I'm thinking for we may not be checking that second column at a glance. So TMDB is providing a country code - not a language code.

ref https://discord.com/channels/264387956343570434/264388019585286144/885258016914542623

ditmarvisser commented 1 year ago

6522 Seems to fix the issue, but the only thing that needs changing in that PR is the numbering of the language. 38 & 39 are available and not reserved as far as I can tell. Is there a reason this can't be done without @chryzsh?

chryzsh commented 1 year ago

What am I needed for here, exactly?

ditmarvisser commented 1 year ago

What am I needed for here, exactly?

In your PR there are some requested changes and the language numbers need changing.

chryzsh commented 1 year ago

In the original PR I was asked to choose different numbers, but it wasn’t clear what number I was supposed to use and still isn’t. Sorry I’m not of help here.

bakerboy448 commented 6 months ago

CN fixed in Feb? https://github.com/Radarr/Radarr/commit/4ad7b60d9dee0f99d0247fce5478b0ceb7841ed3

mynameisbogdan commented 6 months ago

Yes, cn was added.