A4K: adding foreign language changes & img hosters#1260
A4K: adding foreign language changes & img hosters#1260minicoz wants to merge 13 commits intoAudionut:masterfrom
Conversation
|
Thanks for taking the time to contribute to this project. Upload Assistant is currently in a complete rewrite, and no new development is being conducted on this python source at this time. If you have come this far, please feel free to leave open, any pull requests regarding new sites being added to the source, as these can serve as the baseline for later conversion. If your pull request relates to a critical bug, this will be addressed in this code base, and a new release published as needed. If your pull request only addresses a quite minor bug, it is not likely to be addressed in this code base. Details for the new code base will follow at a later date. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds language-aware track naming via Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/trackers/A4K.py (2)
104-127: Extract the repeated "could not determine bitrate" prompt into a helper.Lines 104–111, 112–119, and 120–127 are identical blocks. A small local helper would eliminate the triplication and make future changes (e.g., adjusting the message) less error-prone.
♻️ Suggested helper extraction
Add a helper at the top of
get_additional_checksor as a private method:async def _prompt_bitrate_unknown(self, meta: dict[str, Any]) -> bool: """Returns True if upload should continue, False otherwise.""" if not meta['unattended'] or (meta['unattended'] and meta.get('unattended_confirm', False)): console.print(f"[bold red]Could not determine video bitrate from mediainfo for {self.tracker} upload.[/bold red]") console.print("[yellow]Bitrate must be above 15000 kbps for movies and 10000 kbps for TV shows.[/yellow]") if cli_ui.ask_yes_no("Do you want to upload anyway?", default=False): return True return False return FalseThen replace each duplicated block with:
- else: - if not meta['unattended'] or (meta['unattended'] and meta.get('unattended_confirm', False)): - console.print(f"[bold red]Could not determine video bitrate from mediainfo for {self.tracker} upload.[/bold red]") - console.print("[yellow]Bitrate must be above 15000 kbps for movies and 10000 kbps for TV shows.[/yellow]") - if cli_ui.ask_yes_no("Do you want to upload anyway?", default=False): - pass - else: - return False + else: + if not await self._prompt_bitrate_unknown(meta): + return False🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trackers/A4K.py` around lines 104 - 127, Extract the repeated "Could not determine video bitrate" prompt into a small helper (e.g., async def _prompt_bitrate_unknown(self, meta: dict) -> bool) and call it from get_additional_checks where the three identical blocks currently live; the helper should encapsulate the console.print calls and cli_ui.ask_yes_no logic and return True to continue upload or False to abort, then replace each duplicated block (the three identical if-not-meta['unattended']... blocks) with a single call to self._prompt_bitrate_unknown(meta) and act on its boolean result.
159-160: Consider defensive access foraudio_languages.If
language_checkedwas set toTrueby an unexpected code path without populatingaudio_languages, line 159 would raiseKeyError. Usingmeta.get('audio_languages', [])would be a safer alternative.Suggested change
- audio_languages: list[str] = meta['audio_languages'] - if audio_languages and not await languages_manager.has_english_language(audio_languages): + audio_languages: list[str] = meta.get('audio_languages', []) + if audio_languages and not await languages_manager.has_english_language(audio_languages):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trackers/A4K.py` around lines 159 - 160, The code accesses meta['audio_languages'] directly which can raise KeyError if audio_languages wasn't set; instead use a defensive getter (e.g., retrieve audio_languages via meta.get('audio_languages', []) ) before the conditional in the block around audio_languages and the call to languages_manager.has_english_language so that the variable is always defined and safe to pass to has_english_language in the A4K.py logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/trackers/A4K.py`:
- Line 80: The conditional that checks "if not meta['is_disc'] and meta['type']
in ['ENCODE', 'WEBRIP', 'WEBDL']" contains an unreachable 'WEBRIP' branch for
A4K; remove 'WEBRIP' from that list so the check reads only ['ENCODE','WEBDL'],
or if you intend to support WEBRIP instead, add 'WEBRIP' to the accepted types
array where types are validated (the earlier check that allows only
['DISC','REMUX','WEBDL','ENCODE']) and add the corresponding entry to the
get_type_id mapping so A4K actually recognizes WEBRIP. Ensure you update the
conditional around meta['is_disc'] and meta['type'] and the get_type_id mapping
(and the initial accepted types list) consistently.
- Around line 155-165: The get_name coroutine in A4K.py is missing its return
type annotation; update the method signature of async def get_name(self, meta:
dict[str, Any]) to include the return type -> dict[str, str] so it matches the
base class signature and other trackers (e.g., AITHER.py). Ensure only the
signature is changed (keep existing logic using languages_manager, meta keys,
and self.tracker) and leave the returned value as {'name': a4k_name}.
---
Nitpick comments:
In `@src/trackers/A4K.py`:
- Around line 104-127: Extract the repeated "Could not determine video bitrate"
prompt into a small helper (e.g., async def _prompt_bitrate_unknown(self, meta:
dict) -> bool) and call it from get_additional_checks where the three identical
blocks currently live; the helper should encapsulate the console.print calls and
cli_ui.ask_yes_no logic and return True to continue upload or False to abort,
then replace each duplicated block (the three identical
if-not-meta['unattended']... blocks) with a single call to
self._prompt_bitrate_unknown(meta) and act on its boolean result.
- Around line 159-160: The code accesses meta['audio_languages'] directly which
can raise KeyError if audio_languages wasn't set; instead use a defensive getter
(e.g., retrieve audio_languages via meta.get('audio_languages', []) ) before the
conditional in the block around audio_languages and the call to
languages_manager.has_english_language so that the variable is always defined
and safe to pass to has_english_language in the A4K.py logic.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/trackers/A4K.py (1)
80-128:⚠️ Potential issue | 🟠 MajorWEBDL bitrate is never actually validated —
Encoded_Library_Settingsis absent on all WEBDLs.
Encoded_Library_Settings(line 86) is an encoder-metadata field written only by libx264/libx265 and similar; it is always absent on WEB-DL tracks. This means every WEBDL upload reaches theelsefallback at line 120 and prints "Could not determine video bitrate", even whenBitRateis perfectly readable from the track. The numeric threshold check (lines 96–103) is therefore dead code for WEBDL.If WEBDL bitrate validation is intended, the
BitRateread should be moved outside theencoding_settingsgate:🐛 Proposed fix — decouple BitRate read from encoding_settings
- if encoding_settings: - bit_rate = track.get('BitRate') - if bit_rate: - try: - bit_rate_num = int(bit_rate) - except (ValueError, TypeError): - bit_rate_num = None - - if bit_rate_num is not None: - bit_rate_kbps = bit_rate_num / 1000 - if meta.get('category') == "MOVIE" and bit_rate_kbps < 15000: - if not meta.get('unattended', False): - console.print(...) - return False - elif meta.get('category') == "TV" and bit_rate_kbps < 10000: - if not meta.get('unattended', False): - console.print(...) - return False - else: - # prompt - else: - # prompt - else: - # prompt + bit_rate = track.get('BitRate') + if bit_rate: + try: + bit_rate_num = int(bit_rate) + except (ValueError, TypeError): + bit_rate_num = None + + if bit_rate_num is not None: + bit_rate_kbps = bit_rate_num / 1000 + if meta.get('category') == "MOVIE" and bit_rate_kbps < 15000: + if not meta.get('unattended', False): + console.print(f"Video bitrate too low: {bit_rate_kbps:.0f} kbps for A4K movie uploads.") + return False + elif meta.get('category') == "TV" and bit_rate_kbps < 10000: + if not meta.get('unattended', False): + console.print(f"Video bitrate too low: {bit_rate_kbps:.0f} kbps for A4K TV uploads.") + return False + else: + if not meta['unattended'] or (meta['unattended'] and meta.get('unattended_confirm', False)): + console.print(f"[bold red]Could not parse video bitrate from mediainfo for {self.tracker} upload.[/bold red]") + console.print("[yellow]Bitrate must be above 15000 kbps for movies and 10000 kbps for TV shows.[/yellow]") + if not cli_ui.ask_yes_no("Do you want to upload anyway?", default=False): + return False + else: + if not meta['unattended'] or (meta['unattended'] and meta.get('unattended_confirm', False)): + console.print(f"[bold red]Could not determine video bitrate from mediainfo for {self.tracker} upload.[/bold red]") + console.print("[yellow]Bitrate must be above 15000 kbps for movies and 10000 kbps for TV shows.[/yellow]") + if not cli_ui.ask_yes_no("Do you want to upload anyway?", default=False): + return False🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trackers/A4K.py` around lines 80 - 128, The code only checks BitRate when Encoded_Library_Settings exists, so WEBDL tracks (which lack Encoded_Library_Settings) always hit the "Could not determine video bitrate" branch; move the BitRate extraction/parse/threshold logic out of the encoding_settings gate so BitRate is evaluated for every Video track regardless of Encoded_Library_Settings. Concretely: inside the Video track loop in the A4K validation block, read track.get('BitRate') and attempt to parse bit_rate_num before checking encoding_settings, then run the kbps threshold checks (movie 15000, TV 10000) and the same unattended/confirm fallback only if BitRate is missing or unparseable; keep existing messages and use self.tracker, meta, and cli_ui as currently referenced.
🧹 Nitpick comments (1)
src/trackers/A4K.py (1)
104-127: Three identical prompt blocks;if…pass…elseis avoidable.All three fallback paths print the same messages and ask the same question. They can be collapsed into one, and
if cli_ui.ask_yes_no(...): pass else: return Falsesimplifies toif not cli_ui.ask_yes_no(...): return False.♻️ Suggested consolidation (applied after the proposed fix above)
- else: - if not meta['unattended'] or (meta['unattended'] and meta.get('unattended_confirm', False)): - console.print(...) - console.print(...) - if cli_ui.ask_yes_no("Do you want to upload anyway?", default=False): - pass - else: - return False - else: - if not meta['unattended'] or (meta['unattended'] and meta.get('unattended_confirm', False)): - console.print(...) - console.print(...) - if cli_ui.ask_yes_no("Do you want to upload anyway?", default=False): - pass - else: - return False - else: - if not meta['unattended'] or (meta['unattended'] and meta.get('unattended_confirm', False)): - console.print(...) - console.print(...) - if cli_ui.ask_yes_no("Do you want to upload anyway?", default=False): - pass - else: - return False + # (single consolidated block as shown in the fix above) + if not cli_ui.ask_yes_no("Do you want to upload anyway?", default=False): + return False🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trackers/A4K.py` around lines 104 - 127, Multiple identical fallback blocks that print the same messages and prompt the user should be collapsed into one shared path: remove the duplicated blocks that check meta['unattended'] and replace them with a single check that uses console.print (referencing self.tracker) and asks the user once with cli_ui.ask_yes_no; also simplify the prompt logic from "if cli_ui.ask_yes_no(...): pass else: return False" to "if not cli_ui.ask_yes_no(...): return False". Keep the unattended conditional logic (meta['unattended'] and meta.get('unattended_confirm', False)) when deciding whether to show the prompt.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/trackers/A4K.py`:
- Line 159: The code unsafely uses direct bracket access meta['audio_languages']
and annotates it as list[str]; because process_desc_language can leave that key
unset or set it to None, replace the direct access with a safe lookup (e.g.,
meta.get('audio_languages')) and change the type to Optional[List[str]] (or
list[str] | None) for the variable audio_languages; then add a null/empty guard
before any use of audio_languages (in functions or logic that expect a list) so
you handle the unset/None case safely. Ensure you update occurrences that rely
on audio_languages to handle the Optional type and reference
process_desc_language and meta when making the change.
---
Outside diff comments:
In `@src/trackers/A4K.py`:
- Around line 80-128: The code only checks BitRate when Encoded_Library_Settings
exists, so WEBDL tracks (which lack Encoded_Library_Settings) always hit the
"Could not determine video bitrate" branch; move the BitRate
extraction/parse/threshold logic out of the encoding_settings gate so BitRate is
evaluated for every Video track regardless of Encoded_Library_Settings.
Concretely: inside the Video track loop in the A4K validation block, read
track.get('BitRate') and attempt to parse bit_rate_num before checking
encoding_settings, then run the kbps threshold checks (movie 15000, TV 10000)
and the same unattended/confirm fallback only if BitRate is missing or
unparseable; keep existing messages and use self.tracker, meta, and cli_ui as
currently referenced.
---
Duplicate comments:
In `@src/trackers/A4K.py`:
- Line 80: The guard at the conditional using meta['is_disc'] and meta['type']
has been updated to only check ['ENCODE', 'WEBDL'] and the get_name return type
annotation exists; there is nothing to change—confirm the conditional in the
function that checks meta['is_disc'] and meta['type'] (the line with "if not
meta['is_disc'] and meta['type'] in ['ENCODE', 'WEBDL']") remains as shown and
ensure get_name's return type annotation is preserved, then resolve/remove any
stale review TODOs referencing WEBRIP.
---
Nitpick comments:
In `@src/trackers/A4K.py`:
- Around line 104-127: Multiple identical fallback blocks that print the same
messages and prompt the user should be collapsed into one shared path: remove
the duplicated blocks that check meta['unattended'] and replace them with a
single check that uses console.print (referencing self.tracker) and asks the
user once with cli_ui.ask_yes_no; also simplify the prompt logic from "if
cli_ui.ask_yes_no(...): pass else: return False" to "if not
cli_ui.ask_yes_no(...): return False". Keep the unattended conditional logic
(meta['unattended'] and meta.get('unattended_confirm', False)) when deciding
whether to show the prompt.
| a4k_name: str = meta["name"] | ||
| if not meta.get('language_checked', False): | ||
| await languages_manager.process_desc_language(meta, tracker=self.tracker) | ||
| audio_languages: list[str] = meta['audio_languages'] |
There was a problem hiding this comment.
Unsafe direct access to meta['audio_languages'] — can raise KeyError.
process_desc_language can leave meta['audio_languages'] unset if a BDMV exception is raised before the assignment, and sets it to None in some unattended/no-audio paths. Direct bracket access on line 159 throws a KeyError in the former case; the type annotation (list[str]) is also incorrect for the None case.
🛡️ Proposed fix
- audio_languages: list[str] = meta['audio_languages']
+ audio_languages: list[str] = meta.get('audio_languages') or []🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/trackers/A4K.py` at line 159, The code unsafely uses direct bracket
access meta['audio_languages'] and annotates it as list[str]; because
process_desc_language can leave that key unset or set it to None, replace the
direct access with a safe lookup (e.g., meta.get('audio_languages')) and change
the type to Optional[List[str]] (or list[str] | None) for the variable
audio_languages; then add a null/empty guard before any use of audio_languages
(in functions or logic that expect a list) so you handle the unset/None case
safely. Ensure you update occurrences that rely on audio_languages to handle the
Optional type and reference process_desc_language and meta when making the
change.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/trackers/A4K.py (1)
162-162:⚠️ Potential issue | 🟡 Minor
meta['audio_languages']still uses unsafe direct access.
process_desc_languagecan leaveaudio_languagesunset orNoneunder certain code paths (e.g., BDMV exception before assignment, unattended/no-audio paths), making the direct bracket access aKeyErrorrisk. The type annotationlist[str]also doesn't account for theNonecase.🛡️ Proposed fix
- audio_languages: list[str] = meta['audio_languages'] + audio_languages: list[str] = meta.get('audio_languages') or []🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trackers/A4K.py` at line 162, The code unsafely accesses meta['audio_languages'] which can be missing or None (e.g., from process_desc_language); update the access and typing to handle that: change the annotation from list[str] to Optional[list[str]] or list[str] with a safe default, and replace direct bracket access with meta.get('audio_languages') and a fallback (e.g., [] or None) and add null-checks before usage; locate references to audio_languages, meta, and process_desc_language in A4K.py and ensure subsequent logic handles the None/empty case accordingly.
🧹 Nitpick comments (1)
src/trackers/A4K.py (1)
105-128: Collapse three identical "unknown bitrate" blocks into a helper.Lines 105–112, 113–120, and 121–128 have identical bodies — three different code paths that cannot determine the bitrate all print the same messages, ask the same prompt, and behave identically. Each could also use the simpler
if not cli_ui.ask_yes_no(...)form instead of theif ... pass / else: return Falseidiom.♻️ Proposed refactor
+ async def _prompt_unknown_bitrate(self, meta: dict[str, Any]) -> bool: + """Returns True if the upload should proceed despite unknown bitrate.""" + if not meta['unattended'] or (meta['unattended'] and meta.get('unattended_confirm', False)): + console.print(f"[bold red]Could not determine video bitrate from mediainfo for {self.tracker} upload.[/bold red]") + console.print("[yellow]Bitrate must be above 15000 kbps for movies and 10000 kbps for TV shows.[/yellow]") + if not cli_ui.ask_yes_no("Do you want to upload anyway?", default=False): + return False + return TrueThen replace each of the three duplicate blocks:
- else: - if not meta['unattended'] or (meta['unattended'] and meta.get('unattended_confirm', False)): - console.print(f"[bold red]Could not determine video bitrate from mediainfo for {self.tracker} upload.[/bold red]") - console.print("[yellow]Bitrate must be above 15000 kbps for movies and 10000 kbps for TV shows.[/yellow]") - if cli_ui.ask_yes_no("Do you want to upload anyway?", default=False): - pass - else: - return False + else: + if not await self._prompt_unknown_bitrate(meta): + return FalseApply the same replacement for the other two identical blocks (lines 113–120 and 121–128).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trackers/A4K.py` around lines 105 - 128, The three identical "unknown bitrate" blocks in src/trackers/A4K.py should be replaced by a single helper to avoid duplication: create a helper method (e.g. _handle_unknown_bitrate(meta, reason=None)) that prints the two console messages referencing self.tracker, checks the unattended/meta['unattended_confirm'] condition, and returns False when the user declines; then call this helper from the three places that currently contain the duplicated body (they reference meta, self.tracker and cli_ui.ask_yes_no). Also simplify the prompt logic to use the clearer form if not cli_ui.ask_yes_no(...): return False instead of the pass/else pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/trackers/A4K.py`:
- Line 162: The code unsafely accesses meta['audio_languages'] which can be
missing or None (e.g., from process_desc_language); update the access and typing
to handle that: change the annotation from list[str] to Optional[list[str]] or
list[str] with a safe default, and replace direct bracket access with
meta.get('audio_languages') and a fallback (e.g., [] or None) and add
null-checks before usage; locate references to audio_languages, meta, and
process_desc_language in A4K.py and ensure subsequent logic handles the
None/empty case accordingly.
---
Nitpick comments:
In `@src/trackers/A4K.py`:
- Around line 105-128: The three identical "unknown bitrate" blocks in
src/trackers/A4K.py should be replaced by a single helper to avoid duplication:
create a helper method (e.g. _handle_unknown_bitrate(meta, reason=None)) that
prints the two console messages referencing self.tracker, checks the
unattended/meta['unattended_confirm'] condition, and returns False when the user
declines; then call this helper from the three places that currently contain the
duplicated body (they reference meta, self.tracker and cli_ui.ask_yes_no). Also
simplify the prompt logic to use the clearer form if not cli_ui.ask_yes_no(...):
return False instead of the pass/else pattern.
Summary by CodeRabbit