Repository navigation
Code review fixes: faster start, no update wait before playback, Deno on Android - #194
Merged
Merged
Conversation
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MpqNbBhrj8pan7qLdtFbdy
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MpqNbBhrj8pan7qLdtFbdy
- patch_strptime and the stderr workaround only patch once, so a reused interpreter does not stack datetime subclasses. - An empty or invalid maxresolution setting falls back to 1920 instead of raising ValueError before playback. - Drop the deno_enabled value: the setting no longer exists. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MpqNbBhrj8pan7qLdtFbdy
Python on Android reports Linux/aarch64, so the manager downloaded the glibc Linux build of Deno (100+ MB unpacked), which cannot run there, and handed it to yt-dlp. Android is now detected (sys.getandroidapilevel, platform.system() on Python 3.13+, or Kodi's System.Platform.Android), no Deno is configured, a leftover unusable download is removed, and the settings actions say why instead of reporting a failed install. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MpqNbBhrj8pan7qLdtFbdy
The tag archive has no yt_dlp/extractor/lazy_extractors.py, so every start imported all ~1800 extractor modules. The yt-dlp.tar.gz release asset is built with 'make all' and includes it, with the same layout as the archive (checked against the 2026.08.19 asset). Startup of import yt_dlp + YoutubeDL() on x86: tag archive: 1.8 s cold / 0.45 s warm release asset: 0.6 s cold / 0.22 s warm Releases without the asset fall back to the tag archive on HTTP 404. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MpqNbBhrj8pan7qLdtFbdy
download_with_progress collected the whole archive in memory, and the Deno extraction then read the unpacked binary (100+ MB) in one piece, so an install needed well over 150 MB of RAM on top of Kodi. Archives are now streamed to a temporary file next to the versions directory, extracted with shutil.copyfileobj and removed afterwards; a failed download leaves no partial file behind. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MpqNbBhrj8pan7qLdtFbdy
The cause is not a thread race: Kodi runs each add-on invocation in a sub-interpreter that is destroyed afterwards, and up to Python 3.12 the C datetime module caches the _strptime module in a static shared by all interpreters. After the interpreter that filled the cache is gone, every datetime.strptime() call raises "'NoneType' object is not callable" (issues #8 and #177, bpo-27400, fixed in Python 3.13). Reproduced with _testcapi.run_in_subinterp on Python 3.11 and 3.12; a one-time warmup does not help because it poisons the next invocation. The new test replays two Kodi-style invocations in a child process. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MpqNbBhrj8pan7qLdtFbdy
get_runtime_status always resolved the latest yt-dlp release, which goes to the GitHub API once the cached value is older than six hours. Opening the add-on settings (refresh_runtime_displays) and the version actions only need the installed versions, so they could wait up to 20 s on the network. The lookup is now opt-in via include_latest, as for Deno. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MpqNbBhrj8pan7qLdtFbdy
Once the cached update interval (6 h) expired, the next playback first asked GitHub for the latest yt-dlp and Deno release (20 s timeout each) and downloaded new versions before resolving anything. An installed yt-dlp or Deno (also a system Deno) is now used right away without network access. update_runtimes_after_playback runs after setResolvedUrl, also after a failed resolve, where an outdated yt-dlp is the usual cause, and installs new releases silently for the next playback. A first install without any runtime still happens up front. service.py is now a main() function that returns instead of calling the site module's exit(), with smoke tests against fake Kodi modules. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MpqNbBhrj8pan7qLdtFbdy
Subtitles were fetched one after another (20 s timeout each) before setResolvedUrl, so videos with many languages started noticeably later. Downloads now run in a small thread pool; file names are still assigned in order, the result keeps the original order and a failed download still falls back to the remote url. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MpqNbBhrj8pan7qLdtFbdy
When the segment index was not in the first 4 KiB, the probe fetched 16 KiB, 64 KiB and 256 KiB, each again from byte 0. It now requests only the missing range and appends it. A server that ignores Range (HTTP 200) still works: the answer then starts at byte 0 and replaces the buffer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MpqNbBhrj8pan7qLdtFbdy
The playlist's starting video was resolved (and, with auto-download on, downloaded) through a wrapper that ignored its download argument and showed no progress dialog. It now goes through the same helpers as a single video: "Resolving" while extracting and "Downloading" while downloading. resolve_starting_entry no longer passes a download flag. The docs said playlists are never downloaded; in fact every playlist video is downloaded when its turn comes, which the docs now say. Also renames _prompt_preferred_stream_url to _prompt_preferred_stream, since it returns the chosen format, not a url. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MpqNbBhrj8pan7qLdtFbdy
The original-manifest lookup, the DASH manifest result and the source/format_label tagging were each written out two or three times. They are now small helpers; the order of decisions is unchanged. The final original-manifest fallback is no longer computed (and ISA no longer asked) when a chosen stream excludes it anyway. Adds tests for the two branches without a strict resolution limit, which were not covered before; they pass on the old code as well. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MpqNbBhrj8pan7qLdtFbdy
With reuselanguageinvoker, Kodi keeps the interpreter of the add-on, so yt-dlp and requests are imported once instead of on every playback. The add-on is ready for it: service.py runs main() instead of module code and exit(), and the strptime/stderr patches apply only once. A yt-dlp version switch still takes effect immediately: activate_runtime now removes other managed versions from sys.path and unloads a yt_dlp imported from elsewhere (checked by switching between two real yt-dlp trees in one interpreter), and the system source takes managed versions off sys.path before importing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MpqNbBhrj8pan7qLdtFbdy
update_installed_runtime only looked at versioned installs, so a Deno in the old flat addon_data layout would never have been updated (or migrated to the versions folder) once the pre-playback check was gone. It now updates any Deno the add-on installed and still leaves a system Deno alone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MpqNbBhrj8pan7qLdtFbdy
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MpqNbBhrj8pan7qLdtFbdy
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #194 +/- ##
==========================================
+ Coverage 75.88% 84.48% +8.60%
==========================================
Files 14 14
Lines 2380 2591 +211
==========================================
+ Hits 1806 2189 +383
+ Misses 574 402 -172 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MpqNbBhrj8pan7qLdtFbdy
This file contains hidden or 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
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.
Implements the verified points of a code review of the add-on. Every point was checked against the code, and measured or reproduced where possible. Each change is its own commit.
Faster playback start
yt-dlp.tar.gzrelease asset. It is built withmake alland shipslazy_extractors.py; the tag archive does not, so every start imported all ~1800 extractor modules. Checked against the real 2026.08.19 asset. Releases without the asset fall back to the tag archive on HTTP 404.setResolvedUrl, also after a failed resolve, and the new version is used from the next playback. A first install still happens up front.reuselanguageinvokerkeeps the interpreter between calls, so yt-dlp is imported only once. A yt-dlp version switch still takes effect immediately:activate_runtimecleanssys.pathand unloads a staleyt_dlp. This was tested by switching between two real yt-dlp trees in one interpreter.import yt_dlp+YoutubeDL(), cold cacheBugs
maxresolutionsetting raisedValueError. The deaddeno_enabledsetting is removed.Why the strptime patch is needed (#177)
It is not a thread race. Kodi runs every add-on call in a sub-interpreter and destroys it afterwards. Up to Python 3.12, the C
datetimemodule caches_strptimein a static variable shared by all interpreters. After the interpreter that filled the cache is gone, everystrptime()call raises'NoneType' object is not callable(bpo-27400, fixed in 3.13).This was reproduced with
_testcapi.run_in_subinterpon 3.11 and 3.12. The warmup suggested in #177 makes things worse, because it poisons the next call. The patch is now idempotent, and a new test replays two Kodi-style calls; it runs on CI's Python 3.12.Other changes
service.pyis now amain()function that returns instead of callingexit(), and it has smoke tests against fake Kodi modules.select_playback_sourcewere moved into helpers without changing the order of decisions. Two branches that had no tests now have them, and those tests also pass on the old code.Rejected after checking
parse_options: a call takes 2 ms.requestsandurllib: no benefit.Testing
reuselanguageinvokerand the post-playback update on Kodi 21. Check playback, a playlist, the settings actions and a yt-dlp version switch without restarting Kodi.🤖 Generated with Claude Code
https://claude.ai/code/session_01MpqNbBhrj8pan7qLdtFbdy