From 053e41cf5233f99b1462d5aede74e08e4840e8da Mon Sep 17 00:00:00 2001 From: Alex Shnitman Date: Fri, 27 Feb 2026 12:58:50 +0200 Subject: [PATCH] code review fixes --- README.md | 1 - app/main.py | 15 +++++-- app/ytdl.py | 3 +- ui/src/app/app.html | 96 ++++++++++++++++++++++++--------------------- ui/src/app/app.ts | 36 +++++++++++++++++ 5 files changed, 100 insertions(+), 51 deletions(-) diff --git a/README.md b/README.md index 91b7e5e..3812b8d 100644 --- a/README.md +++ b/README.md @@ -59,7 +59,6 @@ Certain values can be set via environment variables, using the `-e` parameter on * __OUTPUT_TEMPLATE_CHANNEL__: The template for the filenames of the downloaded videos when downloaded as a channel. Defaults to `%(channel)s/%(title)s.%(ext)s`. When empty, then `OUTPUT_TEMPLATE` is used. * __YTDL_OPTIONS__: Additional options to pass to yt-dlp in JSON format. [See available options here](https://github.com/yt-dlp/yt-dlp/blob/master/yt_dlp/YoutubeDL.py#L222). They roughly correspond to command-line options, though some do not have exact equivalents here. For example, `--recode-video` has to be specified via `postprocessors`. Also note that dashes are replaced with underscores. You may find [this script](https://github.com/yt-dlp/yt-dlp/blob/master/devscripts/cli_to_api.py) helpful for converting from command-line options to `YTDL_OPTIONS`. * __YTDL_OPTIONS_FILE__: A path to a JSON file that will be loaded and used for populating `YTDL_OPTIONS` above. Please note that if both `YTDL_OPTIONS_FILE` and `YTDL_OPTIONS` are specified, the options in `YTDL_OPTIONS` take precedence. The file will be monitored for changes and reloaded automatically when changes are detected. -* UI format __Captions__: Downloads subtitles/captions only (no media). Subtitle format, language, and source preference are configurable from Advanced Options (defaults: `srt`, `en`, `prefer_manual`). `txt` is generated from `srt` by stripping timestamps and cue numbers. ### 🌐 Web Server & URLs diff --git a/app/main.py b/app/main.py index c28c32c..3240744 100644 --- a/app/main.py +++ b/app/main.py @@ -156,6 +156,9 @@ app = web.Application() sio = socketio.AsyncServer(cors_allowed_origins='*') sio.attach(app, socketio_path=config.URL_PREFIX + 'socket.io') routes = web.RouteTableDef() +VALID_SUBTITLE_FORMATS = {'srt', 'txt', 'vtt', 'ttml', 'sbv', 'scc', 'dfxp'} +VALID_SUBTITLE_MODES = {'auto_only', 'manual_only', 'prefer_manual', 'prefer_auto'} +SUBTITLE_LANGUAGE_RE = re.compile(r'^[A-Za-z0-9][A-Za-z0-9-]{0,34}$') class Notifier(DownloadQueueNotifier): async def added(self, dl): @@ -269,11 +272,17 @@ async def add(request): subtitle_language = 'en' if subtitle_mode is None: subtitle_mode = 'prefer_manual' + subtitle_format = str(subtitle_format).strip().lower() + subtitle_language = str(subtitle_language).strip() + subtitle_mode = str(subtitle_mode).strip() if chapter_template and ('..' in chapter_template or chapter_template.startswith('/') or chapter_template.startswith('\\')): raise web.HTTPBadRequest(reason='chapter_template must not contain ".." or start with a path separator') - valid_subtitle_modes = {'auto_only', 'manual_only', 'prefer_manual', 'prefer_auto'} - if subtitle_mode not in valid_subtitle_modes: - raise web.HTTPBadRequest(reason=f'subtitle_mode must be one of {sorted(valid_subtitle_modes)}') + if subtitle_format not in VALID_SUBTITLE_FORMATS: + raise web.HTTPBadRequest(reason=f'subtitle_format must be one of {sorted(VALID_SUBTITLE_FORMATS)}') + if not SUBTITLE_LANGUAGE_RE.fullmatch(subtitle_language): + raise web.HTTPBadRequest(reason='subtitle_language must match pattern [A-Za-z0-9-] and be at most 35 characters') + if subtitle_mode not in VALID_SUBTITLE_MODES: + raise web.HTTPBadRequest(reason=f'subtitle_mode must be one of {sorted(VALID_SUBTITLE_MODES)}') playlist_item_limit = int(playlist_item_limit) diff --git a/app/ytdl.py b/app/ytdl.py index dd458a9..18e47ab 100644 --- a/app/ytdl.py +++ b/app/ytdl.py @@ -163,6 +163,7 @@ class DownloadInfo: self.subtitle_format = subtitle_format self.subtitle_language = subtitle_language self.subtitle_mode = subtitle_mode + self.subtitle_files = [] class Download: manager = None @@ -379,8 +380,6 @@ class Download: except OSError as exc: log.debug(f"Could not remove temporary SRT file {subtitle_file}: {exc}") - if not hasattr(self.info, 'subtitle_files'): - self.info.subtitle_files = [] rel_path = os.path.relpath(subtitle_output_file, self.download_dir) file_size = os.path.getsize(subtitle_output_file) if os.path.exists(subtitle_output_file) else None existing = next((sf for sf in self.info.subtitle_files if sf['filename'] == rel_path), None) diff --git a/ui/src/app/app.html b/ui/src/app/app.html index 9149a7f..8e7741e 100644 --- a/ui/src/app/app.html +++ b/ui/src/app/app.html @@ -218,54 +218,60 @@ ngbTooltip="Maximum number of items to download from a playlist or channel (0 = no limit)"> -
-
- Subtitles - + @if (format === 'captions') { +
+
+ Subtitles + +
+ @if (subtitleFormat === 'txt') { +
TXT is generated from SRT by stripping timestamps and cue numbers.
+ }
- @if (subtitleFormat === 'txt') { -
TXT is generated from SRT by stripping timestamps and cue numbers.
- } -
-
-
- Language - +
+
+ Language + + + @for (lang of subtitleLanguages; track lang.id) { + + } + +
-
-
-
- Subtitle Source - +
+
+ Subtitle Source + +
-
+ }
diff --git a/ui/src/app/app.ts b/ui/src/app/app.ts index 525d668..e56fd2b 100644 --- a/ui/src/app/app.ts +++ b/ui/src/app/app.ts @@ -108,16 +108,48 @@ export class App implements AfterViewInit, OnInit { ]; subtitleLanguages = [ { id: 'en', text: 'English' }, + { id: 'ar', text: 'Arabic' }, + { id: 'bn', text: 'Bengali' }, + { id: 'bg', text: 'Bulgarian' }, + { id: 'ca', text: 'Catalan' }, + { id: 'cs', text: 'Czech' }, + { id: 'da', text: 'Danish' }, + { id: 'nl', text: 'Dutch' }, { id: 'es', text: 'Spanish' }, + { id: 'et', text: 'Estonian' }, + { id: 'fi', text: 'Finnish' }, { id: 'fr', text: 'French' }, { id: 'de', text: 'German' }, + { id: 'el', text: 'Greek' }, + { id: 'he', text: 'Hebrew' }, + { id: 'hi', text: 'Hindi' }, + { id: 'hu', text: 'Hungarian' }, + { id: 'id', text: 'Indonesian' }, { id: 'it', text: 'Italian' }, + { id: 'lt', text: 'Lithuanian' }, + { id: 'lv', text: 'Latvian' }, + { id: 'ms', text: 'Malay' }, + { id: 'no', text: 'Norwegian' }, + { id: 'pl', text: 'Polish' }, { id: 'pt', text: 'Portuguese' }, + { id: 'pt-BR', text: 'Portuguese (Brazil)' }, + { id: 'ro', text: 'Romanian' }, { id: 'ru', text: 'Russian' }, + { id: 'sk', text: 'Slovak' }, + { id: 'sl', text: 'Slovenian' }, + { id: 'sr', text: 'Serbian' }, + { id: 'sv', text: 'Swedish' }, + { id: 'ta', text: 'Tamil' }, + { id: 'te', text: 'Telugu' }, + { id: 'th', text: 'Thai' }, + { id: 'tr', text: 'Turkish' }, { id: 'uk', text: 'Ukrainian' }, + { id: 'ur', text: 'Urdu' }, + { id: 'vi', text: 'Vietnamese' }, { id: 'ja', text: 'Japanese' }, { id: 'ko', text: 'Korean' }, { id: 'zh-Hans', text: 'Chinese (Simplified)' }, + { id: 'zh-Hant', text: 'Chinese (Traditional)' }, ]; subtitleModes = [ { id: 'prefer_manual', text: 'Prefer Manual' }, @@ -139,9 +171,13 @@ export class App implements AfterViewInit, OnInit { this.subtitleLanguage = this.cookieService.get('metube_subtitle_language') || 'en'; this.subtitleMode = this.cookieService.get('metube_subtitle_mode') || 'prefer_manual'; const allowedSubtitleFormats = new Set(this.subtitleFormats.map(fmt => fmt.id)); + const allowedSubtitleModes = new Set(this.subtitleModes.map(mode => mode.id)); if (!allowedSubtitleFormats.has(this.subtitleFormat)) { this.subtitleFormat = 'srt'; } + if (!allowedSubtitleModes.has(this.subtitleMode)) { + this.subtitleMode = 'prefer_manual'; + } this.activeTheme = this.getPreferredTheme(this.cookieService);