refactor get_subtitle_sources #136
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
'settings.subtitles_language' is now a ',' seperated list of languages that the user understands. This allows to eliminate cases in which a translated caption would be preferred over captions with the native language of the video, even tough a multilingual user might understand them.
user234683
reviewed
Jul 30, 2022
| native_video_lang = None | ||
| if info['automatic_caption_languages']: | ||
| native_video_lang = info['automatic_caption_languages'][0] | ||
|
|
||
| highest_fidelity_is_manual = False | ||
| kManual, kNativeVideoLang, kUserSpeaksIt = 1, 2, 4 |
There was a problem hiding this comment.
Shouldn't this be kNativeVideoLang, kManual, kUserSpeaksIt = 1, 2, 4?
Manually created subtitles in a different language than the video are always going to be higher quality than automatically generated ones. Comparing the current scores to the comment at the top of the code with the ideal ordering:
010 native_video_lang (Automatic)
001 foreign_langs (Manual)
110 native_video_lang (Automatic) -> pref_lang
101 foreign_langs (Manual) -> pref_lang
111 native_video_lang (Manual) -> pref_lang
110 pref_lang (Automatic)
111 pref_lang (Manual) (101 if pref_lang != native_video_lang)'''
Swapping the last two bits will fix it
Additionally, these constants should be changed to NATIVE_VIDEO_LANG, MANUAL, etc. to match the convention in the rest of the code that constants are indicated with uppercase snakecase; we dont use hungarian notation in the code.
user234683
reviewed
Jul 30, 2022
| if len(sources) == 0: | ||
| assert len(info['automatic_caption_languages']) == 0 and len(info['manual_caption_languages']) == 0 | ||
| if settings.video_player == 1: # Plyr | ||
| sources[-1]['on'] = True |
There was a problem hiding this comment.
Why are we always turning captions on for Plyr only?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
'settings.subtitles_language' is now a ',' seperated list of languages
that the user understands.
This allows to eliminate cases in which a translated caption would
be preferred over captions with the native language of the video, even
tough a multilingual user might understand them.