Skip to content

Code review fixes: faster start, no update wait before playback, Deno on Android - #194

Merged
firsttris merged 17 commits into
masterfrom
claude/gracious-brahmagupta-go1e1y
Oct 8, 2026
Merged

firsttris merged 17 commits into
masterfrom
claude/gracious-brahmagupta-go1e1y

Conversation

@firsttris

@firsttris firsttris commented Oct 8, 2026 •

Copy link
Copy Markdown
Owner

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 stable now installs the yt-dlp.tar.gz release asset. It is built with make all and ships lazy_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.
  • Runtime updates no longer run before playback. An installed yt-dlp or Deno is used without network access. The update check runs after setResolvedUrl, also after a failed resolve, and the new version is used from the next playback. A first install still happens up front.
  • reuselanguageinvoker keeps the interpreter between calls, so yt-dlp is imported only once. A yt-dlp version switch still takes effect immediately: activate_runtime cleans sys.path and unloads a stale yt_dlp. This was tested by switching between two real yt-dlp trees in one interpreter.
  • Subtitles download in parallel. The DASH range probe only fetches the bytes that are missing.
Measurement (x86) Before After
import yt_dlp + YoutubeDL(), cold cache 1.8 s 0.6 s
same, warm cache 0.45 s 0.22 s

Bugs

  • Deno on Android: Python reports Linux/aarch64 there, so the glibc Linux build was downloaded (100+ MB unpacked) even though it cannot run. Android is now detected, the leftover download is removed, and the settings actions explain why.
  • Downloads in RAM: the Deno zip and the unpacked binary were held fully in memory. Downloads now stream to disk.
  • Opening the settings could wait up to 20 s on GitHub, because the yt-dlp status always looked up the latest release.
  • An empty maxresolution setting raised ValueError. The dead deno_enabled setting is removed.
  • The playlist start video was resolved without a progress dialog. The docs claimed playlists are never downloaded, which is wrong and is now fixed.

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 datetime module caches _strptime in a static variable shared by all interpreters. After the interpreter that filled the cache is gone, every strptime() call raises 'NoneType' object is not callable (bpo-27400, fixed in 3.13).

This was reproduced with _testcapi.run_in_subinterp on 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

  • The release zip no longer contains tests, docs or dev config. It shrinks from over 1 MB to 70 KB.
  • service.py is now a main() function that returns instead of calling exit(), and it has smoke tests against fake Kodi modules.
  • The repeated blocks in select_playback_source were 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

  • Caching parse_options: a call takes 2 ms.
  • Splitting the DASH timeout constant: it would change behavior and needs a product decision.
  • Unifying requests and urllib: no benefit.
  • Coverage only in CI: it adds about 1 s locally.

Testing

  • 341 tests: 340 pass and 1 is skipped (that test is also skipped on master, which has 301). Run on Python 3.13 and on 3.12 with coverage, as in CI.
  • Not tested on a real Kodi device. Before merging, please try reuselanguageinvoker and 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

claude added 16 commits October 8, 2026 21:33
- 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
@codecov-commenter

codecov-commenter commented Oct 8, 2026 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 90.60052% with 36 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.48%. Comparing base (b5e27a0) to head (895ab5b).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
service.py 73.40% 25 Missing ⚠️
core/deno_manager.py 90.90% 7 Missing ⚠️
core/playback_selection.py 96.96% 1 Missing ⚠️
core/runtime/playback.py 95.65% 1 Missing ⚠️
core/service_runtime.py 80.00% 1 Missing ⚠️
core/ytdlp_manager.py 98.38% 1 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MpqNbBhrj8pan7qLdtFbdy
@firsttris
firsttris merged commit 4aa2a6e into master Oct 8, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants