-
-
Notifications
You must be signed in to change notification settings - Fork 123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added an option to export channels thumbnails for Jellyfin #449
Conversation
Thanks! This looks reasonable to add poster support for Jellyfin. I'll comment on inline. |
tubesync/sync/models.py
Outdated
@property | ||
def get_thumbnail_url(self): | ||
if self.source_type==Source.SOURCE_TYPE_YOUTUBE_PLAYLIST: | ||
raise Exception('This source is a playlist so it doesn\'t have thumbnail.') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likely this needs nicer error handling. This will bubble up as a 500 probably if someone ticks the tickbox on a playlist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is only called if the tickbox is checked AND the source is not a playlist, so it cannot be triggered until it is called from somewhere it should not
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right, yes I see. Could probably be a Django-flavoured exception at least then?
tubesync/sync/models.py
Outdated
def get_thumbnail_url(self): | ||
if self.source_type==Source.SOURCE_TYPE_YOUTUBE_PLAYLIST: | ||
raise Exception('This source is a playlist so it doesn\'t have thumbnail.') | ||
soup = BeautifulSoup(requests.get(self.url, cookies={'CONSENT': 'YES+1'}).text, "html.parser") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only concern I have with the commit. How stable is using requests.get
to load pages from YouTube? This needs confirmation it's reliable, as well as wrapping in some exception catches.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh and the HTTP status code probably needs a check.
tubesync/sync/models.py
Outdated
soup = BeautifulSoup(requests.get(self.url, cookies={'CONSENT': 'YES+1'}).text, "html.parser") | ||
data = re.search(r"var ytInitialData = ({.*});", str(soup.prettify())).group(1) | ||
json_data = json.loads(data) | ||
return json_data["header"]["c4TabbedHeaderRenderer"]["avatar"]["thumbnails"][2]["url"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This likely needs to be wrapped in a KeyError
catch to politely handle YouTube changing their JSON format.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This reminded me, I believe yt-dlp
extracts the posters/banners for channels and adds them to the metadata for media items. There are usually very large images in the "thumbnails" metadata key with resolutions of 1920x1080. Are these the same as the posters fetched above? Could these be used instead?
Where possible it would be highly preferable to use yt-dlp
's well maintained upstream for features.
tubesync/sync/models.py
Outdated
raise Exception('This source is a playlist so it doesn\'t have thumbnail.') | ||
soup = BeautifulSoup(requests.get(self.url, cookies={'CONSENT': 'YES+1'}).text, "html.parser") | ||
data = re.search(r"var ytInitialData = ({.*});", str(soup.prettify())).group(1) | ||
json_data = json.loads(data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And wrap this to catch ValueError and TypeError in case the JSON found is invalid.
You're right, YT-DLP can do it. I switch to use this Test : import json
import yt_dlp
URL = 'https://www.youtube.com/playlist?list=PLyHWd4LxGoS8cdwh0unZXqVA3AqHoeDPU'
ydl_opts = {'extract_flat': True}
with yt_dlp.YoutubeDL(ydl_opts) as ydl:
channel_info = ydl.extract_info(URL, download=False)
banner_url = None
avatar_url = None
for thumbnail in channel_info['thumbnails']:
if thumbnail['id'] == 'banner_uncropped':
banner_url = thumbnail['url']
if thumbnail['id'] == 'avatar_uncropped':
avatar_url = thumbnail['url']
if banner_url != None and avatar_url != None:
break
print(banner_url)
print(avatar_url) |
It now exports banner as banner.jpg and avatar as poster.jpg and season-poster.jpg
@@ -35,6 +35,35 @@ def get_yt_opts(): | |||
opts.update({'cookiefile': cookie_file_path}) | |||
return opts | |||
|
|||
def get_channel_image_info(url): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One final comment here, this info is almost certainly in the metadata stored in Media.metadata - you can likely just load it straight out of the JSON without having to invoke yt-dlp
at all.
Some exceptions defined in Django are handled better (auto-modify HTTP status codes, logging, etc), that's all I meant. Raise a SuspiciousOperation exception rather than a plain Exception: https://docs.djangoproject.com/en/4.2/ref/exceptions/#suspiciousoperation |
I'll merge this for now, thanks for the feature PR, most appreciated. I'll rework your function that uses |
Added an option to download channel thumbnails to poster.jpg and season-poster.jpg for Youtube channels :
This uses BeautifulSoup to find the thumbnail URL and only works for Youtube channels (playlists don't have thumbnails)
These 2 images are then used by Jellyfin:
I didn't manage to hide this option while adding a Playlist but it is ignore if it is