+- Kodi Community Forum (https://forum.kodi.tv)
+-- Forum: Support (https://forum.kodi.tv/forumdisplay.php?fid=33)
+--- Forum: General Support (https://forum.kodi.tv/forumdisplay.php?fid=111)
+---- Forum: OS independent / Other (https://forum.kodi.tv/forumdisplay.php?fid=228)
+---- Thread: Potential Subtitle Handling Bug (/showthread.php?tid=365802)
RE: Potential Subtitle Handling Bug - SEIKT - 2021-12-05
The damage occurs here:
https://github.com/xbmc/xbmc/blob/Matrix/xbmc/Util.cpp#L2112
Scans the string from left to right.
1st loop: info.language is empty
langTmp = "Back"
2nd loop: info.language is empty
LangTmp = "to"
if statement on line 2119 is met
info.language = "tor"
RE: Potential Subtitle Handling Bug - SEIKT - 2021-12-05
Know next to nothing regarding C++ but I think I fixed it.
https://github.com/JDRIVO/xbmc/blob/Matrix/xbmc/Util.cpp
Code:
2021-12-05 18:01:08.875 T:1880 DEBUG <general>: CUtil::GetExternalStreamDetailsFromFilename - Language = 'eng' / Name = 'Back to the Future Part III (1990) (External)' / Flag = '0' from C:\Users\Jay\Documents\strm\movies\Back to the Future Part III (1990).eng.srtRE: Potential Subtitle Handling Bug - matthuisman - 2021-12-06
i'd probably suggest more like this:
https://github.com/matthuisman/xbmc/commit/59b30b3eda96d78d57c3b6049d7a470054d5edb3
(i haven't compiled to test)
As going backwards, you would want to check for the flags first, then the language.
Also, just prepending the name stops you from needing the "count" variable.
RE: Potential Subtitle Handling Bug - SEIKT - 2021-12-06
I'll try it out and let you know. I think the for loop should stop scanning for languages in the string when the year in the string appears. Otherwise, the possibility of incorrect language identification is still present. If no language is identified up to that point maybe default to the user's default language in case they have forgotten to append the language to the subtitles filename or set the language to unknown.
RE: Potential Subtitle Handling Bug - SEIKT - 2021-12-06
And some other ideas:
Optional parameters for setSubtitles:
1. Video name: Pass the video name so that it can be removed from the subtitles string to improve match accuracy
2. Name preference of subtitles as shown in the GUI: either as only the language detected or the subtitles filename
RE: Potential Subtitle Handling Bug - SEIKT - 2021-12-06
Matt, yours compiles. Just need to change the for loop creation code to this:
Code:
for (std::vector<std::string>::reverse_iterator it = tokens.rbegin(); it != tokens.rend(); ++it)Otherwise it won't compile.
RE: Potential Subtitle Handling Bug - matthuisman - 2021-12-09
oh right - good catch.
updated commit: https://github.com/matthuisman/xbmc/commit/cfb29106fe96c56c4fa15769b72e3d4552eee48a
Did it work as intended?
RE: Potential Subtitle Handling Bug - SEIKT - 2021-12-11
@matthuisman confirming your code solves the problem. Just may want to double-check that the identified flag number is correct as I don't know what "flag" refers to and the old and new code produce the same flag number.
New:
Code:
2021-12-11 11:42:47.264 T:6912 DEBUG <general>: CUtil::GetExternalStreamDetailsFromFilename - Language = 'eng' / Name = 'Back to the Future Part III (1990) [tt0099088] (External)' / Flag = '0' from C:\Users\Jay\Documents\strm\movies\Back to the Future Part III (1990) [tt0099088] English.srtOld:
Code:
2021-12-11 11:57:47.644 T:9404 DEBUG <general>: CUtil::GetExternalStreamDetailsFromFilename - Language = 'ton' / Name = 'Back the Future Part III (1990) [tt0099088] English (External)' / Flag = '0' from C:\Users\Jay\Documents\strm\movies\Back to the Future Part III (1990) [tt0099088] English.srtNew:
Code:
2021-12-11 11:43:05.448 T:4748 DEBUG <general>: CUtil::GetExternalStreamDetailsFromFilename - Language = 'eng' / Name = 'No Time to Die (2021) (External)' / Flag = '64' from C:\Users\Jay\Documents\strm\movies\No Time to Die (2021) English Forced.srtOld:
Code:
2021-12-11 11:48:06.175 T:13508 DEBUG <general>: CUtil::GetExternalStreamDetailsFromFilename - Language = 'nor' / Name = 'Time to Die (2021) English (External)' / Flag = '64' from C:\Users\Jay\Documents\strm\movies\No Time to Die (2021) English Forced.srtRE: Potential Subtitle Handling Bug - matthuisman - 2021-12-11
The flag relates to forced and or default.
64 is decimal of the hex flag for forced (0x0040)
RE: Potential Subtitle Handling Bug - SEIKT - 2021-12-11
Understood. Cheers for the insight. Would you mind creating a pull request? Seems the devs gave up on this thread.
RE: Potential Subtitle Handling Bug - matthuisman - 2021-12-12
PR opened here:
https://github.com/xbmc/xbmc/pull/20666
RE: Potential Subtitle Handling Bug - SEIKT - 2021-12-12
Champ, cheers boss.
RE: Potential Subtitle Handling Bug - matthuisman - 2021-12-13
@SEIKT
the commit is in latest 20 nightly now
http://mirrors.kodi.tv/nightlies/windows/win64/master/KodiSetup-20211212-1b1e86a6-master-x64.exe
I noticed li.setSubtitles(['D:\\Back to the Future Part III (1990).eng.srt']) seems to not work on Kodi 20 but does in Kodi 19.
I tried earlier nightly builds (1st dcember) to make sure it wasnt that PR that caused it and same issue.
On Kodi 20, I need to browse for the subtitle.
Doing that I could confirm the language is then detected correctly
Found the issue with setSubtitles and have new PR to fix that:
https://github.com/xbmc/xbmc/pull/20671
In the meantime, you can just do li.setSubtitles([None, 'D:\\Back to the Future Part III (1990).eng.srt']) to test
RE: Potential Subtitle Handling Bug - scott967 - 2021-12-14
Don't we still have a problem that the requirement is to get the filename of the video item and strip that from the subtitle filename prior to parsing for the language? What this has done is better, but I think in the case of No Time To Die.chs.srt for example, chs isn't recognized by Kodi (unless added by user in advancedsettings) so instead of getting language "unknown" it will still return "Norwegian". (I use chs as example as it seems to be a common work around for limitations of using ISO 639 language codes.)
scott s.
.
RE: Potential Subtitle Handling Bug - matthuisman - 2021-12-14
That is way out of scope of this issue.
You'd need to make your own PR adding more language codes etc