-
-
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
Upgrade to video.js 8 #4439
base: master
Are you sure you want to change the base?
Upgrade to video.js 8 #4439
Conversation
I can answer that one for you, it is because the CSS selector in video.js got slightly more specific due to an unrelated change videojs/video.js#7724 which was released in 7.19.1. Thus the Invidious CSS no longer takes precedence. Not being a CSS expert myself I don't know what the most future-proof way to increase the precedence of the selector on the Invidious side would be. Adding the extra |
Issue #4199 is not fixed. I just tested this PR on macOS 14.4 with Safari 17.4 and still get the error message. |
assets/js/player.js
Outdated
if (!video_data.params.listen && video_data.params.quality === 'dash') { | ||
var quality_menu_button = document.getElementsByClassName('vjs-quality-menu-button'); | ||
for (var i = 0; i < quality_menu_button.length; i++) { | ||
quality_menu_button[i].className += ' vjs-icon-cog'; |
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.
need to remove this when videojs-contrib-quality-menu
is upgraded to 1.0.2
Maybe an upstream bug: videojs/video.js#8641. Thank you for the info. |
I use code like this in my fork, it's a combination of your work and my own: Diff listingdiff --git a/assets/js/player.js b/assets/js/player.js
index 71c5e7da..c83d9d5e 100644
--- a/assets/js/player.js
+++ b/assets/js/player.js
@@ -18,7 +18,7 @@ var options = {
'Spacer',
'captionsButton',
'audioTrackButton',
- 'qualitySelector',
+ 'qualityMenuButton',
'playbackRateMenuButton',
'fullscreenToggle'
]
@@ -157,6 +157,16 @@ player.on('timeupdate', function () {
elem_iv_other.href = addCurrentTimeToURL(base_url_iv_other, domain);
});
+player.one('playing', function () {
+
+ if (!video_data.params.listen && video_data.params.quality === 'dash') {
+ var quality_menu_button = document.getElementsByClassName('vjs-quality-menu-button');
+ for (var i = 0; i < quality_menu_button.length; i++) {
+ quality_menu_button[i].className += ' vjs-icon-cog';
+ }
+ }
+});
+
var shareOptions = {
socials: ['fbFeed', 'tw', 'reddit', 'email'],
@@ -193,42 +203,47 @@ function isMobile() {
}
if (isMobile()) {
- player.mobileUi({ touchControls: { seekSeconds: 5 * player.playbackRate() } });
-
- var buttons = ['playToggle', 'volumePanel', 'captionsButton'];
-
- if (!video_data.params.listen && video_data.params.quality === 'dash') buttons.push('audioTrackButton');
- if (video_data.params.listen || video_data.params.quality !== 'dash') buttons.push('qualitySelector');
-
- // Create new control bar object for operation buttons
- const ControlBar = videojs.getComponent('controlBar');
- let operations_bar = new ControlBar(player, {
- children: [],
- playbackRates: [0.25, 0.5, 0.75, 1.0, 1.25, 1.5, 1.75, 2.0]
+ player.mobileUi({
+ touchControls: {
+ seekSeconds: 5 * player.playbackRate(),
+ },
+ fullscreen: {
+ enterOnRotate: false,
+ exitOnRotate: false,
+ lockOnRotate: false,
+ lockToLandscapeOnEnter: false,
+ },
});
- buttons.slice(1).forEach(function (child) {operations_bar.addChild(child);});
-
- // Remove operation buttons from primary control bar
- var primary_control_bar = player.getChild('controlBar');
- buttons.forEach(function (child) {primary_control_bar.removeChild(child);});
-
- var operations_bar_element = operations_bar.el();
- operations_bar_element.classList.add('mobile-operations-bar');
- player.addChild(operations_bar);
- // Playback menu doesn't work when it's initialized outside of the primary control bar
- var playback_element = document.getElementsByClassName('vjs-playback-rate')[0];
- operations_bar_element.append(playback_element);
-
- // The share and http source selector element can't be fetched till the players ready.
player.one('playing', function () {
- var share_element = document.getElementsByClassName('vjs-share-control')[0];
- operations_bar_element.append(share_element);
-
- if (!video_data.params.listen && video_data.params.quality === 'dash') {
- var http_source_selector = document.getElementsByClassName('vjs-http-source-selector vjs-menu-button')[0];
- operations_bar_element.append(http_source_selector);
- }
+ var buttons = [
+ 'playToggle',
+ 'volumePanel',
+ 'captionsButton',
+ 'audioTrackButton',
+ 'qualityMenuButton',
+ 'playbackRateMenuButton',
+ ];
+
+ // Create new control bar object for operation buttons
+ const ControlBar = videojs.getComponent('controlBar');
+ let operations_bar = new ControlBar(player, {
+ children: [],
+ name: "mobileOperationsBar",
+ playbackRates: [0.25, 0.5, 0.75, 1.0, 1.25, 1.5, 1.75, 2.0]
+ });
+ operations_bar.addClass("mobile-operations-bar");
+ player.addChild(operations_bar);
+
+ // Move operation buttons from to secondary control bar
+ var primary_control_bar = player.getChild('controlBar');
+ buttons.forEach(function (child) {
+ const elt = primary_control_bar.getChild(child);
+ primary_control_bar.removeChild(elt);
+ if (child !== "playToggle") {
+ operations_bar.addChild(elt);
+ }
+ });
});
}
@@ -386,7 +436,7 @@ if (video_data.params.autoplay) {
}
if (!video_data.params.listen && video_data.params.quality === 'dash') {
- player.httpSourceSelector();
+ var qualityMenuOptions = {}
if (video_data.params.quality_dash !== 'auto') {
player.ready(function () {
@@ -409,12 +459,13 @@ if (!video_data.params.listen && video_data.params.quality === 'dash') {
break;
}
}
- qualityLevels.forEach(function (level, index) {
- level.enabled = (index === targetQualityLevel);
- });
+ qualityMenuOptions.defaultResolution = (qualityLevels[targetQualityLevel].height + "p");
});
});
}
+
+ console.log(qualityMenuOptions)
+ player.qualityMenu(qualityMenuOptions);
}
player.vttThumbnails({ It gives me a more navigable user interface where only the fullscreen button is in the bottom bar, and space is saved by moving all the other controls to the mobile operations bar. I found that I was getting some errors with the video.js 8 upgrade before making these changes. It's not a complete solution, however, and I think the quality selector is still a bit broken even with this implementation. May be helpful. |
What errors? Can you tell us, it's quite important for achieving an ideal upgrade to video.js 8.
Apart from the |
Yes, of course, sorry for being vague the first time. I have created a branch on my fork that matches your PR here, rolling back all of the video.js related changes that I had made. For reference, the exact diff from your PR to the branch I used for the following test case is unixfox/invidious@videojs8...raxod502:invidious:rr-without-videojs-hacks. One problem that I notice almost immediately is that although the control bar renders fine on desktop, I see an unexpected Using Firefox remote debugger, the following stacktrace appears immediately on page load: The other buttons on the mobile UI work okay. |
Thank you for the feedback! Did you try to replicate this issue on a "vanilla" video.js project? Like with just video.js and the plugin videojs-contrib-quality-menu. Because if you have the same issue on mobile then we have a bug in the plugin that need a ticket creation in https://github.com/videojs/videojs-contrib-quality-menu/issues! |
No, the issue is around the quality menu failing to initialize properly when it's moved from its default location to the mobile operations bar by Invidious in this code: Lines 214 to 220 in 44df12f
There is even a comment saying that a similar problem occurs with other components: Line 230 in 44df12f
So this is not a problem that could be reproduced outside of Invidious (without repeating the same logic to manually move components around). I did set up this test case though to import the relevant plugins: index.html<!doctype html>
<html>
<head>
<meta charset="utf-8" />
<title>Test</title>
<link
rel="stylesheet"
href="https://cdnjs.cloudflare.com/ajax/libs/video.js/8.14.0/video-js.min.css"
integrity="sha512-K8LjnAlu87KsLcGLq5Y6cpwpLkGUgYpEBCtxiEjLTGZzXih1CSSfDHeo2RQTVi4mVq2KCnXAHMoL3D1XwOz0Dg=="
crossorigin="anonymous"
referrerpolicy="no-referrer"
/>
</head>
<body>
<video
id="myvideo"
class="video-js"
controls
preload="auto"
width="640"
height="264"
>
<source
src="https://stream.mux.com/UZMwOY6MgmhFNXLbSFXAuPKlRPss5XNA.m3u8"
/>
</video>
<script
src="https://cdnjs.cloudflare.com/ajax/libs/video.js/8.14.0/video.min.js"
integrity="sha512-TuwqcgFUxZeSmVdVYuGxu8jfbeJHBW/Q25/p15XPRwqx/YjOWHhCGghk03ZFfO0MDvuIRCiuawSaZaIsEVTqsw=="
crossorigin="anonymous"
referrerpolicy="no-referrer"
></script>
<script
src="https://unpkg.com/[email protected]/dist/videojs-contrib-quality-menu.js"
integrity="sha512-/7iX6nvO3kvMsf+rS/qml3Iv8q9Huch0UYFEaQtT9r7bSHNqqMtJmQA+stRd5CrBQYVsOsI7dDgFjDzMhX3lYw=="
crossorigin="anonymous"
referrerpolicy="no-referrer"
></script>
<script
src="https://unpkg.com/[email protected]/dist/videojs-mobile-ui.min.js"
integrity="sha512-i9YYtHHqBLdHJo3eQa880kia/3I8R/XPzOvCgpVobFrE3pi7+JkYSegl/5g2PHs2w6+dgAQkREJQJuqJxuDdVw=="
crossorigin="anonymous"
referrerpolicy="no-referrer"
></script>
<script>
function isMobile() {
try {
document.createEvent("TouchEvent");
return true;
} catch (e) {
return false;
}
}
let player = videojs("myvideo");
player.qualityMenu();
if (isMobile()) {
player.mobileUi();
}
</script>
</body>
</html> |
/** | ||
* videojs-share | ||
* @version 3.2.1 | ||
* @copyright 2024 Mikhail Khazov <[email protected]> | ||
* @license MIT | ||
*/ |
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.
Why are you adding videojs-share
to the invidious repository, when it's already part of the VideoJS dependencies, and downloaded from NPM?
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.
Just testing stuff, hence why it's still in a draft.
I did find another alternative to videojs-share for videojs 8 here: https://www.npmjs.com/package/@viostream/videojs-share-menu but it's not ready out of the box.
The old video-js annotations extension also needs to be tested on whether or not it works on VideoJS 8 |
Description
The final plugin that was stopping us from upgrading to a newer video.js version was https://github.com/jfujita/videojs-http-source-selector. There is now a replacement plugin available officially by video.js: https://github.com/videojs/videojs-contrib-quality-menu.
This PR aims to use this new plugin and upgrade to video.js 8.
Drawbacks
IE will still work on Invidious, just that it will use the default video player of IE with no way to switch between qualities without going through the preferences.
Fixes
Confirmed
Unsure
PR depends on
TODO
Representation
with the same ID, video.js will complain.Related to [Enhancement] Add support for multiples audio tracks #2007
videojs-share
: Add custom share widget to replace videojs-share #5080padding-top
for.player-dimensions.vjs-fluid
/watch?v=LXb3EKWsInQ
videojs-contrib-quality-menu
to1.0.2
but waiting for the version to be released on NPM: https://www.npmjs.com/package/videojs-contrib-quality-menu