-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Fix an issue with the HLS manifest check for livestream videos #5189
Fix an issue with the HLS manifest check for livestream videos #5189
Conversation
Fixes: iv-org/invidious-companion#35 There's also another place where the simple check for an HLS manifest exists:
Does this also need to be changed? The alternative approach to this would be to add a boolean property to the This change has been tested on my local instance and is working well with:
Let me know if anything makes sense to change. |
@@ -4,7 +4,7 @@ | |||
<% if params.autoplay %>autoplay<% end %> | |||
<% if params.video_loop %>loop<% end %> | |||
<% if params.controls %>controls<% end %>> | |||
<% if (hlsvp = video.hls_manifest_url) && !CONFIG.disabled?("livestreams") %> | |||
<% if (hlsvp = video.hls_manifest_url) && hlsvp.includes?("/source/yt_live_broadcast/") && !CONFIG.disabled?("livestreams") %> |
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.
You should just be able to check whether video.live_now
is true instead of the 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.
Thanks - great catch. I'm still digging through the codebase, this does a better job that checking the URL.
I've updated to that in case it's worth merging this in in the meantime as some videos are broken at the moment.
If I'm understanding everything correctly this is something that should be fixed by #4118, whenever that is merged. This part of the PR prevents Invidious's frontend from using the HLS manifest unless the video is a livestream or the user has selected HLS quality invidious/src/invidious/views/components/player.ecr Lines 7 to 10 in ffc44b0
|
Originally, the HLS manifest check was essentially a boolean: if the HLS manifest field was present, it was assumed to be a livestream. Some videos include the HLS Manifest but aren't livestreams. In the case where they are livestreams, the video contains a videoType field with the value "Livestream". In the case that they're normal videos, the videoType is "Video". This is exposed via the video.live_now method. This commit just checks that video.live_now is true before treating it as a livestream
2cc4444
to
49afbf2
Compare
Originally, the HLS manifest check was essentially a boolean: if the HLS manifest field was present, it was assumed to be a livestream. Some videos include the HLS Manifest but aren't livestreams.
In the case where they are livestreams, the video contains a videoType field with the value "Livestream". In the case that they're normal videos, the videoType is "Video". This is exposed via the video.live_now method.
This commit just checks that video.live_now is true before treating it as a livestream