Deno Integration - #165
Closed
firsttris wants to merge 9 commits into
Closed
Conversation
Closed
|
@firsttris Thank you! I can confirm it downloaded Deno nicely to |
Owner
Author
|
did you try again? |
|
Something strange is happening, I get 403 on the stream URL (https://rr1---sn-ivgt1vou2pm-2v1s.googlevideo.com/videoplayback?expire=1768431922&ei=0sxnad....) with the deno PR, but I also get it when built from master. And cherry on top. yt-dlp works correctly on command line on the same machine (even without deno). |
|
But, hmm, I don't think deno is working. |
Owner
Author
|
yes i think the path for deno was not correct, also found some potential bug regarding 403. pushed another commit to this branch. |
|
It played one video, failed with another, but I think Deno is in use. I added verbose: True to ydl options. See first line - deno is not listed as unavailable. And the following log on a non-playing stream: |
Owner
Author
|
thx for log, i found some more issues releated to headers, check again, if your issues are gone |
|
Tried 2 videos, both failed unfortunately |
|
I vibe coded a solution but struggling with pushing to the PR. Basically Claude found out that special characters aren't xml-encoded in DASH manifest. |
|
Sorry I don't know how else to push here, I'm unable to push to this branch and it's a trivial change. |
Owner
Author
|
good work mate! i applied your fix 91220eb is everything working, do you think we can ship it? |
There was a problem hiding this comment.
Pull request overview
This PR adds automatic Deno JavaScript runtime integration to SendToKodi to enable proper YouTube extraction with yt-dlp. Modern yt-dlp requires a JavaScript runtime for YouTube video extraction, and this implementation automatically downloads and configures Deno (~100MB) on first use, with fallback options for manual installation.
Changes:
- Implemented automatic Deno binary download and management system with platform detection
- Added configuration settings for Deno auto-download and YouTube JS warning suppression
- Fixed HTTP header handling in DASH manifest generation and non-ISA streams to prevent 403 errors
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/deno_manager.py | Core Deno management module with download, detection, and configuration |
| service.py | Integration with yt-dlp, Deno configuration, and improved HTTP header handling |
| resources/settings.xml | Added settings for Deno auto-download and JS warning suppression |
| resources/language/resource.language.en_gb/strings.po | Language strings for new settings |
| dash_builder.py | Fixed XML escaping and added HTTP headers support for DASH manifest generation |
| docs/DENO_INTEGRATION.md | Comprehensive documentation for Deno integration |
| docs/DEVELOPMENT.md | Added reference to Deno integration documentation |
| README.md | Added YouTube/Deno support information |
| test_deno_manager.py | Test script for Deno manager functionality |
| example_deno_usage.py | Example usage with yt-dlp |
| verify_integration.sh | Bash script to verify integration completeness |
| changelog.txt | Version 100.0.1 changelog |
| .gitignore | Added Deno-related ignore patterns |
Comments suppressed due to low confidence (1)
service.py:236
- Variable 'f' is out of scope here. The loop variable 'f' from the for loop at line 158 is not available in this context (line 236). Should use 'filtered_format.get('http_headers')' instead.
headers = f.get('http_headers')
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ```python | ||
| { | ||
| 'js_runtimes': { | ||
| 'deno': {'location': '/path/to/deno'} |
There was a problem hiding this comment.
The key should be 'path', not 'location'. According to yt-dlp's implementation in lib/yt_dlp/YoutubeDL.py (line 544 and 880), the correct configuration key is 'path'. The actual implementation in lib/deno_manager.py line 325 correctly uses 'path'.
|
|
||
| You don't need to configure these manually - yt-dlp passes the required permissions when executing Deno. The integration is transparent and secure. | ||
|
|
||
| ## For Developers |
There was a problem hiding this comment.
The heading '## For Developers' appears twice in the document (lines 88 and 117). The first occurrence at line 88 should likely be a different heading or merged with the second section to avoid duplicate headings.
Suggested change
| ## For Developers | |
| ## Developer Resources |
| import zipfile | ||
| import tarfile | ||
| import tempfile | ||
| import shutil |
There was a problem hiding this comment.
Import of 'shutil' is not used.
Suggested change
| import shutil |
Comment on lines
+25
to
+27
| from urllib.error import URLError | ||
| except ImportError: | ||
| from urllib2 import urlopen, Request, URLError |
There was a problem hiding this comment.
Import of 'URLError' is not used.
Suggested change
| from urllib.error import URLError | |
| except ImportError: | |
| from urllib2 import urlopen, Request, URLError | |
| except ImportError: | |
| from urllib2 import urlopen, Request |
Comment on lines
+25
to
+27
| from urllib.error import URLError | ||
| except ImportError: | ||
| from urllib2 import urlopen, Request, URLError |
There was a problem hiding this comment.
Import of 'URLError' is not used.
Suggested change
| from urllib.error import URLError | |
| except ImportError: | |
| from urllib2 import urlopen, Request, URLError | |
| except ImportError: | |
| from urllib2 import urlopen, Request |
| # Clean up the archive | ||
| try: | ||
| os.remove(archive_path) | ||
| except: |
There was a problem hiding this comment.
Except block directly handles BaseException.
Suggested change
| except: | |
| except OSError: |
| # Clean up the archive | ||
| try: | ||
| os.remove(archive_path) | ||
| except: |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
Suggested change
| except: | |
| except OSError: | |
| # Ignore errors while removing temporary archive file |
|
I can't explain how, but it works (with the most blurry resolution) even though the Python version in Kodi is old enough to throw this exception: To make things stranger, if I remove the parameter and try playing a stream this way, it doesn't play because: So somehow it only works without these sources!? |
|
So, looking at the code, the way claude fixed the playback, was by effectively disabling the DASH builder? xD I just disabled DASH in settings and it plays now, with the "quote" argument removed. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Owner
Author
|
i did some testing, installed kodi from flatpak installed my extension. send a youtube video with high quality. and it just works without deno. it plays the highest resolution vp9 2560x1350, |
Contributor
|
@firsttris My kodi instance cannot access videos without deno. I had claude patch sendtokodi locally with deno and it works. Are you interested to revive this PR or I can also PR a fresh one if you are willing to review |
Owner
Author
|
@glogiotatidis yes sure please create PR, can you ellaborate in which cases you need deno? |
Contributor
|
I's trying to play this for example https://www.youtube.com/watch?v=1_Vlcr-ihkY |
Collaborator
Isn't |
Owner
Author
|
Last time I checked, it was still working without Deno, but it now seems to be deprecated: @glogiotatidis could you share the changes you made? I can then publish a new version. |
Contributor
|
Here you go #168. This is vibe coded but tested on rpi4 / libreelec |
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.
#164